From 13fa0bcdd13d94047a5591b7ae2530d6c84938a8 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 10 Aug 2020 12:20:31 +0700 Subject: [PATCH] implement writing of ACK frames containing ECN counts --- fuzzing/frames/cmd/corpus.go | 7 +++++++ internal/wire/ack_frame.go | 22 ++++++++++++++++++-- internal/wire/ack_frame_test.go | 36 ++++++++++++++++++++++++--------- internal/wire/log.go | 9 +++++++-- internal/wire/log_test.go | 12 +++++++++++ qlog/frame.go | 5 +++++ qlog/frame_test.go | 18 +++++++++++++++++ 7 files changed, 95 insertions(+), 14 deletions(-) diff --git a/fuzzing/frames/cmd/corpus.go b/fuzzing/frames/cmd/corpus.go index c65fe98f2..8ea956ed0 100644 --- a/fuzzing/frames/cmd/corpus.go +++ b/fuzzing/frames/cmd/corpus.go @@ -115,6 +115,13 @@ func getFrames() []wire.Frame { AckRanges: getAckRanges(300), DelayTime: time.Duration(getRandomNumber()), }, + &wire.AckFrame{ + AckRanges: getAckRanges(3), + DelayTime: time.Duration(getRandomNumber()), + ECT0: getRandomNumber(), + ECT1: getRandomNumber(), + ECNCE: getRandomNumber(), + }, &wire.PingFrame{}, &wire.ResetStreamFrame{ StreamID: protocol.StreamID(getRandomNumber()), diff --git a/internal/wire/ack_frame.go b/internal/wire/ack_frame.go index 0f9b2d224..b3017ecbe 100644 --- a/internal/wire/ack_frame.go +++ b/internal/wire/ack_frame.go @@ -16,6 +16,8 @@ var errInvalidAckRanges = errors.New("AckFrame: ACK frame contains invalid ACK r type AckFrame struct { AckRanges []AckRange // has to be ordered. The highest ACK range goes first, the lowest ACK range goes last DelayTime time.Duration + + ECT0, ECT1, ECNCE uint64 } // parseAckFrame reads an ACK frame @@ -104,8 +106,13 @@ func parseAckFrame(r *bytes.Reader, ackDelayExponent uint8, _ protocol.VersionNu } // Write writes an ACK frame. -func (f *AckFrame) Write(b *bytes.Buffer, version protocol.VersionNumber) error { - b.WriteByte(0x2) +func (f *AckFrame) Write(b *bytes.Buffer, _ protocol.VersionNumber) error { + hasECN := f.ECT0 > 0 || f.ECT1 > 0 || f.ECNCE > 0 + if hasECN { + b.WriteByte(0x3) + } else { + b.WriteByte(0x2) + } utils.WriteVarInt(b, uint64(f.LargestAcked())) utils.WriteVarInt(b, encodeAckDelay(f.DelayTime)) @@ -122,6 +129,12 @@ func (f *AckFrame) Write(b *bytes.Buffer, version protocol.VersionNumber) error utils.WriteVarInt(b, gap) utils.WriteVarInt(b, len) } + + if hasECN { + utils.WriteVarInt(b, f.ECT0) + utils.WriteVarInt(b, f.ECT1) + utils.WriteVarInt(b, f.ECNCE) + } return nil } @@ -141,6 +154,11 @@ func (f *AckFrame) Length(version protocol.VersionNumber) protocol.ByteCount { length += utils.VarIntLen(gap) length += utils.VarIntLen(len) } + if f.ECT0 > 0 || f.ECT1 > 0 || f.ECNCE > 0 { + length += utils.VarIntLen(f.ECT0) + length += utils.VarIntLen(f.ECT1) + length += utils.VarIntLen(f.ECNCE) + } return length } diff --git a/internal/wire/ack_frame_test.go b/internal/wire/ack_frame_test.go index df7d486e8..1d9f37708 100644 --- a/internal/wire/ack_frame_test.go +++ b/internal/wire/ack_frame_test.go @@ -207,8 +207,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { f := &AckFrame{ AckRanges: []AckRange{{Smallest: 100, Largest: 1337}}, } - err := f.Write(buf, versionIETFFrames) - Expect(err).ToNot(HaveOccurred()) + Expect(f.Write(buf, versionIETFFrames)).To(Succeed()) expected := []byte{0x2} expected = append(expected, encodeVarInt(1337)...) // largest acked expected = append(expected, 0) // delay @@ -217,14 +216,34 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { Expect(buf.Bytes()).To(Equal(expected)) }) + It("writes an ACK-ECN frame", func() { + buf := &bytes.Buffer{} + f := &AckFrame{ + AckRanges: []AckRange{{Smallest: 10, Largest: 2000}}, + ECT0: 13, + ECT1: 37, + ECNCE: 12345, + } + Expect(f.Write(buf, versionIETFFrames)).To(Succeed()) + Expect(f.Length(versionIETFFrames)).To(BeEquivalentTo(buf.Len())) + expected := []byte{0x3} + expected = append(expected, encodeVarInt(2000)...) // largest acked + expected = append(expected, 0) // delay + expected = append(expected, encodeVarInt(0)...) // num ranges + expected = append(expected, encodeVarInt(2000-10)...) + expected = append(expected, encodeVarInt(13)...) + expected = append(expected, encodeVarInt(37)...) + expected = append(expected, encodeVarInt(12345)...) + Expect(buf.Bytes()).To(Equal(expected)) + }) + It("writes a frame that acks a single packet", func() { buf := &bytes.Buffer{} f := &AckFrame{ AckRanges: []AckRange{{Smallest: 0x2eadbeef, Largest: 0x2eadbeef}}, DelayTime: 18 * time.Millisecond, } - err := f.Write(buf, versionIETFFrames) - Expect(err).ToNot(HaveOccurred()) + Expect(f.Write(buf, versionIETFFrames)).To(Succeed()) Expect(f.Length(versionIETFFrames)).To(BeEquivalentTo(buf.Len())) b := bytes.NewReader(buf.Bytes()) frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) @@ -240,8 +259,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { f := &AckFrame{ AckRanges: []AckRange{{Smallest: 0x1337, Largest: 0x2eadbeef}}, } - err := f.Write(buf, versionIETFFrames) - Expect(err).ToNot(HaveOccurred()) + Expect(f.Write(buf, versionIETFFrames)).To(Succeed()) Expect(f.Length(versionIETFFrames)).To(BeEquivalentTo(buf.Len())) b := bytes.NewReader(buf.Bytes()) frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) @@ -282,8 +300,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { }, } Expect(f.validateAckRanges()).To(BeTrue()) - err := f.Write(buf, versionIETFFrames) - Expect(err).ToNot(HaveOccurred()) + Expect(f.Write(buf, versionIETFFrames)).To(Succeed()) Expect(f.Length(versionIETFFrames)).To(BeEquivalentTo(buf.Len())) b := bytes.NewReader(buf.Bytes()) frame, err := parseAckFrame(b, protocol.AckDelayExponent, versionIETFFrames) @@ -302,8 +319,7 @@ var _ = Describe("ACK Frame (for IETF QUIC)", func() { } f := &AckFrame{AckRanges: ackRanges} Expect(f.validateAckRanges()).To(BeTrue()) - err := f.Write(buf, versionIETFFrames) - Expect(err).ToNot(HaveOccurred()) + Expect(f.Write(buf, versionIETFFrames)).To(Succeed()) Expect(f.Length(versionIETFFrames)).To(BeEquivalentTo(buf.Len())) // make sure the ACK frame is *a little bit* smaller than the MaxAckFrameSize Expect(buf.Len()).To(BeNumerically(">", protocol.MaxAckFrameSize-5)) diff --git a/internal/wire/log.go b/internal/wire/log.go index 78a99318f..30cf94243 100644 --- a/internal/wire/log.go +++ b/internal/wire/log.go @@ -26,14 +26,19 @@ func LogFrame(logger utils.Logger, frame Frame, sent bool) { case *ResetStreamFrame: logger.Debugf("\t%s &wire.ResetStreamFrame{StreamID: %d, ErrorCode: %#x, FinalSize: %d}", dir, f.StreamID, f.ErrorCode, f.FinalSize) case *AckFrame: + hasECN := f.ECT0 > 0 || f.ECT1 > 0 || f.ECNCE > 0 + var ecn string + if hasECN { + ecn = fmt.Sprintf(", ECT0: %d, ECT1: %d, CE: %d", f.ECT0, f.ECT1, f.ECNCE) + } if len(f.AckRanges) > 1 { ackRanges := make([]string, len(f.AckRanges)) for i, r := range f.AckRanges { ackRanges[i] = fmt.Sprintf("{Largest: %d, Smallest: %d}", r.Largest, r.Smallest) } - logger.Debugf("\t%s &wire.AckFrame{LargestAcked: %d, LowestAcked: %d, AckRanges: {%s}, DelayTime: %s}", dir, f.LargestAcked(), f.LowestAcked(), strings.Join(ackRanges, ", "), f.DelayTime.String()) + logger.Debugf("\t%s &wire.AckFrame{LargestAcked: %d, LowestAcked: %d, AckRanges: {%s}, DelayTime: %s%s}", dir, f.LargestAcked(), f.LowestAcked(), strings.Join(ackRanges, ", "), f.DelayTime.String(), ecn) } else { - logger.Debugf("\t%s &wire.AckFrame{LargestAcked: %d, LowestAcked: %d, DelayTime: %s}", dir, f.LargestAcked(), f.LowestAcked(), f.DelayTime.String()) + logger.Debugf("\t%s &wire.AckFrame{LargestAcked: %d, LowestAcked: %d, DelayTime: %s%s}", dir, f.LargestAcked(), f.LowestAcked(), f.DelayTime.String(), ecn) } case *MaxDataFrame: logger.Debugf("\t%s &wire.MaxDataFrame{MaximumData: %d}", dir, f.MaximumData) diff --git a/internal/wire/log_test.go b/internal/wire/log_test.go index b0925888e..92874d24c 100644 --- a/internal/wire/log_test.go +++ b/internal/wire/log_test.go @@ -75,6 +75,18 @@ var _ = Describe("Frame logging", func() { Expect(buf.String()).To(ContainSubstring("\t<- &wire.AckFrame{LargestAcked: 1337, LowestAcked: 42, DelayTime: 1ms}\n")) }) + It("logs ACK frames with ECN", func() { + frame := &AckFrame{ + AckRanges: []AckRange{{Smallest: 42, Largest: 1337}}, + DelayTime: 1 * time.Millisecond, + ECT0: 5, + ECT1: 66, + ECNCE: 777, + } + LogFrame(logger, frame, false) + Expect(buf.String()).To(ContainSubstring("\t<- &wire.AckFrame{LargestAcked: 1337, LowestAcked: 42, DelayTime: 1ms, ECT0: 5, ECT1: 66, CE: 777}\n")) + }) + It("logs ACK frames with missing packets", func() { frame := &AckFrame{ AckRanges: []AckRange{ diff --git a/qlog/frame.go b/qlog/frame.go index 4c19123c0..2f8449750 100644 --- a/qlog/frame.go +++ b/qlog/frame.go @@ -102,6 +102,11 @@ func marshalAckFrame(enc *gojay.Encoder, f *logging.AckFrame) { enc.StringKey("frame_type", "ack") enc.FloatKeyOmitEmpty("ack_delay", milliseconds(f.DelayTime)) enc.ArrayKey("acked_ranges", ackRanges(f.AckRanges)) + if hasECN := f.ECT0 > 0 || f.ECT1 > 0 || f.ECNCE > 0; hasECN { + enc.Uint64Key("ect0", f.ECT0) + enc.Uint64Key("ect1", f.ECT1) + enc.Uint64Key("ce", f.ECNCE) + } } func marshalResetStreamFrame(enc *gojay.Encoder, f *logging.ResetStreamFrame) { diff --git a/qlog/frame_test.go b/qlog/frame_test.go index 159ad4705..f2235e866 100644 --- a/qlog/frame_test.go +++ b/qlog/frame_test.go @@ -60,6 +60,24 @@ var _ = Describe("Frames", func() { ) }) + It("marshals ACK frames with ECN counts", func() { + check( + &logging.AckFrame{ + AckRanges: []logging.AckRange{{Smallest: 120, Largest: 120}}, + ECT0: 10, + ECT1: 100, + ECNCE: 1000, + }, + map[string]interface{}{ + "frame_type": "ack", + "acked_ranges": [][]float64{{120}}, + "ect0": 10, + "ect1": 100, + "ce": 1000, + }, + ) + }) + It("marshals ACK frames with a range acknowledging ranges of packets", func() { check( &logging.AckFrame{