Repository navigation
netstack hands the node a pass's received frames as one batch, about 15% fewer pipe calls on the T14, and a reset keeps the text received before it, as Linux and the BSDs do - #828
Conversation
|
Mutation patches at b1-a-pass-a-framediff --git a/userland/netstack/node/src/lib.rs b/userland/netstack/node/src/lib.rs
--- a/userland/netstack/node/src/lib.rs
+++ b/userland/netstack/node/src/lib.rs
@@ -158,6 +158,7 @@ impl Node {
while next(&mut |frame| {
self.stack.receive(now, frame);
self.settle(now, &mut draw);
+ self.bridge(now);
}) {}
self.bridge(now);
}b2-no-pass-after-the-batchdiff --git a/userland/netstack/node/src/lib.rs b/userland/netstack/node/src/lib.rs
--- a/userland/netstack/node/src/lib.rs
+++ b/userland/netstack/node/src/lib.rs
@@ -159,7 +159,6 @@ impl Node {
self.stack.receive(now, frame);
self.settle(now, &mut draw);
}) {}
- self.bridge(now);
}
/// A transmit opportunity with room for `credit` frames, each handed to `sink` as it is built.The profile's instrumentation and the host profile test
|
|
Logs this PR's body rests on: each log's first line (its head) and the lines the body cites. Paths are scrubbed. prof-host-node-base.logprof-host-node-after.logprof-qemu-base.logprof-qemu-after.lognode-tests-1.lognode-tests-2.logmut-b1-a-pass-a-frame.logmut-b2-no-pass-after-the-batch.logmut2-b1-a-pass-a-frame.logmut2-b2-no-pass-after-the-batch.logbuild-only-x86_64.logbuild-only-aarch64.logci-host.logci-host-2.logguest.logmetal-stage.log |
|
T14 staged, not run (the T14 is the orchestrator's): measurement branch
What the reading decides: |
|
T14 at |
|
Review of #828, round 1, at Net lines ( Checked, no finding:
BLOCKER
NOTE
SEND BACK |
…ed by one pass over the streams Node::receive took one frame and ended in a pass over every stream, so a bulk download cost netstack one write of the client's pipe and one empty read of its send pipe per frame, and the client woke once per write. A QEMU e1000e profile of a 64 MiB download from the host at 6df8222 counted, per 64 MiB: 46,609 frames in 1,081 passes (43 a pass), 46,604 pipe writes, 46,669 empty send-pipe reads and 1,077 ACKs; the reader made 21,748 calls of a 16,389-byte buffer (3,086 bytes each) and 19,956 of a 64 KiB one (3,363 each), so its read size was set by netstack's writes and not by its buffer. Node::receive now takes the pass's frames through a pull closure, each frame handed to the stack and settled as before, and one pass over the streams follows the last. netstack passes Card::rx to it. The pass still precedes the transmit opportunity, so the ACK carries the window the drain opened, as before. What it buys is set by how many frames a pass holds. Under QEMU's TCG, 43 a pass, the same download made 2,103 pipe writes over 1,089 passes. On the T14's I219, which is programmed with no interrupt moderation, almost every pass held 0 to 2 frames: one warm download counted 71,384 pipe writes over 101,403 passes and 119,890 frames, about 15% fewer pipe calls than one a frame. No CPU or bandwidth change is measured: one boot, CPU per MB 13.15 ms warm and 13.49 cold, against 13.5 to 15.4 across #820's. The batches grow only once the card holds its interrupt for more frames, which is a change of its own. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
…y's type-complexity lint asks for Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
…ads ahead of the failure, as Linux and the BSDs do
A peer's reset in a synchronized state ended the connection with its
receive buffer dropped, as RFC 9293 section 3.10.7.4's SHOULD says of every
queue: recv answered Failed(Reset) at once, whatever text was in order and
unread. With netstack handing the node a pass's frames as one batch, text
and the reset after it in the same pass were both taken before the stream's
pass, so the client lost the text, where a pass between the two frames had
already written it to its pipe: what a client read depended on how the card
batched frames. On the T14 44,612 of 101,403 passes held two frames.
Linux keeps the receive queue readable ahead of ECONNRESET (tcp_reset purges
the write queue only, and tcp_recvmsg walks sk_receive_queue before it
checks sk_err), and the BSDs' soreceive returns buffered data before
so_error. ToyOS's clients are programs written against those stacks, so the
reset now ends the connection with its receive buffer kept
(Ended { failure: Some(Reset), rx }), and recv and recv_with give its text
before Err(Failed). Text already in a pipe cannot be flushed afterwards, so
keeping it is also the only rule that makes what a client reads independent
of batch boundaries. The orchestrator ruled for it.
s_rx_023 asserted the flush and now asserts the text, then the failure.
s_ac_005's claim is that an RST at the right edge of a zero window lands; it
reads the status, since the unread window is now readable ahead of the
failure. The node test holds the batch: text and a reset in one batch put
the text in the client's pipe before both ends go. Both new assertions red
with the tcp change reverted.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
…the I219's strands what it leaves VirtioNet::poll_rx answers frames while the used ring holds one, so a device refilling the ring as fast as it is read holds the pass in receive, now with every stream's pass behind it too. The I219's RX_BUDGET is no shape to copy: the frames past it raised interrupts the pass already took, and netstack's loop does not wait zero after such a pass. The fix is one bound at Node::receive answering that a pass is owed, with the I219's budget deleted; that touches toyos-i219, outside this branch's brief. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
f4898ce to
350fb23
Compare
|
Round 2 negative control at diff --git b/toyos-net-shard/tcp/src/stack.rs a/toyos-net-shard/tcp/src/stack.rs
index 6fdd2cf6a..acb35480f 100644
--- b/toyos-net-shard/tcp/src/stack.rs
+++ a/toyos-net-shard/tcp/src/stack.rs
@@ -100,8 +100,7 @@ enum User {
struct Ended {
failure: Option<Failure>,
- /// After both FINs or the peer's reset: what the user has still to read, ahead of the failure
- /// (Linux and the BSDs, not RFC 9293 §3.10.7.4's SHOULD-flush).
+ /// After both FINs: what the user has still to read.
rx: Option<Box<Rx>>,
}
@@ -475,7 +474,7 @@ impl Tcp {
}
/// A connection's end: the user who holds it keeps the socket to learn why (and to read what
- /// both FINs or a reset left); anyone else's is freed.
+ /// both FINs left); anyone else's is freed.
fn end(&mut self, index: u32, failure: Option<Failure>, rx: Option<Box<Rx>>) {
let Some(conn) = value(&mut self.conns, index) else { return };
match conn.user {
@@ -888,7 +887,7 @@ impl Tcp {
self.settle(index, now);
}
Verdict::Closed => self.end(index, None, Some(Box::new(sync.rx))),
- Verdict::Reset => self.end(index, Some(Failure::Reset), Some(Box::new(sync.rx))),
+ Verdict::Reset => self.end(index, Some(Failure::Reset), None),
Verdict::TimeWait => {
let tw = TimeWait::from_sync(&sync, now);
let owed = sync.rx.ack_now || sync.rx.dup_owed > 0 || sync.rx.delayed.is_some();
@@ -1011,9 +1010,10 @@ impl Tcp {
let result = match &mut conn.state {
Tcb::SynSent(_) | Tcb::SynRcvd(_) => Err(Error::WouldBlock),
Tcb::Sync(sync) => sync.recv(out, now),
- Tcb::Ended(Ended { failure, rx }) => match rx.as_mut().map(|rx| rx.read(out)) {
+ Tcb::Ended(Ended { failure: Some(failure), .. }) => Err(Error::Failed(*failure)),
+ Tcb::Ended(Ended { failure: None, rx }) => match rx.as_mut().map(|rx| rx.read(out)) {
Some(n) if n > 0 => Ok(Received::Data(n)),
- _ => failure.map_or(Ok(Received::End), |f| Err(Error::Failed(f))),
+ _ => Ok(Received::End),
},
};
self.settle(id.index, now);
@@ -1029,9 +1029,10 @@ impl Tcp {
let result = match &mut conn.state {
Tcb::SynSent(_) | Tcb::SynRcvd(_) => Err(Error::WouldBlock),
Tcb::Sync(sync) => sync.recv_with(take, now),
- Tcb::Ended(Ended { failure, rx }) => match rx.as_mut() {
+ Tcb::Ended(Ended { failure: Some(failure), .. }) => Err(Error::Failed(*failure)),
+ Tcb::Ended(Ended { failure: None, rx }) => match rx.as_mut() {
Some(rx) if rx.unread() > 0 => Ok(Received::Data(rx.read_with(take))),
- _ => failure.map_or(Ok(Received::End), |f| Err(Error::Failed(f))),
+ _ => Ok(Received::End),
},
};
self.settle(id.index, now);negative-control.log |
|
Round 2 gate logs at crates.logbuild-x86_64.logbuild-aarch64.logci-host.logguest.log |
|
Review of #828, round 2, at Round 1, checked against the evidence
Batch code: Gates at
Net lines ( BLOCKER
NOTE
SEND BACK |
…nsole wire (#805), into wt/toyos-netperf Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
…that text is read The review of round 2 named the gap: every reset test had room in the client's pipe, so nothing showed the text outliving the pass that saw the reset. text_the_pipe_had_no_room_for_outlives_the_reset is the FIN sibling's reset version; it reds when a failure drops the to-client pipe with the from-client pipe. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
…h and reset lines as they now hold RFC 9293 section 3.10.7.4 SHOULD-flushes on a reset; [tcp] keeps the text before it, the orchestrator's ruling to follow Linux and the BSDs, and the track now lists that among its departures with an exit. A stream its peer reset goes once its client's pipe has taken that text, not at once, in the track and in the nodelay issue. The node passes every stream once per batch of received frames, not per frame. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
|
Round 3 mutation at mutation.patch--- a/userland/netstack/node/src/streams.rs
+++ b/userland/netstack/node/src/streams.rs
@@ -303,3 +303,4 @@
if status.failure.is_some() {
self.from_client = None;
+ self.to_client = None;
}mutation.shset -u
cd <dev>/toyos-netperf
P=<logs>/mutation.patch
echo "head: $(git rev-parse HEAD)"
git apply --check "$P"; echo CHECK=$?
git apply "$P"; echo APPLIED=$?
git diff
cargo test -p toyos-net-node --test streams text_the_pipe_had_no_room_for_outlives_the_reset; echo MUTANT_EXIT=$?
git apply -R "$P"; echo RESTORED=$?
echo "porcelain: [$(git status --porcelain --ignore-submodules=none)]"mutation.log |
|
Round 3 gate logs at crates.loghost.logbuild-x86_64.logbuild-aarch64.log |
|
Review of #828, round 3, at Round 2, checked against the evidence
Merge Net lines ( BLOCKER
NOTENone. SEND BACK |
|
Guest suite at |
|
Review of #828, round 4, at Round 3, checked against the evidence
Nothing changed since round 3's head, so there are no new findings. BLOCKERNone. NOTENone. LAND |
, #826, #835, #833, #831 and #822, into wt/toyos-netperf No hunk conflicted. userland/netstack/src/main.rs took both sides: main's removal of `mod device` and the branch's batched `node.receive` and its module-doc line. The TCP window-scaling and loss-probe commits main carries were already in the branch from #820, so their files merged to main's text plus the branch's own delta. Both lockfiles are main's and pass `cargo metadata --locked`. The branch's new issue still cites `VirtioNet::poll_rx` and `toyos_i219::RX_BUDGET` as they stand on main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
|
build-x86_64 log, part 1 of 1: |
|
build-aarch64 log, part 1 of 1: |
|
guest log, part 1 of 2: |
|
guest log, part 2 of 2: |
|
host log, part 1 of 10: |
|
host log, part 2 of 10: |
|
host log, part 3 of 10: |
|
host log, part 4 of 10: |
|
host log, part 5 of 10: |
|
host log, part 6 of 10: |
|
host log, part 7 of 10: |
|
host log, part 8 of 10: |
|
host log, part 9 of 10: |
|
host log, part 10 of 10: |
|
rerun log, part 1 of 1: |
|
Resolution note: Hunks. Git reported no conflicts. Only Lockfiles. Gates at
Every log is posted whole and unedited, and its parts concatenate byte for byte to the file (checked with |
#846, #843, #849, #850, #840, #839, #837) into the batch: the icons' and wallpaper's digests hash with toyos-sha2-hw, and the loader's wall clock reads through its own UEFI bindings The batch's merge of main at f72d53d moved #813's wallpaper and #814's icon digest tests onto `toyos_sha2`. #833, already on main, had replaced the root package's `toyos-sha2` dependency with `toyos-sha2-hw`, so CI's merge of the two compiled no `toyos-build` lib test and both the build system's tests and clippy went red with E0432. Both tests now hash through `toyos_sha2_hw`, as every other SHA-256 the build takes does. #842's `bootloader/src/wallclock.rs` was written on the `uefi` crate, which #815 removes; it now calls `efi`'s `RuntimeServices::get_time`, whose `Time` fields are plain and whose error is the `Status` itself. `start_kernel` takes main's `wall_clock` and the batch's `SystemTable`; the batch's `armed_at` goes, as main replaced it with `wallclock::now`. Cargo.lock takes main's `ntapi`; `nonempty` goes with `gix`, which the batch removed and which was its only user (`cargo metadata --offline`). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
, #836, #839, #840, #842 through #847, #849 and #850, into consent system.toml: sshserver and shell start both toyfetch (#843) and grants. tests/common/qemu.rs: Profile carries both Desktop and MetalAmdVi (#837). tests/toyos.rs: SCREEN_TESTS carries consent_prompt beside virt_wall_clock_utc (#842) and virt_low_ecam (#840). Beyond the conflict lines: #833 replaced the build's toyos-sha2 dependency with toyos-sha2-hw, so consent_prompt digests its job with toyos_sha2_hw::sha256_digest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
Built on #820, which has since landed on
mainas6ab2af582. This branch is #820's reviewed head49dca4e02merged withorigin/main(22df2c551, again ata1eb2c0b9in85415457e, and at1079e854aineb07e6ee0), plus its own commits, sogit diff origin/main...HEADshows only this branch's own change: 13 files, +127/−41; production (toyos-net-shard/tcp/src,userland/netstack/node/src,userland/netstack/src) +22/−18, tests +68/−20, one new issue file and two issue files edited.What a pass's batch buys, measured
QEMU, the e1000e (
Part::E82574, the driver the T14's I219 runs), x86-64 under TCG: a measurement-only test with a host thread sending 64 MiB over slirp to a guest job that reads the stream to its end, in calls of 16,389 bytes and then of 64 KiB. netstack counts its own passes. The instrumentation patch is in this PR's comments, and it never lands.6df8222c1T14, the I219, read by the orchestrator at
a61698510: this change'sf4898ced9with measurement-only commits that never land. One boot:download, judge EXIT=0. Warm download:mbps=844.9 cpu_ms_per_mb=13.15. Cold download:mbps=393.8 cpu_ms_per_mb=13.49.[prof]counted 101,403 passes, 119,890 frames and 71,384 pipe writes.What changed and why
Node::receivetakes a pass's frames as one batch, and one pass over the streams follows the last of them (userland/netstack/node/src/lib.rs).nexthands its frame to the sink it is given, and answersfalsewhen it had none. The card gives a buffer back as its frame is read, so it cannot lend a slice of frames.Card::rx(userland/netstack/src/main.rs).A peer's reset keeps the in-order text received before it, and the user reads that text before the failure (
toyos-net-shard/tcp/src/stack.rs). The orchestrator ruled for this, following Linux and the BSDs.rx: None, RFC 9293 §3.10.7.4's SHOULD-flush. Text and a reset in the same batch were both taken before the stream's pass, so the client lost the text. A pass between the two frames would already have written it to the client's pipe. What a client read therefore depended on how the card batched frames.Ended { failure: Some(Reset), rx }.recvandrecv_withgive that text first, thenErr(Failed(Reset)).ECONNRESET:tcp_resetpurges only the write queue, andtcp_recvmsgwalkssk_receive_queuebefore it checkssk_err. The BSDs'soreceivereturns buffered data beforeso_error. ToyOS's clients are programs written against those stacks.tcp_recv→recv_with, so a socket client also reads the text beforeECONNRESET.issues/toyos-has-its-own-network-stack.md), with the orchestrator's ruling and an exit. Its nodelay line andissues/a-stream-its-peer-reset-refuses-the-option-requests-a-host-answers.mdno longer say a reset ends a stream at once, and its pass line counts per batch, not per frame.virtio's receive has no per-pass budget: filed, not fixed (
issues/netstacks-virtio-receive-has-no-per-pass-budget.md).RX_BUDGETwould copy a defect, read from the code and not measured. A pass that stops at the budget leaves frames whose interrupts it already took, and netstack's loop does not make its next wait zero. Those frames wait for the next interrupt or deadline.Node::receivethat answers whether a pass is owed, with the I219's own budget deleted. That deletion is intoyos-i219, which this branch was not briefed to change.Tests and their controls
s_rx_023_a_reset_comes_after_the_text_before_it(crate,toyos-net-shard/tcp/tests/receive.rs, replacess_rx_023_a_reset_is_never_end_of_stream, which asserted the flush): 100 bytes in order, then an in-window RST.recvgivesData(100), thenFailed(Reset).a_reset_in_the_batch_of_the_text_before_it_ends_the_pipes_after_the_text(node,userland/netstack/node/tests/streams.rs):batch(&[&far.text(b"last"), &far.rst()])putsb"last"in the client's pipe, andlet_gois[FromClient, ToClient].text_the_pipe_had_no_room_for_outlives_the_reset(node, beside its FIN siblingtext_the_pipe_had_no_room_for_outlives_both_fins):room = 0, thenbatch(&[&far.text(b"held back"), &far.rst()]). The inbox is empty,let_gois[FromClient]and the stream is held. Thenroom = 65_536andbridge(): the inbox isb"held back",let_gois[FromClient, ToClient], and the stream is gone.s_ac_005_zero_window_rstmakes the claim that an RST at the right edge of a zero window lands. It now reads the status rather thanrecv, because the unread window is now readable ahead of the failure, which iss_rx_023's claim.a_batch_of_frames_moves_an_established_stream_once_with_no_opportunity_after_it: three segments in one batch reach the client's pipe whole, in one write, and its send pipe is read once.Negative control, the reset (
negative-control.log):git apply --check, thengit apply), and that patch is in this PR's comments.s_rx_023gave EXIT=101 (left: Err(Failed(Reset)),right: Ok(Data(100))), and the node test gave EXIT=101 (left: ([], [FromClient, ToClient])).git apply -R, and the tree was clean after.350fb236d, this head, and printedRESTORED=0andporcelain: [].Mutation, the full pipe (
mutation.log, patch in this PR's comments): at884f5fa6d,streams.rs:304's failure arm also dropsto_client. Applied as a checked patch, built, andtext_the_pipe_had_no_room_for_outlives_the_resetgave MUTANT_EXIT=101 (left: (0, [FromClient, ToClient], 0),right: (0, [FromClient], 1)). Restored withgit apply -R: RESTORED=0,porcelain: [].Mutations, the batch, from round 1 at
f4898ced9, with patches in this PR's comments:a_batch_of_frames_moves_…reds.Independent oracle:
netstack_streamsandnetstack_streams_e1000e,libc_sockets(stream_endsin C and std against the host's own report) andnetstack_socket_churn, in the guest suite below.No new guest test. Both claims are the stack's and the node's behaviour on given segments, which host tests reach with the existing fakes.
Gates
At
eb07e6ee0, this head, the merge oforigin/mainat1079e854a, run one after another. The resolution note and links to every log part are in #828 (comment).cargo run -- --ci host: EXIT=0,[ci] Host: 78 step(s), all green.cargo run -- --build-only: EXIT=0;cargo run -- --build-only --arch aarch64: EXIT=0.cargo test: EXIT=1,39 passed, 1 failed, 15 invalidated, 55 total. The one red isvirtio_sound_counts, which came with virtio-sound moves into soundserver as a PCI claim; a claim's record says when its messages landed, and the virtio drivers share toyos-pci-claim #822. By the owner's ruling it is invalid, because host load can turn it red and timings are only valid on metal, and it is being deleted on its own branch. This branch changes no sound code. The 15 tests were invalidated by host suspends.15 passed, 15 total.Before that merge, at
9d068f194. Each log's first line records the head it ran at, and the logs are in this PR's log comment. Run one after another; load averages fromsysctl vm.loadavgin each log.cargo test -p toyos-net-tcp -p toyos-net-shard -p toyos-net-node -p netstack --no-fail-fast: EXIT=0 (crates.log; 1-minute load 42.79 before, 45.22 after).cargo run -- --ci host: EXIT=0,[ci] Host: 78 step(s), all green(host.log; 45.22 before, 47.22 after).cargo run -- --build-only: EXIT=0 (build-x86_64.log);cargo run -- --build-only --arch aarch64: EXIT=0 (build-aarch64.log).cargo test, at9d068f194: EXIT=0,test result: ok. 50 passed, 50 total(suite.log, posted in netstack hands the node a pass's received frames as one batch, about 15% fewer pipe calls on the T14, and a reset keeps the text received before it, as Linux and the BSDs do #828 (comment)). This run is The stop takes the console wire from klogd for good and holds it to the machine's end; flush_final goes #805's kernel and harness together with this branch's netstack.netstack_streams,netstack_streams_e1000e,libc_sockets,netstack_socket_churnandhttps_fetchall pass in it.recvanswers after a reset.What I am unsure of
mbps) are no measurement.🤖 Generated with Claude Code
https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C