From b25b2f6921799ccc7469feb460ef53350b34a457 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 23 Jun 2019 13:46:18 +0800 Subject: [PATCH] use the same packet number space for sent 0-RTT and 1-RTT packets --- internal/ackhandler/sent_packet_handler.go | 20 +++++----- .../ackhandler/sent_packet_handler_test.go | 39 +++++++++++-------- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/internal/ackhandler/sent_packet_handler.go b/internal/ackhandler/sent_packet_handler.go index 2ebf17fd..730d135e 100644 --- a/internal/ackhandler/sent_packet_handler.go +++ b/internal/ackhandler/sent_packet_handler.go @@ -46,7 +46,7 @@ type sentPacketHandler struct { initialPackets *packetNumberSpace handshakePackets *packetNumberSpace - oneRTTPackets *packetNumberSpace + appDataPackets *packetNumberSpace handshakeComplete bool @@ -94,7 +94,7 @@ func NewSentPacketHandler( return &sentPacketHandler{ initialPackets: newPacketNumberSpace(initialPacketNumber), handshakePackets: newPacketNumberSpace(0), - oneRTTPackets: newPacketNumberSpace(0), + appDataPackets: newPacketNumberSpace(0), rttStats: rttStats, congestion: congestion, traceCallback: traceCallback, @@ -137,8 +137,8 @@ func (h *sentPacketHandler) getPacketNumberSpace(encLevel protocol.EncryptionLev return h.initialPackets case protocol.EncryptionHandshake: return h.handshakePackets - case protocol.Encryption1RTT: - return h.oneRTTPackets + case protocol.Encryption0RTT, protocol.Encryption1RTT: + return h.appDataPackets default: panic("invalid packet number space") } @@ -295,8 +295,8 @@ func (h *sentPacketHandler) getEarliestLossTimeAndSpace() (time.Time, protocol.E encLevel = protocol.EncryptionHandshake } if h.handshakeComplete && - (lossTime.IsZero() || (!h.oneRTTPackets.lossTime.IsZero() && h.oneRTTPackets.lossTime.Before(lossTime))) { - lossTime = h.oneRTTPackets.lossTime + (lossTime.IsZero() || (!h.appDataPackets.lossTime.IsZero() && h.appDataPackets.lossTime.Before(lossTime))) { + lossTime = h.appDataPackets.lossTime encLevel = protocol.Encryption1RTT } return lossTime, encLevel @@ -316,8 +316,8 @@ func (h *sentPacketHandler) getEarliestSentTimeAndSpace() (time.Time, protocol.E encLevel = protocol.EncryptionHandshake } if h.handshakeComplete && - (sentTime.IsZero() || (!h.oneRTTPackets.lastSentAckElicitingPacketTime.IsZero() && h.oneRTTPackets.lastSentAckElicitingPacketTime.Before(sentTime))) { - sentTime = h.oneRTTPackets.lastSentAckElicitingPacketTime + (sentTime.IsZero() || (!h.appDataPackets.lastSentAckElicitingPacketTime.IsZero() && h.appDataPackets.lastSentAckElicitingPacketTime.Before(sentTime))) { + sentTime = h.appDataPackets.lastSentAckElicitingPacketTime encLevel = protocol.Encryption1RTT } return sentTime, encLevel @@ -337,7 +337,7 @@ func (h *sentPacketHandler) hasOutstandingCryptoPackets() bool { func (h *sentPacketHandler) hasOutstandingPackets() bool { // We only send application data probe packets once the handshake completes, // because before that, we don't have the keys to decrypt ACKs sent in 1-RTT packets. - return (h.handshakeComplete && h.oneRTTPackets.history.HasOutstandingPackets()) || + return (h.handshakeComplete && h.appDataPackets.history.HasOutstandingPackets()) || h.hasOutstandingCryptoPackets() } @@ -514,7 +514,7 @@ func (h *sentPacketHandler) PopPacketNumber(encLevel protocol.EncryptionLevel) p } func (h *sentPacketHandler) SendMode() SendMode { - numTrackedPackets := h.oneRTTPackets.history.Len() + numTrackedPackets := h.appDataPackets.history.Len() if h.initialPackets != nil { numTrackedPackets += h.initialPackets.history.Len() } diff --git a/internal/ackhandler/sent_packet_handler_test.go b/internal/ackhandler/sent_packet_handler_test.go index 184cc6ad..2ef5dc02 100644 --- a/internal/ackhandler/sent_packet_handler_test.go +++ b/internal/ackhandler/sent_packet_handler_test.go @@ -91,11 +91,18 @@ var _ = Describe("SentPacketHandler", func() { Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(2))) }) + It("uses the same packet number space for 0-RTT and 1-RTT packets", func() { + handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1, EncryptionLevel: protocol.Encryption0RTT})) + handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 2, EncryptionLevel: protocol.Encryption1RTT})) + Expect(handler.appDataPackets.largestSent).To(Equal(protocol.PacketNumber(2))) + Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(2))) + }) + It("accepts packet number 0", func() { handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 0, EncryptionLevel: protocol.Encryption1RTT})) - Expect(handler.oneRTTPackets.largestSent).To(BeZero()) + Expect(handler.appDataPackets.largestSent).To(BeZero()) handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1, EncryptionLevel: protocol.Encryption1RTT})) - Expect(handler.oneRTTPackets.largestSent).To(Equal(protocol.PacketNumber(1))) + Expect(handler.appDataPackets.largestSent).To(Equal(protocol.PacketNumber(1))) expectInPacketHistory([]protocol.PacketNumber{0, 1}, protocol.Encryption1RTT) Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(2))) }) @@ -103,7 +110,7 @@ var _ = Describe("SentPacketHandler", func() { It("stores the sent time", func() { sendTime := time.Now().Add(-time.Minute) handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1, SendTime: sendTime})) - Expect(handler.oneRTTPackets.lastSentAckElicitingPacketTime).To(Equal(sendTime)) + Expect(handler.appDataPackets.lastSentAckElicitingPacketTime).To(Equal(sendTime)) }) It("stores the sent time of Initial packets", func() { @@ -115,8 +122,8 @@ var _ = Describe("SentPacketHandler", func() { It("does not store non-ack-eliciting packets", func() { handler.SentPacket(nonAckElicitingPacket(&Packet{PacketNumber: 1})) - Expect(handler.oneRTTPackets.history.Len()).To(BeZero()) - Expect(handler.oneRTTPackets.lastSentAckElicitingPacketTime).To(BeZero()) + Expect(handler.appDataPackets.history.Len()).To(BeZero()) + Expect(handler.appDataPackets.lastSentAckElicitingPacketTime).To(BeZero()) Expect(handler.bytesInFlight).To(BeZero()) }) }) @@ -136,18 +143,18 @@ var _ = Describe("SentPacketHandler", func() { ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 0, Largest: 5}}} err := handler.ReceivedAck(ack, 0, protocol.Encryption1RTT, time.Now()) Expect(err).ToNot(HaveOccurred()) - Expect(handler.oneRTTPackets.largestAcked).To(Equal(protocol.PacketNumber(5))) + Expect(handler.appDataPackets.largestAcked).To(Equal(protocol.PacketNumber(5))) }) It("accepts multiple ACKs sent in the same packet", func() { ack1 := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 0, Largest: 3}}} ack2 := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 0, Largest: 4}}} Expect(handler.ReceivedAck(ack1, 1337, protocol.Encryption1RTT, time.Now())).To(Succeed()) - Expect(handler.oneRTTPackets.largestAcked).To(Equal(protocol.PacketNumber(3))) + Expect(handler.appDataPackets.largestAcked).To(Equal(protocol.PacketNumber(3))) // this wouldn't happen in practice // for testing purposes, we pretend send a different ACK frame in a duplicated packet, to be able to verify that it actually doesn't get processed Expect(handler.ReceivedAck(ack2, 1337, protocol.Encryption1RTT, time.Now())).To(Succeed()) - Expect(handler.oneRTTPackets.largestAcked).To(Equal(protocol.PacketNumber(4))) + Expect(handler.appDataPackets.largestAcked).To(Equal(protocol.PacketNumber(4))) }) It("rejects ACKs with a too high LargestAcked packet number", func() { @@ -162,7 +169,7 @@ var _ = Describe("SentPacketHandler", func() { Expect(handler.ReceivedAck(ack, 1337, protocol.Encryption1RTT, time.Now())).To(Succeed()) Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(6))) Expect(handler.ReceivedAck(ack, 1337+1, protocol.Encryption1RTT, time.Now())).To(Succeed()) - Expect(handler.oneRTTPackets.largestAcked).To(Equal(protocol.PacketNumber(3))) + Expect(handler.appDataPackets.largestAcked).To(Equal(protocol.PacketNumber(3))) Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(6))) }) }) @@ -188,7 +195,7 @@ var _ = Describe("SentPacketHandler", func() { It("adjusts the LargestAcked, and adjusts the bytes in flight", func() { ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 0, Largest: 5}}} Expect(handler.ReceivedAck(ack, 1, protocol.Encryption1RTT, time.Now())).To(Succeed()) - Expect(handler.oneRTTPackets.largestAcked).To(Equal(protocol.PacketNumber(5))) + Expect(handler.appDataPackets.largestAcked).To(Equal(protocol.PacketNumber(5))) expectInPacketHistoryOrLost([]protocol.PacketNumber{6, 7, 8, 9}, protocol.Encryption1RTT) Expect(handler.bytesInFlight).To(Equal(protocol.ByteCount(4))) }) @@ -653,7 +660,7 @@ var _ = Describe("SentPacketHandler", func() { handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 2})) updateRTT(time.Hour) - Expect(handler.oneRTTPackets.lossTime.IsZero()).To(BeTrue()) + Expect(handler.appDataPackets.lossTime.IsZero()).To(BeTrue()) Expect(handler.OnLossDetectionTimeout()).To(Succeed()) // TLP Expect(handler.ptoCount).To(BeEquivalentTo(1)) @@ -738,12 +745,12 @@ var _ = Describe("SentPacketHandler", func() { now := time.Now() handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1, SendTime: now.Add(-time.Hour)})) handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 2, SendTime: now.Add(-time.Second)})) - Expect(handler.oneRTTPackets.lossTime.IsZero()).To(BeTrue()) + Expect(handler.appDataPackets.lossTime.IsZero()).To(BeTrue()) ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 2, Largest: 2}}} Expect(handler.ReceivedAck(ack, 1, protocol.Encryption1RTT, now)).To(Succeed()) // no need to set an alarm, since packet 1 was already declared lost - Expect(handler.oneRTTPackets.lossTime.IsZero()).To(BeTrue()) + Expect(handler.appDataPackets.lossTime.IsZero()).To(BeTrue()) Expect(handler.bytesInFlight).To(BeZero()) }) @@ -752,15 +759,15 @@ var _ = Describe("SentPacketHandler", func() { handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1, SendTime: now.Add(-2 * time.Second)})) handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 2, SendTime: now.Add(-2 * time.Second)})) handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 3, SendTime: now.Add(-time.Second)})) - Expect(handler.oneRTTPackets.lossTime.IsZero()).To(BeTrue()) + Expect(handler.appDataPackets.lossTime.IsZero()).To(BeTrue()) ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 2, Largest: 2}}} Expect(handler.ReceivedAck(ack, 1, protocol.Encryption1RTT, now.Add(-time.Second))).To(Succeed()) Expect(handler.rttStats.SmoothedRTT()).To(Equal(time.Second)) // Packet 1 should be considered lost (1+1/8) RTTs after it was sent. - Expect(handler.oneRTTPackets.lossTime.IsZero()).To(BeFalse()) - Expect(handler.oneRTTPackets.lossTime.Sub(getPacket(1, protocol.Encryption1RTT).SendTime)).To(Equal(time.Second * 9 / 8)) + Expect(handler.appDataPackets.lossTime.IsZero()).To(BeFalse()) + Expect(handler.appDataPackets.lossTime.Sub(getPacket(1, protocol.Encryption1RTT).SendTime)).To(Equal(time.Second * 9 / 8)) }) It("sets the early retransmit alarm for crypto packets", func() {