From ee75f5e2f224a6125b42dacec672e16442abba5a Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 7 Jan 2019 14:32:57 +0700 Subject: [PATCH] implement ACK frame parsing using an ack delay exponent --- internal/protocol/params.go | 3 +++ internal/wire/ack_frame.go | 7 ++--- internal/wire/ack_frame_test.go | 48 ++++++++++++++++++++++----------- internal/wire/frame_parser.go | 2 +- 4 files changed, 38 insertions(+), 22 deletions(-) diff --git a/internal/protocol/params.go b/internal/protocol/params.go index e6f9493f..7077d825 100644 --- a/internal/protocol/params.go +++ b/internal/protocol/params.go @@ -117,3 +117,6 @@ const MinPacingDelay time.Duration = 100 * time.Microsecond // DefaultConnectionIDLength is the connection ID length that is used for multiplexed connections // if no other value is configured. const DefaultConnectionIDLength = 4 + +// AckDelayExponent is the ack delay exponent used when sending ACKs. +const AckDelayExponent = 3 diff --git a/internal/wire/ack_frame.go b/internal/wire/ack_frame.go index e2e8f471..7909bffa 100644 --- a/internal/wire/ack_frame.go +++ b/internal/wire/ack_frame.go @@ -10,9 +10,6 @@ import ( "github.com/lucas-clemente/quic-go/internal/utils" ) -// TODO: use the value sent in the transport parameters -const ackDelayExponent = 3 - var errInvalidAckRanges = errors.New("AckFrame: ACK frame contains invalid ACK ranges") // An AckFrame is an ACK frame @@ -22,7 +19,7 @@ type AckFrame struct { } // parseAckFrame reads an ACK frame -func parseAckFrame(r *bytes.Reader, version protocol.VersionNumber) (*AckFrame, error) { +func parseAckFrame(r *bytes.Reader, ackDelayExponent uint8, version protocol.VersionNumber) (*AckFrame, error) { typeByte, err := r.ReadByte() if err != nil { return nil, err @@ -225,5 +222,5 @@ func (f *AckFrame) AcksPacket(p protocol.PacketNumber) bool { } func encodeAckDelay(delay time.Duration) uint64 { - return uint64(delay.Nanoseconds() / (1000 * (1 << ackDelayExponent))) + return uint64(delay.Nanoseconds() / (1000 * (1 << protocol.AckDelayExponent))) } diff --git a/internal/wire/ack_frame_test.go b/internal/wire/ack_frame_test.go index 65a06fee..41d7382d 100644 --- a/internal/wire/ack_frame_test.go +++ b/internal/wire/ack_frame_test.go @@ -19,7 +19,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { data = append(data, encodeVarInt(0)...) // num blocks data = append(data, encodeVarInt(10)...) // first ack block b := bytes.NewReader(data) - frame, err := parseAckFrame(b, versionIETFFrames) + frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) Expect(err).ToNot(HaveOccurred()) Expect(frame.LargestAcked()).To(Equal(protocol.PacketNumber(100))) Expect(frame.LowestAcked()).To(Equal(protocol.PacketNumber(90))) @@ -34,7 +34,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { data = append(data, encodeVarInt(0)...) // num blocks data = append(data, encodeVarInt(0)...) // first ack block b := bytes.NewReader(data) - frame, err := parseAckFrame(b, versionIETFFrames) + frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) Expect(err).ToNot(HaveOccurred()) Expect(frame.LargestAcked()).To(Equal(protocol.PacketNumber(55))) Expect(frame.LowestAcked()).To(Equal(protocol.PacketNumber(55))) @@ -49,7 +49,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { data = append(data, encodeVarInt(0)...) // num blocks data = append(data, encodeVarInt(20)...) // first ack block b := bytes.NewReader(data) - frame, err := parseAckFrame(b, versionIETFFrames) + frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) Expect(err).ToNot(HaveOccurred()) Expect(frame.LargestAcked()).To(Equal(protocol.PacketNumber(20))) Expect(frame.LowestAcked()).To(Equal(protocol.PacketNumber(0))) @@ -64,7 +64,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { data = append(data, encodeVarInt(0)...) // num blocks data = append(data, encodeVarInt(21)...) // first ack block b := bytes.NewReader(data) - _, err := parseAckFrame(b, versionIETFFrames) + _, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) Expect(err).To(MatchError("invalid first ACK range")) }) @@ -77,7 +77,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { data = append(data, encodeVarInt(98)...) // gap data = append(data, encodeVarInt(50)...) // ack block b := bytes.NewReader(data) - frame, err := parseAckFrame(b, versionIETFFrames) + frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) Expect(err).ToNot(HaveOccurred()) Expect(frame.LargestAcked()).To(Equal(protocol.PacketNumber(1000))) Expect(frame.LowestAcked()).To(Equal(protocol.PacketNumber(750))) @@ -100,7 +100,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { data = append(data, encodeVarInt(1)...) // gap data = append(data, encodeVarInt(1)...) // ack block b := bytes.NewReader(data) - frame, err := parseAckFrame(b, versionIETFFrames) + frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) Expect(err).ToNot(HaveOccurred()) Expect(frame.LargestAcked()).To(Equal(protocol.PacketNumber(100))) Expect(frame.LowestAcked()).To(Equal(protocol.PacketNumber(94))) @@ -113,6 +113,22 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { Expect(b.Len()).To(BeZero()) }) + It("uses the ack delay exponent", func() { + const delayTime = 1 << 10 * time.Millisecond + buf := &bytes.Buffer{} + f := &AckFrame{ + AckRanges: []AckRange{{Smallest: 1, Largest: 1}}, + DelayTime: delayTime, + } + Expect(f.Write(buf, versionIETFFrames)).To(Succeed()) + for i := uint8(0); i < 8; i++ { + b := bytes.NewReader(buf.Bytes()) + frame, err := parseAckFrame(b, protocol.AckDelayExponent+i, versionIETFFrames) + Expect(err).ToNot(HaveOccurred()) + Expect(frame.DelayTime).To(Equal(delayTime * (1 << i))) + } + }) + It("errors on EOF", func() { data := []byte{0x2} data = append(data, encodeVarInt(1000)...) // largest acked @@ -121,10 +137,10 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { data = append(data, encodeVarInt(100)...) // first ack block data = append(data, encodeVarInt(98)...) // gap data = append(data, encodeVarInt(50)...) // ack block - _, err := parseAckFrame(bytes.NewReader(data), versionIETFFrames) + _, err := parseAckFrame(bytes.NewReader(data), protocol.AckDelayExponent, versionIETFFrames) Expect(err).NotTo(HaveOccurred()) for i := range data { - _, err := parseAckFrame(bytes.NewReader(data[0:i]), versionIETFFrames) + _, err := parseAckFrame(bytes.NewReader(data[0:i]), protocol.AckDelayExponent, versionIETFFrames) Expect(err).To(MatchError(io.EOF)) } }) @@ -140,7 +156,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { data = append(data, encodeVarInt(0x12345)...) // ECT(1) data = append(data, encodeVarInt(0x12345678)...) // ECN-CE b := bytes.NewReader(data) - frame, err := parseAckFrame(b, versionIETFFrames) + frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) Expect(err).ToNot(HaveOccurred()) Expect(frame.LargestAcked()).To(Equal(protocol.PacketNumber(100))) Expect(frame.LowestAcked()).To(Equal(protocol.PacketNumber(90))) @@ -159,10 +175,10 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { data = append(data, encodeVarInt(0x42)...) // ECT(0) data = append(data, encodeVarInt(0x12345)...) // ECT(1) data = append(data, encodeVarInt(0x12345678)...) // ECN-CE - _, err := parseAckFrame(bytes.NewReader(data), versionIETFFrames) + _, err := parseAckFrame(bytes.NewReader(data), protocol.AckDelayExponent, versionIETFFrames) Expect(err).NotTo(HaveOccurred()) for i := range data { - _, err := parseAckFrame(bytes.NewReader(data[0:i]), versionIETFFrames) + _, err := parseAckFrame(bytes.NewReader(data[0:i]), protocol.AckDelayExponent, versionIETFFrames) Expect(err).To(MatchError(io.EOF)) } }) @@ -196,7 +212,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { Expect(err).ToNot(HaveOccurred()) Expect(f.Length(versionIETFFrames)).To(BeEquivalentTo(buf.Len())) b := bytes.NewReader(buf.Bytes()) - frame, err := parseAckFrame(b, versionIETFFrames) + frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) Expect(err).ToNot(HaveOccurred()) Expect(frame).To(Equal(f)) Expect(frame.HasMissingRanges()).To(BeFalse()) @@ -213,7 +229,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { Expect(err).ToNot(HaveOccurred()) Expect(f.Length(versionIETFFrames)).To(BeEquivalentTo(buf.Len())) b := bytes.NewReader(buf.Bytes()) - frame, err := parseAckFrame(b, versionIETFFrames) + frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) Expect(err).ToNot(HaveOccurred()) Expect(frame).To(Equal(f)) Expect(frame.HasMissingRanges()).To(BeFalse()) @@ -233,7 +249,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { Expect(err).ToNot(HaveOccurred()) Expect(f.Length(versionIETFFrames)).To(BeEquivalentTo(buf.Len())) b := bytes.NewReader(buf.Bytes()) - frame, err := parseAckFrame(b, versionIETFFrames) + frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) Expect(err).ToNot(HaveOccurred()) Expect(frame).To(Equal(f)) Expect(frame.HasMissingRanges()).To(BeTrue()) @@ -255,7 +271,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { Expect(err).ToNot(HaveOccurred()) Expect(f.Length(versionIETFFrames)).To(BeEquivalentTo(buf.Len())) b := bytes.NewReader(buf.Bytes()) - frame, err := parseAckFrame(b, versionIETFFrames) + frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) Expect(err).ToNot(HaveOccurred()) Expect(frame).To(Equal(f)) Expect(frame.HasMissingRanges()).To(BeTrue()) @@ -278,7 +294,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { Expect(buf.Len()).To(BeNumerically(">", protocol.MaxAckFrameSize-5)) Expect(buf.Len()).To(BeNumerically("<=", protocol.MaxAckFrameSize)) b := bytes.NewReader(buf.Bytes()) - frame, err := parseAckFrame(b, versionIETFFrames) + frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) Expect(err).ToNot(HaveOccurred()) Expect(frame.HasMissingRanges()).To(BeTrue()) Expect(b.Len()).To(BeZero()) diff --git a/internal/wire/frame_parser.go b/internal/wire/frame_parser.go index caecc25d..5dedf1f5 100644 --- a/internal/wire/frame_parser.go +++ b/internal/wire/frame_parser.go @@ -46,7 +46,7 @@ func (p *frameParser) parseFrame(r *bytes.Reader, typeByte byte) (Frame, error) case 0x1: frame, err = parsePingFrame(r, p.version) case 0x2, 0x3: - frame, err = parseAckFrame(r, p.version) + frame, err = parseAckFrame(r, protocol.AckDelayExponent, p.version) case 0x4: frame, err = parseResetStreamFrame(r, p.version) case 0x5: