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

implement spurious RTO detection

Browse Source
This commit is contained in:
Marten Seemann
2018-03-30 23:59:19 +07:00
parent 0ad3c17506
commit 605607d39b
2 changed files with 96 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
internal/ackhandler/sent_packet_handler.go
View File

@@ -42,6 +42,7 @@ type sentPacketHandler struct {
// example: we send an ACK for packets 90-100 with packet number 20
// once we receive an ACK from the peer for packet 20, the lowestPacketNotConfirmedAcked is 101
lowestPacketNotConfirmedAcked protocol.PacketNumber
largestSentBeforeRTO protocol.PacketNumber
packetHistory *sentPacketHistory
stopWaitingManager stopWaitingManager
@@ -316,6 +317,8 @@ func (h *sentPacketHandler) detectLostPackets(now time.Time) error {
if err := h.queuePacketForRetransmission(p); err != nil {
return err
}
}
if p.includedInBytesInFlight {
h.congestion.OnPacketLost(p.PacketNumber, p.Length, h.bytesInFlight)
}
h.packetHistory.Remove(p.PacketNumber)
@@ -357,9 +360,6 @@ func (h *sentPacketHandler) onPacketAcked(p *Packet) error {
if packet := h.packetHistory.GetPacket(p.PacketNumber); packet == nil {
return nil
}
h.rtoCount = 0
h.handshakeCount = 0
// TODO(#497): h.tlpCount = 0
// only report the acking of this packet to the congestion controller if:
// * it is a retransmittable packet
@@ -381,16 +381,20 @@ func (h *sentPacketHandler) onPacketAcked(p *Packet) error {
}
}
}
// this also applies to packets that have been retransmitted as probe packets
if p.includedInBytesInFlight {
h.bytesInFlight -= p.Length
}
// TODO: this will need to change once we implement sending of probe packets
if p.canBeRetransmitted {
h.congestion.OnPacketAcked(p.PacketNumber, p.Length, h.bytesInFlight)
}
if h.rtoCount > 0 {
h.verifyRTO(p.PacketNumber)
}
if err := h.stopRetransmissionsFor(p); err != nil {
return err
}
h.rtoCount = 0
h.handshakeCount = 0
// TODO(#497): h.tlpCount = 0
return h.packetHistory.Remove(p.PacketNumber)
}
@@ -408,6 +412,18 @@ func (h *sentPacketHandler) stopRetransmissionsFor(p *Packet) error {
return nil
}
func (h *sentPacketHandler) verifyRTO(pn protocol.PacketNumber) {
if pn <= h.largestSentBeforeRTO {
h.logger.Debugf("Spurious RTO detected. Received an ACK for %#x (largest sent before RTO: %#x)", pn, h.largestSentBeforeRTO)
// Replace SRTT with latest_rtt and increase the variance to prevent
// a spurious RTO from happening again.
h.rttStats.ExpireSmoothedMetrics()
return
}
h.logger.Debugf("RTO verified. Received an ACK for %#x (largest sent before RTO: %#x", pn, h.largestSentBeforeRTO)
h.congestion.OnRetransmissionTimeout(true)
}
func (h *sentPacketHandler) DequeuePacketForRetransmission() *Packet {
if len(h.retransmissionQueue) == 0 {
return nil
@@ -469,18 +485,16 @@ func (h *sentPacketHandler) ShouldSendNumPackets() int {
// retransmit the oldest two packets
func (h *sentPacketHandler) queueRTOs() error {
h.largestSentBeforeRTO = h.lastSentPacketNumber
// Queue the first two outstanding packets for retransmission.
// This does NOT declare this packets as lost:
// They are still tracked in the packet history and count towards the bytes in flight.
// TODO: don't report them as lost to the congestion controller
for i := 0; i < 2; i++ {
if p := h.packetHistory.FirstOutstanding(); p != nil {
h.logger.Debugf("\tQueueing packet %#x for retransmission (RTO)", p.PacketNumber)
if err := h.queuePacketForRetransmission(p); err != nil {
return err
}
h.congestion.OnPacketLost(p.PacketNumber, p.Length, h.bytesInFlight)
h.congestion.OnRetransmissionTimeout(true)
}
}
return nil

87
internal/ackhandler/sent_packet_handler_test.go
View File

@@ -541,24 +541,62 @@ var _ = Describe("SentPacketHandler", func() {
Expect(err).NotTo(HaveOccurred())
})
It("should call MaybeExitSlowStart and OnPacketLost", func() {
It("doesn't call OnPacketLost and OnRetransmissionTimeout when queuing RTOs", func() {
cong.EXPECT().OnPacketSent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(3)
cong.EXPECT().TimeUntilSend(gomock.Any()).Times(3)
cong.EXPECT().OnRetransmissionTimeout(true).Times(2)
cong.EXPECT().OnPacketLost(
protocol.PacketNumber(1),
protocol.ByteCount(1),
protocol.ByteCount(3),
)
cong.EXPECT().OnPacketLost(
protocol.PacketNumber(2),
protocol.ByteCount(1),
protocol.ByteCount(3),
)
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 1}))
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 2}))
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 3}))
handler.OnAlarm() // RTO, meaning 2 lost packets
Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil())
Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil())
})
It("declares all lower packets lost and call OnRetransmissionTimeout when verifying an RTO", func() {
cong.EXPECT().OnPacketSent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(5)
cong.EXPECT().TimeUntilSend(gomock.Any()).Times(5)
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 1, SendTime: time.Now().Add(-time.Hour)}))
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 2, SendTime: time.Now().Add(-time.Hour)}))
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 3, SendTime: time.Now().Add(-time.Hour)}))
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 4, SendTime: time.Now().Add(-time.Hour)}))
handler.OnAlarm() // RTO, meaning 2 lost packets
Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil())
Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil())
// send one probe packet and receive an ACK for it
gomock.InOrder(
cong.EXPECT().MaybeExitSlowStart(),
cong.EXPECT().OnPacketAcked(protocol.PacketNumber(5), protocol.ByteCount(1), protocol.ByteCount(4)),
cong.EXPECT().OnRetransmissionTimeout(true),
cong.EXPECT().OnPacketLost(protocol.PacketNumber(1), protocol.ByteCount(1), protocol.ByteCount(3)),
cong.EXPECT().OnPacketLost(protocol.PacketNumber(2), protocol.ByteCount(1), protocol.ByteCount(2)),
cong.EXPECT().OnPacketLost(protocol.PacketNumber(3), protocol.ByteCount(1), protocol.ByteCount(1)),
cong.EXPECT().OnPacketLost(protocol.PacketNumber(4), protocol.ByteCount(1), protocol.ByteCount(0)),
)
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 5}))
err := handler.ReceivedAck(&wire.AckFrame{LargestAcked: 5, LowestAcked: 5}, 1, protocol.EncryptionForwardSecure, time.Now())
Expect(err).ToNot(HaveOccurred())
})
It("doesn't call OnRetransmissionTimeout when a spurious RTO occurs", func() {
cong.EXPECT().OnPacketSent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(3)
cong.EXPECT().TimeUntilSend(gomock.Any()).Times(3)
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 1, SendTime: time.Now().Add(-time.Hour)}))
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 2, SendTime: time.Now()}))
handler.OnAlarm() // RTO, meaning 2 lost packets
Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil())
Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil())
// send one probe packet
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 3}))
// receive an ACK for a packet send *before* the probe packet
// don't EXPECT any call to OnRetransmissionTimeout
gomock.InOrder(
cong.EXPECT().MaybeExitSlowStart(),
cong.EXPECT().OnPacketAcked(protocol.PacketNumber(2), protocol.ByteCount(1), protocol.ByteCount(2)),
cong.EXPECT().OnPacketLost(protocol.PacketNumber(1), protocol.ByteCount(1), protocol.ByteCount(1)),
)
err := handler.ReceivedAck(&wire.AckFrame{LargestAcked: 2, LowestAcked: 2}, 1, protocol.EncryptionForwardSecure, time.Now())
Expect(err).ToNot(HaveOccurred())
})
It("doesn't call OnPacketAcked when a retransmitted packet is acked", func() {
@@ -705,13 +743,34 @@ var _ = Describe("SentPacketHandler", func() {
Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil())
expectInPacketHistory([]protocol.PacketNumber{1, 2})
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(2)))
// send another packet and receive an ACK for it
// This will trigger ack-based loss detection, and declare 1 and 2 lost, and remove them from bytes in flight
// Send a probe packet and receive an ACK for it.
// This verifies the RTO.
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 3}))
err = handler.ReceivedAck(&wire.AckFrame{LargestAcked: 3, LowestAcked: 3}, 1, protocol.EncryptionForwardSecure, time.Now())
Expect(err).ToNot(HaveOccurred())
Expect(handler.packetHistory.Len()).To(BeZero())
Expect(handler.bytesInFlight).To(BeZero())
Expect(handler.retransmissionQueue).To(BeEmpty()) // 1 and 2 were already sent as probe packets
})
It("queues packets sent before the probe packet for retransmission", func() {
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 1, SendTime: time.Now().Add(-time.Hour)}))
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 2, SendTime: time.Now().Add(-time.Hour)}))
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 3, SendTime: time.Now().Add(-time.Hour)}))
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 4, SendTime: time.Now().Add(-time.Hour)}))
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 5, SendTime: time.Now().Add(-time.Hour)}))
handler.OnAlarm()
Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil())
Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil())
expectInPacketHistory([]protocol.PacketNumber{1, 2, 3, 4, 5})
// Send a probe packet and receive an ACK for it.
// This verifies the RTO.
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 6}))
err := handler.ReceivedAck(&wire.AckFrame{LargestAcked: 6, LowestAcked: 6}, 1, protocol.EncryptionForwardSecure, time.Now())
Expect(err).ToNot(HaveOccurred())
Expect(handler.packetHistory.Len()).To(BeZero())
Expect(handler.bytesInFlight).To(BeZero())
Expect(handler.retransmissionQueue).To(HaveLen(3)) // packets 3, 4, 5
})
It("handles ACKs for the original packet", func() {