From c455ae0a052f6c27828b8625b71eb97708d091e8 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 20 Apr 2016 11:57:09 +0700 Subject: [PATCH] add support for NACK ranges when writing ACK frames --- frames/ack_frame.go | 36 +++++++++++-- frames/ack_frame_test.go | 110 +++++++++++++++++++++++++++++++++------ utils/utils.go | 10 ++++ utils/utils_test.go | 53 +++++++++++++------ 4 files changed, 175 insertions(+), 34 deletions(-) diff --git a/frames/ack_frame.go b/frames/ack_frame.go index bb9ba7fe2..4cbd46c7d 100644 --- a/frames/ack_frame.go +++ b/frames/ack_frame.go @@ -19,14 +19,44 @@ type AckFrame struct { // Write writes an ACK frame. func (f *AckFrame) Write(b *bytes.Buffer) error { - typeByte := uint8(0x48) + typeByte := uint8(0x40 | 0x0C) + + if f.HasNACK() { + typeByte |= (0x20 | 0x03) + } + b.WriteByte(typeByte) b.WriteByte(f.Entropy) - utils.WriteUint32(b, uint32(f.LargestObserved)) // TODO: send the correct length + utils.WriteUint48(b, uint64(f.LargestObserved)) // TODO: send the correct length utils.WriteUint16(b, 1) // TODO: Ack delay time b.WriteByte(0x01) // Just one timestamp - b.WriteByte(0x00) // Largest observed + b.WriteByte(0x00) // Delta Largest observed utils.WriteUint32(b, 0) // First timestamp + + if f.HasNACK() { + numRanges := uint8(len(f.NackRanges)) + b.WriteByte(numRanges) + + for i, nackRange := range f.NackRanges { + var missingPacketSequenceNumberDelta uint64 + if i == 0 { + if protocol.PacketNumber(uint64(nackRange.FirstPacketNumber)+uint64(nackRange.Length)) > f.LargestObserved { + return errors.New("AckFrame: Invalid NACK ranges") + } + missingPacketSequenceNumberDelta = uint64(f.LargestObserved-nackRange.FirstPacketNumber) - uint64(nackRange.Length) + 1 + } else { + lastNackRange := f.NackRanges[i-1] + missingPacketSequenceNumberDelta = uint64(lastNackRange.FirstPacketNumber-nackRange.FirstPacketNumber) - uint64(nackRange.Length) + } + rangeLength := nackRange.Length - 1 + if rangeLength > 255 { + return errors.New("AckFrame: NACK ranges larger 256 packets not yet supported") + } + utils.WriteUint48(b, missingPacketSequenceNumberDelta) + b.WriteByte(rangeLength) + } + } + return nil } diff --git a/frames/ack_frame_test.go b/frames/ack_frame_test.go index 6da7f2a6d..d3dccd4cc 100644 --- a/frames/ack_frame_test.go +++ b/frames/ack_frame_test.go @@ -3,6 +3,7 @@ package frames import ( "bytes" + "github.com/lucas-clemente/quic-go/ackhandler" "github.com/lucas-clemente/quic-go/protocol" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -44,6 +45,7 @@ var _ = Describe("AckFrame", func() { Expect(len(frame.NackRanges)).To(Equal(1)) Expect(frame.NackRanges[0].FirstPacketNumber).To(Equal(protocol.PacketNumber(1))) Expect(frame.NackRanges[0].Length).To(Equal(uint8(2))) + Expect(b.Len()).To(Equal(0)) }) It("parses a frame containing one NACK range with a 48 bit missingPacketSequenceNumberDelta", func() { @@ -70,31 +72,107 @@ var _ = Describe("AckFrame", func() { Expect(frame.NackRanges[1].Length).To(Equal(uint8(3))) Expect(frame.NackRanges[2].FirstPacketNumber).To(Equal(protocol.PacketNumber(2))) Expect(frame.NackRanges[2].Length).To(Equal(uint8(1))) + Expect(b.Len()).To(Equal(0)) }) }) Context("when writing", func() { - It("writes simple frames", func() { + It("writes simple frames without NACK ranges", func() { b := &bytes.Buffer{} - (&AckFrame{ + frame := AckFrame{ Entropy: 2, LargestObserved: 1, - }).Write(b) - Expect(b.Bytes()).To(Equal([]byte{0x48, 0x02, 0x01, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 0})) + } + err := frame.Write(b) + Expect(err).ToNot(HaveOccurred()) + Expect(b.Bytes()).To(Equal([]byte{0x4c, 0x02, 0x01, 0, 0, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 0})) + }) + + It("writes a frame with one NACK range", func() { + b := &bytes.Buffer{} + nackRange := ackhandler.NackRange{ + FirstPacketNumber: 2, + Length: 1, + } + frame := AckFrame{ + Entropy: 2, + LargestObserved: 4, + NackRanges: []*ackhandler.NackRange{&nackRange}, + } + err := frame.Write(b) + Expect(err).ToNot(HaveOccurred()) + missingPacketBytes := b.Bytes()[b.Len()-8:] + Expect(missingPacketBytes[0]).To(Equal(uint8(1))) // numRanges + Expect(missingPacketBytes[7]).To(Equal(uint8(0))) // rangeLength + packetNumber := make([]byte, 6) + copy(packetNumber, missingPacketBytes[1:6]) + Expect(packetNumber).To(BeEquivalentTo([]byte{2, 0, 0, 0, 0, 0})) + }) + + It("writes a frame with multiple NACK ranges", func() { + b := &bytes.Buffer{} + nackRange1 := ackhandler.NackRange{ + FirstPacketNumber: 4, + Length: 3, + } + nackRange2 := ackhandler.NackRange{ + FirstPacketNumber: 2, + Length: 1, + } + frame := AckFrame{ + Entropy: 2, + LargestObserved: 7, + NackRanges: []*ackhandler.NackRange{&nackRange1, &nackRange2}, + } + err := frame.Write(b) + Expect(err).ToNot(HaveOccurred()) + missingPacketBytes := b.Bytes()[b.Len()-(1+2*7):] + Expect(missingPacketBytes[0]).To(Equal(uint8(2))) // numRanges + Expect(missingPacketBytes[7]).To(Equal(uint8(3 - 1))) // rangeLength #1 + Expect(missingPacketBytes[14]).To(Equal(uint8(1 - 1))) // rangeLength #2 + packetNumber1 := make([]byte, 6) + packetNumber2 := make([]byte, 6) + copy(packetNumber1, missingPacketBytes[1:6]) + copy(packetNumber2, missingPacketBytes[8:13]) + Expect(packetNumber1).To(BeEquivalentTo([]byte{1, 0, 0, 0, 0, 0})) + Expect(packetNumber2).To(BeEquivalentTo([]byte{1, 0, 0, 0, 0, 0})) }) }) - It("is self-consistent", func() { - b := &bytes.Buffer{} - frame := &AckFrame{ - Entropy: 0xDE, - LargestObserved: 6789, - } - err := frame.Write(b) - Expect(err).ToNot(HaveOccurred()) - readframe, err := ParseAckFrame(bytes.NewReader(b.Bytes())) - Expect(err).ToNot(HaveOccurred()) - Expect(readframe.Entropy).To(Equal(frame.Entropy)) - Expect(readframe.LargestObserved).To(Equal(frame.LargestObserved)) + Context("self-consistency checks", func() { + It("is self-consistent for ACK frames without NACK ranges", func() { + b := &bytes.Buffer{} + frameOrig := &AckFrame{ + Entropy: 0xDE, + LargestObserved: 6789, + } + err := frameOrig.Write(b) + Expect(err).ToNot(HaveOccurred()) + frame, err := ParseAckFrame(bytes.NewReader(b.Bytes())) + Expect(err).ToNot(HaveOccurred()) + Expect(frame.Entropy).To(Equal(frameOrig.Entropy)) + Expect(frame.LargestObserved).To(Equal(frameOrig.LargestObserved)) + }) + + It("is self-consistent for ACK frames with NACK ranges", func() { + b := &bytes.Buffer{} + nackRanges := []*ackhandler.NackRange{ + &ackhandler.NackRange{FirstPacketNumber: 9, Length: 3}, + &ackhandler.NackRange{FirstPacketNumber: 7, Length: 1}, + &ackhandler.NackRange{FirstPacketNumber: 2, Length: 2}, + } + frameOrig := &AckFrame{ + LargestObserved: 15, + NackRanges: nackRanges, + } + err := frameOrig.Write(b) + 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)) + }) }) }) diff --git a/utils/utils.go b/utils/utils.go index 99e2b5b3b..4b7033c32 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -107,6 +107,16 @@ func WriteUint64(b *bytes.Buffer, i uint64) { b.WriteByte(uint8(i >> 56)) } +// WriteUint48 writes 48 bit of a uint64 +func WriteUint48(b *bytes.Buffer, i uint64) { + b.WriteByte(uint8(i & 0xff)) + b.WriteByte(uint8((i >> 8) & 0xff)) + b.WriteByte(uint8((i >> 16) & 0xff)) + b.WriteByte(uint8((i >> 24) & 0xff)) + b.WriteByte(uint8((i >> 32) & 0xff)) + b.WriteByte(uint8((i >> 40))) +} + // WriteUint32 writes a uint32 func WriteUint32(b *bytes.Buffer, i uint32) { b.WriteByte(uint8(i & 0xff)) diff --git a/utils/utils_test.go b/utils/utils_test.go index ac20e9299..eb1758696 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -8,21 +8,6 @@ import ( ) var _ = Describe("Utils", func() { - Context("WriteUint64", func() { - It("outputs 8 bytes", func() { - b := &bytes.Buffer{} - WriteUint64(b, uint64(1)) - Expect(b.Len()).To(Equal(8)) - }) - - It("outputs a little endian", func() { - num := uint64(0xFFEEDDCCBBAA9988) - b := &bytes.Buffer{} - WriteUint64(b, num) - Expect(b.Bytes()).To(Equal([]byte{0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF})) - }) - }) - Context("ReadUint16", func() { It("reads a little endian", func() { b := []byte{0x13, 0xEF} @@ -98,6 +83,44 @@ var _ = Describe("Utils", func() { }) }) + Context("WriteUint48", func() { + It("outputs 6 bytes", func() { + b := &bytes.Buffer{} + WriteUint48(b, uint64(1)) + Expect(b.Len()).To(Equal(6)) + }) + + It("outputs a little endian", func() { + num := uint64(0xDEADBEEFCAFE) + b := &bytes.Buffer{} + WriteUint48(b, num) + Expect(b.Bytes()).To(Equal([]byte{0xFE, 0xCA, 0xEF, 0xBE, 0xAD, 0xDE})) + }) + + It("doesn't care about the two higher order bytes", func() { + num := uint64(0x1337DEADBEEFCAFE) + b := &bytes.Buffer{} + WriteUint48(b, num) + Expect(b.Len()).To(Equal(6)) + Expect(b.Bytes()).To(Equal([]byte{0xFE, 0xCA, 0xEF, 0xBE, 0xAD, 0xDE})) + }) + }) + + Context("WriteUint64", func() { + It("outputs 8 bytes", func() { + b := &bytes.Buffer{} + WriteUint64(b, uint64(1)) + Expect(b.Len()).To(Equal(8)) + }) + + It("outputs a little endian", func() { + num := uint64(0xFFEEDDCCBBAA9988) + b := &bytes.Buffer{} + WriteUint64(b, num) + Expect(b.Bytes()).To(Equal([]byte{0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF})) + }) + }) + Context("Max", func() { It("returns the maximum", func() { Expect(Max(5, 7)).To(Equal(7))