From a4debcac71da13e872ef22850c36096c72d6c0c0 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 27 Apr 2016 15:39:07 +0700 Subject: [PATCH] implement contiguous NACK frame writing --- frames/ack_frame.go | 39 +++++++-- frames/ack_frame_test.go | 180 ++++++++++++++++++++++++++++++--------- 2 files changed, 171 insertions(+), 48 deletions(-) diff --git a/frames/ack_frame.go b/frames/ack_frame.go index 1235bb336..a01cd5f84 100644 --- a/frames/ack_frame.go +++ b/frames/ack_frame.go @@ -41,9 +41,23 @@ func (f *AckFrame) Write(b *bytes.Buffer, packetNumber protocol.PacketNumber, pa utils.WriteUint32(b, 0) // First timestamp if f.HasNACK() { - numRanges := uint8(len(f.NackRanges)) - b.WriteByte(numRanges) + numRanges := uint64(0) + // calculate the number of NackRanges that are about to be written + // this number is different from len(f.NackRanges) for the case of contiguous NACK ranges + for _, nackRange := range f.NackRanges { + rangeLength := uint64(nackRange.LastPacketNumber - nackRange.FirstPacketNumber) + numRanges += rangeLength/0xFF + 1 + if rangeLength > 0 && rangeLength%0xFF == 0 { + numRanges-- + } + } + if numRanges > 0xFF { + panic("Too many NACK ranges. Truncating not yet implemented.") + } + b.WriteByte(uint8(numRanges)) + + rangeCounter := uint8(0) for i, nackRange := range f.NackRanges { var missingPacketSequenceNumberDelta uint64 if i == 0 { @@ -55,12 +69,23 @@ func (f *AckFrame) Write(b *bytes.Buffer, packetNumber protocol.PacketNumber, pa lastNackRange := f.NackRanges[i-1] missingPacketSequenceNumberDelta = uint64(lastNackRange.FirstPacketNumber) - uint64(nackRange.LastPacketNumber) - 1 } - rangeLength := uint8(nackRange.LastPacketNumber - nackRange.FirstPacketNumber) - if rangeLength > 255 { - return errors.New("AckFrame: NACK ranges larger 256 packets not yet supported") - } + rangeLength := nackRange.LastPacketNumber - nackRange.FirstPacketNumber + utils.WriteUint48(b, missingPacketSequenceNumberDelta) - b.WriteByte(rangeLength) + b.WriteByte(uint8(rangeLength % 0x100)) + rangeCounter++ + + rangeLength = rangeLength - (rangeLength % 0x100) + for rangeLength > 0 { + rangeCounter++ + utils.WriteUint48(b, 0) + b.WriteByte(uint8(0xFF)) + rangeLength -= 0x100 + } + } + + if rangeCounter != uint8(numRanges) { + panic("Inconsistent number of NACK ranges written.") } } diff --git a/frames/ack_frame_test.go b/frames/ack_frame_test.go index 119cd31dd..892c2f630 100644 --- a/frames/ack_frame_test.go +++ b/frames/ack_frame_test.go @@ -219,8 +219,12 @@ var _ = Describe("AckFrame", func() { }) Context("when writing", func() { + var b *bytes.Buffer + BeforeEach(func() { + b = &bytes.Buffer{} + }) + It("writes simple frames without NACK ranges", func() { - b := &bytes.Buffer{} frame := AckFrame{ Entropy: 2, LargestObserved: 1, @@ -231,15 +235,10 @@ var _ = Describe("AckFrame", func() { }) It("writes a frame with one NACK range", func() { - b := &bytes.Buffer{} - nackRange := NackRange{ - FirstPacketNumber: 2, - LastPacketNumber: 2, - } frame := AckFrame{ Entropy: 2, LargestObserved: 4, - NackRanges: []NackRange{nackRange}, + NackRanges: []NackRange{NackRange{FirstPacketNumber: 2, LastPacketNumber: 2}}, } err := frame.Write(b, 1, 6) Expect(err).ToNot(HaveOccurred()) @@ -252,15 +251,8 @@ var _ = Describe("AckFrame", func() { }) It("writes a frame with multiple NACK ranges", func() { - b := &bytes.Buffer{} - nackRange1 := NackRange{ - FirstPacketNumber: 4, - LastPacketNumber: 6, - } - nackRange2 := NackRange{ - FirstPacketNumber: 2, - LastPacketNumber: 2, - } + nackRange1 := NackRange{FirstPacketNumber: 4, LastPacketNumber: 6} + nackRange2 := NackRange{FirstPacketNumber: 2, LastPacketNumber: 2} frame := AckFrame{ Entropy: 2, LargestObserved: 7, @@ -280,37 +272,124 @@ var _ = Describe("AckFrame", func() { Expect(packetNumber2).To(BeEquivalentTo([]byte{1, 0, 0, 0, 0, 0})) }) - It("has proper min length", func() { - b := &bytes.Buffer{} - f := &AckFrame{ - Entropy: 2, - LargestObserved: 1, - } - f.Write(b, 1, 6) - Expect(f.MinLength()).To(Equal(b.Len())) + Context("contiguous NACK ranges", func() { + It("writes the largest possible NACK range that does not require to be written in contiguous form", func() { + frame := AckFrame{ + Entropy: 2, + LargestObserved: 258, + NackRanges: []NackRange{NackRange{FirstPacketNumber: 2, LastPacketNumber: 257}}, + } + err := frame.Write(b, 1, 6) + Expect(err).ToNot(HaveOccurred()) + missingPacketBytes := b.Bytes()[b.Len()-(1+7):] + Expect(missingPacketBytes[0]).To(Equal(uint8(1))) // numRanges + Expect(missingPacketBytes[1:7]).To(Equal([]byte{1, 0, 0, 0, 0, 0})) // missingPacketSequenceNumberDelta + Expect(missingPacketBytes[7]).To(Equal(uint8(0xFF))) // rangeLength + }) + + It("writes a frame with a contiguous NACK range", func() { + frame := AckFrame{ + Entropy: 2, + LargestObserved: 302, + NackRanges: []NackRange{NackRange{FirstPacketNumber: 2, LastPacketNumber: 301}}, + } + err := frame.Write(b, 1, 6) + Expect(err).ToNot(HaveOccurred()) + missingPacketBytes := b.Bytes()[b.Len()-(1+2*7):] + Expect(missingPacketBytes[0]).To(Equal(uint8(2))) // numRanges + Expect(missingPacketBytes[1:7]).To(Equal([]byte{1, 0, 0, 0, 0, 0})) // missingPacketSequenceNumberDelta #1 + Expect(missingPacketBytes[7]).To(Equal(uint8(43))) // rangeLength #1 + Expect(missingPacketBytes[8:14]).To(Equal([]byte{0, 0, 0, 0, 0, 0})) // missingPacketSequenceNumberDelta #2 + Expect(missingPacketBytes[14]).To(Equal(uint8(0xFF))) // rangeLength #2 + }) + + It("writes a frame with the smallest NACK ranges that requires a contiguous NACK range", func() { + frame := AckFrame{ + Entropy: 2, + LargestObserved: 259, + NackRanges: []NackRange{NackRange{FirstPacketNumber: 2, LastPacketNumber: 258}}, + } + err := frame.Write(b, 1, 6) + Expect(err).ToNot(HaveOccurred()) + missingPacketBytes := b.Bytes()[b.Len()-(1+2*7):] + Expect(missingPacketBytes[0]).To(Equal(uint8(2))) // numRanges + Expect(missingPacketBytes[1:7]).To(Equal([]byte{1, 0, 0, 0, 0, 0})) // missingPacketSequenceNumberDelta #1 + Expect(missingPacketBytes[7]).To(Equal(uint8(0))) // rangeLength #1 + Expect(missingPacketBytes[8:14]).To(Equal([]byte{0, 0, 0, 0, 0, 0})) // missingPacketSequenceNumberDelta #2 + Expect(missingPacketBytes[14]).To(Equal(uint8(0xFF))) // rangeLength #2 + }) + + It("writes a frame with a long contiguous NACK range", func() { + frame := AckFrame{ + Entropy: 2, + LargestObserved: 603, + NackRanges: []NackRange{NackRange{FirstPacketNumber: 2, LastPacketNumber: 601}}, + } + err := frame.Write(b, 1, 6) + Expect(err).ToNot(HaveOccurred()) + missingPacketBytes := b.Bytes()[b.Len()-(1+3*7):] + Expect(missingPacketBytes[0]).To(Equal(uint8(3))) // numRanges + Expect(missingPacketBytes[1:7]).To(Equal([]byte{2, 0, 0, 0, 0, 0})) // missingPacketSequenceNumberDelta #1 + Expect(missingPacketBytes[7]).To(Equal(uint8(87))) // rangeLength #1 + Expect(missingPacketBytes[8:14]).To(Equal([]byte{0, 0, 0, 0, 0, 0})) // missingPacketSequenceNumberDelta #2 + Expect(missingPacketBytes[14]).To(Equal(uint8(0xFF))) // rangeLength #2 + Expect(missingPacketBytes[15:21]).To(Equal([]byte{0, 0, 0, 0, 0, 0})) // missingPacketSequenceNumberDelta #3 + Expect(missingPacketBytes[21]).To(Equal(uint8(0xFF))) // rangeLength #3 + }) + + It("writes a frame with two contiguous NACK range", func() { + nackRange1 := NackRange{FirstPacketNumber: 2, LastPacketNumber: 351} + nackRange2 := NackRange{FirstPacketNumber: 355, LastPacketNumber: 654} + frame := AckFrame{ + Entropy: 2, + LargestObserved: 655, + NackRanges: []NackRange{nackRange2, nackRange1}, + } + err := frame.Write(b, 1, 6) + Expect(err).ToNot(HaveOccurred()) + missingPacketBytes := b.Bytes()[b.Len()-(1+4*7):] + Expect(missingPacketBytes[0]).To(Equal(uint8(4))) // numRanges + Expect(missingPacketBytes[1:7]).To(Equal([]byte{1, 0, 0, 0, 0, 0})) // missingPacketSequenceNumberDelta #1 + Expect(missingPacketBytes[7]).To(Equal(uint8(43))) // rangeLength #1 + Expect(missingPacketBytes[8:14]).To(Equal([]byte{0, 0, 0, 0, 0, 0})) // missingPacketSequenceNumberDelta #2 + Expect(missingPacketBytes[14]).To(Equal(uint8(0xFF))) // rangeLength #2 + Expect(missingPacketBytes[15:21]).To(Equal([]byte{3, 0, 0, 0, 0, 0})) // missingPacketSequenceNumberDelta #3 + Expect(missingPacketBytes[21]).To(Equal(uint8(93))) // rangeLength #3 + Expect(missingPacketBytes[22:28]).To(Equal([]byte{0, 0, 0, 0, 0, 0})) // missingPacketSequenceNumberDelta #4 + Expect(missingPacketBytes[28]).To(Equal(uint8(0xFF))) // rangeLength #4 + }) }) - It("has proper min length with nack ranges", func() { - b := &bytes.Buffer{} - f := &AckFrame{ - Entropy: 2, - LargestObserved: 4, - NackRanges: []NackRange{ - NackRange{ - FirstPacketNumber: 2, - LastPacketNumber: 2, - }, - }, - } - err := f.Write(b, 1, 6) - Expect(err).ToNot(HaveOccurred()) - Expect(f.MinLength()).To(Equal(b.Len())) + Context("min length", func() { + It("has proper min length", func() { + f := &AckFrame{ + Entropy: 2, + LargestObserved: 1, + } + f.Write(b, 1, 6) + Expect(f.MinLength()).To(Equal(b.Len())) + }) + + It("has proper min length with nack ranges", func() { + f := &AckFrame{ + Entropy: 2, + LargestObserved: 4, + NackRanges: []NackRange{NackRange{FirstPacketNumber: 2, LastPacketNumber: 2}}, + } + err := f.Write(b, 1, 6) + Expect(err).ToNot(HaveOccurred()) + Expect(f.MinLength()).To(Equal(b.Len())) + }) }) }) Context("self-consistency checks", func() { + var b *bytes.Buffer + BeforeEach(func() { + b = &bytes.Buffer{} + }) + It("is self-consistent for ACK frames without NACK ranges", func() { - b := &bytes.Buffer{} frameOrig := &AckFrame{ Entropy: 0xDE, LargestObserved: 6789, @@ -324,7 +403,6 @@ var _ = Describe("AckFrame", func() { }) It("is self-consistent for ACK frames with NACK ranges", func() { - b := &bytes.Buffer{} nackRanges := []NackRange{ NackRange{FirstPacketNumber: 9, LastPacketNumber: 11}, NackRange{FirstPacketNumber: 7, LastPacketNumber: 7}, @@ -343,5 +421,25 @@ var _ = Describe("AckFrame", func() { Expect(len(frame.NackRanges)).To(Equal(len(frameOrig.NackRanges))) Expect(frame.NackRanges).To(Equal(frameOrig.NackRanges)) }) + + It("is self-consistent for ACK frames with contiguous NACK ranges", func() { + nackRanges := []NackRange{ + NackRange{FirstPacketNumber: 500, LastPacketNumber: 1500}, + NackRange{FirstPacketNumber: 350, LastPacketNumber: 351}, + NackRange{FirstPacketNumber: 2, LastPacketNumber: 306}, + } + frameOrig := &AckFrame{ + LargestObserved: 1600, + NackRanges: nackRanges, + } + err := frameOrig.Write(b, 1, 6) + Expect(err).ToNot(HaveOccurred()) + r := bytes.NewReader(b.Bytes()) + frame, err := ParseAckFrame(r) + Expect(err).ToNot(HaveOccurred()) + Expect(frame.LargestObserved).To(Equal(frameOrig.LargestObserved)) + Expect(len(frame.NackRanges)).To(Equal(len(frameOrig.NackRanges))) + Expect(frame.NackRanges).To(Equal(frameOrig.NackRanges)) + }) }) })