quic: move ack_delay_exponent handling out of frame parsing
The ACK Delay field of ACK frames contains a duration.
The field contains an integer which is multiplied by two to the
power of the sender's ack_delay_exponent transport parameter to
arrive at the delay in microseconds.
Change the frame parsing and encoding layer to operate on the
unscaled field value, rather than passing the ack_delay_exponent
and a duration. This better expresses the fact that we may
parse an ACK frame without knowing the ack_delay_exponent, if
the ACK is received before transport parameters.
For golang/go#58547
Change-Id: Ic26256761961ce89aea0618b849e5661b0502b12
Reviewed-on: https://go-review.googlesource.com/c/net/+/504855
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
diff --git a/internal/quic/ack_delay.go b/internal/quic/ack_delay.go
new file mode 100644
index 0000000..66bdf3c
--- /dev/null
+++ b/internal/quic/ack_delay.go
@@ -0,0 +1,28 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build go1.21
+
+package quic
+
+import (
+ "math"
+ "time"
+)
+
+// An unscaledAckDelay is an ACK Delay field value from an ACK packet,
+// without the ack_delay_exponent scaling applied.
+type unscaledAckDelay int64
+
+func unscaledAckDelayFromDuration(d time.Duration, ackDelayExponent uint8) unscaledAckDelay {
+ return unscaledAckDelay(d.Microseconds() >> ackDelayExponent)
+}
+
+func (d unscaledAckDelay) Duration(ackDelayExponent uint8) time.Duration {
+ if int64(d) > (math.MaxInt64>>ackDelayExponent)/int64(time.Microsecond) {
+ // If scaling the delay would overflow, ignore the delay.
+ return 0
+ }
+ return time.Duration(d<<ackDelayExponent) * time.Microsecond
+}
diff --git a/internal/quic/ack_delay_test.go b/internal/quic/ack_delay_test.go
new file mode 100644
index 0000000..038964a
--- /dev/null
+++ b/internal/quic/ack_delay_test.go
@@ -0,0 +1,81 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build go1.21
+
+package quic
+
+import (
+ "math"
+ "testing"
+ "time"
+)
+
+func TestAckDelayFromDuration(t *testing.T) {
+ for _, test := range []struct {
+ d time.Duration
+ ackDelayExponent uint8
+ want unscaledAckDelay
+ }{{
+ d: 8 * time.Microsecond,
+ ackDelayExponent: 3,
+ want: 1,
+ }, {
+ d: 1 * time.Nanosecond,
+ ackDelayExponent: 3,
+ want: 0, // rounds to zero
+ }, {
+ d: 3 * (1 << 20) * time.Microsecond,
+ ackDelayExponent: 20,
+ want: 3,
+ }} {
+ got := unscaledAckDelayFromDuration(test.d, test.ackDelayExponent)
+ if got != test.want {
+ t.Errorf("unscaledAckDelayFromDuration(%v, %v) = %v, want %v",
+ test.d, test.ackDelayExponent, got, test.want)
+ }
+ }
+}
+
+func TestAckDelayToDuration(t *testing.T) {
+ for _, test := range []struct {
+ d unscaledAckDelay
+ ackDelayExponent uint8
+ want time.Duration
+ }{{
+ d: 1,
+ ackDelayExponent: 3,
+ want: 8 * time.Microsecond,
+ }, {
+ d: 0,
+ ackDelayExponent: 3,
+ want: 0,
+ }, {
+ d: 3,
+ ackDelayExponent: 20,
+ want: 3 * (1 << 20) * time.Microsecond,
+ }, {
+ d: math.MaxInt64 / 1000,
+ ackDelayExponent: 0,
+ want: (math.MaxInt64 / 1000) * time.Microsecond,
+ }, {
+ d: (math.MaxInt64 / 1000) + 1,
+ ackDelayExponent: 0,
+ want: 0, // return 0 on overflow
+ }, {
+ d: math.MaxInt64 / 1000 / 8,
+ ackDelayExponent: 3,
+ want: (math.MaxInt64 / 1000 / 8) * 8 * time.Microsecond,
+ }, {
+ d: (math.MaxInt64 / 1000 / 8) + 1,
+ ackDelayExponent: 3,
+ want: 0, // return 0 on overflow
+ }} {
+ got := test.d.Duration(test.ackDelayExponent)
+ if got != test.want {
+ t.Errorf("unscaledAckDelay(%v).Duration(%v) = %v, want %v",
+ test.d, test.ackDelayExponent, int64(got), int64(test.want))
+ }
+ }
+}
diff --git a/internal/quic/frame_debug.go b/internal/quic/frame_debug.go
index 93ddf55..945bb9d 100644
--- a/internal/quic/frame_debug.go
+++ b/internal/quic/frame_debug.go
@@ -8,7 +8,6 @@
import (
"fmt"
- "time"
)
// A debugFrame is a representation of the contents of a QUIC frame,
@@ -115,13 +114,13 @@
// debugFrameAck is an ACK frame.
type debugFrameAck struct {
- ackDelay time.Duration
+ ackDelay unscaledAckDelay
ranges []i64range[packetNumber]
}
func parseDebugFrameAck(b []byte) (f debugFrameAck, n int) {
f.ranges = nil
- _, f.ackDelay, n = consumeAckFrame(b, ackDelayExponent, func(start, end packetNumber) {
+ _, f.ackDelay, n = consumeAckFrame(b, func(start, end packetNumber) {
f.ranges = append(f.ranges, i64range[packetNumber]{
start: start,
end: end,
@@ -144,7 +143,7 @@
}
func (f debugFrameAck) write(w *packetWriter) bool {
- return w.appendAckFrame(rangeset[packetNumber](f.ranges), ackDelayExponent, f.ackDelay)
+ return w.appendAckFrame(rangeset[packetNumber](f.ranges), f.ackDelay)
}
// debugFrameResetStream is a RESET_STREAM frame.
diff --git a/internal/quic/packet_codec_test.go b/internal/quic/packet_codec_test.go
index efd519b..499ec4d 100644
--- a/internal/quic/packet_codec_test.go
+++ b/internal/quic/packet_codec_test.go
@@ -11,7 +11,6 @@
"crypto/tls"
"reflect"
"testing"
- "time"
)
func TestParseLongHeaderPacket(t *testing.T) {
@@ -219,9 +218,9 @@
0x01, // TYPE(i) = 0x01
},
}, {
- s: "ACK Delay=80µs [0,16) [17,32) [48,64)",
+ s: "ACK Delay=10 [0,16) [17,32) [48,64)",
f: debugFrameAck{
- ackDelay: (10 << ackDelayExponent) * time.Microsecond,
+ ackDelay: 10,
ranges: []i64range[packetNumber]{
{0x00, 0x10},
{0x11, 0x20},
@@ -594,7 +593,7 @@
}, {
desc: "ACK frame with ECN counts",
want: debugFrameAck{
- ackDelay: (10 << ackDelayExponent) * time.Microsecond,
+ ackDelay: 10,
ranges: []i64range[packetNumber]{
{0, 1},
},
diff --git a/internal/quic/packet_parser.go b/internal/quic/packet_parser.go
index cc025b6..e910e0e 100644
--- a/internal/quic/packet_parser.go
+++ b/internal/quic/packet_parser.go
@@ -6,10 +6,6 @@
package quic
-import (
- "time"
-)
-
// parseLongHeaderPacket parses a QUIC long header packet.
//
// It does not parse Version Negotiation packets.
@@ -166,7 +162,7 @@
// which includes both general parse failures and specific violations of frame
// constraints.
-func consumeAckFrame(frame []byte, ackDelayExponent uint8, f func(start, end packetNumber)) (largest packetNumber, ackDelay time.Duration, n int) {
+func consumeAckFrame(frame []byte, f func(start, end packetNumber)) (largest packetNumber, ackDelay unscaledAckDelay, n int) {
b := frame[1:] // type
largestAck, n := consumeVarint(b)
@@ -175,12 +171,12 @@
}
b = b[n:]
- ackDelayScaled, n := consumeVarint(b)
+ v, n := consumeVarintInt64(b)
if n < 0 {
return 0, 0, -1
}
b = b[n:]
- ackDelay = time.Duration(ackDelayScaled*(1<<ackDelayExponent)) * time.Microsecond
+ ackDelay = unscaledAckDelay(v)
ackRangeCount, n := consumeVarint(b)
if n < 0 {
diff --git a/internal/quic/packet_writer.go b/internal/quic/packet_writer.go
index bfe9af7..97987e0 100644
--- a/internal/quic/packet_writer.go
+++ b/internal/quic/packet_writer.go
@@ -8,7 +8,6 @@
import (
"encoding/binary"
- "time"
)
// A packetWriter constructs QUIC datagrams.
@@ -257,21 +256,20 @@
// to the peer potentially failing to receive an acknowledgement
// for an older packet during a period of high packet loss or
// reordering. This may result in unnecessary retransmissions.
-func (w *packetWriter) appendAckFrame(seen rangeset[packetNumber], ackDelayExponent uint8, delay time.Duration) (added bool) {
+func (w *packetWriter) appendAckFrame(seen rangeset[packetNumber], delay unscaledAckDelay) (added bool) {
if len(seen) == 0 {
return false
}
var (
largest = uint64(seen.max())
- mdelay = uint64(delay.Microseconds() / (1 << ackDelayExponent))
firstRange = uint64(seen[len(seen)-1].size() - 1)
)
- if w.avail() < 1+sizeVarint(largest)+sizeVarint(mdelay)+1+sizeVarint(firstRange) {
+ if w.avail() < 1+sizeVarint(largest)+sizeVarint(uint64(delay))+1+sizeVarint(firstRange) {
return false
}
w.b = append(w.b, frameTypeAck)
w.b = appendVarint(w.b, largest)
- w.b = appendVarint(w.b, mdelay)
+ w.b = appendVarint(w.b, uint64(delay))
// The range count is technically a varint, but we'll reserve a single byte for it
// and never add more than 62 ranges (the maximum varint that fits in a byte).
rangeCountOff := len(w.b)
