http2: fix weight overflow in RFC 7540 write scheduler

We use uint8 (0-255, inclusive) to represent the RFC 7540 priorities
weight (1-256, inclusive). To account for the difference, we add 1 to
the uint8 weight value within sortPriorityNodeSiblingsRFC7540.

However, the addition was done before converting the uint8 type to
float. As a result, when provided a maximum weight value, overflow will
happen and will cause the scheduler to treat the maximum weight as a
minimum weight instead.

This CL fixes the issue by making sure the addition happens after the
type conversion.

Change-Id: I404e87e5ad85fa06d5fa49cda613c93ac8847bdc
Reviewed-on: https://go-review.googlesource.com/c/net/+/714742
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
This commit is contained in:
Nicholas S. Husin
2025-10-24 16:26:54 -04:00
committed by Nicholas Husin
parent 89adc90ac4
commit f35e3a4dd2
2 changed files with 35 additions and 2 deletions

View File

@@ -214,8 +214,8 @@ func (z sortPriorityNodeSiblingsRFC7540) Swap(i, k int) { z[i], z[k] = z[k], z[i
func (z sortPriorityNodeSiblingsRFC7540) Less(i, k int) bool {
// Prefer the subtree that has sent fewer bytes relative to its weight.
// See sections 5.3.2 and 5.3.4.
wi, bi := float64(z[i].weight+1), float64(z[i].subtreeBytes)
wk, bk := float64(z[k].weight+1), float64(z[k].subtreeBytes)
wi, bi := float64(z[i].weight)+1, float64(z[i].subtreeBytes)
wk, bk := float64(z[k].weight)+1, float64(z[k].subtreeBytes)
if bi == 0 && bk == 0 {
return wi >= wk
}

View File

@@ -548,6 +548,39 @@ func TestPriorityWeights(t *testing.T) {
}
}
func TestPriorityWeightsMinMax(t *testing.T) {
ws := defaultPriorityWriteScheduler()
ws.OpenStream(1, OpenStreamOptions{})
ws.OpenStream(2, OpenStreamOptions{})
sc := &serverConn{maxFrameSize: 8}
st1 := &stream{id: 1, sc: sc}
st2 := &stream{id: 2, sc: sc}
st1.flow.add(40)
st2.flow.add(40)
// st2 gets 256x the bandwidth of st1 (256 = (255+1)/(0+1)).
// The maximum frame size is 8 bytes. The write sequence should be:
// st2, total bytes so far is (st1=0, st=8)
// st1, total bytes so far is (st1=8, st=8)
// st2, total bytes so far is (st1=8, st=16)
// st2, total bytes so far is (st1=8, st=24)
// st2, total bytes so far is (st1=8, st=32)
// st2, total bytes so far is (st1=8, st=40) // 5x bandwidth
// st1, total bytes so far is (st1=16, st=40)
// st1, total bytes so far is (st1=24, st=40)
// st1, total bytes so far is (st1=32, st=40)
// st1, total bytes so far is (st1=40, st=40)
ws.Push(FrameWriteRequest{&writeData{1, make([]byte, 40), false}, st1, nil})
ws.Push(FrameWriteRequest{&writeData{2, make([]byte, 40), false}, st2, nil})
ws.AdjustStream(1, PriorityParam{StreamDep: 0, Weight: 0})
ws.AdjustStream(2, PriorityParam{StreamDep: 0, Weight: 255})
if err := checkPopAll(ws, []uint32{2, 1, 2, 2, 2, 2, 1, 1, 1, 1}); err != nil {
t.Error(err)
}
}
func TestPriorityRstStreamOnNonOpenStreams(t *testing.T) {
ws := NewPriorityWriteScheduler(&PriorityWriteSchedulerConfig{
MaxClosedNodesInTree: 0,