From 2765a2c14ecb2a326384ab923c53115998603e14 Mon Sep 17 00:00:00 2001 From: CMGS Date: Tue, 10 Nov 2026 21:54:03 +0800 Subject: [PATCH 1/3] test: two v0.1.31 flakes get their root causes fixed TestChangingTheLimitMovesAnOpenTunnel failed 2/40 under -race on linux/arm64: a limit switch lets through the unlimited chunk already in flight (splice checks the kicked deadline between chunks), which reached the test's 512 KiB target in under a millisecond. The test now measures the new rate from 50 ms after the switch, and egress.md and SetLimit say the switch lands after the chunk in flight. TestDisablingARunningClaimRevokesItsPacketPath timed out under full-suite load: the claim's first tun_up runs in a goroutine, and a disable that beats it records down, up, down, while the test waited for up, down. It now waits for the first tun_up before disabling, and a new test pins that a disable beating the first tun_up still leaves the guest down. --- docs/egress.md | 5 +++-- sandboxd/egress/limit.go | 2 +- sandboxd/egress/limit_test.go | 7 ++++--- sandboxd/pool/tun_test.go | 20 ++++++++++++++++++++ 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/docs/egress.md b/docs/egress.md index 117930c3a..70e2761cd 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -592,8 +592,9 @@ map update per tunnel and an atomic load per request read. A changed rate starts with a full bucket, a direction whose rate did not change keeps its bucket, and a repeated cap changes nothing. A change reaches every open connection at once: the proxy sets a past read deadline on every open tunnel, -whose copy resumes in the matching mode, and a request reads the cap on every -read. A claim in [advanced network mode](advanced-network.md#host-side) keeps +whose copy resumes in the matching mode once the chunk in flight is through (a +spliced chunk can run to a few hundred KiB at the old rate), and a request reads +the cap on every read. A claim in [advanced network mode](advanced-network.md#host-side) keeps separate buckets on its tun path, so its proxy and tun traffic are capped apart. diff --git a/sandboxd/egress/limit.go b/sandboxd/egress/limit.go index 7e1211f1c..6c69b2ec6 100644 --- a/sandboxd/egress/limit.go +++ b/sandboxd/egress/limit.go @@ -26,7 +26,7 @@ const ( var limitBufs = sync.Pool{New: func() any { b := make([]byte, limitBurst); return &b }} -// SetLimit caps the claim's bytes per second each way and moves every open tunnel onto the new cap at once. +// SetLimit caps the claim's bytes per second each way and moves every open tunnel onto the new cap after the chunk it is copying. func (p *Proxy) SetLimit(l types.EgressLimit) { cur := p.limits.Load() if cur == nil && l == (types.EgressLimit{}) || cur != nil && cur.set == l { diff --git a/sandboxd/egress/limit_test.go b/sandboxd/egress/limit_test.go index 2fc3e3905..0663928e8 100644 --- a/sandboxd/egress/limit_test.go +++ b/sandboxd/egress/limit_test.go @@ -62,11 +62,12 @@ func TestChangingTheLimitMovesAnOpenTunnel(t *testing.T) { waitBytes(t, s, 1<<20) p.SetLimit(types.EgressLimit{TxBytesPerSecond: testRate / 2}) - start := time.Now() go writeUntilClosed(conn) - waitBytes(t, s, 1<<20+limitBurst+testRate/2) + time.Sleep(50 * time.Millisecond) + from, start := s.received.Load(), time.Now() + waitBytes(t, s, from+testRate/2) if took := time.Since(start); took < 700*time.Millisecond { - t.Errorf("a burst and a second's worth past it took %v after the limit dropped to %d/s", took, testRate/2) + t.Errorf("a second's worth took %v once the limit dropped to %d/s", took, testRate/2) } p.SetLimit(types.EgressLimit{}) diff --git a/sandboxd/pool/tun_test.go b/sandboxd/pool/tun_test.go index e8694def2..673242dcb 100644 --- a/sandboxd/pool/tun_test.go +++ b/sandboxd/pool/tun_test.go @@ -154,6 +154,7 @@ func TestDisablingARunningClaimRevokesItsPacketPath(t *testing.T) { m := tunManager(t, eng, addrs, config.PoolSpec{PoolKey: testKey, Tun: true, Egress: anyPolicy}) sb := mustClaim(t, m, testKey) stream := openStream(t, sb) + waitFor(t, func() bool { return slices.Equal(tunVerbsOf(eng), []string{"up " + sb.VsockSocket}) }) if err := m.SetTun(t.Context(), sb.ID, new(false), nil); err != nil { t.Fatalf("disable: %v", err) } @@ -175,6 +176,25 @@ func TestDisablingARunningClaimRevokesItsPacketPath(t *testing.T) { } } +func TestADisableBeforeTheFirstTunUpLeavesTheGuestDown(t *testing.T) { + eng, addrs := newFakeEngine(), &fakeAddrs{} + eng.tunUpGate = make(chan struct{}) + m := tunManager(t, eng, addrs, config.PoolSpec{PoolKey: testKey, Tun: true, Egress: anyPolicy}) + sb := mustClaim(t, m, testKey) + if err := m.SetTun(t.Context(), sb.ID, new(false), nil); err != nil { + t.Fatalf("disable: %v", err) + } + waitFor(t, func() bool { return slices.Contains(tunVerbsOf(eng), "down "+sb.VsockSocket) }) + close(eng.tunUpGate) + waitFor(t, func() bool { + verbs := tunVerbsOf(eng) + return len(verbs) == 3 && verbs[len(verbs)-1] == "down "+sb.VsockSocket + }) + if doorOpen(sb) { + t.Error("the packet door accepts after a disable that beat the first tun_up") + } +} + func TestARearmWhileTunUpIsInFlightLeavesTheGuestUp(t *testing.T) { eng, addrs := newFakeEngine(), &fakeAddrs{} eng.tunUpGate = make(chan struct{}) From 1b0a47da7b60e58a62c93b80be09d5555bb61426 Mon Sep 17 00:00:00 2001 From: CMGS Date: Tue, 10 Nov 2026 22:19:44 +0800 Subject: [PATCH 2/3] test: the limit-switch test pins its bound, and the tun test tolerates a redundant down The diagnostic showed which copy moved the extra bytes: the tx spliced copy, parked in waitRead when the kick set its past deadline, woke on data instead. runtime.poll_runtime_pollWait returns ready when the fd is readable even with the deadline expired, so spliceDrain moved what the socket held, up to maxSpliceSize (1 MiB), before the next prepareRead returned the timeout. The test now runs at 2 MiB/s, requires a burst, one splice chunk and 0.75 s at the new cap to take 700 ms or more, and fails when more than the burst and one chunk pass the cap's pace, which a 50 ms late kick does. egress.md and SetLimit name that bound. The tun test accepts the down the arm goroutine repeats when a disable lands between tun_up and its armed check. --- docs/egress.md | 6 +++--- sandboxd/egress/limit.go | 2 +- sandboxd/egress/limit_test.go | 17 +++++++++++------ sandboxd/pool/tun_test.go | 3 ++- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/docs/egress.md b/docs/egress.md index 70e2761cd..3ecbc06b2 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -592,9 +592,9 @@ map update per tunnel and an atomic load per request read. A changed rate starts with a full bucket, a direction whose rate did not change keeps its bucket, and a repeated cap changes nothing. A change reaches every open connection at once: the proxy sets a past read deadline on every open tunnel, -whose copy resumes in the matching mode once the chunk in flight is through (a -spliced chunk can run to a few hundred KiB at the old rate), and a request reads -the cap on every read. A claim in [advanced network mode](advanced-network.md#host-side) keeps +whose copy resumes in the matching mode; an uncapped direction can pass one more +splice, up to 1 MiB at the old, uncapped rate, when its data arrives together +with the deadline, and a request reads the cap on every read. A claim in [advanced network mode](advanced-network.md#host-side) keeps separate buckets on its tun path, so its proxy and tun traffic are capped apart. diff --git a/sandboxd/egress/limit.go b/sandboxd/egress/limit.go index 6c69b2ec6..35fc9aa04 100644 --- a/sandboxd/egress/limit.go +++ b/sandboxd/egress/limit.go @@ -26,7 +26,7 @@ const ( var limitBufs = sync.Pool{New: func() any { b := make([]byte, limitBurst); return &b }} -// SetLimit caps the claim's bytes per second each way and moves every open tunnel onto the new cap after the chunk it is copying. +// SetLimit caps the claim's bytes per second each way and moves every open tunnel onto the new cap after at most one more splice of an uncapped direction. func (p *Proxy) SetLimit(l types.EgressLimit) { cur := p.limits.Load() if cur == nil && l == (types.EgressLimit{}) || cur != nil && cur.set == l { diff --git a/sandboxd/egress/limit_test.go b/sandboxd/egress/limit_test.go index 0663928e8..1220fd231 100644 --- a/sandboxd/egress/limit_test.go +++ b/sandboxd/egress/limit_test.go @@ -61,13 +61,18 @@ func TestChangingTheLimitMovesAnOpenTunnel(t *testing.T) { } waitBytes(t, s, 1<<20) - p.SetLimit(types.EgressLimit{TxBytesPerSecond: testRate / 2}) + const rate, spliceChunk = 2 << 20, 1 << 20 + base := s.received.Load() + p.SetLimit(types.EgressLimit{TxBytesPerSecond: rate}) + start := time.Now() go writeUntilClosed(conn) - time.Sleep(50 * time.Millisecond) - from, start := s.received.Load(), time.Now() - waitBytes(t, s, from+testRate/2) - if took := time.Since(start); took < 700*time.Millisecond { - t.Errorf("a second's worth took %v once the limit dropped to %d/s", took, testRate/2) + waitBytes(t, s, base+rate+spliceChunk+rate*3/4) + took := time.Since(start) + if took < 700*time.Millisecond { + t.Errorf("a burst, a splice chunk and 0.75 s at the new cap took %v once the limit dropped to %d/s", took, rate) + } + if over := s.received.Load() - base - int64(took.Seconds()*rate); over > rate+spliceChunk+rate/limitSlices { + t.Errorf("%d bytes past the cap's pace once the limit dropped, want at most its burst and one splice chunk", over) } p.SetLimit(types.EgressLimit{}) diff --git a/sandboxd/pool/tun_test.go b/sandboxd/pool/tun_test.go index 673242dcb..dd85036e3 100644 --- a/sandboxd/pool/tun_test.go +++ b/sandboxd/pool/tun_test.go @@ -166,7 +166,8 @@ func TestDisablingARunningClaimRevokesItsPacketPath(t *testing.T) { t.Error("the packet door still accepts after the disable") } waitFor(t, func() bool { - return slices.Equal(tunVerbsOf(eng), []string{"up " + sb.VsockSocket, "down " + sb.VsockSocket}) + verbs := tunVerbsOf(eng) + return len(verbs) >= 2 && verbs[0] == "up "+sb.VsockSocket && !slices.ContainsFunc(verbs[1:], func(v string) bool { return v != "down "+sb.VsockSocket }) }) if err := m.SetTun(t.Context(), sb.ID, new(true), nil); err != nil { t.Fatalf("enable: %v", err) From da5fc215718873e8e70f7acaaf3ea98a7391a73e Mon Sep 17 00:00:00 2001 From: CMGS Date: Tue, 10 Nov 2026 22:25:41 +0800 Subject: [PATCH 3/3] review: the limit-switch test reads its byte count before its clock, and the remaining at-once docs name the splice bound Taking the time before loading the sink count let a scheduler stall between them count as bytes past the pace. The local rate constant no longer shadows x/time/rate. sandboxd-api.md and SetEgressLimit carry the same one-more-splice caveat as egress.md, which is rewrapped. --- docs/egress.md | 6 +++--- docs/sandboxd-api.md | 3 ++- sandboxd/egress/limit_test.go | 11 ++++++----- sandboxd/pool/egress.go | 2 +- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/docs/egress.md b/docs/egress.md index 3ecbc06b2..04d37f30d 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -594,9 +594,9 @@ bucket, and a repeated cap changes nothing. A change reaches every open connection at once: the proxy sets a past read deadline on every open tunnel, whose copy resumes in the matching mode; an uncapped direction can pass one more splice, up to 1 MiB at the old, uncapped rate, when its data arrives together -with the deadline, and a request reads the cap on every read. A claim in [advanced network mode](advanced-network.md#host-side) keeps -separate buckets on its tun path, so its proxy and tun traffic are capped -apart. +with the deadline, and a request reads the cap on every read. A claim in +[advanced network mode](advanced-network.md#host-side) keeps separate buckets on +its tun path, so its proxy and tun traffic are capped apart. A limited download paces what the proxy reads, not what the far end sends: the upstream can run ahead by up to the node's TCP receive buffer per connection diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index 75cc628ed..eda87c223 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -1274,7 +1274,8 @@ cap. It covers the claim's [proxy doors](egress.md#bandwidth-limit) and, in [advanced network mode](advanced-network.md#host-side), replaces `tun.limits.bytes_per_second` for the claim in each direction it sets. Preview, port relay and exec traffic is not capped. On a running claim the change -applies at once to open connections, without a reconnect. The cap is persisted +applies at once to open connections, without a reconnect; an uncapped direction +can pass one more splice, up to 1 MiB at the old rate. The cap is persisted with the claim, follows it through hibernate, archive, a live move, an archive row and an adoption, and is accepted on a hibernated or archived claim without waking it; its next wake applies it. A running claim's replica row under diff --git a/sandboxd/egress/limit_test.go b/sandboxd/egress/limit_test.go index 1220fd231..223e7874a 100644 --- a/sandboxd/egress/limit_test.go +++ b/sandboxd/egress/limit_test.go @@ -61,17 +61,18 @@ func TestChangingTheLimitMovesAnOpenTunnel(t *testing.T) { } waitBytes(t, s, 1<<20) - const rate, spliceChunk = 2 << 20, 1 << 20 + const perSecond, spliceChunk = 2 << 20, 1 << 20 base := s.received.Load() - p.SetLimit(types.EgressLimit{TxBytesPerSecond: rate}) + p.SetLimit(types.EgressLimit{TxBytesPerSecond: perSecond}) start := time.Now() go writeUntilClosed(conn) - waitBytes(t, s, base+rate+spliceChunk+rate*3/4) + waitBytes(t, s, base+perSecond+spliceChunk+perSecond*3/4) + got := s.received.Load() took := time.Since(start) if took < 700*time.Millisecond { - t.Errorf("a burst, a splice chunk and 0.75 s at the new cap took %v once the limit dropped to %d/s", took, rate) + t.Errorf("a burst, a splice chunk and 0.75 s at the new cap took %v once the limit dropped to %d/s", took, perSecond) } - if over := s.received.Load() - base - int64(took.Seconds()*rate); over > rate+spliceChunk+rate/limitSlices { + if over := got - base - int64(took.Seconds()*perSecond); over > perSecond+spliceChunk+perSecond/limitSlices { t.Errorf("%d bytes past the cap's pace once the limit dropped, want at most its burst and one splice chunk", over) } diff --git a/sandboxd/pool/egress.go b/sandboxd/pool/egress.go index e6336ece7..56863c1a8 100644 --- a/sandboxd/pool/egress.go +++ b/sandboxd/pool/egress.go @@ -14,7 +14,7 @@ import ( "github.com/cocoonstack/sandbox/sandboxd/usage" ) -// SetEgressLimit replaces a claim's bandwidth cap without waking a hibernated or archived claim; open connections move onto it at once. +// SetEgressLimit replaces a claim's bandwidth cap without waking a hibernated or archived claim; open connections move onto it at once, after at most one more splice of an uncapped direction. func (m *Manager) SetEgressLimit(ctx context.Context, id string, l types.EgressLimit) error { m.mu.Lock() sb := m.claimed[id]