accept duplicate and delayed packets

We used to reject duplicate and packets with packet numbers lower than
the LeastUnacked we received in a STOP_WAITING frame, because we didn't
accept overlapping stream data. For all other frames, duplicates never
were an issue. Now that we accept overlapping stream data, there's no
need to reject those packets, in fact, processing a delayed packet will
be beneficial for performance.
This commit is contained in:
Marten Seemann 2017-06-08 14:40:55 +02:00
parent 94843e1919
commit 14fa2dc7dd
No known key found for this signature in database
GPG key ID: 3603F40B121FCDEA
4 changed files with 20 additions and 53 deletions

View file

@ -8,13 +8,6 @@ import (
"github.com/lucas-clemente/quic-go/protocol"
)
var (
// ErrDuplicatePacket occurres when a duplicate packet is received
ErrDuplicatePacket = errors.New("ReceivedPacketHandler: Duplicate Packet")
// ErrPacketSmallerThanLastStopWaiting occurs when a packet arrives with a packet number smaller than the largest LeastUnacked of a StopWaitingFrame. If this error occurs, the packet should be ignored
ErrPacketSmallerThanLastStopWaiting = errors.New("ReceivedPacketHandler: Packet number smaller than highest StopWaiting")
)
var errInvalidPacketNumber = errors.New("ReceivedPacketHandler: Invalid packet number")
type receivedPacketHandler struct {
@ -52,19 +45,10 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe
return errInvalidPacketNumber
}
// if the packet number is smaller than the largest LeastUnacked value of a StopWaiting we received, we cannot detect if this packet has a duplicate number
// the packet has to be ignored anyway
if packetNumber <= h.ignorePacketsBelow {
return ErrPacketSmallerThanLastStopWaiting
}
if h.packetHistory.IsDuplicate(packetNumber) {
return ErrDuplicatePacket
}
err := h.packetHistory.ReceivedPacket(packetNumber)
if err != nil {
return err
if packetNumber > h.ignorePacketsBelow {
if err := h.packetHistory.ReceivedPacket(packetNumber); err != nil {
return err
}
}
if packetNumber > h.largestObserved {

View file

@ -40,24 +40,6 @@ var _ = Describe("receivedPacketHandler", func() {
Expect(err).To(MatchError(errInvalidPacketNumber))
})
It("rejects a duplicate package", func() {
for i := 1; i < 5; i++ {
err := handler.ReceivedPacket(protocol.PacketNumber(i), true)
Expect(err).ToNot(HaveOccurred())
}
err := handler.ReceivedPacket(4, true)
Expect(err).To(MatchError(ErrDuplicatePacket))
})
It("ignores a packet with PacketNumber less than the LeastUnacked of a previously received StopWaiting", func() {
err := handler.ReceivedPacket(5, true)
Expect(err).ToNot(HaveOccurred())
err = handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: 10})
Expect(err).ToNot(HaveOccurred())
err = handler.ReceivedPacket(9, true)
Expect(err).To(MatchError(ErrPacketSmallerThanLastStopWaiting))
})
It("does not ignore a packet with PacketNumber equal to LeastUnacked of a previously received StopWaiting", func() {
err := handler.ReceivedPacket(5, true)
Expect(err).ToNot(HaveOccurred())
@ -274,6 +256,19 @@ var _ = Describe("receivedPacketHandler", func() {
Expect(ack.AckRanges[1]).To(Equal(frames.AckRange{FirstPacketNumber: 1, LastPacketNumber: 1}))
})
It("doesn't add delayed packets to the packetHistory", func() {
err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(6)})
Expect(err).ToNot(HaveOccurred())
err = handler.ReceivedPacket(4, true)
Expect(err).ToNot(HaveOccurred())
err = handler.ReceivedPacket(10, true)
Expect(err).ToNot(HaveOccurred())
ack := handler.GetAckFrame()
Expect(ack).ToNot(BeNil())
Expect(ack.LargestAcked).To(Equal(protocol.PacketNumber(10)))
Expect(ack.LowestAcked).To(Equal(protocol.PacketNumber(10)))
})
It("deletes packets from the packetHistory after receiving a StopWaiting, after continuously received packets", func() {
for i := 1; i <= 12; i++ {
err := handler.ReceivedPacket(protocol.PacketNumber(i), true)

View file

@ -396,19 +396,7 @@ func (s *session) handlePacketImpl(p *receivedPacket) error {
// Only do this after decrypting, so we are sure the packet is not attacker-controlled
s.largestRcvdPacketNumber = utils.MaxPacketNumber(s.largestRcvdPacketNumber, hdr.PacketNumber)
err = s.receivedPacketHandler.ReceivedPacket(hdr.PacketNumber, packet.IsRetransmittable())
// ignore duplicate packets
if err == ackhandler.ErrDuplicatePacket {
utils.Infof("Ignoring packet 0x%x due to ErrDuplicatePacket", hdr.PacketNumber)
return nil
}
// ignore packets with packet numbers smaller than the LeastUnacked of a StopWaiting
if err == ackhandler.ErrPacketSmallerThanLastStopWaiting {
utils.Infof("Ignoring packet 0x%x due to ErrPacketSmallerThanLastStopWaiting", hdr.PacketNumber)
return nil
}
if err != nil {
if err = s.receivedPacketHandler.ReceivedPacket(hdr.PacketNumber, packet.IsRetransmittable()); err != nil {
return err
}

View file

@ -813,7 +813,7 @@ var _ = Describe("Session", func() {
Expect(sess.largestRcvdPacketNumber).To(Equal(protocol.PacketNumber(5)))
})
It("ignores duplicate packets", func() {
It("handles duplicate packets", func() {
hdr.PacketNumber = 5
err := sess.handlePacketImpl(&receivedPacket{publicHeader: hdr})
Expect(err).ToNot(HaveOccurred())
@ -821,7 +821,7 @@ var _ = Describe("Session", func() {
Expect(err).ToNot(HaveOccurred())
})
It("ignores packets smaller than the highest LeastUnacked of a StopWaiting", func() {
It("handles packets smaller than the highest LeastUnacked of a StopWaiting", func() {
err := sess.receivedPacketHandler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: 10})
Expect(err).ToNot(HaveOccurred())
hdr.PacketNumber = 5