gr1ffon/quic-go
1
0
Fork 0
forked from quic-go/quic-go
Code Pull Requests Activity

Merge pull request #2689 from lucas-clemente/dont-arm-app-data-pto-before-handshake-confirmed

Browse Source 
only arm the application data PTO timer after the handshake is confirmed
This commit is contained in:
Marten Seemann
2020-07-29 07:58:29 +07:00
committed by GitHub
parent 6068883fa4 8db76ab9c2
commit 03a1db641f
6 changed files with 23 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
internal/ackhandler/interfaces.go
View File

@@ -28,7 +28,7 @@ type SentPacketHandler interface {
ReceivedBytes(protocol.ByteCount)
DropPackets(protocol.EncryptionLevel)
ResetForRetry() error
SetHandshakeComplete()
SetHandshakeConfirmed()
// The SendMode determines if and what kind of packets can be sent.
SendMode() SendMode

10
internal/ackhandler/sent_packet_handler.go
View File

@@ -58,7 +58,7 @@ type sentPacketHandler struct {
// Always true for the client.
peerAddressValidated bool
handshakeComplete bool
handshakeConfirmed bool
// lowestNotConfirmedAcked is the lowest packet number that we sent an ACK for, but haven't received confirmation, that this ACK actually arrived
// example: we send an ACK for packets 90-100 with packet number 20
@@ -444,7 +444,7 @@ func (h *sentPacketHandler) getPTOTimeAndSpace() (time.Time, protocol.Encryption
encLevel = protocol.EncryptionHandshake
}
}
if h.handshakeComplete && !h.appDataPackets.lastAckElicitingPacketTime.IsZero() {
if h.handshakeConfirmed && !h.appDataPackets.lastAckElicitingPacketTime.IsZero() {
t := h.appDataPackets.lastAckElicitingPacketTime.Add(h.rttStats.PTO(true) << h.ptoCount)
if pto.IsZero() || (!t.IsZero() && t.Before(pto)) {
pto = t
@@ -468,7 +468,7 @@ func (h *sentPacketHandler) hasOutstandingCryptoPackets() bool {
func (h *sentPacketHandler) hasOutstandingPackets() bool {
// We only send application data probe packets once the handshake completes,
// because before that, we don't have the keys to decrypt ACKs sent in 1-RTT packets.
return (h.handshakeComplete && h.appDataPackets.history.HasOutstandingPackets()) ||
return (h.handshakeConfirmed && h.appDataPackets.history.HasOutstandingPackets()) ||
h.hasOutstandingCryptoPackets()
}
@@ -802,8 +802,8 @@ func (h *sentPacketHandler) ResetForRetry() error {
return nil
}
func (h *sentPacketHandler) SetHandshakeComplete() {
h.handshakeComplete = true
func (h *sentPacketHandler) SetHandshakeConfirmed() {
h.handshakeConfirmed = true
// We don't send PTOs for application data packets before the handshake completes.
// Make sure the timer is armed now, if necessary.
h.setLossDetectionTimer()

18
internal/ackhandler/sent_packet_handler_test.go
View File

@@ -604,7 +604,7 @@ var _ = Describe("SentPacketHandler", func() {
})
It("implements exponential backoff", func() {
handler.SetHandshakeComplete()
handler.SetHandshakeConfirmed()
sendTime := time.Now().Add(-time.Hour)
handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1, SendTime: sendTime}))
timeout := handler.GetLossDetectionTimeout().Sub(sendTime)
@@ -620,7 +620,7 @@ var _ = Describe("SentPacketHandler", func() {
It("reset the PTO count when receiving an ACK", func() {
handler.ReceivedPacket(protocol.EncryptionHandshake)
now := time.Now()
handler.SetHandshakeComplete()
handler.SetHandshakeConfirmed()
handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1, SendTime: now.Add(-time.Minute)}))
handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 2, SendTime: now.Add(-time.Minute)}))
Expect(handler.GetLossDetectionTimeout()).To(BeTemporally("~", now.Add(-time.Minute), time.Second))
@@ -659,7 +659,7 @@ var _ = Describe("SentPacketHandler", func() {
Expect(handler.ptoCount).To(BeEquivalentTo(1))
Expect(handler.SendMode()).To(Equal(SendPTOHandshake))
Expect(handler.GetLossDetectionTimeout()).To(Equal(sendTimeHandshake.Add(handler.rttStats.PTO(false) << 1)))
handler.SetHandshakeComplete()
handler.SetHandshakeConfirmed()
handler.DropPackets(protocol.EncryptionHandshake)
// PTO timer based on the 1-RTT packet
Expect(handler.GetLossDetectionTimeout()).To(Equal(sendTimeAppData.Add(handler.rttStats.PTO(true)))) // no backoff. PTO count = 0
@@ -669,7 +669,7 @@ var _ = Describe("SentPacketHandler", func() {
It("allows two 1-RTT PTOs", func() {
handler.ReceivedPacket(protocol.EncryptionHandshake)
handler.SetHandshakeComplete()
handler.SetHandshakeConfirmed()
var lostPackets []protocol.PacketNumber
handler.SentPacket(ackElicitingPacket(&Packet{
PacketNumber: 1,
@@ -688,7 +688,7 @@ var _ = Describe("SentPacketHandler", func() {
It("only counts ack-eliciting packets as probe packets", func() {
handler.ReceivedPacket(protocol.EncryptionHandshake)
handler.SetHandshakeComplete()
handler.SetHandshakeConfirmed()
handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1, SendTime: time.Now().Add(-time.Hour)}))
Expect(handler.OnLossDetectionTimeout()).To(Succeed())
Expect(handler.SendMode()).To(Equal(SendPTOAppData))
@@ -704,7 +704,7 @@ var _ = Describe("SentPacketHandler", func() {
It("gets two probe packets if PTO expires", func() {
handler.ReceivedPacket(protocol.EncryptionHandshake)
handler.SetHandshakeComplete()
handler.SetHandshakeConfirmed()
handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1}))
handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 2}))
@@ -752,7 +752,7 @@ var _ = Describe("SentPacketHandler", func() {
Expect(handler.OnLossDetectionTimeout()).To(Succeed()) // TLP
Expect(handler.GetLossDetectionTimeout()).To(BeZero())
Expect(handler.SendMode()).To(Equal(SendAny))
handler.SetHandshakeComplete()
handler.SetHandshakeConfirmed()
Expect(handler.GetLossDetectionTimeout()).ToNot(BeZero())
Expect(handler.OnLossDetectionTimeout()).To(Succeed())
Expect(handler.SendMode()).To(Equal(SendPTOAppData))
@@ -760,7 +760,7 @@ var _ = Describe("SentPacketHandler", func() {
It("resets the send mode when it receives an acknowledgement after queueing probe packets", func() {
handler.ReceivedPacket(protocol.EncryptionHandshake)
handler.SetHandshakeComplete()
handler.SetHandshakeConfirmed()
handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1, SendTime: time.Now().Add(-time.Hour)}))
handler.rttStats.UpdateRTT(time.Second, 0, time.Now())
Expect(handler.OnLossDetectionTimeout()).To(Succeed())
@@ -902,7 +902,7 @@ var _ = Describe("SentPacketHandler", func() {
It("sets the early retransmit alarm", func() {
handler.ReceivedPacket(protocol.EncryptionHandshake)
handler.handshakeComplete = true
handler.handshakeConfirmed = true
now := time.Now()
handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1, SendTime: now.Add(-2 * time.Second)}))
handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 2, SendTime: now.Add(-2 * time.Second)}))

12
internal/mocks/ackhandler/sent_packet_handler.go
View File

@@ -229,16 +229,16 @@ func (mr *MockSentPacketHandlerMockRecorder) SentPacket(arg0 interface{}) *gomoc
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SentPacket", reflect.TypeOf((*MockSentPacketHandler)(nil).SentPacket), arg0)
}
// SetHandshakeComplete mocks base method
func (m *MockSentPacketHandler) SetHandshakeComplete() {
// SetHandshakeConfirmed mocks base method
func (m *MockSentPacketHandler) SetHandshakeConfirmed() {
m.ctrl.T.Helper()
m.ctrl.Call(m, "SetHandshakeComplete")
m.ctrl.Call(m, "SetHandshakeConfirmed")
}
// SetHandshakeComplete indicates an expected call of SetHandshakeComplete
func (mr *MockSentPacketHandlerMockRecorder) SetHandshakeComplete() *gomock.Call {
// SetHandshakeConfirmed indicates an expected call of SetHandshakeConfirmed
func (mr *MockSentPacketHandlerMockRecorder) SetHandshakeConfirmed() *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetHandshakeComplete", reflect.TypeOf((*MockSentPacketHandler)(nil).SetHandshakeComplete))
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetHandshakeConfirmed", reflect.TypeOf((*MockSentPacketHandler)(nil).SetHandshakeConfirmed))
}
// TimeUntilSend mocks base method