diff --git a/internal/ackhandler/packet.go b/internal/ackhandler/packet.go index 5dfa608b..0ace4d65 100644 --- a/internal/ackhandler/packet.go +++ b/internal/ackhandler/packet.go @@ -10,15 +10,12 @@ import ( // A Packet is a packet type Packet struct { PacketNumber protocol.PacketNumber - PacketType protocol.PacketType - Ack *wire.AckFrame Frames []wire.Frame + LargestAcked protocol.PacketNumber // InvalidPacketNumber if the packet doesn't contain an ACK Length protocol.ByteCount EncryptionLevel protocol.EncryptionLevel SendTime time.Time - largestAcked protocol.PacketNumber // if the packet contains an ACK, the LargestAcked value of that ACK - // 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 diff --git a/internal/ackhandler/sent_packet_handler.go b/internal/ackhandler/sent_packet_handler.go index 2747ea98..1bcb4bd4 100644 --- a/internal/ackhandler/sent_packet_handler.go +++ b/internal/ackhandler/sent_packet_handler.go @@ -172,13 +172,6 @@ func (h *sentPacketHandler) sentPacketImpl(packet *Packet) bool /* is ack-elicit } pnSpace.largestSent = packet.PacketNumber - - packet.largestAcked = protocol.InvalidPacketNumber - if packet.Ack != nil { - packet.largestAcked = packet.Ack.LargestAcked() - } - packet.Ack = nil // no need to save the ACK - isAckEliciting := len(packet.Frames) > 0 if isAckEliciting { @@ -237,8 +230,8 @@ func (h *sentPacketHandler) ReceivedAck(ackFrame *wire.AckFrame, withPacketNumbe priorInFlight := h.bytesInFlight for _, p := range ackedPackets { - if p.largestAcked != protocol.InvalidPacketNumber && encLevel == protocol.Encryption1RTT { - h.lowestNotConfirmedAcked = utils.MaxPacketNumber(h.lowestNotConfirmedAcked, p.largestAcked+1) + if p.LargestAcked != protocol.InvalidPacketNumber && encLevel == protocol.Encryption1RTT { + h.lowestNotConfirmedAcked = utils.MaxPacketNumber(h.lowestNotConfirmedAcked, p.LargestAcked+1) } if err := h.onPacketAcked(p, rcvTime); err != nil { return err diff --git a/internal/ackhandler/sent_packet_handler_test.go b/internal/ackhandler/sent_packet_handler_test.go index 83040f8e..d3f88625 100644 --- a/internal/ackhandler/sent_packet_handler_test.go +++ b/internal/ackhandler/sent_packet_handler_test.go @@ -31,7 +31,7 @@ func ackElicitingPacket(p *Packet) *Packet { func nonAckElicitingPacket(p *Packet) *Packet { p = ackElicitingPacket(p) p.Frames = nil - p.Ack = &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 1, Largest: 1}}} + p.LargestAcked = 1 return p } @@ -332,11 +332,9 @@ var _ = Describe("SentPacketHandler", func() { Context("determining which ACKs we have received an ACK for", func() { BeforeEach(func() { - ack1 := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 80, Largest: 100}}} - ack2 := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 50, Largest: 200}}} morePackets := []*Packet{ - {PacketNumber: 13, Ack: ack1, Frames: []wire.Frame{&streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT}, - {PacketNumber: 14, Ack: ack2, Frames: []wire.Frame{&streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT}, + {PacketNumber: 13, LargestAcked: 100, Frames: []wire.Frame{&streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT}, + {PacketNumber: 14, LargestAcked: 200, Frames: []wire.Frame{&streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT}, {PacketNumber: 15, Frames: []wire.Frame{&streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT}, } for _, packet := range morePackets { diff --git a/internal/protocol/packet_number.go b/internal/protocol/packet_number.go index 0fa62ff3..82ac88f2 100644 --- a/internal/protocol/packet_number.go +++ b/internal/protocol/packet_number.go @@ -5,7 +5,7 @@ type PacketNumber int64 // InvalidPacketNumber is a packet number that is never sent. // In QUIC, 0 is a valid packet number. -const InvalidPacketNumber = -1 +const InvalidPacketNumber PacketNumber = -1 // PacketNumberLen is the length of the packet number in bytes type PacketNumberLen uint8 diff --git a/packet_packer.go b/packet_packer.go index ed9589c8..8a4548a2 100644 --- a/packet_packer.go +++ b/packet_packer.go @@ -63,10 +63,13 @@ func (p *packedPacket) IsAckEliciting() bool { } func (p *packedPacket) ToAckHandlerPacket() *ackhandler.Packet { + largestAcked := protocol.InvalidPacketNumber + if p.ack != nil { + largestAcked = p.ack.LargestAcked() + } return &ackhandler.Packet{ PacketNumber: p.header.PacketNumber, - PacketType: p.header.Type, - Ack: p.ack, + LargestAcked: largestAcked, Frames: p.frames, Length: protocol.ByteCount(len(p.raw)), EncryptionLevel: p.EncryptionLevel(), diff --git a/packet_packer_test.go b/packet_packer_test.go index e016870a..1c152100 100644 --- a/packet_packer_test.go +++ b/packet_packer_test.go @@ -5,6 +5,7 @@ import ( "errors" "math/rand" "net" + "time" "github.com/golang/mock/gomock" "github.com/lucas-clemente/quic-go/internal/ackhandler" @@ -931,7 +932,6 @@ var _ = Describe("Packet packer", func() { pnManager.EXPECT().PopPacketNumber(protocol.EncryptionInitial).Return(protocol.PacketNumber(0x42)) sealingManager.EXPECT().GetInitialSealer().Return(sealer, nil) packet := &ackhandler.Packet{ - PacketType: protocol.PacketTypeHandshake, EncryptionLevel: protocol.EncryptionInitial, Frames: []wire.Frame{cf}, } @@ -951,7 +951,6 @@ var _ = Describe("Packet packer", func() { sealingManager.EXPECT().GetInitialSealer().Return(sealer, nil) packer.perspective = protocol.PerspectiveClient packet := &ackhandler.Packet{ - PacketType: protocol.PacketTypeInitial, EncryptionLevel: protocol.EncryptionInitial, Frames: []wire.Frame{cf}, } @@ -969,3 +968,29 @@ var _ = Describe("Packet packer", func() { }) }) }) + +var _ = Describe("Converting to AckHandler packets", func() { + It("convert a packet", func() { + packet := &packedPacket{ + header: &wire.ExtendedHeader{Header: wire.Header{}}, + frames: []wire.Frame{&wire.MaxDataFrame{}, &wire.PingFrame{}}, + ack: &wire.AckFrame{AckRanges: []wire.AckRange{{Largest: 100, Smallest: 80}}}, + raw: []byte("foobar"), + } + p := packet.ToAckHandlerPacket() + Expect(p.Length).To(Equal(protocol.ByteCount(6))) + Expect(p.Frames).To(Equal(packet.frames)) + Expect(p.LargestAcked).To(Equal(protocol.PacketNumber(100))) + Expect(p.SendTime).To(BeTemporally("~", time.Now(), 50*time.Millisecond)) + }) + + It("sets the LargestAcked to invalid, if the packet doesn't have an ACK frame", func() { + packet := &packedPacket{ + header: &wire.ExtendedHeader{Header: wire.Header{}}, + frames: []wire.Frame{&wire.MaxDataFrame{}, &wire.PingFrame{}}, + raw: []byte("foobar"), + } + p := packet.ToAckHandlerPacket() + Expect(p.LargestAcked).To(Equal(protocol.InvalidPacketNumber)) + }) +}) diff --git a/session_test.go b/session_test.go index 82e606cc..56d13fab 100644 --- a/session_test.go +++ b/session_test.go @@ -867,10 +867,7 @@ var _ = Describe("Session", func() { }) It("sends a retransmission and a regular packet in the same run", func() { - packetToRetransmit := &ackhandler.Packet{ - PacketNumber: 10, - PacketType: protocol.PacketTypeHandshake, - } + packetToRetransmit := &ackhandler.Packet{PacketNumber: 10} retransmittedPacket := getPacket(123) newPacket := getPacket(234) sess.windowUpdateQueue.callback(&wire.MaxDataFrame{}) @@ -921,10 +918,7 @@ var _ = Describe("Session", func() { }) It("sends a probe packet", func() { - packetToRetransmit := &ackhandler.Packet{ - PacketNumber: 0x42, - PacketType: protocol.PacketTypeHandshake, - } + packetToRetransmit := &ackhandler.Packet{PacketNumber: 0x42} retransmittedPacket := getPacket(123) sph := mockackhandler.NewMockSentPacketHandler(mockCtrl) sph.EXPECT().TimeUntilSend()