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

Merge pull request #1273 from lucas-clemente/bytes-in-flight-fixes

Browse Source 
only delete the acked packet, but none its retransmissions
This commit is contained in:
Marten Seemann
2018-04-04 15:58:14 +07:00
committed by GitHub
parent 89e719f03b 42d35360bb
commit 0ad3c17506
5 changed files with 169 additions and 164 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
internal/ackhandler/packet.go
View File

@@ -18,7 +18,10 @@ type Packet struct {
largestAcked protocol.PacketNumber // if the packet contains an ACK, the LargestAcked value of that ACK
queuedForRetransmission bool
// There are two reasons why a packet cannot be retransmitted:
// * it was already retransmitted
// * this packet is a retransmission, and we already received an ACK for the original packet
canBeRetransmitted bool
includedInBytesInFlight bool
retransmittedAs []protocol.PacketNumber
isRetransmission bool // we need a separate bool here because 0 is a valid packet number

118
internal/ackhandler/sent_packet_handler.go
View File

@@ -144,10 +144,9 @@ func (h *sentPacketHandler) sentPacketImpl(packet *Packet) bool /* isRetransmitt
h.lastSentPacketNumber = packet.PacketNumber
var largestAcked protocol.PacketNumber
if len(packet.Frames) > 0 {
if ackFrame, ok := packet.Frames[0].(*wire.AckFrame); ok {
largestAcked = ackFrame.LargestAcked
packet.largestAcked = ackFrame.LargestAcked
}
}
@@ -155,10 +154,10 @@ func (h *sentPacketHandler) sentPacketImpl(packet *Packet) bool /* isRetransmitt
isRetransmittable := len(packet.Frames) != 0
if isRetransmittable {
packet.largestAcked = largestAcked
h.lastSentRetransmittablePacketTime = packet.SendTime
packet.includedInBytesInFlight = true
h.bytesInFlight += packet.Length
packet.canBeRetransmitted = true
}
h.congestion.OnPacketSent(packet.SendTime, h.bytesInFlight, packet.PacketNumber, packet.Length, isRetransmittable)
@@ -205,9 +204,6 @@ func (h *sentPacketHandler) ReceivedAck(ackFrame *wire.AckFrame, withPacketNumbe
if err := h.onPacketAcked(p); err != nil {
return err
}
if len(p.retransmittedAs) == 0 {
h.congestion.OnPacketAcked(p.PacketNumber, p.Length, h.bytesInFlight)
}
}
if err := h.detectLostPackets(rcvTime); err != nil {
@@ -298,9 +294,6 @@ func (h *sentPacketHandler) detectLostPackets(now time.Time) error {
if packet.PacketNumber > h.largestAcked {
return false, nil
}
if packet.queuedForRetransmission { // don't retransmit packets twice
return true, nil
}
timeSinceSent := now.Sub(packet.SendTime)
if timeSinceSent > delayUntilLost {
@@ -313,12 +306,19 @@ func (h *sentPacketHandler) detectLostPackets(now time.Time) error {
})
for _, p := range lostPackets {
if err := h.queuePacketForRetransmission(p); err != nil {
return err
// the bytes in flight need to be reduced no matter if this packet will be retransmitted
if p.includedInBytesInFlight {
h.bytesInFlight -= p.Length
}
h.bytesInFlight -= p.Length
p.includedInBytesInFlight = false
h.congestion.OnPacketLost(p.PacketNumber, p.Length, h.bytesInFlight)
if p.canBeRetransmitted {
// queue the packet for retransmission, and report the loss to the congestion controller
h.logger.Debugf("\tQueueing packet %#x because it was detected lost", p.PacketNumber)
if err := h.queuePacketForRetransmission(p); err != nil {
return err
}
h.congestion.OnPacketLost(p.PacketNumber, p.Length, h.bytesInFlight)
}
h.packetHistory.Remove(p.PacketNumber)
}
return nil
}
@@ -361,55 +361,53 @@ func (h *sentPacketHandler) onPacketAcked(p *Packet) error {
h.handshakeCount = 0
// TODO(#497): h.tlpCount = 0
// find the first packet, from which on we can delete all retransmissions
// example: packet 10 was retransmitted as packet 11 and 12, and
// packet 12 was then retransmitted as 13.
// When receiving an ACK for packet 13, we can remove packets 12 and 13,
// but still need to keep 10 and 11.
first := p
for first.isRetransmission {
previous := h.packetHistory.GetPacket(first.retransmissionOf)
if previous == nil {
return fmt.Errorf("sent packet handler BUG: retransmitted packet for %d not found (should have been %d)", first.PacketNumber, first.retransmissionOf)
}
// if the retransmission of a packet was split, we can't remove it yet
if len(previous.retransmittedAs) > 1 {
break
}
first = previous
}
if first.isRetransmission {
root := h.packetHistory.GetPacket(first.retransmissionOf)
retransmittedAs := make([]protocol.PacketNumber, 0, len(root.retransmittedAs)-1)
for _, pn := range root.retransmittedAs {
if pn != first.PacketNumber {
retransmittedAs = append(retransmittedAs, pn)
// only report the acking of this packet to the congestion controller if:
// * it is a retransmittable packet
// * this packet wasn't retransmitted yet
if p.isRetransmission {
// that the parent doesn't exist is expected to happen every time the original packet was already acked
if parent := h.packetHistory.GetPacket(p.retransmissionOf); parent != nil {
if len(parent.retransmittedAs) == 1 {
parent.retransmittedAs = nil
} else {
// remove this packet from the slice of retransmission
retransmittedAs := make([]protocol.PacketNumber, 0, len(parent.retransmittedAs)-1)
for _, pn := range parent.retransmittedAs {
if pn != p.PacketNumber {
retransmittedAs = append(retransmittedAs, pn)
}
}
parent.retransmittedAs = retransmittedAs
}
}
root.retransmittedAs = retransmittedAs
}
return h.removeAllRetransmissions(first)
}
func (h *sentPacketHandler) removeAllRetransmissions(p *Packet) error {
if p.includedInBytesInFlight {
h.bytesInFlight -= p.Length
p.includedInBytesInFlight = false
}
if p.queuedForRetransmission {
for _, r := range p.retransmittedAs {
packet := h.packetHistory.GetPacket(r)
if packet == nil {
return fmt.Errorf("sent packet handler BUG: removing packet %d (retransmission of %d) not found in history", r, p.PacketNumber)
}
if err := h.removeAllRetransmissions(packet); err != nil {
return err
}
}
// 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 err := h.stopRetransmissionsFor(p); err != nil {
return err
}
return h.packetHistory.Remove(p.PacketNumber)
}
func (h *sentPacketHandler) stopRetransmissionsFor(p *Packet) error {
if err := h.packetHistory.MarkCannotBeRetransmitted(p.PacketNumber); err != nil {
return err
}
for _, r := range p.retransmittedAs {
packet := h.packetHistory.GetPacket(r)
if packet == nil {
return fmt.Errorf("sent packet handler BUG: marking packet as not retransmittable %d (retransmission of %d) not found in history", r, p.PacketNumber)
}
h.stopRetransmissionsFor(packet)
}
return nil
}
func (h *sentPacketHandler) DequeuePacketForRetransmission() *Packet {
if len(h.retransmissionQueue) == 0 {
return nil
@@ -471,9 +469,13 @@ func (h *sentPacketHandler) ShouldSendNumPackets() int {
// retransmit the oldest two packets
func (h *sentPacketHandler) queueRTOs() error {
// 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), %d outstanding", p.PacketNumber, h.packetHistory.Len())
h.logger.Debugf("\tQueueing packet %#x for retransmission (RTO)", p.PacketNumber)
if err := h.queuePacketForRetransmission(p); err != nil {
return err
}
@@ -487,12 +489,13 @@ func (h *sentPacketHandler) queueRTOs() error {
func (h *sentPacketHandler) queueHandshakePacketsForRetransmission() error {
var handshakePackets []*Packet
h.packetHistory.Iterate(func(p *Packet) (bool, error) {
if !p.queuedForRetransmission && p.EncryptionLevel < protocol.EncryptionForwardSecure {
if p.canBeRetransmitted && p.EncryptionLevel < protocol.EncryptionForwardSecure {
handshakePackets = append(handshakePackets, p)
}
return true, nil
})
for _, p := range handshakePackets {
h.logger.Debugf("\tQueueing packet %#x as a handshake retransmission", p.PacketNumber)
if err := h.queuePacketForRetransmission(p); err != nil {
return err
}
@@ -501,7 +504,10 @@ func (h *sentPacketHandler) queueHandshakePacketsForRetransmission() error {
}
func (h *sentPacketHandler) queuePacketForRetransmission(p *Packet) error {
if _, err := h.packetHistory.QueuePacketForRetransmission(p.PacketNumber); err != nil {
if !p.canBeRetransmitted {
return fmt.Errorf("sent packet handler BUG: packet %d already queued for retransmission", p.PacketNumber)
}
if err := h.packetHistory.MarkCannotBeRetransmitted(p.PacketNumber); err != nil {
return err
}
h.retransmissionQueue = append(h.retransmissionQueue, p)

159
internal/ackhandler/sent_packet_handler_test.go
View File

@@ -77,6 +77,19 @@ var _ = Describe("SentPacketHandler", func() {
return nil
}
losePacket := func(pn protocol.PacketNumber) {
p := getPacket(pn)
ExpectWithOffset(1, p).ToNot(BeNil())
handler.queuePacketForRetransmission(p)
if p.includedInBytesInFlight {
p.includedInBytesInFlight = false
handler.bytesInFlight -= p.Length
}
r := handler.DequeuePacketForRetransmission()
ExpectWithOffset(1, r).ToNot(BeNil())
ExpectWithOffset(1, r.PacketNumber).To(Equal(pn))
}
expectInPacketHistory := func(expected []protocol.PacketNumber) {
ExpectWithOffset(1, handler.packetHistory.Len()).To(Equal(len(expected)))
for _, p := range expected {
@@ -120,6 +133,7 @@ var _ = Describe("SentPacketHandler", func() {
handler.SentPacket(nonRetransmittablePacket(&Packet{PacketNumber: 1}))
Expect(handler.packetHistory.Len()).To(BeZero())
Expect(handler.lastSentRetransmittablePacketTime).To(BeZero())
Expect(handler.bytesInFlight).To(BeZero())
})
Context("skipped packet numbers", func() {
@@ -268,12 +282,13 @@ var _ = Describe("SentPacketHandler", func() {
})
Context("acks and nacks the right packets", func() {
It("adjusts the LargestAcked", func() {
It("adjusts the LargestAcked, and adjusts the bytes in flight", func() {
ack := createAck([]wire.AckRange{{First: 0, Last: 5}})
err := handler.ReceivedAck(ack, 1, protocol.EncryptionForwardSecure, time.Now())
Expect(err).ToNot(HaveOccurred())
Expect(handler.largestAcked).To(Equal(protocol.PacketNumber(5)))
expectInPacketHistory([]protocol.PacketNumber{6, 7, 8, 9})
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(4)))
})
It("acks packet 0", func() {
@@ -423,19 +438,6 @@ var _ = Describe("SentPacketHandler", func() {
})
Context("ACK processing, for retransmitted packets", func() {
losePacket := func(pn protocol.PacketNumber) {
p := getPacket(pn)
ExpectWithOffset(1, p).ToNot(BeNil())
handler.queuePacketForRetransmission(p)
if p.includedInBytesInFlight {
p.includedInBytesInFlight = false
handler.bytesInFlight -= p.Length
}
r := handler.DequeuePacketForRetransmission()
ExpectWithOffset(1, r).ToNot(BeNil())
ExpectWithOffset(1, r.PacketNumber).To(Equal(pn))
}
It("sends a packet as retransmission", func() {
// packet 5 was retransmitted as packet 6
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 5, Length: 10}))
@@ -446,66 +448,17 @@ var _ = Describe("SentPacketHandler", func() {
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(11)))
})
It("removes all retransmissions when the original packet is acked", func() {
// packet 5 was retransmitted as packet 6, which was then retransmitted as packet 8
It("removes a packet when it is acked", func() {
// packet 5 was retransmitted as packet 6
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 5, Length: 10}))
losePacket(5)
handler.SentPacketsAsRetransmission([]*Packet{retransmittablePacket(&Packet{PacketNumber: 6, Length: 11})}, 5)
losePacket(6)
handler.SentPacketsAsRetransmission([]*Packet{retransmittablePacket(&Packet{PacketNumber: 8, Length: 12})}, 6)
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(12)))
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(11)))
// ack 5
err := handler.ReceivedAck(&wire.AckFrame{LowestAcked: 5, LargestAcked: 5}, 1, protocol.EncryptionForwardSecure, time.Now())
Expect(err).ToNot(HaveOccurred())
Expect(handler.packetHistory.Len()).To(BeZero())
Expect(handler.bytesInFlight).To(BeZero())
})
It("removes all retransmissions when the retransmission is acked", func() {
// the retransmission for packet 5 was split into packets 6 and 8
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 5, Length: 10}))
losePacket(5)
handler.SentPacketsAsRetransmission([]*Packet{retransmittablePacket(&Packet{PacketNumber: 6, Length: 11})}, 5)
losePacket(6)
handler.SentPacketsAsRetransmission([]*Packet{retransmittablePacket(&Packet{PacketNumber: 8, Length: 12})}, 6)
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(12)))
// ack 8
err := handler.ReceivedAck(&wire.AckFrame{LowestAcked: 8, LargestAcked: 8}, 1, protocol.EncryptionForwardSecure, time.Now())
Expect(err).ToNot(HaveOccurred())
Expect(handler.packetHistory.Len()).To(BeZero())
Expect(handler.bytesInFlight).To(BeZero())
})
It("removes split retransmissions when the original packet is acked", func() {
// the retransmission for packet 5 was split into 8 and 9
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 5, Length: 10}))
losePacket(5)
handler.SentPacketsAsRetransmission([]*Packet{
retransmittablePacket(&Packet{PacketNumber: 8, Length: 6}),
retransmittablePacket(&Packet{PacketNumber: 9, Length: 7}),
}, 5)
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(6 + 7)))
// ack 5
err := handler.ReceivedAck(&wire.AckFrame{LowestAcked: 5, LargestAcked: 5}, 1, protocol.EncryptionForwardSecure, time.Now())
Expect(err).ToNot(HaveOccurred())
Expect(handler.packetHistory.Len()).To(BeZero())
Expect(handler.bytesInFlight).To(BeZero())
})
It("doesn't remove the original packet if a split retransmission is acked", func() {
// the retransmission for packet 5 was split into 10 and 12
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 5, Length: 10}))
losePacket(5)
handler.SentPacketsAsRetransmission([]*Packet{
retransmittablePacket(&Packet{PacketNumber: 10, Length: 6}),
retransmittablePacket(&Packet{PacketNumber: 12, Length: 7})},
5)
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(6 + 7)))
// ack 10
err := handler.ReceivedAck(&wire.AckFrame{LowestAcked: 10, LargestAcked: 10}, 1, protocol.EncryptionForwardSecure, time.Now())
Expect(err).ToNot(HaveOccurred())
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(7)))
expectInPacketHistory([]protocol.PacketNumber{5, 12})
expectInPacketHistory([]protocol.PacketNumber{6})
Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(11)))
})
It("handles ACKs that ack the original packet as well as the retransmission", func() {
@@ -608,6 +561,24 @@ var _ = Describe("SentPacketHandler", func() {
handler.OnAlarm() // RTO, meaning 2 lost packets
})
It("doesn't call OnPacketAcked when a retransmitted packet is acked", func() {
cong.EXPECT().OnPacketSent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(2)
cong.EXPECT().TimeUntilSend(gomock.Any()).Times(2)
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 1, SendTime: time.Now().Add(-time.Hour)}))
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 2}))
// lose packet 1
gomock.InOrder(
cong.EXPECT().MaybeExitSlowStart(),
cong.EXPECT().OnPacketAcked(protocol.PacketNumber(2), protocol.ByteCount(1), protocol.ByteCount(1)),
cong.EXPECT().OnPacketLost(protocol.PacketNumber(1), protocol.ByteCount(1), protocol.ByteCount(0)),
)
err := handler.ReceivedAck(&wire.AckFrame{LargestAcked: 2, LowestAcked: 2}, 1, protocol.EncryptionForwardSecure, time.Now())
Expect(err).ToNot(HaveOccurred())
// don't EXPECT any further calls to the congestion controller
err = handler.ReceivedAck(&wire.AckFrame{LargestAcked: 2, LowestAcked: 1}, 2, protocol.EncryptionForwardSecure, time.Now())
Expect(err).ToNot(HaveOccurred())
})
It("only allows sending of ACKs when congestion limited", func() {
handler.bytesInFlight = 100
cong.EXPECT().GetCongestionWindow().Return(protocol.ByteCount(200))
@@ -711,7 +682,8 @@ var _ = Describe("SentPacketHandler", func() {
Expect(handler.lossTime.IsZero()).To(BeTrue())
Expect(time.Until(handler.GetAlarmTimeout())).To(BeNumerically("~", handler.computeRTOTimeout(), time.Minute))
handler.OnAlarm()
err := handler.OnAlarm()
Expect(err).ToNot(HaveOccurred())
p := handler.DequeuePacketForRetransmission()
Expect(p).ToNot(BeNil())
Expect(p.PacketNumber).To(Equal(protocol.PacketNumber(1)))
@@ -722,6 +694,47 @@ var _ = Describe("SentPacketHandler", func() {
Expect(handler.rtoCount).To(BeEquivalentTo(1))
})
It("doesn't delete packets transmitted as RTO from the history", 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.rttStats.UpdateRTT(time.Second, 0, time.Now())
err := handler.OnAlarm()
Expect(err).ToNot(HaveOccurred())
Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil())
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
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())
})
It("handles ACKs for the original packet", func() {
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 5, SendTime: time.Now().Add(-time.Hour)}))
handler.rttStats.UpdateRTT(time.Second, 0, time.Now())
err := handler.OnAlarm()
Expect(err).ToNot(HaveOccurred())
Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil())
handler.SentPacketsAsRetransmission([]*Packet{retransmittablePacket(&Packet{PacketNumber: 6})}, 5)
err = handler.ReceivedAck(&wire.AckFrame{LargestAcked: 5, LowestAcked: 5}, 1, protocol.EncryptionForwardSecure, time.Now())
Expect(err).ToNot(HaveOccurred())
err = handler.OnAlarm()
Expect(err).ToNot(HaveOccurred())
})
It("handles ACKs for the original packet", func() {
handler.SentPacket(retransmittablePacket(&Packet{PacketNumber: 5, SendTime: time.Now().Add(-time.Hour)}))
handler.rttStats.UpdateRTT(time.Second, 0, time.Now())
err := handler.OnAlarm()
Expect(err).ToNot(HaveOccurred())
err = handler.OnAlarm()
Expect(err).ToNot(HaveOccurred())
})
})
Context("Delay-based loss detection", func() {
@@ -734,8 +747,10 @@ var _ = Describe("SentPacketHandler", func() {
err := handler.ReceivedAck(&wire.AckFrame{LargestAcked: 2, LowestAcked: 2}, 1, protocol.EncryptionForwardSecure, now)
Expect(err).NotTo(HaveOccurred())
Expect(handler.DequeuePacketForRetransmission()).ToNot(BeNil())
Expect(handler.DequeuePacketForRetransmission()).To(BeNil())
// no need to set an alarm, since packet 1 was already declared lost
Expect(handler.lossTime.IsZero()).To(BeTrue())
Expect(handler.bytesInFlight).To(BeZero())
})
It("sets the early retransmit alarm", func() {
@@ -754,7 +769,8 @@ var _ = Describe("SentPacketHandler", func() {
Expect(handler.lossTime.Sub(getPacket(1).SendTime)).To(Equal(time.Second * 9 / 8))
// Expect(time.Until(handler.GetAlarmTimeout())).To(BeNumerically("~", time.Hour*9/8, time.Minute))
handler.OnAlarm()
err = handler.OnAlarm()
Expect(err).ToNot(HaveOccurred())
Expect(handler.DequeuePacketForRetransmission()).NotTo(BeNil())
// make sure this is not an RTO: only packet 1 is retransmissted
Expect(handler.DequeuePacketForRetransmission()).To(BeNil())
@@ -784,7 +800,8 @@ var _ = Describe("SentPacketHandler", func() {
Expect(handler.lossTime.IsZero()).To(BeTrue())
Expect(handler.GetAlarmTimeout().Sub(lastHandshakePacketSendTime)).To(Equal(2 * time.Minute))
handler.OnAlarm()
err = handler.OnAlarm()
Expect(err).ToNot(HaveOccurred())
p := handler.DequeuePacketForRetransmission()
Expect(p).ToNot(BeNil())
Expect(p.PacketNumber).To(Equal(protocol.PacketNumber(2)))

13
internal/ackhandler/sent_packet_history.go
View File

@@ -87,26 +87,23 @@ func (h *sentPacketHistory) FirstOutstanding() *Packet {
// QueuePacketForRetransmission marks a packet for retransmission.
// A packet can only be queued once.
func (h *sentPacketHistory) QueuePacketForRetransmission(pn protocol.PacketNumber) (*Packet, error) {
func (h *sentPacketHistory) MarkCannotBeRetransmitted(pn protocol.PacketNumber) error {
el, ok := h.packetMap[pn]
if !ok {
return nil, fmt.Errorf("sent packet history: packet %d not found", pn)
return fmt.Errorf("sent packet history: packet %d not found", pn)
}
if el.Value.queuedForRetransmission {
return nil, fmt.Errorf("sent packet history BUG: packet %d already queued for retransmission", pn)
}
el.Value.queuedForRetransmission = true
el.Value.canBeRetransmitted = false
if el == h.firstOutstanding {
h.readjustFirstOutstanding()
}
return &el.Value, nil
return nil
}
// readjustFirstOutstanding readjusts the pointer to the first outstanding packet.
// This is necessary every time the first outstanding packet is deleted or retransmitted.
func (h *sentPacketHistory) readjustFirstOutstanding() {
el := h.firstOutstanding.Next()
for el != nil && el.Value.queuedForRetransmission {
for el != nil && !el.Value.canBeRetransmitted {
el = el.Next()
}
h.firstOutstanding = el

38
internal/ackhandler/sent_packet_history_test.go
View File

@@ -55,15 +55,15 @@ var _ = Describe("SentPacketHistory", func() {
})
It("gets the second packet if the first one is retransmitted", func() {
hist.SentPacket(&Packet{PacketNumber: 1})
hist.SentPacket(&Packet{PacketNumber: 3})
hist.SentPacket(&Packet{PacketNumber: 4})
hist.SentPacket(&Packet{PacketNumber: 1, canBeRetransmitted: true})
hist.SentPacket(&Packet{PacketNumber: 3, canBeRetransmitted: true})
hist.SentPacket(&Packet{PacketNumber: 4, canBeRetransmitted: true})
front := hist.FirstOutstanding()
Expect(front).ToNot(BeNil())
Expect(front.PacketNumber).To(Equal(protocol.PacketNumber(1)))
// Queue the first packet for retransmission.
// The first outstanding packet should now be 3.
_, err := hist.QueuePacketForRetransmission(1)
err := hist.MarkCannotBeRetransmitted(1)
Expect(err).ToNot(HaveOccurred())
front = hist.FirstOutstanding()
Expect(front).ToNot(BeNil())
@@ -71,22 +71,22 @@ var _ = Describe("SentPacketHistory", func() {
})
It("gets the third packet if the first two are retransmitted", func() {
hist.SentPacket(&Packet{PacketNumber: 1})
hist.SentPacket(&Packet{PacketNumber: 3})
hist.SentPacket(&Packet{PacketNumber: 4})
hist.SentPacket(&Packet{PacketNumber: 1, canBeRetransmitted: true})
hist.SentPacket(&Packet{PacketNumber: 3, canBeRetransmitted: true})
hist.SentPacket(&Packet{PacketNumber: 4, canBeRetransmitted: true})
front := hist.FirstOutstanding()
Expect(front).ToNot(BeNil())
Expect(front.PacketNumber).To(Equal(protocol.PacketNumber(1)))
// Queue the second packet for retransmission.
// The first outstanding packet should still be 3.
_, err := hist.QueuePacketForRetransmission(3)
err := hist.MarkCannotBeRetransmitted(3)
Expect(err).ToNot(HaveOccurred())
front = hist.FirstOutstanding()
Expect(front).ToNot(BeNil())
Expect(front.PacketNumber).To(Equal(protocol.PacketNumber(1)))
// Queue the first packet for retransmission.
// The first outstanding packet should still be 4.
_, err = hist.QueuePacketForRetransmission(1)
err = hist.MarkCannotBeRetransmitted(1)
Expect(err).ToNot(HaveOccurred())
front = hist.FirstOutstanding()
Expect(front).ToNot(BeNil())
@@ -168,26 +168,8 @@ var _ = Describe("SentPacketHistory", func() {
}
})
It("gets packets for retransmission", func() {
p, err := hist.QueuePacketForRetransmission(3)
Expect(err).ToNot(HaveOccurred())
Expect(p.PacketNumber).To(Equal(protocol.PacketNumber(3)))
Expect(p.queuedForRetransmission).To(BeTrue())
Expect(p.retransmissionOf).To(BeZero())
Expect(p.isRetransmission).To(BeFalse())
Expect(p.retransmittedAs).To(BeNil())
})
It("errors if the packet was already queued for retransmission", func() {
p, err := hist.QueuePacketForRetransmission(5)
Expect(err).ToNot(HaveOccurred())
Expect(p).ToNot(BeNil())
_, err = hist.QueuePacketForRetransmission(5)
Expect(err).To(MatchError("sent packet history BUG: packet 5 already queued for retransmission"))
})
It("errors if the packet doesn't exist", func() {
_, err := hist.QueuePacketForRetransmission(100)
err := hist.MarkCannotBeRetransmitted(100)
Expect(err).To(MatchError("sent packet history: packet 100 not found"))
})