From 805c21cb773843d9fc3ddd2a48f7b1f4aad4633f Mon Sep 17 00:00:00 2001 From: Lucas Clemente Date: Wed, 10 Aug 2016 13:29:30 +0200 Subject: [PATCH] use LeastUnacked in packet number derivation fixes #271 --- ackhandler/interfaces.go | 2 +- ackhandler/sent_packet_handler.go | 4 ++-- ackhandler/sent_packet_handler_test.go | 6 +++--- ackhandlerlegacy/interfaces.go | 2 +- ackhandlerlegacy/sent_packet_handler.go | 4 ++-- packet_packer.go | 12 ++++++------ protocol/packet_number.go | 4 ++-- protocol/packet_number_test.go | 24 ++++++++++++------------ session.go | 4 ++-- session_test.go | 2 +- 10 files changed, 32 insertions(+), 32 deletions(-) diff --git a/ackhandler/interfaces.go b/ackhandler/interfaces.go index 414c4787..d478838e 100644 --- a/ackhandler/interfaces.go +++ b/ackhandler/interfaces.go @@ -19,7 +19,7 @@ type SentPacketHandler interface { DequeuePacketForRetransmission() (packet *ackhandlerlegacy.Packet) BytesInFlight() protocol.ByteCount - GetLargestAcked() protocol.PacketNumber + GetLeastUnacked() protocol.PacketNumber CongestionAllowsSending() bool CheckForError() error diff --git a/ackhandler/sent_packet_handler.go b/ackhandler/sent_packet_handler.go index e8329c75..453341e1 100644 --- a/ackhandler/sent_packet_handler.go +++ b/ackhandler/sent_packet_handler.go @@ -273,8 +273,8 @@ func (h *sentPacketHandler) BytesInFlight() protocol.ByteCount { return h.bytesInFlight } -func (h *sentPacketHandler) GetLargestAcked() protocol.PacketNumber { - return h.LargestAcked +func (h *sentPacketHandler) GetLeastUnacked() protocol.PacketNumber { + return h.LargestInOrderAcked + 1 } func (h *sentPacketHandler) GetStopWaitingFrame() *frames.StopWaitingFrame { diff --git a/ackhandler/sent_packet_handler_test.go b/ackhandler/sent_packet_handler_test.go index 322a44c2..d5f52eb5 100644 --- a/ackhandler/sent_packet_handler_test.go +++ b/ackhandler/sent_packet_handler_test.go @@ -74,9 +74,9 @@ var _ = Describe("SentPacketHandler", func() { return nil } - It("gets the LargestAcked packet number", func() { - handler.LargestAcked = 0x1337 - Expect(handler.GetLargestAcked()).To(Equal(protocol.PacketNumber(0x1337))) + It("gets the LeastUnacked packet number", func() { + handler.LargestInOrderAcked = 0x1337 + Expect(handler.GetLeastUnacked()).To(Equal(protocol.PacketNumber(0x1337 + 1))) }) Context("registering sent packets", func() { diff --git a/ackhandlerlegacy/interfaces.go b/ackhandlerlegacy/interfaces.go index 0c7253a2..51299b7d 100644 --- a/ackhandlerlegacy/interfaces.go +++ b/ackhandlerlegacy/interfaces.go @@ -18,7 +18,7 @@ type SentPacketHandler interface { DequeuePacketForRetransmission() (packet *Packet) BytesInFlight() protocol.ByteCount - GetLargestAcked() protocol.PacketNumber + GetLeastUnacked() protocol.PacketNumber CongestionAllowsSending() bool CheckForError() error diff --git a/ackhandlerlegacy/sent_packet_handler.go b/ackhandlerlegacy/sent_packet_handler.go index d609ec85..8ea5efe0 100644 --- a/ackhandlerlegacy/sent_packet_handler.go +++ b/ackhandlerlegacy/sent_packet_handler.go @@ -308,8 +308,8 @@ func (h *sentPacketHandler) BytesInFlight() protocol.ByteCount { return h.bytesInFlight } -func (h *sentPacketHandler) GetLargestAcked() protocol.PacketNumber { - return h.LargestObserved +func (h *sentPacketHandler) GetLeastUnacked() protocol.PacketNumber { + return h.highestInOrderAckedPacketNumber + 1 } func (h *sentPacketHandler) GetStopWaitingFrame() *frames.StopWaitingFrame { diff --git a/packet_packer.go b/packet_packer.go index 63d70fc7..cba74a2b 100644 --- a/packet_packer.go +++ b/packet_packer.go @@ -40,15 +40,15 @@ func newPacketPacker(connectionID protocol.ConnectionID, cryptoSetup *handshake. } } -func (p *packetPacker) PackConnectionClose(frame *frames.ConnectionCloseFrame, largestObserved protocol.PacketNumber) (*packedPacket, error) { - return p.packPacket(nil, []frames.Frame{frame}, largestObserved, true, false) +func (p *packetPacker) PackConnectionClose(frame *frames.ConnectionCloseFrame, leastUnacked protocol.PacketNumber) (*packedPacket, error) { + return p.packPacket(nil, []frames.Frame{frame}, leastUnacked, true, false) } -func (p *packetPacker) PackPacket(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame, largestObserved protocol.PacketNumber, maySendOnlyAck bool) (*packedPacket, error) { - return p.packPacket(stopWaitingFrame, controlFrames, largestObserved, false, maySendOnlyAck) +func (p *packetPacker) PackPacket(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame, leastUnacked protocol.PacketNumber, maySendOnlyAck bool) (*packedPacket, error) { + return p.packPacket(stopWaitingFrame, controlFrames, leastUnacked, false, maySendOnlyAck) } -func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame, largestObserved protocol.PacketNumber, onlySendOneControlFrame, maySendOnlyAck bool) (*packedPacket, error) { +func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame, leastUnacked protocol.PacketNumber, onlySendOneControlFrame, maySendOnlyAck bool) (*packedPacket, error) { if len(controlFrames) > 0 { p.controlFrames = append(p.controlFrames, controlFrames...) } @@ -60,7 +60,7 @@ func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, con p.cryptoSetup.LockForSealing() defer p.cryptoSetup.UnlockForSealing() - packetNumberLen := protocol.GetPacketNumberLengthForPublicHeader(currentPacketNumber, largestObserved) + packetNumberLen := protocol.GetPacketNumberLengthForPublicHeader(currentPacketNumber, leastUnacked) responsePublicHeader := &PublicHeader{ ConnectionID: p.connectionID, PacketNumber: currentPacketNumber, diff --git a/protocol/packet_number.go b/protocol/packet_number.go index fb9e5588..3900464d 100644 --- a/protocol/packet_number.go +++ b/protocol/packet_number.go @@ -28,8 +28,8 @@ func delta(a, b PacketNumber) PacketNumber { } // GetPacketNumberLengthForPublicHeader gets the length of the packet number for the public header -func GetPacketNumberLengthForPublicHeader(packetNumber PacketNumber, highestAckedPacketNumber PacketNumber) PacketNumberLen { - diff := uint64(packetNumber - highestAckedPacketNumber) +func GetPacketNumberLengthForPublicHeader(packetNumber PacketNumber, leastUnacked PacketNumber) PacketNumberLen { + diff := uint64(packetNumber - leastUnacked) if diff < (2 << (uint8(PacketNumberLen1)*8 - 2)) { return PacketNumberLen1 } diff --git a/protocol/packet_number_test.go b/protocol/packet_number_test.go index 88539b94..f179504f 100644 --- a/protocol/packet_number_test.go +++ b/protocol/packet_number_test.go @@ -148,11 +148,11 @@ var _ = Describe("packet number calculation", func() { It("works for small packet numbers", func() { for i := uint64(1); i < 10000; i++ { packetNumber := PacketNumber(i) - highestAcked := PacketNumber(1) - length := GetPacketNumberLengthForPublicHeader(packetNumber, highestAcked) + leastUnacked := PacketNumber(1) + length := GetPacketNumberLengthForPublicHeader(packetNumber, leastUnacked) wirePacketNumber := (uint64(packetNumber) << (64 - length*8)) >> (64 - length*8) - inferedPacketNumber := InferPacketNumber(length, highestAcked, PacketNumber(wirePacketNumber)) + inferedPacketNumber := InferPacketNumber(length, leastUnacked, PacketNumber(wirePacketNumber)) Expect(inferedPacketNumber).To(Equal(packetNumber)) } }) @@ -160,11 +160,11 @@ var _ = Describe("packet number calculation", func() { It("works for small packet numbers and increasing ACKed packets", func() { for i := uint64(1); i < 10000; i++ { packetNumber := PacketNumber(i) - highestAcked := PacketNumber(i / 2) - length := GetPacketNumberLengthForPublicHeader(packetNumber, highestAcked) + leastUnacked := PacketNumber(i / 2) + length := GetPacketNumberLengthForPublicHeader(packetNumber, leastUnacked) wirePacketNumber := (uint64(packetNumber) << (64 - length*8)) >> (64 - length*8) - inferedPacketNumber := InferPacketNumber(length, highestAcked, PacketNumber(wirePacketNumber)) + inferedPacketNumber := InferPacketNumber(length, leastUnacked, PacketNumber(wirePacketNumber)) Expect(inferedPacketNumber).To(Equal(packetNumber)) } }) @@ -173,11 +173,11 @@ var _ = Describe("packet number calculation", func() { increment := uint64(1 << (8 - 3)) for i := uint64(1); i < (2 << 46); i += increment { packetNumber := PacketNumber(i) - highestAcked := PacketNumber(1) - length := GetPacketNumberLengthForPublicHeader(packetNumber, highestAcked) + leastUnacked := PacketNumber(1) + length := GetPacketNumberLengthForPublicHeader(packetNumber, leastUnacked) wirePacketNumber := (uint64(packetNumber) << (64 - length*8)) >> (64 - length*8) - inferedPacketNumber := InferPacketNumber(length, highestAcked, PacketNumber(wirePacketNumber)) + inferedPacketNumber := InferPacketNumber(length, leastUnacked, PacketNumber(wirePacketNumber)) Expect(inferedPacketNumber).To(Equal(packetNumber)) switch length { @@ -194,11 +194,11 @@ var _ = Describe("packet number calculation", func() { It("works for packet numbers larger than 2^48", func() { for i := (uint64(1) << 48); i < ((uint64(1) << 63) - 1); i += (uint64(1) << 48) { packetNumber := PacketNumber(i) - highestAcked := PacketNumber(i - 1000) - length := GetPacketNumberLengthForPublicHeader(packetNumber, highestAcked) + leastUnacked := PacketNumber(i - 1000) + length := GetPacketNumberLengthForPublicHeader(packetNumber, leastUnacked) wirePacketNumber := (uint64(packetNumber) << (64 - length*8)) >> (64 - length*8) - inferedPacketNumber := InferPacketNumber(length, highestAcked, PacketNumber(wirePacketNumber)) + inferedPacketNumber := InferPacketNumber(length, leastUnacked, PacketNumber(wirePacketNumber)) Expect(inferedPacketNumber).To(Equal(packetNumber)) } }) diff --git a/session.go b/session.go index 1c0c213d..5692e6a7 100644 --- a/session.go +++ b/session.go @@ -492,7 +492,7 @@ func (s *Session) sendPacket() error { stopWaitingFrame = s.sentPacketHandler.GetStopWaitingFrame() } } - packet, err := s.packer.PackPacket(stopWaitingFrame, controlFrames, s.sentPacketHandler.GetLargestAcked(), maySendOnlyAck) + packet, err := s.packer.PackPacket(stopWaitingFrame, controlFrames, s.sentPacketHandler.GetLeastUnacked(), maySendOnlyAck) if err != nil { return err } @@ -535,7 +535,7 @@ func (s *Session) sendPacket() error { } func (s *Session) sendConnectionClose(quicErr *qerr.QuicError) error { - packet, err := s.packer.PackConnectionClose(&frames.ConnectionCloseFrame{ErrorCode: quicErr.ErrorCode, ReasonPhrase: quicErr.ErrorMessage}, s.sentPacketHandler.GetLargestAcked()) + packet, err := s.packer.PackConnectionClose(&frames.ConnectionCloseFrame{ErrorCode: quicErr.ErrorCode, ReasonPhrase: quicErr.ErrorMessage}, s.sentPacketHandler.GetLeastUnacked()) if err != nil { return err } diff --git a/session_test.go b/session_test.go index 433b850c..a80849b6 100644 --- a/session_test.go +++ b/session_test.go @@ -55,7 +55,7 @@ func (h *mockSentPacketHandler) ReceivedAck(ackFrame *frames.AckFrame, withPacke return nil } func (h *mockSentPacketHandler) BytesInFlight() protocol.ByteCount { return 0 } -func (h *mockSentPacketHandler) GetLargestAcked() protocol.PacketNumber { return 1 } +func (h *mockSentPacketHandler) GetLeastUnacked() protocol.PacketNumber { return 1 } func (h *mockSentPacketHandler) GetStopWaitingFrame() *frames.StopWaitingFrame { panic("not implemented") }