From c91e72b8352252cded5cd9b51bfaf0ac198a33e2 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 11 Oct 2026 21:24:44 +0200 Subject: [PATCH 1/9] netstack wakes only on a change: a pipe's leaving has its own watch, and a watch the kernel holds armed is not submitted again Two costs every connection paid on every pass, both read off issues/netstack-watches-both-pipes-of-every-connection-on-every-pass.md. The kernel: an OTHER_END_GONE poll sat on its own end's data watch, which the other end's every read or write posts, so each read a client made of its receive pipe fired netstack's poll on that pipe for a look that found the reader still there. Each pipe end now has a leaving watch that only the other end's last holder posts (pipe::left_watch); release posts it beside the data watch it always posted, and a close cancels the polls on it with the end's others. A watch that asks a direction as well needs no entry there: the same release posts that direction's watch. netstack: every pass submitted a watch for the acceptor, the card's claim, both pipes of every stream and every other place's pipe, each one replacing an identical poll the kernel still held armed, and each paying its registration on every watch it names. A kernel watch stays armed until it is answered or replaced, and is answered by what the object holds at the look that answers it, so watches::Armed records what is armed by token and a pass submits only a watch whose handle or interest changed or whose answer it has read. Every armed watch is answered once, a closed handle's with NotFound, so no record outlives its poll. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- kernel/src/inbox/mod.rs | 7 +- kernel/src/object/ops.rs | 10 +-- kernel/src/pipe.rs | 29 ++++++-- system/netstack/src/main.rs | 5 +- system/netstack/src/serve.rs | 5 +- system/netstack/src/watches.rs | 120 +++++++++++++++++++++++++++++++++ 6 files changed, 160 insertions(+), 16 deletions(-) create mode 100644 system/netstack/src/watches.rs diff --git a/kernel/src/inbox/mod.rs b/kernel/src/inbox/mod.rs index 32310a66ae0..d542b06e522 100644 --- a/kernel/src/inbox/mod.rs +++ b/kernel/src/inbox/mod.rs @@ -536,9 +536,10 @@ fn arm( let ready = readiness_of(object, flags).any(); let read = if flags.readable() { ops::read_watch(object) } else { None }; let write = if flags.writable() { ops::write_watch(object) } else { None }; - // A pipe end's own watch is the one its other end's last holder posts; - // an object with no other end is refused the question, whatever else the - // watch asks. A direction asked above has taken that watch already. + // A pipe end's leaving watch is posted by its other end's last holder + // alone; an object with no other end is refused the question, whatever + // else the watch asks. A direction asked above needs no second entry: that + // last holder posts the end's own data watch too. let own = match flags.other_end_gone().then(|| ops::pipe_end_watch(object)) { Some(None) => return Err(SyscallError::NotSupported), Some(Some(_)) if read.is_some() || write.is_some() => None, diff --git a/kernel/src/object/ops.rs b/kernel/src/object/ops.rs index 959490458f7..e01a7cab605 100644 --- a/kernel/src/object/ops.rs +++ b/kernel/src/object/ops.rs @@ -188,7 +188,7 @@ pub fn close( // Only a watch this handle's object ends is answered: one it shares (every `Console` and a // `Device(Keyboard)` name the keyboard's) cannot cancel another's poll. if close_ends_polls(&object) { - for watch in [read_watch(&object), write_watch(&object)].into_iter().flatten() { + for watch in [read_watch(&object), write_watch(&object), pipe_end_watch(&object)].into_iter().flatten() { watch.cancel_polls(); } } @@ -315,12 +315,12 @@ pub fn write_watch(object: &KObjectRef) -> Option { } } -/// The watch of a pipe end, which its other end's last holder posts as it -/// lets go; `None` for an object that is no pipe end. +/// A pipe end's leaving watch, which only its other end's last holder posts as +/// it lets go; `None` for an object that is no pipe end. pub fn pipe_end_watch(object: &KObjectRef) -> Option { match object { - KObjectRef::PipeRead(_) => read_watch(object), - KObjectRef::PipeWrite(_) => write_watch(object), + KObjectRef::PipeRead(r) => pipe::left_watch(r.id(), End::Read).map(WatchRef::Shared), + KObjectRef::PipeWrite(w) => pipe::left_watch(w.id(), End::Write).map(WatchRef::Shared), KObjectRef::Connection(_) | KObjectRef::Acceptor(_) | KObjectRef::Process(_) | KObjectRef::Console(_) | KObjectRef::Device(_) | KObjectRef::SysCap(_) | KObjectRef::File(_) | KObjectRef::Inbox(_) | KObjectRef::Connector(_) diff --git a/kernel/src/pipe.rs b/kernel/src/pipe.rs index de41909970d..396c50dcf5b 100644 --- a/kernel/src/pipe.rs +++ b/kernel/src/pipe.rs @@ -117,6 +117,10 @@ struct Pipe { /// poll registration can clone one out from under the table lock and hold it across its own park. read_watch: Arc, write_watch: Arc, + /// Each end's leaving watch, by [`End`]: an `OTHER_END_GONE` poll on that + /// end sits on it, and only the other end's last holder posts it — a byte + /// moved through the pipe wakes no poll that asked about a leaving. + left: [Arc; 2], /// Set when a write should hand the next reader transient RT priority — covers readers already runnable, which the wake-time boost misses. rt_boost_pending: bool, } @@ -132,6 +136,7 @@ impl Pipe { holders: Holders::new(), read_watch: Arc::new(Watch::new()), write_watch: Arc::new(Watch::new()), + left: [Arc::new(Watch::new()), Arc::new(Watch::new())], rt_boost_pending: false, } } @@ -283,23 +288,33 @@ pub fn set_rt_boost_pending(pipe_id: PipeId) { }); } -/// One reference to `end` lets go; its end's last posts the other end's watch. +/// One reference to `end` lets go; its end's last posts the other end's watch +/// and its leaving watch. fn release(pipe_id: PipeId, end: End) { - let released = with_pipes_mut(|pipes| { + let (released, left) = with_pipes_mut(|pipes| { let pipe = pipes.get_mut(pipe_id).expect("release: pipe not found"); let released = pipe.holders.release(end); pipe.publish_ends(); + let left = pipe.left[slot(end.other())].clone(); if released == Released::PipeGone { let pipe = pipes.remove(pipe_id).unwrap(); free_pipe(pipe); } - released + (released, left) }); match (released, end) { - (Released::Held | Released::PipeGone, _) => {} + (Released::Held | Released::PipeGone, _) => return, (Released::EndGone, End::Read) => crate::scheduler::wake_pipe_writers(pipe_id), (Released::EndGone, End::Write) => crate::scheduler::wake_pipe_readers(pipe_id), } + left.post(); +} + +fn slot(end: End) -> usize { + match end { + End::Read => 0, + End::Write => 1, + } } fn free_pipe(pipe: Pipe) { @@ -315,3 +330,9 @@ pub fn read_watch(pipe_id: PipeId) -> Option> { pub fn write_watch(pipe_id: PipeId) -> Option> { with_pipes(|pipes| pipes.get(pipe_id).map(|p| p.write_watch.clone())) } + +/// `end`'s leaving watch, the same way: posted once, by the other end's last +/// holder. +pub fn left_watch(pipe_id: PipeId, end: End) -> Option> { + with_pipes(|pipes| pipes.get(pipe_id).map(|p| p.left[slot(end)].clone())) +} diff --git a/system/netstack/src/main.rs b/system/netstack/src/main.rs index 73960cddf28..5024b8512fd 100644 --- a/system/netstack/src/main.rs +++ b/system/netstack/src/main.rs @@ -48,6 +48,7 @@ mod i219; mod pipes; mod serve; mod virtio_net; +mod watches; use card::Card; use client::{Client, ClientRx, HANDSHAKE_TIMEOUT, MAX_KEPT_REQUEST, MAX_PENDING_CONNS, PendingConn, Request}; @@ -356,7 +357,7 @@ fn main() { node.link(clock(), true, draw); } - let poller = Poller::new( + let mut poller = watches::Watches::new( FIXED_WATCHES + serve::WATCHES_PER_PLACE * MAX_PLACES + MAX_PENDING_CONNS + serve::LOOKUP_WATCHES, ); const TOKEN_ACCEPTOR: u64 = 0; @@ -410,7 +411,7 @@ fn main() { poller.watch(&acceptor, READABLE, TOKEN_ACCEPTOR); poller.watch(card.claim(), READABLE, TOKEN_NIC); - sockets.watch(&node, &poller); + sockets.watch(&node, &mut poller); for p in pending.iter() { poller.watch(&p.conn, READABLE, TOKEN_PENDING_BASE + p.conn.as_handle().0 as u64); } diff --git a/system/netstack/src/serve.rs b/system/netstack/src/serve.rs index 7a95dbf023e..b6fd96f5e4c 100644 --- a/system/netstack/src/serve.rs +++ b/system/netstack/src/serve.rs @@ -28,7 +28,7 @@ use std::net::Ipv4Addr; use std::time::Duration; use toyos::net::*; -use toyos::poller::{OTHER_END_GONE, READABLE, WRITABLE, Poller}; +use toyos::poller::{OTHER_END_GONE, READABLE, WRITABLE}; use toyos::{ipc, say, AsHandle, Pipe}; use toyos_abi::syscall::SyscallError; use toyos_inspect::Snapshot; @@ -42,6 +42,7 @@ use toyos_net_wire::{Instant, Port}; use crate::client::{Client, Request}; use crate::pipes::{self, Watched}; +use crate::watches::Watches; use crate::HOSTNAME; /// A watch's token names the socket it was asked of by its id, in the low @@ -737,7 +738,7 @@ impl Sockets { /// one pipe of each datagram socket for its owner's leaving, and the /// connection of each client that waits for a lookup, which hangs up by /// closing it. - pub fn watch(&self, node: &Node, poller: &Poller) { + pub fn watch(&self, node: &Node, poller: &mut Watches) { for (stream, watch) in node.watches() { let Some((socket_id, ends)) = self.by_stream.get(&stream).and_then(|id| Some((*id, self.ends.get(id)?))) else { unreachable!("the node holds a stream whose ends netstack does not") diff --git a/system/netstack/src/watches.rs b/system/netstack/src/watches.rs new file mode 100644 index 00000000000..7dd5f28323f --- /dev/null +++ b/system/netstack/src/watches.rs @@ -0,0 +1,120 @@ +//! What netstack has asked the kernel to watch, so that a pass submits a watch +//! only where its interest changed or the kernel has answered it. +//! +//! **A kernel watch stays armed until it is answered or replaced**, and its +//! answer is what the object holds at the look that answers it +//! (`kernel/src/inbox/polls.rs`). So a watch submitted again with the same +//! handle, interest and token replaces a poll with its twin and costs the +//! submit the poll's registration on every watch it names, for nothing. +//! +//! **Every armed watch is answered, once**: by readiness, by a refusal, or by +//! `NotFound` when a handle it names is closed, so a record here is let go by +//! the answer under its token and never outlives the poll it records. A record +//! whose poll was answered and whose answer is not yet read costs one +//! needless submit after that answer, and never a missed one. + +use std::collections::HashMap; + +use toyos::poller::Poller; +use toyos::AsHandle; +use toyos_abi::syscall::SyscallError; + +/// The watches the kernel holds armed for this process, by token. +#[derive(Default)] +pub struct Armed(HashMap); + +impl Armed { + /// Whether a watch of `flags` on `handle` under `token` has to be + /// submitted, recording it as armed where it does. + pub fn submit(&mut self, token: u64, handle: u32, flags: u32) -> bool { + self.0.insert(token, (handle, flags)) != Some((handle, flags)) + } + + /// The kernel answered the watch under `token`: it is armed no more. + pub fn answered(&mut self, token: u64) { + self.0.remove(&token); + } +} + +/// A [`Poller`] that submits only what [`Armed`] says has to be. +pub struct Watches { + poller: Poller, + armed: Armed, +} + +impl Watches { + pub fn new(capacity: u32) -> Self { + Self { poller: Poller::new(capacity), armed: Armed::default() } + } + + pub fn watch(&mut self, handle: &impl AsHandle, flags: u32, token: u64) { + let raw = handle.as_handle(); + if self.armed.submit(token, raw.0, flags) { + self.poller.watch_raw(raw, flags, token); + } + } + + /// [`Poller::wait_answers`], each answer letting its watch's record go + /// before `f` sees it. + pub fn wait_answers(&mut self, min_complete: u32, timeout_nanos: u64, mut f: impl FnMut(u64, Result)) { + let armed = &mut self.armed; + self.poller.wait_answers(min_complete, timeout_nanos, |token, answer| { + armed.answered(token); + f(token, answer); + }); + } +} + +#[cfg(test)] +mod tests { + use super::Armed; + + const TOKEN: u64 = 0x1_0000_0007; + const READABLE: u32 = 1; + const WRITABLE: u32 = 2; + + #[test] + fn an_armed_watch_is_not_submitted_again() { + let mut armed = Armed::default(); + assert!(armed.submit(TOKEN, 7, READABLE)); + assert!(!armed.submit(TOKEN, 7, READABLE)); + assert!(!armed.submit(TOKEN, 7, READABLE)); + } + + #[test] + fn an_answered_watch_is_submitted_again() { + let mut armed = Armed::default(); + assert!(armed.submit(TOKEN, 7, READABLE)); + armed.answered(TOKEN); + assert!(armed.submit(TOKEN, 7, READABLE)); + } + + #[test] + fn a_changed_interest_is_submitted() { + let mut armed = Armed::default(); + assert!(armed.submit(TOKEN, 7, READABLE)); + assert!(armed.submit(TOKEN, 7, READABLE | WRITABLE)); + assert!(armed.submit(TOKEN, 7, READABLE)); + assert!(!armed.submit(TOKEN, 7, READABLE)); + } + + /// A handle number its closed holder freed, named again under the same + /// token: the closed one's poll ended with it and the new one has none. + #[test] + fn a_token_on_another_handle_is_submitted() { + let mut armed = Armed::default(); + assert!(armed.submit(TOKEN, 7, READABLE)); + assert!(armed.submit(TOKEN, 8, READABLE)); + } + + /// The answer of one token lets no other's record go. + #[test] + fn an_answer_is_its_own_tokens_alone() { + let mut armed = Armed::default(); + assert!(armed.submit(TOKEN, 7, READABLE)); + assert!(armed.submit(TOKEN + 1, 8, READABLE)); + armed.answered(TOKEN); + assert!(!armed.submit(TOKEN + 1, 8, READABLE)); + assert!(armed.submit(TOKEN, 7, READABLE)); + } +} From ba97b5c3956d2793040a9989927ab2ae00640023 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 11 Oct 2026 22:42:31 +0200 Subject: [PATCH 2/9] netstack's pass is the one cadence: the card's interrupt interval follows the least round trip a batch moved, and the I219's ring fills its grant The ACK, the pipe write and the client's wake each follow the pass one for one (round 1 on the T14: frames_tx, passes, pipe writes and app reads all about 60 per MB), so the pass's interval is the one thing to time. Round 1 measured about 4.2 Mcyc/MB of per-pass cost at 60 passes/MB, under a fixed 128 us ITR that set the cadence whatever the path. - `cadence` (netstack/src/cadence.rs), pure: the interval after a pass that took frames is min(least RTT / K, the ring's bound), never under FLOOR unless the ring's bound is. K = 16 is the proposal the T14 round settles. FLOOR = 30 us estimates one wake: round 1's 19.6 us of netstack CPU per pass plus about 9 us of kernel. The ring wins over the floor, because an overrun loses frames and a wake too many costs only cycles. The design put the floor above the ring; at 400 Gb/s with this ring that would overrun. - The least RTT is toyos-net-tcp's `take_least_rtt`: the receiver's own estimate where in-order text arrived, the sender's SRTT where an ACK moved SND.UNA. The design named only the receiver. The sender's side applies the same ACK-clock bound to an upload, which a receiver-only rule would leave at the floor. - The I219 applies the interval as ITR, rounded down and kept to three leading bits, and written only when that changes. The design said powers of two. That rounding loses up to half of every bound, and it halves the floor, which is a lower bound: 30 us would apply as 16.4 us. Three bits lose at most a fifth, and a step is an eighth to a quarter, which still stops the register flapping. - The receive ring grows from 256 to 1000 descriptors, filling the 2 MiB span the kernel maps every grant in, of which the old layout used 552 KiB: about 12 ms of full-sized frames at a gigabit instead of 3.1. The node's receive budget, 64 frames before, is the card's ring. - F1: a given-back buffer reaches the part at the batch's end in one RDT write, or once HELD_MAX (a quarter of the half-ring reserve) wait. Filed: the receiver's RTT reads no finer than its 1 ms TSval tick, and a flow with no TCP RTT (UDP, QUIC, a flood) is paced at the floor. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...no-tcp-round-trip-is-paced-at-the-floor.md | 24 +++ ...-trip-reads-no-finer-than-a-millisecond.md | 26 ++++ issues/the-lan-is-not-yet-production-grade.md | 4 +- system/netstack/i219/src/lib.rs | 126 +++++++++++---- system/netstack/i219/src/tests.rs | 133 +++++++++++++++- system/netstack/node/src/lease.rs | 4 + system/netstack/node/src/lib.rs | 35 +++-- system/netstack/node/tests/common/mod.rs | 7 +- system/netstack/node/tests/lan/mod.rs | 6 +- system/netstack/node/tests/listeners.rs | 8 +- system/netstack/node/tests/streams.rs | 28 ++-- system/netstack/shard/src/lib.rs | 6 + system/netstack/shard/tcp/src/rx.rs | 5 + system/netstack/shard/tcp/src/stack.rs | 22 ++- system/netstack/shard/tcp/tests/rtt.rs | 44 ++++++ system/netstack/src/cadence.rs | 146 ++++++++++++++++++ system/netstack/src/card.rs | 46 +++++- system/netstack/src/i219.rs | 15 ++ system/netstack/src/main.rs | 12 +- 19 files changed, 625 insertions(+), 72 deletions(-) create mode 100644 issues/a-receive-flow-with-no-tcp-round-trip-is-paced-at-the-floor.md create mode 100644 issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md create mode 100644 system/netstack/src/cadence.rs diff --git a/issues/a-receive-flow-with-no-tcp-round-trip-is-paced-at-the-floor.md b/issues/a-receive-flow-with-no-tcp-round-trip-is-paced-at-the-floor.md new file mode 100644 index 00000000000..94ba0529e42 --- /dev/null +++ b/issues/a-receive-flow-with-no-tcp-round-trip-is-paced-at-the-floor.md @@ -0,0 +1,24 @@ +--- +status: open +kind: defect +opened: 2026-10-11 +--- + +# A receive flow with no TCP round trip is paced at the floor + +The pass's cadence (`system/netstack/src/cadence.rs`) bounds the card's +interrupt interval by the least round trip of the TCP streams a batch moved, +and holds a batch that moved none to `FLOOR` (30 µs). A sustained flow that +moves no TCP stream gets that floor: UDP, which includes QUIC, and a flood of +frames for nobody. The I219 takes 30 µs as 28.7 µs, so on such a flow the card +may wake netstack about 35,000 times a second. Before the cadence the fixed +128 µs throttle held it to about 7,800. The floor is an estimate of one wake's +cost, so at the floor the wakes can take a whole CPU. + +Netstack has no round trip for such a flow. QUIC's round trip is its +endpoint's, inside the app, and a flood has none. Pacing it at the ring's +bound would add up to that bound to every datagram's latency. + +**Exit**: a sustained receive flow that moves no TCP stream costs no more +wakes per megabyte than a TCP download at the same rate, with its latency +bound stated. A T14 row reads both. diff --git a/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md b/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md new file mode 100644 index 00000000000..36dae1ab4d2 --- /dev/null +++ b/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md @@ -0,0 +1,26 @@ +--- +status: open +kind: defect +opened: 2026-10-11 +--- + +# netstack's receiver round trip reads no finer than a millisecond + +A downloading stream's round trip is the receiver's own estimate +(`system/netstack/shard/tcp/src/rx.rs`): the age of an echoed TSval. TSval +ticks once a millisecond, so an echo's age is a whole number of milliseconds +and never less than one tick. A path whose round trip is under a millisecond, +a LAN's, reads as at least 1 ms. + +The pass's cadence (`system/netstack/src/cadence.rs`) holds the card's +interrupt for a `K`th of the least round trip. On a 200 µs path that round +trip reads 1 ms, so the cadence asks for 62.5 µs where a `K`th of the true +one, 12.5 µs, would give the 30 µs floor. The I219 takes 62.5 µs as 57.3 µs, +its three leading bits. So the interval is about 29% of the round trip, not a +`K`th: the sender gets about three acknowledgments a round trip, and each +round trip grows by about 29%. These figures are derived from `K = 16`, the +tick and the rounding. No machine has measured a sub-millisecond path. + +**Exit**: a stream on a sub-millisecond path reports its round trip to within +a `K`th of the path's own, read by a host test that delivers echoes on such a +path. diff --git a/issues/the-lan-is-not-yet-production-grade.md b/issues/the-lan-is-not-yet-production-grade.md index 3785e7e0efe..a62432f400d 100644 --- a/issues/the-lan-is-not-yet-production-grade.md +++ b/issues/the-lan-is-not-yet-production-grade.md @@ -39,8 +39,8 @@ why, and the boot still leases. and raise it under traffic. Nothing has one, and `issues/no-machine-has-read-an-intel-nic-across-a-link-change.md` waits on it. - **Throughput** is measured against Ubuntu on the same machine and cable, with - line rate (about 940 Mb/s of TCP) as the target: interrupt moderation (a - fixed 128 µs throttle today, not adaptive), ring sizes, checksum and + line rate (about 940 Mb/s of TCP) as the target: interrupt moderation (the + pass's cadence, `system/netstack/src/cadence.rs`), ring sizes, checksum and segmentation offload. - **The part is handed back** in the state the next operating system and the management firmware that shares it expect. Ubuntu has had link after every diff --git a/system/netstack/i219/src/lib.rs b/system/netstack/i219/src/lib.rs index a7983803ce5..4249bfad034 100644 --- a/system/netstack/i219/src/lib.rs +++ b/system/netstack/i219/src/lib.rs @@ -148,25 +148,38 @@ pub trait Interrupts { fn taken(&self) -> Result; } -/// How many receive descriptors the ring holds. §7.1.8: `RDLEN` "must be a -/// multiple of 128", so the count is a multiple of eight. -pub const RX_RING: usize = 256; +/// How much memory the driver asks to be granted, in one region: one 2 MiB +/// span, which is what the kernel maps a grant of any smaller size in, so +/// every byte of it is the receive ring's that the transmit ring does not +/// need. +pub const GRANT_BYTES: u64 = 2 << 20; +/// How many receive descriptors the ring holds: the most whose descriptors +/// and buffers fit the grant beside the transmit ring's. §7.1.8: `RDLEN` +/// "must be a multiple of 128", so the count is a multiple of eight. +pub const RX_RING: usize = 1000; /// How many transmit descriptors the ring holds. A frame is handed to the /// device the moment it is filled and reclaimed when room is next asked for, /// so what this bounds is how many may be in flight at once: one fewer than /// this, [`I219::tx_room`]'s most. pub const TX_RING: usize = 16; -/// `ITR` (§10.2.4.2): the least interval between two of the function's -/// interrupts, in 256 ns units — 128 µs. It runs from the last interrupt, so a -/// frame after a quiet spell that long interrupts at once, and a gigabit -/// stream's full-sized frames, one each 12.3 µs, wake their reader about ten at -/// a time. -const THROTTLE: u32 = 500; +/// Free receive descriptors at which the part reports itself short: +/// `RCTL.RDMTS = 00b`, "1/2" of `RDLEN` (§10.2.5.1), raising `RXDMT0`. +pub const RX_SHORT: usize = RX_RING / 2; +/// Receive buffers given back and not yet handed to the part at most: the +/// tail moves once a batch ([`I219::rx_flush`]), and sooner once this many +/// wait. A quarter of the [`RX_SHORT`] descriptors a pacing caller keeps in +/// reserve, so what a batch holds back never brings the part to its report +/// alone. +pub const HELD_MAX: usize = RX_SHORT / 4; /// Bytes per receive buffer — `RCTL.BSIZE = 00b` with `BSEX` clear (§10.2.5.1). pub const RX_BUF_BYTES: usize = 2048; /// Bytes per transmit buffer, and therefore the longest frame [`I219::tx_reserve`] /// takes. pub const TX_BUF_BYTES: usize = 2048; +/// `ITR` (§10.2.4.2) counts the least interval between two of the function's +/// interrupts in units of this many nanoseconds, in sixteen bits; zero is no +/// throttle at all. +pub const THROTTLE_UNIT_NANOS: u64 = 256; const _: () = { // §7.1.8 and §7.2.4: both ring lengths are programmed in bytes and "must be @@ -181,20 +194,30 @@ const _: () = { /// The grant's layout. One grant, because every byte of it is this process's /// own: what a wrong offset here corrupts is this driver's memory and nobody -/// else's. +/// else's. Each buffer starts on a multiple of its own size. const OFF_RX_RING: usize = 0x0000; const OFF_TX_RING: usize = OFF_RX_RING + RX_RING * rx_desc::BYTES; -const OFF_RX_BUFS: usize = 0x2000; +const OFF_RX_BUFS: usize = layout(RX_RING).0; const OFF_TX_BUFS: usize = OFF_RX_BUFS + RX_RING * RX_BUF_BYTES; -/// How much memory the driver needs granted, in one region. -pub const GRANT_BYTES: u64 = (OFF_TX_BUFS + TX_RING * TX_BUF_BYTES) as u64; + +/// Where the receive buffers start, and where the grant's last byte ends, for +/// a receive ring of `rx` descriptors. +const fn layout(rx: usize) -> (usize, usize) { + let rings = rx * rx_desc::BYTES + TX_RING * tx_desc::BYTES; + let rx_bufs = rings.div_ceil(RX_BUF_BYTES) * RX_BUF_BYTES; + (rx_bufs, rx_bufs + rx * RX_BUF_BYTES + TX_RING * TX_BUF_BYTES) +} const _: () = { assert!(OFF_TX_RING + TX_RING * tx_desc::BYTES <= OFF_RX_BUFS); + // The grant holds this ring and not the next multiple of eight. + assert!(layout(RX_RING).1 <= GRANT_BYTES as usize); + assert!(layout(RX_RING + 8).1 > GRANT_BYTES as usize); // §7.1.8 and §7.2.4: the ring base "is aligned on a 16-byte boundary" and // "hardware ignores the lower 4 bits", so a ring at an unaligned offset // would be programmed somewhere else entirely. assert!(OFF_RX_RING.is_multiple_of(16) && OFF_TX_RING.is_multiple_of(16)); + assert!(HELD_MAX > 0); }; /// Why the function was not brought up. Each keeps its own word: a caller asks @@ -765,6 +788,10 @@ pub struct I219 { /// receive ring keeps in software's hands (§7.1.8: the queue is empty when /// head equals tail). rx_tail: usize, + /// Buffers given back since the tail last moved. + rx_held: usize, + /// What `ITR` holds, in its own units. + throttle: u32, /// Next transmit descriptor to hand out, which is also `TDT`. tx_next: usize, /// Next transmit descriptor to reclaim. @@ -913,6 +940,8 @@ impl I219 { link_up_at: None, rx_next: 0, rx_tail: RX_RING - 1, + rx_held: 0, + throttle: 0, tx_next: 0, tx_clean: 0, tx_wake: false, @@ -930,10 +959,10 @@ impl I219 { nic.regs.write32(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO); nic.accepted(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO)?; - // The throttle alone: the two receive timers hold back every frame's - // interrupt, the first after a quiet spell too, and the throttle none - // of those. - nic.regs.write32(regs::ITR, THROTTLE); + // The throttle alone, and the caller's to set ([`Self::throttle`]): + // the two receive timers hold back every frame's interrupt, the first + // after a quiet spell too, and the throttle none of those. + nic.regs.write32(regs::ITR, 0); nic.regs.write32(regs::RDTR, 0); nic.regs.write32(regs::RADV, 0); nic.regs.write32(regs::TIDV, 0); @@ -1253,19 +1282,27 @@ impl I219 { } /// Give a frame's buffer back to the device, its bytes having been read. + /// It reaches the device at the next [`Self::rx_flush`], or here once + /// [`HELD_MAX`] buffers wait. pub fn rx_done(&mut self, frame: Frame) { self.give_back(frame.index); } - /// Republish descriptor `index` and move the tail over every ready - /// descriptor from where it stands. - /// - /// **The tail moves over a run, never to an index.** §7.1.8's tail - /// "identifies the location beyond the last descriptor hardware can - /// process", so writing it with the descriptor that came back last would - /// hand the device every descriptor between — including one whose bytes - /// their reader still holds — and, for a buffer returned before an older - /// one, walk `RDT` backwards over the rest of the ring. + /// Hand the device every buffer given back since the tail last moved: the + /// end of a batch, and one `RDT` write for all of it. + pub fn rx_flush(&mut self) { + self.move_tail(); + } + + /// Receive descriptors the device holds empty, every buffer given back + /// handed to it: what it can take before the next pass without running + /// short. Read after [`Self::rx_flush`], with nothing left to poll. + pub fn rx_free(&self) -> usize { + (self.rx_tail + RX_RING - self.rx_next) % RX_RING + } + + /// Republish descriptor `index`, and move the tail once [`HELD_MAX`] + /// buffers wait for it. /// /// **Ready is read out of the descriptor, so there is no second record of /// it to disagree.** [`publish_rx`] zeroes the status word, and a @@ -1274,6 +1311,43 @@ impl I219 { /// touches neither until the tail passes it. fn give_back(&mut self, index: usize) { self.publish_rx(index); + self.rx_held += 1; + if self.rx_held >= HELD_MAX { + self.move_tail(); + } + } + + /// Hold the function's interrupts at least `nanos` apart (§10.2.4.2's + /// `ITR`): its units at or below `nanos`, between one and the field's + /// largest, kept to their three leading bits; the register is written only + /// when that changes. + /// + /// **Rounded down**, so the interval never exceeds what the caller bounded + /// it by, and **to three leading bits**, so an interval the caller + /// recomputes every pass moves the register only on a step of an eighth + /// to a quarter, and loses at most a fifth of what was asked. The throttle + /// runs from the last interrupt, so a frame after a quiet spell that long + /// interrupts at once. + pub fn throttle(&mut self, nanos: u64) { + let units = (nanos / THROTTLE_UNIT_NANOS).clamp(1, u64::from(u16::MAX)) as u32; + let low = units.ilog2().saturating_sub(2); + let units = units >> low << low; + if units != self.throttle { + self.throttle = units; + self.regs.write32(regs::ITR, units); + } + } + + /// Move the tail over every ready descriptor from where it stands. + /// + /// **The tail moves over a run, never to an index.** §7.1.8's tail + /// "identifies the location beyond the last descriptor hardware can + /// process", so writing it with the descriptor that came back last would + /// hand the device every descriptor between — including one whose bytes + /// their reader still holds — and, for a buffer returned before an older + /// one, walk `RDT` backwards over the rest of the ring. + fn move_tail(&mut self) { + self.rx_held = 0; let mut tail = self.rx_tail; loop { let next = (tail + 1) % RX_RING; diff --git a/system/netstack/i219/src/tests.rs b/system/netstack/i219/src/tests.rs index 7a197b77a24..ca72f9c8913 100644 --- a/system/netstack/i219/src/tests.rs +++ b/system/netstack/i219/src/tests.rs @@ -47,6 +47,7 @@ fn drain(nic: &Nic, driver: &mut Driver) -> Vec> { for frame in taken { driver.rx_done(frame); } + driver.rx_flush(); got } @@ -122,11 +123,11 @@ fn bring_up_programs_what_the_initialization_sections_name() { assert_eq!(nic.peek(regs::TDLEN) as usize, TX_RING * tx_desc::BYTES); assert_eq!(nic.peek(regs::TDT), 0); - // Both timers off and the throttle on: the first frame after a quiet - // spell is not held back, and a stream's are taken a batch a wake. + // Both timers off, and the throttle the caller's to set: the first frame + // after a quiet spell is not held back. assert_eq!(nic.peek(regs::RDTR), 0); assert_eq!(nic.peek(regs::RADV), 0); - assert_eq!(u64::from(nic.peek(regs::ITR)) * 256, 128_000, "§10.2.4.2 counts in 256 ns"); + assert_eq!(nic.peek(regs::ITR), 0); assert_eq!(driver.mac(), NVM_MAC); } @@ -528,6 +529,7 @@ fn a_buffer_given_back_out_of_turn_does_not_carry_the_tail_over_an_older_one() { assert_eq!(armed as usize, RX_RING - 1, "{}", nic.because("the ring did not arm its tail")); driver.rx_done(second); + driver.rx_flush(); assert_eq!( nic.peek(regs::RDT), armed, @@ -536,6 +538,7 @@ fn a_buffer_given_back_out_of_turn_does_not_carry_the_tail_over_an_older_one() { ); driver.rx_done(first); + driver.rx_flush(); assert_eq!( nic.peek(regs::RDT), 1, @@ -574,6 +577,7 @@ fn a_refused_descriptor_does_not_carry_the_tail_over_a_frame_still_held() { nic.because("the refused descriptor was handed up as a frame") ); assert_eq!(driver.counters().over_length, 1, "{}", nic.because("the length was believed")); + driver.rx_flush(); assert_eq!( nic.peek(regs::RDT) as usize, RX_RING - 1, @@ -582,6 +586,7 @@ fn a_refused_descriptor_does_not_carry_the_tail_over_a_frame_still_held() { ); driver.rx_done(held); + driver.rx_flush(); assert_eq!( nic.peek(regs::RDT), 1, @@ -590,6 +595,128 @@ fn a_refused_descriptor_does_not_carry_the_tail_over_a_frame_still_held() { ); } +/// The ring that fills the grant takes frames whole and in order through its wrap, twice over, +/// in batches of up to a third of it with a flush at each end. +#[test] +fn frames_come_up_whole_and_in_order_through_the_rings_wrap() { + let nic = Nic::new(26); + let mut driver = open(&nic); + nic.set_link(true); + let mut sent = Vec::new(); + let mut got = Vec::new(); + for batch in 0..8 { + for i in 0..RX_RING / 3 { + let tag = (batch * RX_RING / 3 + i) as u32; + let mut bytes = frame(tag as u8, 64 + (tag % 1400) as usize); + bytes[7..11].copy_from_slice(&tag.to_le_bytes()); + nic.deliver(&bytes); + sent.push(bytes); + } + for _ in 0..RX_RING { + nic.run(); + one_pass(&mut driver); + got.extend(drain(&nic, &mut driver)); + if got.len() == sent.len() { + break; + } + } + } + assert!(sent.len() > 2 * RX_RING, "the ring did not wrap twice"); + assert_eq!(got.len(), sent.len(), "{}", nic.because("frames were lost or invented")); + assert!(got == sent, "{}", nic.because("the frames did not come up whole and in order")); +} + +/// How many times the driver has written `reg`. +fn writes_of(nic: &Nic, reg: usize) -> usize { + nic.trace().iter().filter(|access| **access == Access::Write { reg }).count() +} + +/// A batch's buffers reach the part in one `RDT` write at its end, and sooner +/// only once [`HELD_MAX`] wait: the part is never kept from more than that. +#[test] +fn the_tail_moves_once_a_batch_and_sooner_only_once_held_max_wait() { + let nic = Nic::with( + 24, + Part::E82574, + Permits { batched_writeback: false, spurious_interrupts: false, ..Permits::default() }, + ); + let mut driver = open(&nic); + nic.set_link(true); + let late = 3; + for i in 0..HELD_MAX + late { + nic.deliver(&frame(i as u8, 64)); + } + nic.run(); + one_pass(&mut driver); + let frames: Vec = core::iter::from_fn(|| driver.poll_rx()).collect(); + assert_eq!(frames.len(), HELD_MAX + late, "{}", nic.because("the frames did not all arrive")); + let (armed, writes) = (nic.peek(regs::RDT), writes_of(&nic, regs::RDT)); + + let mut frames = frames.into_iter(); + for frame in frames.by_ref().take(HELD_MAX - 1) { + driver.rx_done(frame); + } + assert_eq!( + (nic.peek(regs::RDT), writes_of(&nic, regs::RDT)), + (armed, writes), + "{}", + nic.because("the tail moved before a batch ended or HELD_MAX buffers waited") + ); + + driver.rx_done(frames.next().expect("the HELD_MAX-th frame")); + assert_eq!( + (nic.peek(regs::RDT) as usize, writes_of(&nic, regs::RDT)), + (HELD_MAX - 1, writes + 1), + "{}", + nic.because("HELD_MAX buffers waited and the tail did not move over them once") + ); + + for frame in frames { + driver.rx_done(frame); + } + driver.rx_flush(); + assert_eq!( + (nic.peek(regs::RDT) as usize, writes_of(&nic, regs::RDT)), + (HELD_MAX + late - 1, writes + 2), + "{}", + nic.because("the batch's end did not hand the rest over in one write") + ); + driver.rx_flush(); + assert_eq!(writes_of(&nic, regs::RDT), writes + 2, "{}", nic.because("a flush with nothing held wrote RDT")); + assert_eq!(driver.rx_free(), RX_RING - 1, "{}", nic.because("the part does not hold the whole ring")); +} + +/// §10.2.4.2's `ITR` takes its 256 ns units at or below what the caller +/// asked, inside its sixteen bits, to three leading bits, and is written only +/// when that changes: an interval recomputed every pass costs no write unless +/// it moves by a step of an eighth to a quarter. +#[test] +fn the_throttle_is_written_only_when_its_three_leading_bits_change() { + let nic = Nic::new(25); + let mut driver = open(&nic); + let opened = writes_of(&nic, regs::ITR); + for (nanos, units, written) in [ + // 390 units, 0b110000110: 0b110 << 6. + (100_000, 384, 1), + (99_000, 384, 1), + (98_303, 320, 2), + (65_536, 256, 3), + (65_535, 224, 4), + (7 * 256, 7, 5), + (0, 1, 6), + (255, 1, 6), + (u64::MAX, 0b111 << 13, 7), + ] { + driver.throttle(nanos); + assert_eq!( + (nic.peek(regs::ITR), writes_of(&nic, regs::ITR) - opened), + (units, written), + "{}", + nic.because(&format!("asked for {nanos} ns")) + ); + } +} + // --- transmit --- #[test] diff --git a/system/netstack/node/src/lease.rs b/system/netstack/node/src/lease.rs index 9398ca4d322..2e63ca4fb4f 100644 --- a/system/netstack/node/src/lease.rs +++ b/system/netstack/node/src/lease.rs @@ -107,6 +107,10 @@ impl Stack { self.shard.receive(now, frame); } + pub(crate) fn take_least_rtt(&mut self) -> Option { + self.shard.take_least_rtt() + } + pub(crate) fn transmit(&mut self, now: Instant, credit: usize, sink: impl FnMut(&[u8])) -> usize { self.shard.transmit(now, credit, sink) } diff --git a/system/netstack/node/src/lib.rs b/system/netstack/node/src/lib.rs index 3a25ececfe0..014858f06f2 100644 --- a/system/netstack/node/src/lib.rs +++ b/system/netstack/node/src/lib.rs @@ -69,10 +69,17 @@ toyos_net_wire::counters! { QueryFailed = "node.query-failed"; } -/// The most frames one [`Node::receive`] takes. A device may refill its ring as fast as it is -/// read, and a batch that waited for it to empty would keep the deadlines, the transmit -/// opportunity and every stream's pass waiting with it. -const RECEIVE_BUDGET: usize = 64; +/// What one [`Node::receive`] took. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Batch { + pub frames: usize, + /// The batch stopped at its bound, and another is owed without waiting for the device to + /// say so. + pub owed: bool, + /// The least round trip among the streams whose data the batch moved + /// ([`toyos_net_tcp::Tcp::take_least_rtt`]). + pub least_rtt: Option, +} /// A line for the log. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -155,15 +162,19 @@ impl Node { // ---- the device ---- - /// The frames the device received since the last call, one by one and at most - /// `RECEIVE_BUDGET`: `next` hands its next frame to the sink it is given and answers - /// `false` once it had none. One pass over the streams follows the last frame, so what the - /// batch brought reaches each client in one write and not one a frame. `true` is a batch - /// that stopped at the bound, and another is owed without waiting for the device to say so. - pub fn receive(&mut self, now: Instant, mut next: impl FnMut(&mut dyn FnMut(&[u8])) -> bool, mut draw: impl FnMut() -> u32) -> bool { + /// The frames the device received since the last call, one by one and at most `budget`: + /// `next` hands its next frame to the sink it is given and answers `false` once it had none. + /// One pass over the streams follows the last frame, so what the batch brought reaches each + /// client in one write and not one a frame. + /// + /// **The budget is the device's ring**: a device may refill it as fast as it is read, and a + /// batch that waited for it to empty would keep the deadlines, the transmit opportunity and + /// every stream's pass waiting with it, while a bound below what one interrupt's wait can + /// bring would owe a second pass for every long wait. + pub fn receive(&mut self, now: Instant, budget: usize, mut next: impl FnMut(&mut dyn FnMut(&[u8])) -> bool, mut draw: impl FnMut() -> u32) -> Batch { let mut taken = 0usize; let owed = loop { - if taken == RECEIVE_BUDGET { + if taken == budget { break true; } if !next(&mut |frame| { @@ -175,7 +186,7 @@ impl Node { taken = taken.saturating_add(1); }; self.pass(now, false); - owed + Batch { frames: taken, owed, least_rtt: self.stack.take_least_rtt() } } /// A transmit opportunity with room for `credit` frames, each handed to `sink` as it is built. diff --git a/system/netstack/node/tests/common/mod.rs b/system/netstack/node/tests/common/mod.rs index 33c130e1345..f5114b2517c 100644 --- a/system/netstack/node/tests/common/mod.rs +++ b/system/netstack/node/tests/common/mod.rs @@ -294,6 +294,9 @@ pub fn echo_request(destination: Ipv4Addr, id: u16, seq: u16, data: &[u8]) -> Ve /// What [`Node::receive`] hands each frame to. type Sink<'s> = dyn FnMut(&[u8]) + 's; +/// The most frames a test's [`Node::receive`] takes: a ring of a device's size. +pub const BUDGET: usize = 256; + /// `frames`, in order, as the batch [`Node::receive`] takes. pub fn batch<'a>(frames: &'a [&'a [u8]]) -> impl FnMut(&mut Sink<'_>) -> bool + 'a { let mut left = frames.iter(); @@ -350,7 +353,7 @@ impl Wire { } let asked = self.sent[from..].iter().filter(|seen| matches!(seen, Seen::Arp { request: true, sender, target, .. } if *target == R && *sender == A)).count(); for _ in 0..asked { - self.node.receive(self.now, batch(&[&arp(MAC, false, MAC_R, R, A)]), draw(&mut self.draws)); + self.node.receive(self.now, BUDGET, batch(&[&arp(MAC, false, MAC_R, R, A)]), draw(&mut self.draws)); } } } @@ -361,7 +364,7 @@ impl Wire { } pub fn deliver(&mut self, frame: &[u8]) { - self.node.receive(self.now, batch(&[frame]), draw(&mut self.draws)); + self.node.receive(self.now, BUDGET, batch(&[frame]), draw(&mut self.draws)); self.pump(); } diff --git a/system/netstack/node/tests/lan/mod.rs b/system/netstack/node/tests/lan/mod.rs index 5d00ae9dd57..a1e5b3889a3 100644 --- a/system/netstack/node/tests/lan/mod.rs +++ b/system/netstack/node/tests/lan/mod.rs @@ -17,7 +17,7 @@ use etherparse::{ArpOperation, Icmpv4Type, LinkSlice, NetSlice, SlicedPacket, Tr use toyos_net_node::Node; use toyos_net_wire::Instant; -use crate::common::{arp, batch, from_server, message_of, option, sum, terms, xid, Wire, A, ACK, BROADCAST, DISCOVER, ELSEWHERE, MAC, MAC_B, MAC_R, OFFER, R}; +use crate::common::{arp, batch, BUDGET, from_server, message_of, option, sum, terms, xid, Wire, A, ACK, BROADCAST, DISCOVER, ELSEWHERE, MAC, MAC_B, MAC_R, OFFER, R}; pub const B: Ipv4Addr = ELSEWHERE; pub const OFF_LINK: Ipv4Addr = Ipv4Addr::new(198, 51, 100, 7); @@ -161,13 +161,13 @@ impl Lan { .collect(); for target in asked { let mac = if target == R { MAC_R } else { MAC_B }; - self.node.receive(self.now, batch(&[&arp(MAC, false, mac, target, A)]), draw(&mut self.draws)); + self.node.receive(self.now, BUDGET, batch(&[&arp(MAC, false, mac, target, A)]), draw(&mut self.draws)); } } } pub fn deliver(&mut self, frame: &[u8]) { - self.node.receive(self.now, batch(&[frame]), draw(&mut self.draws)); + self.node.receive(self.now, BUDGET, batch(&[frame]), draw(&mut self.draws)); self.pump(); } diff --git a/system/netstack/node/tests/listeners.rs b/system/netstack/node/tests/listeners.rs index ae863e13f70..f6f59cf7bd4 100644 --- a/system/netstack/node/tests/listeners.rs +++ b/system/netstack/node/tests/listeners.rs @@ -18,7 +18,7 @@ use std::net::Ipv4Addr; use std::rc::Rc; use std::time::Duration; -use common::{arp, batch, terms, Segment, Wire, A, ELSEWHERE, MAC, MAC_R, R}; +use common::{arp, batch, terms, Segment, Wire, A, BUDGET, ELSEWHERE, MAC, MAC_R, R}; use etherparse::{ArpOperation, LinkSlice, NetSlice, PacketBuilder, SlicedPacket, TcpOptionElement, TransportSlice}; use toyos_net_node::{AcceptRefused, Accepted, ConnectRefused, FromClient, ListenRefused, ListenerId, Node, PipeEnd, Pipes, ReadRefusal, Refused, StreamEvent, StreamId, ToClient, Wake, WriteRefusal}; use toyos_net_tcp::{limits, Counter, Endpoint}; @@ -227,7 +227,7 @@ impl Net { } for frame in &frames { if let Some(answer) = self.hears(frame) { - self.node.receive(self.now, batch(&[&answer]), draw(&mut self.draws)); + self.node.receive(self.now, BUDGET, batch(&[&answer]), draw(&mut self.draws)); } } } @@ -263,7 +263,7 @@ impl Net { } fn deliver(&mut self, frame: &[u8]) { - self.node.receive(self.now, batch(&[frame]), draw(&mut self.draws)); + self.node.receive(self.now, BUDGET, batch(&[frame]), draw(&mut self.draws)); self.pump(); } @@ -809,7 +809,7 @@ fn a_listener_ended_for_its_wake_gives_its_place_to_another_in_the_same_pass() { refusing.borrow_mut().refusal = Some(WriteRefusal::Gone); net.syn(P3); let ack = net.iss(P3).wrapping_add(1); - net.node.receive(net.now, batch(&[&frame(P3, SSH, ISS + 1, Some(ack), 0, &[])]), draw(&mut net.draws)); + net.node.receive(net.now, BUDGET, batch(&[&frame(P3, SSH, ISS + 1, Some(ack), 0, &[])]), draw(&mut net.draws)); assert_eq!(wakes(&owner), 2); let ended: Vec<(ListenerId, WriteRefusal)> = net.node.drain_ended_listeners().collect(); assert_eq!((ended, net.node.listeners(), net.node.held()), (vec![(first, WriteRefusal::Gone)], 1, 1)); diff --git a/system/netstack/node/tests/streams.rs b/system/netstack/node/tests/streams.rs index 8af4d0e46b9..b18ce57bdb9 100644 --- a/system/netstack/node/tests/streams.rs +++ b/system/netstack/node/tests/streams.rs @@ -16,7 +16,7 @@ use std::net::Ipv4Addr; use std::rc::Rc; use std::time::Duration; -use common::{arp, batch, terms, Segment, Wire, A, MAC, MAC_B, MAC_R, R}; +use common::{arp, batch, terms, Segment, Wire, A, BUDGET, MAC, MAC_B, MAC_R, R}; use etherparse::{ArpOperation, LinkSlice, NetSlice, PacketBuilder, SlicedPacket, TcpOptionElement, TransportSlice}; use toyos_net_node::{ConnectRefused, FromClient, Node, PipeEnd, Pipes, ReadRefusal, StreamEvent, StreamId, ToClient, Watch, WriteRefusal}; use toyos_net_shard::ConnectError; @@ -424,14 +424,14 @@ impl Net { self.node.transmit(self.now, usize::MAX, |frame| frames.push(frame.to_vec()), draw(&mut self.draws)); for frame in &frames { for answer in self.hears(frame) { - self.node.receive(self.now, batch(&[&answer]), draw(&mut self.draws)); + self.node.receive(self.now, BUDGET, batch(&[&answer]), draw(&mut self.draws)); } } frames.len() } fn deliver(&mut self, frame: &[u8]) { - self.node.receive(self.now, batch(&[frame]), draw(&mut self.draws)); + self.node.receive(self.now, BUDGET, batch(&[frame]), draw(&mut self.draws)); self.pump(); } @@ -660,33 +660,33 @@ fn a_batch_of_frames_moves_an_established_stream_once_with_no_opportunity_after_ let (mut net, _, client) = established(); let frames = [net.far.text(b"all "), net.far.text(b"at "), net.far.text(b"once")]; let (writes, reads) = (client.borrow().writes, client.borrow().reads); - net.node.receive(net.now, batch(&[&frames[0], &frames[1], &frames[2]]), draw(&mut net.draws)); + net.node.receive(net.now, BUDGET, batch(&[&frames[0], &frames[1], &frames[2]]), draw(&mut net.draws)); let ends = client.borrow(); assert_eq!(ends.inbox, b"all at once"); assert_eq!((ends.writes - writes, ends.reads - reads), (1, 0), "the pipes' calls in the batch's pass"); } // A device that refills its ring as fast as it is read never says it has no frame: a batch -// stops at the bound and answers that another pass is owed, and one that found the ring empty -// owes none, a ring one short of the bound included. +// stops at the budget it was given and answers that another pass is owed, and one that found the +// ring empty owes none, a ring one short of the budget included. #[test] -fn a_batch_stops_at_its_bound_and_says_a_pass_is_owed() { +fn a_batch_stops_at_its_budget_and_says_a_pass_is_owed() { let (mut net, _, client) = established(); let frame = net.far.text(b"again"); let mut handed = 0usize; let endless = |sink: &mut dyn FnMut(&[u8])| { handed += 1; - assert!(handed <= 1 << 16, "a batch with no bound"); sink(&frame); true }; - assert!(net.node.receive(net.now, endless, draw(&mut net.draws)), "a pass is owed"); - assert!(handed > 1, "a bound of one frame is a pass a frame"); + let took = net.node.receive(net.now, BUDGET, endless, draw(&mut net.draws)); + assert_eq!((took.frames, took.owed, handed), (BUDGET, true, BUDGET), "the batch is the budget, and a pass is owed"); assert_eq!(client.borrow().inbox, b"again", "the rest of the batch was the same segment again"); let next = net.far.text(b"once"); - let short: Vec<&[u8]> = vec![&next; handed - 1]; - assert!(!net.node.receive(net.now, batch(&short), draw(&mut net.draws)), "the ring was emptied"); + let short: Vec<&[u8]> = vec![&next; BUDGET - 1]; + let took = net.node.receive(net.now, BUDGET, batch(&short), draw(&mut net.draws)); + assert_eq!((took.frames, took.owed), (BUDGET - 1, false), "the ring was emptied"); assert_eq!(client.borrow().inbox, b"againonce"); } @@ -1084,7 +1084,7 @@ fn a_reset_in_the_batch_of_the_text_before_it_ends_the_pipes_after_the_text() { let (mut net, _, client) = established(); let text = net.far.text(b"last"); let rst = net.far.rst(); - net.node.receive(net.now, batch(&[&text, &rst]), draw(&mut net.draws)); + net.node.receive(net.now, BUDGET, batch(&[&text, &rst]), draw(&mut net.draws)); assert_eq!((client.borrow().inbox.clone(), let_go(&client)), (b"last".to_vec(), vec![PipeEnd::FromClient, PipeEnd::ToClient])); } @@ -1096,7 +1096,7 @@ fn text_the_pipe_had_no_room_for_outlives_the_reset() { client.borrow_mut().room = 0; let text = net.far.text(b"held back"); let rst = net.far.rst(); - net.node.receive(net.now, batch(&[&text, &rst]), draw(&mut net.draws)); + net.node.receive(net.now, BUDGET, batch(&[&text, &rst]), draw(&mut net.draws)); assert_eq!((client.borrow().inbox.len(), let_go(&client), net.node.streams()), (0, vec![PipeEnd::FromClient], 1)); client.borrow_mut().room = 65_536; diff --git a/system/netstack/shard/src/lib.rs b/system/netstack/shard/src/lib.rs index 9a3a7d90c66..e6592c4b633 100644 --- a/system/netstack/shard/src/lib.rs +++ b/system/netstack/shard/src/lib.rs @@ -242,6 +242,12 @@ impl Shard { // ---- the device ---- + /// The least round trip among the TCP connections whose data moved since the last call + /// ([`Tcp::take_least_rtt`]). + pub fn take_least_rtt(&mut self) -> Option { + self.tcp.take_least_rtt() + } + /// A frame the device received. pub fn receive(&mut self, now: Instant, frame: &[u8]) { match self.ip.receive(now, self.iface, frame) { diff --git a/system/netstack/shard/tcp/src/rx.rs b/system/netstack/shard/tcp/src/rx.rs index ec4873b08b8..afa55416017 100644 --- a/system/netstack/shard/tcp/src/rx.rs +++ b/system/netstack/shard/tcp/src/rx.rs @@ -150,6 +150,11 @@ impl Rx { self.buf.capacity() } + /// The receiver's round-trip estimate, once one has been sampled. + pub const fn rtt(&self) -> Option { + self.rtt + } + /// After in-order text of `len` bytes arrived at `now`, with its TSecr and that echo's age /// where it is one this end sent. pub fn sample_rtt(&mut self, len: u32, echo: Option<(u32, Option)>, now: Instant) { diff --git a/system/netstack/shard/tcp/src/stack.rs b/system/netstack/shard/tcp/src/stack.rs index 6fdd2cf6a83..6ad47360e8b 100644 --- a/system/netstack/shard/tcp/src/stack.rs +++ b/system/netstack/shard/tcp/src/stack.rs @@ -232,6 +232,8 @@ pub struct Tcp { port_table: [u16; 16], log: Log, scratch: Vec, + /// The least round trip of a connection whose data moved since [`Tcp::take_least_rtt`]. + least_rtt: Option, } fn slot(slots: &mut [Slot], index: u32, generation: u32) -> Option<&mut T> { @@ -389,6 +391,7 @@ impl Tcp { port_table, log: Log::default(), scratch: Vec::new(), + least_rtt: None, }) } @@ -396,6 +399,18 @@ impl Tcp { &self.log.counters } + /// The least round trip among the connections whose data moved since the last call: the + /// receiver's own estimate where in-order text arrived, the sender's SRTT where an + /// acknowledgment moved SND.UNA. `None` where none moved or none has an estimate yet. + /// + /// A caller that holds segments back by a fraction of it before they reach [`Self::receive`] + /// lengthens no such connection's round trip by more than that fraction, and holds back no + /// more than that fraction of a round trip's acknowledgments: the least wins, so a short path + /// beside a long one keeps its own clock. + pub fn take_least_rtt(&mut self) -> Option { + self.least_rtt.take() + } + /// Refusals and reachability advice since the last call. pub fn drain_events(&mut self) -> impl Iterator + '_ { self.log.drain() @@ -867,11 +882,16 @@ impl Tcp { } } Tcb::Sync(mut sync) => { - let una = sync.tx.una; + let (una, next) = (sync.tx.una, sync.rx.next); let verdict = sync.receive(seg, &mut ctx); if sync.tx.una != una { conn.soft = None; } + let received = (sync.rx.next != next).then(|| sync.rx.rtt()).flatten(); + let acknowledged = (sync.tx.una != una).then(|| sync.rtt.srtt()).flatten(); + for rtt in received.into_iter().chain(acknowledged) { + self.least_rtt = Some(self.least_rtt.map_or(rtt, |least| least.min(rtt))); + } return self.after_sync(index, tuple, sync, verdict, now); } ended @ Tcb::Ended(_) => conn.state = ended, diff --git a/system/netstack/shard/tcp/tests/rtt.rs b/system/netstack/shard/tcp/tests/rtt.rs index 4093801542c..08e62e278c8 100644 --- a/system/netstack/shard/tcp/tests/rtt.rs +++ b/system/netstack/shard/tcp/tests/rtt.rs @@ -121,3 +121,47 @@ fn s_rt_016_no_timer_when_all_is_acknowledged() { h.input(10, seg(5001).ack(1101)); assert_eq!(h.info().rtx_timer, None); } + +// ---- the least round trip of the connections whose data moved ---- + +/// Text that arrives in order reports the receiver's own estimate: an echo of the ACK sent at 0, +/// aged 15 ms. It is reported once, and the handshake that came before it moved no data. +#[test] +fn text_arriving_in_order_reports_the_receivers_round_trip_once() { + let mut h = fixture_ef(); + assert_eq!(h.tcp.take_least_rtt(), None, "the handshake moved no data"); + h.input(15, seg(5001).ack(1001).wnd(512).ts(50_015, 1000).len(1448)); + assert_eq!(h.tcp.take_least_rtt(), Some(ms(15))); + assert_eq!(h.tcp.take_least_rtt(), None, "taken"); +} + +/// Text above a hole moves nothing in order and reports nothing; the segment that fills the hole +/// does, with the estimate its own echo moved: the dup ACK of 20, aged 5 ms, an eighth of the way. +#[test] +fn text_above_a_hole_reports_nothing_until_the_hole_fills() { + let mut h = fixture_ef(); + h.input(15, seg(5001).ack(1001).wnd(512).ts(50_015, 1000).len(1448)); + h.tcp.take_least_rtt(); + h.input(20, seg(5001 + 2 * 1448).ack(1001).wnd(512).ts(50_020, 1015).len(1448)); + assert_eq!(h.tcp.take_least_rtt(), None, "nothing arrived in order"); + h.input(25, seg(5001 + 1448).ack(1001).wnd(512).ts(50_025, 1020).len(1448)); + assert_eq!(h.tcp.take_least_rtt(), Some(std::time::Duration::from_micros(13_750))); +} + +/// An acknowledgment that moves SND.UNA reports the sender's SRTT, and one segment that moves +/// both directions reports the lesser: text echoing 0 aged 30 ms, beside an SRTT of 12.5 ms. +#[test] +fn an_acknowledgment_reports_the_senders_round_trip_and_the_least_wins() { + let mut h = fixture_ef(); + h.send(0, 100); + h.input(30, seg(5001).ack(1101).wnd(512).ts(50_030, 1000)); + let srtt = h.info().srtt; + assert_eq!(srtt, Some(std::time::Duration::from_micros(12_500))); + assert_eq!(h.tcp.take_least_rtt(), srtt); + + h.send(30, 100); + h.input(60, seg(5001).ack(1201).wnd(512).ts(50_060, 1030).len(1448)); + let srtt = h.info().srtt.expect("an estimate"); + assert!(srtt < ms(30), "the sender's estimate is the lesser: {srtt:?}"); + assert_eq!(h.tcp.take_least_rtt(), Some(srtt)); +} diff --git a/system/netstack/src/cadence.rs b/system/netstack/src/cadence.rs new file mode 100644 index 00000000000..81eee826054 --- /dev/null +++ b/system/netstack/src/cadence.rs @@ -0,0 +1,146 @@ +//! The pass's one cadence: how long the card holds its interrupt after a pass that took frames. +//! Every frame of the next batch waits up to that long, and so does every acknowledgment the +//! batch owes, every pipe write to a client and every wake of one: they follow the pass one for +//! one, so nothing else is timed. +//! +//! **The interval is the lesser of two bounds, and never under [`FLOOR`] unless the ring says so:** +//! - **The ACK clock**: the least round trip among the streams whose data the batch moved +//! (`toyos_net_tcp::Tcp::take_least_rtt`), over [`K`]. The interval is added to the delay of +//! every acknowledgment and to every round-trip sample the peer takes, so it keeps at least `K` +//! acknowledgments a round trip on the sender's clock and lengthens the round trip by at most +//! one `K`th. A receiver's estimate already holds the interval, so under that feedback it +//! settles at `rtt · K / (K − 1)` and does not climb. A batch that moved no stream with an +//! estimate is held to [`FLOOR`]. +//! - **The ring**: the descriptors the device holds empty after the pass, above the +//! [`Ring::reserve`] it reports itself short at, must take one interval and one interval of +//! lateness of full-sized frames at the link's rate. +//! - **[`FLOOR`]**: interrupts come no faster than the wake path costs, so a flood cannot make +//! the wakes themselves the load. The ring wins over it: a wake too many costs cycles, and a +//! ring that overruns loses frames. + +use std::num::NonZeroU32; +use std::time::Duration; + +/// Acknowledgments a round trip, at the least, and the inverse of the most the interval +/// lengthens a round trip by. +pub const K: u32 = 16; + +/// The least interval: an estimate of one wake's cost — the kernel's delivery of the interrupt +/// and netstack's pass around its batch — on the hardware ToyOS targets. +pub const FLOOR: Duration = Duration::from_micros(30); + +/// A full-sized frame as the link carries it, in bits: the largest frame the stack sends or +/// takes (`toyos_net_ip::FRAME`), its FCS, the preamble and start delimiter, and the interframe +/// gap (IEEE 802.3 clause 4: 4, 8 and 12 bytes). +const WIRE_FRAME_BITS: u64 = (toyos_net_ip::FRAME as u64 + 4 + 8 + 12) * 8; + +/// The receive ring, as the pass leaves it. +#[derive(Clone, Copy, Debug)] +pub struct Ring { + /// Descriptors the device holds empty, every buffer the pass took given back. + pub free: usize, + /// Free descriptors at which the device reports itself short. + pub reserve: usize, + /// The link's rate. + pub mbps: NonZeroU32, +} + +/// How long the card holds its interrupt after a pass whose batch moved streams with +/// `least_rtt` as their least round trip, on `ring`. +pub fn cadence(least_rtt: Option, ring: Ring) -> Duration { + let clock = least_rtt.map_or(FLOOR, |rtt| (rtt / K).max(FLOOR)); + let frames = ring.free.saturating_sub(ring.reserve) as u64; + let ring_nanos = frames * WIRE_FRAME_BITS * 1000 / u64::from(ring.mbps.get()) / 2; + clock.min(Duration::from_nanos(ring_nanos)) +} + +#[cfg(test)] +mod tests { + use super::*; + + const GIGABIT: NonZeroU32 = NonZeroU32::new(1000).unwrap(); + + /// A ring of 1,000 that reports itself short at half, as the pass leaves it. + fn ring(mbps: NonZeroU32) -> Ring { + Ring { free: 999, reserve: 500, mbps } + } + + fn ms(n: u64) -> Duration { + Duration::from_millis(n) + } + + /// What `ring` holds above its reserve, at a gigabit: 499 full-sized frames, over two. + const GIGABIT_RING: Duration = Duration::from_nanos(499 * 12_304 / 2); + + /// 1,538 bytes on the wire for a 1,500-byte packet, 12,304 ns of a gigabit link. + #[test] + fn a_full_sized_frame_takes_12_304_ns_at_a_gigabit() { + assert_eq!(WIRE_FRAME_BITS, 1538 * 8); + assert_eq!(cadence(Some(Duration::MAX), Ring { free: 2, reserve: 0, mbps: GIGABIT }), Duration::from_nanos(12_304)); + } + + /// At every link rate from 10 Mb/s to 400 Gb/s, every round trip and every ring, the full-sized + /// frames that arrive at line rate in two intervals — one and its lateness — fit in what the + /// ring holds above its reserve, the floor included. + #[test] + fn the_interval_never_exceeds_what_the_ring_holds_at_any_link_rate() { + for mbps in [10, 100, 1000, 2500, 10_000, 25_000, 100_000, 400_000] { + let mbps = NonZeroU32::new(mbps).unwrap(); + for free in [0, 1, 64, 255, 500, 501, 999, 4095, 65_535] { + let ring = Ring { free, reserve: free.min(500), mbps }; + for rtt in [None, Some(Duration::ZERO), Some(Duration::from_micros(200)), Some(ms(15)), Some(ms(300)), Some(Duration::MAX)] { + let interval = cadence(rtt, ring); + let bits = 2 * interval.as_nanos() * u128::from(mbps.get()) / 1000; + let arrive = bits / u128::from(WIRE_FRAME_BITS); + let room = (free - ring.reserve) as u128; + assert!(arrive <= room, "{mbps} Mb/s, {free} free, rtt {rtt:?}: {interval:?} brings {arrive} frames into {room}"); + } + } + } + } + + #[test] + fn a_batch_with_no_round_trip_is_held_to_the_floor() { + assert_eq!(cadence(None, ring(GIGABIT)), FLOOR); + } + + /// The bound is a `K`th of the least round trip, and a round trip short enough to put that + /// under the floor gets the floor. + #[test] + fn the_least_round_trip_over_k_is_the_interval_above_the_floor() { + assert_eq!(cadence(Some(ms(16)), ring(GIGABIT)), ms(1)); + assert_eq!(cadence(Some(ms(15)), ring(GIGABIT)), Duration::from_micros(937) + Duration::from_nanos(500)); + assert_eq!(cadence(Some(FLOOR * K - Duration::from_nanos(K.into())), ring(GIGABIT)), FLOOR); + } + + /// A short path's stream beside a long path's sets the interval: the batch reports its least + /// round trip, and that is what bounds it. A long path alone is bounded by the ring. + #[test] + fn a_short_round_trip_bounds_the_interval_and_a_long_one_meets_the_ring() { + assert_eq!(cadence(Some(ms(2)), ring(GIGABIT)), Duration::from_micros(125)); + assert_eq!(cadence(Some(ms(300)), ring(GIGABIT)), GIGABIT_RING); + assert!(GIGABIT_RING < ms(300) / K); + } + + /// The interval lengthens the round trip the receiver measures by up to itself, and the + /// next interval is taken from that estimate. From the path's own round trip and from an + /// estimate far above it alike, the estimate moves only toward `rtt · K / (K − 1)` and + /// settles there: it never ratchets past it. + #[test] + fn under_its_own_feedback_the_round_trip_settles_and_does_not_ratchet() { + for path in [Duration::from_micros(500), ms(15), ms(40)] { + let settled = path * K / (K - 1); + for start in [path, ms(10_000)] { + let mut estimate = start; + for _ in 0..200 { + let next = path + cadence(Some(estimate), ring(GIGABIT)); + let toward = if start < settled { estimate <= next && next <= settled } else { settled <= next && next <= estimate }; + assert!(toward, "{path:?} from {start:?}: {estimate:?} moved to {next:?}, away from {settled:?}"); + estimate = next; + } + let off = settled.abs_diff(estimate); + assert!(off <= Duration::from_micros(1), "{path:?} from {start:?}: settled at {estimate:?}, not {settled:?}"); + } + } + } +} diff --git a/system/netstack/src/card.rs b/system/netstack/src/card.rs index e95a53dfaca..6223daf328b 100644 --- a/system/netstack/src/card.rs +++ b/system/netstack/src/card.rs @@ -1,6 +1,10 @@ +use std::num::NonZeroU32; +use std::time::Duration; + use toyos_i219::Part; use toyos_inspect::Snapshot; +use crate::cadence::{cadence, Ring, FLOOR}; use crate::i219; use crate::virtio_net::VirtioNet; @@ -21,9 +25,14 @@ impl Card { panic!("netstack: the NIC this program was given is not one it can drive — {why}") } + /// Paced from the start, before any batch has said what it moved. pub fn intel(claim: toyos::PciDev, part: Part) -> Self { match i219::Nic::open(claim, part) { - Ok(nic) => Self::Intel(nic), + Ok(nic) => { + let card = Self::Intel(nic); + card.pace(None); + card + } Err(why) => Self::undrivable(why), } } @@ -130,9 +139,42 @@ impl Card { snap.put("errors.crc", wire.crc_errors); } + /// One batch of received frames: `batch` is given the most it may take — the card's ring, + /// all of what one wait can bring — and the card's next frame on each call, and every buffer + /// it gave back reaches the card once it returns. + pub fn receive(&self, batch: impl FnOnce(usize, &mut dyn FnMut(&mut dyn FnMut(&[u8])) -> bool) -> R) -> R { + let budget = match self { + Self::Virtio(_) => crate::virtio_net::RX_BUF_COUNT, + Self::Intel(_) => toyos_i219::RX_RING - 1, + }; + let took = batch(budget, &mut |sink| self.rx(sink)); + if let Self::Intel(nic) = self { + nic.rx_flush(); + } + took + } + + /// Hold the card's interrupts after a batch that moved streams whose least round trip is + /// `least_rtt`, by [`cadence`] over the ring the batch left. virtio's device has no throttle. + pub fn pace(&self, least_rtt: Option) { + let Self::Intel(nic) = self else { return }; + let interval = match nic.link() { + toyos_i219::Link::Up { speed, .. } => cadence( + least_rtt, + Ring { + free: nic.rx_free(), + reserve: toyos_i219::RX_SHORT, + mbps: NonZeroU32::new(speed.mbps().into()).expect("a link's rate is not zero"), + }, + ), + toyos_i219::Link::Down => FLOOR, + }; + nic.throttle(u64::try_from(interval.as_nanos()).unwrap_or(u64::MAX)); + } + /// Hands the next received frame to `take` and gives its buffer back to /// the card once `take` returns; `false` when none waits. - pub fn rx(&self, take: impl FnOnce(&[u8])) -> bool { + fn rx(&self, take: &mut dyn FnMut(&[u8])) -> bool { match self { Self::Virtio(nic) => { let Some((index, len)) = nic.poll_rx() else { return false }; diff --git a/system/netstack/src/i219.rs b/system/netstack/src/i219.rs index a71fbc5a289..9dd79ac66c0 100644 --- a/system/netstack/src/i219.rs +++ b/system/netstack/src/i219.rs @@ -262,6 +262,21 @@ impl Nic { self.driver.borrow_mut().rx_done(frame); } + /// Hand the part every buffer the batch gave back. + pub fn rx_flush(&self) { + self.driver.borrow_mut().rx_flush(); + } + + /// Receive descriptors the part holds empty. + pub fn rx_free(&self) -> usize { + self.driver.borrow().rx_free() + } + + /// Hold the part's interrupts at least `nanos` apart. + pub fn throttle(&self, nanos: u64) { + self.driver.borrow_mut().throttle(nanos); + } + /// How many frames the transmit ring takes now. pub fn tx_room(&self) -> usize { self.driver.borrow_mut().tx_room() diff --git a/system/netstack/src/main.rs b/system/netstack/src/main.rs index 5024b8512fd..dcbb29c8272 100644 --- a/system/netstack/src/main.rs +++ b/system/netstack/src/main.rs @@ -9,7 +9,8 @@ //! random source, the kernel's pipes (`pipes`), the clients' connections //! (`client`) and their requests (`serve`). //! -//! **One pass**: the card's link and its received frames, as one batch, go to the node, then +//! **One pass**: the card's link and its received frames, as one batch, go to the node, and +//! what the batch moved sets how long the card holds its next interrupt (`cadence`); then //! every deadline that is due, then the node is offered the card's transmit //! room until it has no frame left or the card no room; what the node has to //! say is written to the log and to the clients that waited for it; and the @@ -42,6 +43,7 @@ use toyos_net_shard::{Config, Secrets}; use toyos_net_wire::ethernet::{IndividualMac, MacAddr}; use toyos_net_wire::Instant; +mod cadence; mod card; mod client; mod i219; @@ -380,7 +382,11 @@ fn main() { node.link(clock(), link_up, draw); } } - let owed = node.receive(clock(), |sink| card.rx(sink), draw); + let batch = card.receive(|budget, next| node.receive(clock(), budget, next, draw)); + // A pass that took no frame says nothing new about the streams the card's wait is for. + if batch.frames > 0 { + card.pace(batch.least_rtt); + } let now = clock(); if node.next_deadline().is_some_and(|at| at <= now) { node.fire(now, draw); @@ -422,7 +428,7 @@ fn main() { // were taken with this pass's. let nanos = |left: Duration| u64::try_from(left.as_nanos()).unwrap_or(u64::MAX); let mut timeout = node.next_deadline().map_or(u64::MAX, |at| nanos(at.since(clock()))); - if owed { + if batch.owed { timeout = 0; } if let Some(left) = leases.due_in() { From db771cfbda42bf54740c5d7eb0912b487b386ba9 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 11 Oct 2026 22:54:04 +0200 Subject: [PATCH 3/9] issues: acpi_press_across_a_lock_wait went red under load beside this branch's guest run Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...-across-a-lock-wait-went-red-under-load.md | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 issues/acpi-press-across-a-lock-wait-went-red-under-load.md diff --git a/issues/acpi-press-across-a-lock-wait-went-red-under-load.md b/issues/acpi-press-across-a-lock-wait-went-red-under-load.md new file mode 100644 index 00000000000..75bc0250341 --- /dev/null +++ b/issues/acpi-press-across-a-lock-wait-went-red-under-load.md @@ -0,0 +1,24 @@ +--- +status: open +kind: defect +opened: 2026-10-11 +--- + +# acpi_press_across_a_lock_wait went red under load + +`cargo test`, the whole guest suite, went red on one test at +`wt/toyos-lan` 777c96b2b: `acpi_press_across_a_lock_wait: "acpi: the ACPI +row: PM1a events " never reached the the boot`, 9 s after it started, with 60 +of 61 green. The guest log is posted whole on pull request #861. + +The host was a 14-core macOS machine. Two `--metal-readback` image builds ran +beside the suite, and the load average read 34.6 / 26.7 / 18.8 a minute after +the red. The boot it reads, `tests/images/acpicase.toml`, starts no netstack +and claims only the NVMe controller. That branch changes netstack, its TCP +stack and its I219 driver, so the change does not reach the boot. Nobody has +run the test again, because the tree's rule is that a red is not re-run to +green. + +**Exit**: the cause is found and fixed, and the fix is shown by a run under +at least this load, or the test is deleted with this file recording the +commit that restores it. From 614f709e9a3aab8861c98a046b3ff18f7ae336b2 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 11 Oct 2026 23:21:12 +0200 Subject: [PATCH 4/9] netstack's cadence takes K = 32, the value round 3 on the T14 chose by the rule stated before it ran MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit K is the passing value with the least median system cycles per MB, each K held against stage D's eighteen runs (stage D: this branch's first stage, the cadence's base). Round 3: three boots of each arm, alternated D B D B D B in one session on the T14, six 170,439,044-byte downloads a boot, two a boot at each K. - K = 8: fails guard 3. Its median LAN round trip during the download, 9.58 ms, is above D's highest run, 9.34 ms. Median 8.906 Mcyc/MB. - K = 16: fails guard 4. Its median D-SACKs sent, 0.141 per MB, are above D's highest run, 0.129. Median 9.659 Mcyc/MB. - K = 32: passes all four. No overrun in any of its runs (B logged none in three boots; D logged 25 in eighteen runs). Median 771.4 Mb/s against D's lowest run, 610.9. Medians tcp 22.95 ms, tls 41.1 ms, handshake RTT 15.5 ms, rcv_rtt at close 15.27 ms, LAN round trip 7.75 ms, each at or under D's highest run (23.7, 44.7, 15.6, 18.0, 9.34). Out-of-order segments 9.91 and D-SACKs 0.038 per MB, under D's highest (30.4, 0.129). Median 10.309 Mcyc/MB (APERF, at 1.913 GHz effective), against D's 14.289 (at 2.599 GHz). Both failures are narrow margins on six runs; the rule took them as stated. The settling test's shortest path moves from 500 µs to 1 ms: at K = 32 a 500 µs path's Kth is under the 30 µs floor, so the floor, not the feedback it tests, sets its interval. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...round-trip-reads-no-finer-than-a-millisecond.md | 14 ++++++++------ system/netstack/src/cadence.rs | 12 ++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md b/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md index 36dae1ab4d2..7870b0c665d 100644 --- a/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md +++ b/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md @@ -14,12 +14,14 @@ a LAN's, reads as at least 1 ms. The pass's cadence (`system/netstack/src/cadence.rs`) holds the card's interrupt for a `K`th of the least round trip. On a 200 µs path that round -trip reads 1 ms, so the cadence asks for 62.5 µs where a `K`th of the true -one, 12.5 µs, would give the 30 µs floor. The I219 takes 62.5 µs as 57.3 µs, -its three leading bits. So the interval is about 29% of the round trip, not a -`K`th: the sender gets about three acknowledgments a round trip, and each -round trip grows by about 29%. These figures are derived from `K = 16`, the -tick and the rounding. No machine has measured a sub-millisecond path. +trip reads 1 ms, so the cadence asks for 31.25 µs where a `K`th of the true +one, 6.25 µs, would give the 30 µs floor. The I219 keeps three leading bits +and applies both as 28.7 µs, so on that part the tick costs such a path +nothing today; a card that applies an interval exactly would hold 31.25 µs +for 30. Either is about 15% of the round trip, not a `K`th: under about +960 µs the floor sets the interval, and the tick only moves it above. These +figures are derived from `K = 32`, the tick and the rounding. No machine has +measured a sub-millisecond path. **Exit**: a stream on a sub-millisecond path reports its round trip to within a `K`th of the path's own, read by a host test that delivers echoes on such a diff --git a/system/netstack/src/cadence.rs b/system/netstack/src/cadence.rs index 81eee826054..430854b7335 100644 --- a/system/netstack/src/cadence.rs +++ b/system/netstack/src/cadence.rs @@ -23,7 +23,7 @@ use std::time::Duration; /// Acknowledgments a round trip, at the least, and the inverse of the most the interval /// lengthens a round trip by. -pub const K: u32 = 16; +pub const K: u32 = 32; /// The least interval: an estimate of one wake's cost — the kernel's delivery of the interrupt /// and netstack's pass around its batch — on the hardware ToyOS targets. @@ -108,8 +108,8 @@ mod tests { /// under the floor gets the floor. #[test] fn the_least_round_trip_over_k_is_the_interval_above_the_floor() { - assert_eq!(cadence(Some(ms(16)), ring(GIGABIT)), ms(1)); - assert_eq!(cadence(Some(ms(15)), ring(GIGABIT)), Duration::from_micros(937) + Duration::from_nanos(500)); + assert_eq!(cadence(Some(ms(16)), ring(GIGABIT)), Duration::from_micros(500)); + assert_eq!(cadence(Some(ms(15)), ring(GIGABIT)), Duration::from_nanos(468_750)); assert_eq!(cadence(Some(FLOOR * K - Duration::from_nanos(K.into())), ring(GIGABIT)), FLOOR); } @@ -117,7 +117,7 @@ mod tests { /// round trip, and that is what bounds it. A long path alone is bounded by the ring. #[test] fn a_short_round_trip_bounds_the_interval_and_a_long_one_meets_the_ring() { - assert_eq!(cadence(Some(ms(2)), ring(GIGABIT)), Duration::from_micros(125)); + assert_eq!(cadence(Some(ms(2)), ring(GIGABIT)), Duration::from_nanos(62_500)); assert_eq!(cadence(Some(ms(300)), ring(GIGABIT)), GIGABIT_RING); assert!(GIGABIT_RING < ms(300) / K); } @@ -125,10 +125,10 @@ mod tests { /// The interval lengthens the round trip the receiver measures by up to itself, and the /// next interval is taken from that estimate. From the path's own round trip and from an /// estimate far above it alike, the estimate moves only toward `rtt · K / (K − 1)` and - /// settles there: it never ratchets past it. + /// settles there: it never ratchets past it. The shortest path's `K`th clears the floor. #[test] fn under_its_own_feedback_the_round_trip_settles_and_does_not_ratchet() { - for path in [Duration::from_micros(500), ms(15), ms(40)] { + for path in [ms(1), ms(15), ms(40)] { let settled = path * K / (K - 1); for start in [path, ms(10_000)] { let mut estimate = start; From a6ac028e05c2e2cf5f5ed1229f16ca3d977af682 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 11 Oct 2026 23:51:25 +0200 Subject: [PATCH 5/9] =?UTF-8?q?netstack's=20cadence=20bounds=20the=20burst?= =?UTF-8?q?=20an=20ACK=20releases=20and=20holds=20every=20wake=20128=20?= =?UTF-8?q?=C2=B5s=20apart=20on=20the=20I219;=20its=20watches=20are=20kept?= =?UTF-8?q?=20by=20handle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 3 on the T14 left B's throughput under D's, with seven of its eighteen runs past D's highest D-SACK rate. Read from round 3's own counters, B's D-SACKs track its long ACK gaps (Spearman +0.94 over its 18 runs), which mark loss episodes, and B's losses come in runs: it filled about three times D's holes, and in its worst runs a hole fill per out-of-order segment and more, the tail of a burst lost upstream. B's ACK stretch was 29 to 120+ segments at every K against D's 12, and K did not order the losses. So the cadence now bounds the stretch itself: an interval of at most STRETCH full-sized frames at the link's rate, whatever the round trip. STRETCH = 16 is the candidate; round 4 on the T14 settles it between 8, 16 and 32 by the rule the pull request states, with an instrument that classes each D-SACK by whether this end had already acknowledged the duplicated data. The floor was an estimate of one wake's cost, 30 µs, so any batch with no round trip, a flood for nobody's among them, and any stream on a path under 960 µs could wake netstack about 35,000 times a second. It is now a wake ceiling: 160 µs, under a fifth of a CPU at 30 µs a wake, which the I219's rounding still applies as 128 µs or more. A batch with no round trip is held to the stretch above it. A host test holds every batch at every I219 rate to 128 µs as the part applies it, through the driver's own rounding, now the pure `toyos_i219::throttle_units`. `watches::Armed` is kept by handle, as the kernel keeps polls: a watch of a handle under another token withdraws its earlier poll unanswered, and a record kept by token would then skip a later identical submit. `cadence::Ring` takes the room above the part's report, which the card computes; the reserve it carried had one production value. `pipe_other_end_gone` holds that an end its own holder closes answers its OTHER_END_GONE watch with NotFound. Issues: the no-round-trip floor issue is deleted, its defect gone; the receiver round trip's figures follow the new floor; the acpi red's issue says what rules the kernel change out; and the pre-existing duplicate-handle cancellation is filed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...no-tcp-round-trip-is-paced-at-the-floor.md | 24 ---- ...-across-a-lock-wait-went-red-under-load.md | 8 +- ...pipe-end-ends-every-holders-polls-on-it.md | 24 ++++ ...-trip-reads-no-finer-than-a-millisecond.md | 16 +-- system/netstack/i219/src/lib.rs | 32 +++-- system/netstack/src/cadence.rs | 126 +++++++++++------- system/netstack/src/card.rs | 3 +- system/netstack/src/watches.rs | 62 +++++++-- .../rust-tests/src/bin/pipe_other_end_gone.rs | 40 ++++++ 9 files changed, 230 insertions(+), 105 deletions(-) delete mode 100644 issues/a-receive-flow-with-no-tcp-round-trip-is-paced-at-the-floor.md create mode 100644 issues/closing-a-pipe-end-ends-every-holders-polls-on-it.md diff --git a/issues/a-receive-flow-with-no-tcp-round-trip-is-paced-at-the-floor.md b/issues/a-receive-flow-with-no-tcp-round-trip-is-paced-at-the-floor.md deleted file mode 100644 index 94ba0529e42..00000000000 --- a/issues/a-receive-flow-with-no-tcp-round-trip-is-paced-at-the-floor.md +++ /dev/null @@ -1,24 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-11 ---- - -# A receive flow with no TCP round trip is paced at the floor - -The pass's cadence (`system/netstack/src/cadence.rs`) bounds the card's -interrupt interval by the least round trip of the TCP streams a batch moved, -and holds a batch that moved none to `FLOOR` (30 µs). A sustained flow that -moves no TCP stream gets that floor: UDP, which includes QUIC, and a flood of -frames for nobody. The I219 takes 30 µs as 28.7 µs, so on such a flow the card -may wake netstack about 35,000 times a second. Before the cadence the fixed -128 µs throttle held it to about 7,800. The floor is an estimate of one wake's -cost, so at the floor the wakes can take a whole CPU. - -Netstack has no round trip for such a flow. QUIC's round trip is its -endpoint's, inside the app, and a flood has none. Pacing it at the ring's -bound would add up to that bound to every datagram's latency. - -**Exit**: a sustained receive flow that moves no TCP stream costs no more -wakes per megabyte than a TCP download at the same rate, with its latency -bound stated. A T14 row reads both. diff --git a/issues/acpi-press-across-a-lock-wait-went-red-under-load.md b/issues/acpi-press-across-a-lock-wait-went-red-under-load.md index 75bc0250341..471dc6cfc04 100644 --- a/issues/acpi-press-across-a-lock-wait-went-red-under-load.md +++ b/issues/acpi-press-across-a-lock-wait-went-red-under-load.md @@ -15,9 +15,11 @@ The host was a 14-core macOS machine. Two `--metal-readback` image builds ran beside the suite, and the load average read 34.6 / 26.7 / 18.8 a minute after the red. The boot it reads, `tests/images/acpicase.toml`, starts no netstack and claims only the NVMe controller. That branch changes netstack, its TCP -stack and its I219 driver, so the change does not reach the boot. Nobody has -run the test again, because the tree's rule is that a red is not re-run to -green. +stack and its I219 driver, which the boot does not run, and the kernel's pipe +`release` and `close`, which every boot runs. The kernel and the ABI at +777c96b2b are byte for byte those at c91e72b83, where the whole suite was 61 +of 61 green. Nobody has run the test again, because the tree's rule is that a +red is not re-run to green. **Exit**: the cause is found and fixed, and the fix is shown by a run under at least this load, or the test is deleted with this file recording the diff --git a/issues/closing-a-pipe-end-ends-every-holders-polls-on-it.md b/issues/closing-a-pipe-end-ends-every-holders-polls-on-it.md new file mode 100644 index 00000000000..2e467ff1557 --- /dev/null +++ b/issues/closing-a-pipe-end-ends-every-holders-polls-on-it.md @@ -0,0 +1,24 @@ +--- +status: open +kind: defect +opened: 2026-10-11 +--- + +# Closing a pipe end ends every holder's polls on it + +`ops::close` (`kernel/src/object/ops.rs`) cancels every poll on the watches of +the object the closed handle names: a pipe end's data watches and its leaving +watch, and those of a connection, an acceptor, a file and the other objects +`close_ends_polls` lists. `Watch::cancel_rings` (`kernel/pure/sched/watch.rs`) +takes every ring registered on the watch, whoever registered it. So when two +handles name one pipe end — a duplicate, or a handle moved to another process +that kept one — closing either answers the other's poll on its live handle +with `NotFound`, as though that handle had been closed. Its holder reads a +watch it still needs as over. + +Found by reading, in review of pull request #861, which put the pipe end's +leaving watch in that same loop. No test or machine has shown it. + +**Exit**: a close ends only the polls made through the handle it closes, and a +guest test that holds a duplicate of a pipe end, watches it, and closes the +other handle reads no answer on it. diff --git a/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md b/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md index 7870b0c665d..aa6239e8a8a 100644 --- a/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md +++ b/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md @@ -13,15 +13,13 @@ and never less than one tick. A path whose round trip is under a millisecond, a LAN's, reads as at least 1 ms. The pass's cadence (`system/netstack/src/cadence.rs`) holds the card's -interrupt for a `K`th of the least round trip. On a 200 µs path that round -trip reads 1 ms, so the cadence asks for 31.25 µs where a `K`th of the true -one, 6.25 µs, would give the 30 µs floor. The I219 keeps three leading bits -and applies both as 28.7 µs, so on that part the tick costs such a path -nothing today; a card that applies an interval exactly would hold 31.25 µs -for 30. Either is about 15% of the round trip, not a `K`th: under about -960 µs the floor sets the interval, and the tick only moves it above. These -figures are derived from `K = 32`, the tick and the rounding. No machine has -measured a sub-millisecond path. +interrupt for a `K`th of the least round trip, never under its 160 µs floor, +so with `K = 32` every path under 5.12 ms is held to the floor and the tick +moves no interval. What it moves is the receive buffer's growth, twice what the +user read per round trip (`rx.rs`): a 200 µs path that reads 1 ms grows its +buffer to five times what it needs. These figures are derived from the floor, +`K`, the tick and the growth rule. No machine has measured a sub-millisecond +path. **Exit**: a stream on a sub-millisecond path reports its round trip to within a `K`th of the path's own, read by a host test that delivers echoes on such a diff --git a/system/netstack/i219/src/lib.rs b/system/netstack/i219/src/lib.rs index 4249bfad034..f284e2a3807 100644 --- a/system/netstack/i219/src/lib.rs +++ b/system/netstack/i219/src/lib.rs @@ -181,6 +181,21 @@ pub const TX_BUF_BYTES: usize = 2048; /// throttle at all. pub const THROTTLE_UNIT_NANOS: u64 = 256; +/// What [`I219::throttle`] writes `ITR` with for an interval of `nanos`: its +/// units at or below `nanos`, between one and the field's largest, kept to +/// their three leading bits. +/// +/// **Rounded down**, so the interval never exceeds what the caller bounded it +/// by, and **to three leading bits**, so an interval the caller recomputes +/// every pass moves the register only on a step of an eighth to a quarter, and +/// loses at most a fifth of what was asked. +pub const fn throttle_units(nanos: u64) -> u32 { + let units = nanos / THROTTLE_UNIT_NANOS; + let units = if units < 1 { 1 } else if units > u16::MAX as u64 { u16::MAX as u32 } else { units as u32 }; + let low = units.ilog2().saturating_sub(2); + units >> low << low +} + const _: () = { // §7.1.8 and §7.2.4: both ring lengths are programmed in bytes and "must be // a multiple of 128". @@ -1318,20 +1333,11 @@ impl I219 { } /// Hold the function's interrupts at least `nanos` apart (§10.2.4.2's - /// `ITR`): its units at or below `nanos`, between one and the field's - /// largest, kept to their three leading bits; the register is written only - /// when that changes. - /// - /// **Rounded down**, so the interval never exceeds what the caller bounded - /// it by, and **to three leading bits**, so an interval the caller - /// recomputes every pass moves the register only on a step of an eighth - /// to a quarter, and loses at most a fifth of what was asked. The throttle - /// runs from the last interrupt, so a frame after a quiet spell that long - /// interrupts at once. + /// `ITR`), as [`throttle_units`] puts it; the register is written only + /// when that changes. The throttle runs from the last interrupt, so a frame + /// after a quiet spell that long interrupts at once. pub fn throttle(&mut self, nanos: u64) { - let units = (nanos / THROTTLE_UNIT_NANOS).clamp(1, u64::from(u16::MAX)) as u32; - let low = units.ilog2().saturating_sub(2); - let units = units >> low << low; + let units = throttle_units(nanos); if units != self.throttle { self.throttle = units; self.regs.write32(regs::ITR, units); diff --git a/system/netstack/src/cadence.rs b/system/netstack/src/cadence.rs index 430854b7335..f5bb1744e27 100644 --- a/system/netstack/src/cadence.rs +++ b/system/netstack/src/cadence.rs @@ -3,20 +3,24 @@ //! batch owes, every pipe write to a client and every wake of one: they follow the pass one for //! one, so nothing else is timed. //! -//! **The interval is the lesser of two bounds, and never under [`FLOOR`] unless the ring says so:** +//! **The interval is the least of three bounds, never under [`FLOOR`], and the ring's wins over +//! the floor:** //! - **The ACK clock**: the least round trip among the streams whose data the batch moved //! (`toyos_net_tcp::Tcp::take_least_rtt`), over [`K`]. The interval is added to the delay of //! every acknowledgment and to every round-trip sample the peer takes, so it keeps at least `K` //! acknowledgments a round trip on the sender's clock and lengthens the round trip by at most //! one `K`th. A receiver's estimate already holds the interval, so under that feedback it //! settles at `rtt · K / (K − 1)` and does not climb. A batch that moved no stream with an -//! estimate is held to [`FLOOR`]. -//! - **The ring**: the descriptors the device holds empty after the pass, above the -//! [`Ring::reserve`] it reports itself short at, must take one interval and one interval of -//! lateness of full-sized frames at the link's rate. -//! - **[`FLOOR`]**: interrupts come no faster than the wake path costs, so a flood cannot make -//! the wakes themselves the load. The ring wins over it: a wake too many costs cycles, and a -//! ring that overruns loses frames. +//! estimate has no clock to keep. +//! - **The stretch**: [`STRETCH`] full-sized frames at the link's rate. A sender that does not pace +//! answers an acknowledgment with a burst of what it acknowledges, and a queue on the path has +//! to hold that burst or lose its tail; this bounds it whatever the round trip. +//! - **The ring**: the descriptors the device holds empty above the count it reports itself short +//! at must take one interval and one interval of lateness of full-sized frames at the link's +//! rate. +//! - **[`FLOOR`]**: what any peer's frames can make of netstack's wakes, a flood for nobody's or a +//! stream's on a short path. The ring wins over it: a wake too many costs cycles, and a ring +//! that overruns loses frames. use std::num::NonZeroU32; use std::time::Duration; @@ -25,9 +29,15 @@ use std::time::Duration; /// lengthens a round trip by. pub const K: u32 = 32; -/// The least interval: an estimate of one wake's cost — the kernel's delivery of the interrupt -/// and netstack's pass around its batch — on the hardware ToyOS targets. -pub const FLOOR: Duration = Duration::from_micros(30); +/// The most full-sized frames a batch holds at the link's rate, and so the most an +/// acknowledgment releases from a sender that does not pace. +pub const STRETCH: u64 = 16; + +/// The least interval. A wake — the kernel's delivery of the interrupt and netstack's pass around +/// its batch — costs about 30 µs on the hardware ToyOS targets, so wakes this far apart take under +/// a fifth of a CPU; and the I219, which rounds an interval down by at most a fifth, still holds +/// them 128 µs apart. +pub const FLOOR: Duration = Duration::from_micros(160); /// A full-sized frame as the link carries it, in bits: the largest frame the stack sends or /// takes (`toyos_net_ip::FRAME`), its FCS, the preamble and start delimiter, and the interframe @@ -37,10 +47,9 @@ const WIRE_FRAME_BITS: u64 = (toyos_net_ip::FRAME as u64 + 4 + 8 + 12) * 8; /// The receive ring, as the pass leaves it. #[derive(Clone, Copy, Debug)] pub struct Ring { - /// Descriptors the device holds empty, every buffer the pass took given back. - pub free: usize, - /// Free descriptors at which the device reports itself short. - pub reserve: usize, + /// Descriptors the device holds empty, every buffer the pass took given back, above the count + /// at which it reports itself short. + pub room: usize, /// The link's rate. pub mbps: NonZeroU32, } @@ -48,10 +57,10 @@ pub struct Ring { /// How long the card holds its interrupt after a pass whose batch moved streams with /// `least_rtt` as their least round trip, on `ring`. pub fn cadence(least_rtt: Option, ring: Ring) -> Duration { - let clock = least_rtt.map_or(FLOOR, |rtt| (rtt / K).max(FLOOR)); - let frames = ring.free.saturating_sub(ring.reserve) as u64; - let ring_nanos = frames * WIRE_FRAME_BITS * 1000 / u64::from(ring.mbps.get()) / 2; - clock.min(Duration::from_nanos(ring_nanos)) + let frames = |n: u64| Duration::from_nanos(n * WIRE_FRAME_BITS * 1000 / u64::from(ring.mbps.get())); + let stretch = frames(STRETCH); + let clock = least_rtt.map_or(stretch, |rtt| (rtt / K).min(stretch)); + clock.max(FLOOR).min(frames(ring.room as u64) / 2) } #[cfg(test)] @@ -60,23 +69,25 @@ mod tests { const GIGABIT: NonZeroU32 = NonZeroU32::new(1000).unwrap(); - /// A ring of 1,000 that reports itself short at half, as the pass leaves it. + /// The rates an I219 links at. + const I219_RATES: [u32; 3] = [10, 100, 1000]; + + /// A ring of 1,000 that reports itself short at half, as a pass that took every frame leaves it. fn ring(mbps: NonZeroU32) -> Ring { - Ring { free: 999, reserve: 500, mbps } + Ring { room: 499, mbps } } fn ms(n: u64) -> Duration { Duration::from_millis(n) } - /// What `ring` holds above its reserve, at a gigabit: 499 full-sized frames, over two. - const GIGABIT_RING: Duration = Duration::from_nanos(499 * 12_304 / 2); - /// 1,538 bytes on the wire for a 1,500-byte packet, 12,304 ns of a gigabit link. + const GIGABIT_FRAME: Duration = Duration::from_nanos(12_304); + #[test] fn a_full_sized_frame_takes_12_304_ns_at_a_gigabit() { assert_eq!(WIRE_FRAME_BITS, 1538 * 8); - assert_eq!(cadence(Some(Duration::MAX), Ring { free: 2, reserve: 0, mbps: GIGABIT }), Duration::from_nanos(12_304)); + assert_eq!(cadence(Some(Duration::MAX), Ring { room: 2, mbps: GIGABIT }), GIGABIT_FRAME); } /// At every link rate from 10 Mb/s to 400 Gb/s, every round trip and every ring, the full-sized @@ -86,54 +97,79 @@ mod tests { fn the_interval_never_exceeds_what_the_ring_holds_at_any_link_rate() { for mbps in [10, 100, 1000, 2500, 10_000, 25_000, 100_000, 400_000] { let mbps = NonZeroU32::new(mbps).unwrap(); - for free in [0, 1, 64, 255, 500, 501, 999, 4095, 65_535] { - let ring = Ring { free, reserve: free.min(500), mbps }; + for room in [0, 1, 64, 255, 499, 4095, 65_535] { + let ring = Ring { room, mbps }; for rtt in [None, Some(Duration::ZERO), Some(Duration::from_micros(200)), Some(ms(15)), Some(ms(300)), Some(Duration::MAX)] { let interval = cadence(rtt, ring); let bits = 2 * interval.as_nanos() * u128::from(mbps.get()) / 1000; let arrive = bits / u128::from(WIRE_FRAME_BITS); - let room = (free - ring.reserve) as u128; - assert!(arrive <= room, "{mbps} Mb/s, {free} free, rtt {rtt:?}: {interval:?} brings {arrive} frames into {room}"); + assert!(arrive <= room as u128, "{mbps} Mb/s, room {room}, rtt {rtt:?}: {interval:?} brings {arrive} frames into {room}"); } } } } + /// Whatever a batch moved — nothing with a round trip, as a flood for nobody or a datagram flow + /// does, or a stream on any path — a pass that took every frame holds an I219 at any of its + /// rates at least 128 µs, as the part applies it: no peer wakes netstack more often. + #[test] + fn no_batch_holds_an_i219_under_128_us() { + for mbps in I219_RATES { + for rtt in [None, Some(Duration::ZERO), Some(Duration::from_micros(200)), Some(ms(1)), Some(ms(15)), Some(Duration::MAX)] { + let asked = cadence(rtt, ring(NonZeroU32::new(mbps).unwrap())); + let nanos = u64::try_from(asked.as_nanos()).unwrap(); + let applied = u64::from(toyos_i219::throttle_units(nanos)) * toyos_i219::THROTTLE_UNIT_NANOS; + assert!(applied >= 128_000, "{mbps} Mb/s, rtt {rtt:?}: asked {asked:?}, the part holds {applied} ns"); + } + } + } + + /// A batch that moved no stream with a round trip is held to the stretch, and to the floor + /// where the stretch is shorter. #[test] - fn a_batch_with_no_round_trip_is_held_to_the_floor() { - assert_eq!(cadence(None, ring(GIGABIT)), FLOOR); + fn a_batch_with_no_round_trip_is_held_to_the_stretch_above_the_floor() { + assert_eq!(cadence(None, ring(GIGABIT)), GIGABIT_FRAME * STRETCH as u32); + assert_eq!(cadence(None, ring(NonZeroU32::new(10_000).unwrap())), FLOOR); } - /// The bound is a `K`th of the least round trip, and a round trip short enough to put that - /// under the floor gets the floor. + /// The bound is a `K`th of the least round trip between the floor and the stretch: a round + /// trip short enough to put it under the floor gets the floor, and one long enough to put it + /// over the stretch gets the stretch. #[test] - fn the_least_round_trip_over_k_is_the_interval_above_the_floor() { - assert_eq!(cadence(Some(ms(16)), ring(GIGABIT)), Duration::from_micros(500)); - assert_eq!(cadence(Some(ms(15)), ring(GIGABIT)), Duration::from_nanos(468_750)); - assert_eq!(cadence(Some(FLOOR * K - Duration::from_nanos(K.into())), ring(GIGABIT)), FLOOR); + fn the_least_round_trip_over_k_is_the_interval_between_the_floor_and_the_stretch() { + let ten_mbps = ring(NonZeroU32::new(10).unwrap()); + assert_eq!(cadence(Some(ms(16)), ten_mbps), Duration::from_micros(500)); + assert_eq!(cadence(Some(ms(15)), ten_mbps), Duration::from_nanos(468_750)); + assert_eq!(cadence(Some(FLOOR * K - Duration::from_nanos(K.into())), ten_mbps), FLOOR); + assert_eq!(cadence(Some(ms(15)), ring(GIGABIT)), GIGABIT_FRAME * STRETCH as u32); } /// A short path's stream beside a long path's sets the interval: the batch reports its least - /// round trip, and that is what bounds it. A long path alone is bounded by the ring. + /// round trip, and that is what bounds it. A long path alone is bounded by the stretch, and on + /// a ring with less room than the stretch, by the ring. #[test] - fn a_short_round_trip_bounds_the_interval_and_a_long_one_meets_the_ring() { - assert_eq!(cadence(Some(ms(2)), ring(GIGABIT)), Duration::from_nanos(62_500)); - assert_eq!(cadence(Some(ms(300)), ring(GIGABIT)), GIGABIT_RING); - assert!(GIGABIT_RING < ms(300) / K); + fn a_short_round_trip_bounds_the_interval_and_a_long_one_meets_the_stretch_or_the_ring() { + let ten_mbps = ring(NonZeroU32::new(10).unwrap()); + assert_eq!(cadence(Some(ms(8)), ten_mbps), Duration::from_micros(250)); + assert_eq!(cadence(Some(ms(1000)), ten_mbps), Duration::from_nanos(1_230_400) * STRETCH as u32); + let short = Ring { room: 20, mbps: GIGABIT }; + assert_eq!(cadence(Some(ms(1000)), short), GIGABIT_FRAME * 10); } /// The interval lengthens the round trip the receiver measures by up to itself, and the /// next interval is taken from that estimate. From the path's own round trip and from an /// estimate far above it alike, the estimate moves only toward `rtt · K / (K − 1)` and - /// settles there: it never ratchets past it. The shortest path's `K`th clears the floor. + /// settles there: it never ratchets past it. Each path's `K`th clears the floor and is + /// under the stretch of a 10 Mb/s link. #[test] fn under_its_own_feedback_the_round_trip_settles_and_does_not_ratchet() { - for path in [ms(1), ms(15), ms(40)] { + let ten_mbps = ring(NonZeroU32::new(10).unwrap()); + for path in [ms(6), ms(15), ms(40)] { let settled = path * K / (K - 1); for start in [path, ms(10_000)] { let mut estimate = start; for _ in 0..200 { - let next = path + cadence(Some(estimate), ring(GIGABIT)); + let next = path + cadence(Some(estimate), ten_mbps); let toward = if start < settled { estimate <= next && next <= settled } else { settled <= next && next <= estimate }; assert!(toward, "{path:?} from {start:?}: {estimate:?} moved to {next:?}, away from {settled:?}"); estimate = next; diff --git a/system/netstack/src/card.rs b/system/netstack/src/card.rs index 6223daf328b..3e266577531 100644 --- a/system/netstack/src/card.rs +++ b/system/netstack/src/card.rs @@ -162,8 +162,7 @@ impl Card { toyos_i219::Link::Up { speed, .. } => cadence( least_rtt, Ring { - free: nic.rx_free(), - reserve: toyos_i219::RX_SHORT, + room: nic.rx_free().saturating_sub(toyos_i219::RX_SHORT), mbps: NonZeroU32::new(speed.mbps().into()).expect("a link's rate is not zero"), }, ), diff --git a/system/netstack/src/watches.rs b/system/netstack/src/watches.rs index 7dd5f28323f..a3c668c67a8 100644 --- a/system/netstack/src/watches.rs +++ b/system/netstack/src/watches.rs @@ -7,11 +7,16 @@ //! handle, interest and token replaces a poll with its twin and costs the //! submit the poll's registration on every watch it names, for nothing. //! -//! **Every armed watch is answered, once**: by readiness, by a refusal, or by -//! `NotFound` when a handle it names is closed, so a record here is let go by -//! the answer under its token and never outlives the poll it records. A record -//! whose poll was answered and whose answer is not yet read costs one -//! needless submit after that answer, and never a missed one. +//! **The kernel keeps one poll a handle** (`admit` in `kernel/src/inbox/polls.rs`): +//! a watch of a handle under another token withdraws the handle's earlier poll +//! unanswered. So a record is kept by handle, and a watch under another token +//! replaces it as the kernel replaces the poll. +//! +//! **Every poll the kernel keeps is answered, once**: by readiness, by a +//! refusal, or by `NotFound` when a handle it names is closed, so a record here +//! is let go by the answer under its token and never outlives the poll it +//! records. A record whose poll was answered and whose answer is not yet read +//! costs one needless submit after that answer, and never a missed one. use std::collections::HashMap; @@ -19,20 +24,36 @@ use toyos::poller::Poller; use toyos::AsHandle; use toyos_abi::syscall::SyscallError; -/// The watches the kernel holds armed for this process, by token. +/// The watches the kernel holds armed for this process: each handle's token +/// and interest, and each token's handle. #[derive(Default)] -pub struct Armed(HashMap); +pub struct Armed { + by_handle: HashMap, + by_token: HashMap, +} impl Armed { /// Whether a watch of `flags` on `handle` under `token` has to be /// submitted, recording it as armed where it does. pub fn submit(&mut self, token: u64, handle: u32, flags: u32) -> bool { - self.0.insert(token, (handle, flags)) != Some((handle, flags)) + let was = self.by_handle.insert(handle, (token, flags)); + if was == Some((token, flags)) { + return false; + } + if let Some((other, _)) = was.filter(|&(other, _)| other != token) { + self.by_token.remove(&other); + } + if let Some(moved) = self.by_token.insert(token, handle).filter(|&moved| moved != handle) { + self.by_handle.remove(&moved); + } + true } /// The kernel answered the watch under `token`: it is armed no more. pub fn answered(&mut self, token: u64) { - self.0.remove(&token); + if let Some(handle) = self.by_token.remove(&token) { + self.by_handle.remove(&handle); + } } } @@ -107,6 +128,29 @@ mod tests { assert!(armed.submit(TOKEN, 8, READABLE)); } + /// A handle watched under another token is submitted, and so is the first + /// token's watch of it after that: the kernel withdrew that poll when the + /// second replaced it. + #[test] + fn a_handle_under_another_token_is_submitted_and_so_is_the_first_again() { + let mut armed = Armed::default(); + assert!(armed.submit(TOKEN, 7, READABLE)); + assert!(armed.submit(TOKEN + 1, 7, READABLE)); + assert!(armed.submit(TOKEN, 7, READABLE)); + assert!(!armed.submit(TOKEN, 7, READABLE)); + } + + /// The first token's answer, read after the handle moved to a second, + /// lets the second's record stand. + #[test] + fn an_answer_under_a_replaced_token_lets_the_handles_record_stand() { + let mut armed = Armed::default(); + assert!(armed.submit(TOKEN, 7, READABLE)); + assert!(armed.submit(TOKEN + 1, 7, READABLE)); + armed.answered(TOKEN); + assert!(!armed.submit(TOKEN + 1, 7, READABLE)); + } + /// The answer of one token lets no other's record go. #[test] fn an_answer_is_its_own_tokens_alone() { diff --git a/tests/guest/rust-tests/src/bin/pipe_other_end_gone.rs b/tests/guest/rust-tests/src/bin/pipe_other_end_gone.rs index f67c2e683ec..4787208946f 100644 --- a/tests/guest/rust-tests/src/bin/pipe_other_end_gone.rs +++ b/tests/guest/rust-tests/src/bin/pipe_other_end_gone.rs @@ -12,6 +12,10 @@ //! **The refusal.** A connection and a file have no other end, and a watch that //! asks for one is refused whatever else it asks. //! +//! **Its own close.** An end whose holder closes it while the other end lives +//! answers its watch with `NotFound`: the poll is over, and nothing else would +//! end it. +//! //! **Not gone.** The other end has a holder while a duplicate of its handle is //! held, while the handle sits unreceived in a connection's queue, and after //! it moved to a child that still lives: the watch stays armed through each, @@ -75,6 +79,17 @@ fn test() { syscall::close(file); println!(" a connection and a file were refused the question"); + for own in [End::Read, End::Write] { + let (reader, writer) = pipe(""); + let (closed, kept) = match own { + End::Read => (reader, writer), + End::Write => (writer, reader), + }; + assert_eq!(closed_own(closed), [(TOKEN, Err::(SyscallError::NotFound))], "{own:?} end closed by its holder, its other end held"); + drop(kept); + } + println!(" an end closed by its own holder answered its watch with NotFound"); + let (reader, writer) = pipe(""); let second = syscall::dup(writer.as_handle()).expect("a second handle to the write end"); drop(writer); @@ -105,6 +120,31 @@ fn test() { println!("pipe_other_end_gone: an end is told when its other end's last holder goes, and not before"); } +#[derive(Clone, Copy, Debug)] +enum End { + Read, + Write, +} + +/// How long [`closed_own`] waits for the answer its close owes: a close +/// answers it before it returns, so this is a hang's ceiling. +const CLOSE_ANSWERED_NANOS: u64 = 10_000_000_000; + +/// What one `OTHER_END_GONE` watch of `end`, held by the kernel, is answered +/// once `end` is closed: every answer a wait bounded by +/// [`CLOSE_ANSWERED_NANOS`] collects. +fn closed_own(end: Pipe) -> Vec<(u64, Result)> { + let poller = Poller::new(1); + poller.watch(&end, OTHER_END_GONE, TOKEN); + let mut answers = Vec::new(); + // A non-blocking enter, so the watch is the kernel's before the close. + poller.wait_answers(0, 0, |token, said| answers.push((token, said))); + assert!(answers.is_empty(), "an end was told its other end is gone while it is held: {answers:?}"); + syscall::close(end.into_raw()); + poller.wait_answers(1, CLOSE_ANSWERED_NANOS, |token, said| answers.push((token, said))); + answers +} + /// A fresh pipe with `unread` written into it. fn pipe(unread: &str) -> (Pipe, Pipe) { let (reader, writer) = pipe_pair().expect("a pipe"); From a8a14f8cb0975a74458f028e4cacc6aa4199afc9 Mon Sep 17 00:00:00 2001 From: japabu Date: Mon, 12 Oct 2026 00:11:15 +0200 Subject: [PATCH 6/9] issues: acpi_press_across_a_lock_wait's red under load is pull request #864's to close #864 fixes that red in the harness and meets the issue's exit, so this branch no longer carries the file. Nothing outside issues/ cites it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...-across-a-lock-wait-went-red-under-load.md | 26 ------------------- 1 file changed, 26 deletions(-) delete mode 100644 issues/acpi-press-across-a-lock-wait-went-red-under-load.md diff --git a/issues/acpi-press-across-a-lock-wait-went-red-under-load.md b/issues/acpi-press-across-a-lock-wait-went-red-under-load.md deleted file mode 100644 index 471dc6cfc04..00000000000 --- a/issues/acpi-press-across-a-lock-wait-went-red-under-load.md +++ /dev/null @@ -1,26 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-11 ---- - -# acpi_press_across_a_lock_wait went red under load - -`cargo test`, the whole guest suite, went red on one test at -`wt/toyos-lan` 777c96b2b: `acpi_press_across_a_lock_wait: "acpi: the ACPI -row: PM1a events " never reached the the boot`, 9 s after it started, with 60 -of 61 green. The guest log is posted whole on pull request #861. - -The host was a 14-core macOS machine. Two `--metal-readback` image builds ran -beside the suite, and the load average read 34.6 / 26.7 / 18.8 a minute after -the red. The boot it reads, `tests/images/acpicase.toml`, starts no netstack -and claims only the NVMe controller. That branch changes netstack, its TCP -stack and its I219 driver, which the boot does not run, and the kernel's pipe -`release` and `close`, which every boot runs. The kernel and the ABI at -777c96b2b are byte for byte those at c91e72b83, where the whole suite was 61 -of 61 green. Nobody has run the test again, because the tree's rule is that a -red is not re-run to green. - -**Exit**: the cause is found and fixed, and the fix is shown by a run under -at least this load, or the test is deleted with this file recording the -commit that restores it. From 1df3ace223c355f9a6620fadc8deadfada5576ab Mon Sep 17 00:00:00 2001 From: japabu Date: Mon, 12 Oct 2026 00:48:56 +0200 Subject: [PATCH 7/9] netstack's cadence takes STRETCH = 8, the value round 4 on the T14 chose by the rule stated before it ran Round 4 (one session, D and B alternated five times, ten B runs at each of S = 8, 16, 32 against D's thirty) applied the rule posted before the run, byte for byte, and it passes S = 8 alone: - throughput AUC P(B > D): 0.683 / 0.577 / 0.473 at S = 8 / 16 / 32; - D-SACKs per MB AUC P(B < D): 0.475 / 0.395 / 0.177, so S = 16 fails by 0.005 and S = 32 by far; - overruns: none in any of B's thirty runs, against D's in 20 of its thirty; - tcp_ms, tls_ms and the LAN round trip: none above D's highest run. Its median package energy per MB less the idle window's rate is 23.003 mJ against D's 25.542, so B lands at S = 8. Losses, hole fills and D-SACKs rose with S, which is the stretch's signature. At a gigabit, eight full-sized frames take 98.4 us, under the 160 us floor, so on the T14's link the floor sets the interval whatever the round trip, and the I219 applies it as 131.1 us; the stretch binds at 100 and 10 Mb/s. The module doc and STRETCH's own doc now say the stretch bounds the burst above the floor, and the tests that read the stretch at a gigabit read it at 100 Mb/s, with the gigabit case held to the floor. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- system/netstack/src/cadence.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/system/netstack/src/cadence.rs b/system/netstack/src/cadence.rs index f5bb1744e27..1e5d47deafe 100644 --- a/system/netstack/src/cadence.rs +++ b/system/netstack/src/cadence.rs @@ -14,7 +14,9 @@ //! estimate has no clock to keep. //! - **The stretch**: [`STRETCH`] full-sized frames at the link's rate. A sender that does not pace //! answers an acknowledgment with a burst of what it acknowledges, and a queue on the path has -//! to hold that burst or lose its tail; this bounds it whatever the round trip. +//! to hold that burst or lose its tail; this bounds it whatever the round trip, at a rate slow +//! enough that the stretch is longer than the floor. At a gigabit it is shorter, and the floor +//! holds. //! - **The ring**: the descriptors the device holds empty above the count it reports itself short //! at must take one interval and one interval of lateness of full-sized frames at the link's //! rate. @@ -29,9 +31,9 @@ use std::time::Duration; /// lengthens a round trip by. pub const K: u32 = 32; -/// The most full-sized frames a batch holds at the link's rate, and so the most an -/// acknowledgment releases from a sender that does not pace. -pub const STRETCH: u64 = 16; +/// The most full-sized frames a batch holds at the link's rate above [`FLOOR`], and so the most +/// an acknowledgment releases from a sender that does not pace. +pub const STRETCH: u64 = 8; /// The least interval. A wake — the kernel's delivery of the interrupt and netstack's pass around /// its batch — costs about 30 µs on the hardware ToyOS targets, so wakes this far apart take under @@ -68,6 +70,7 @@ mod tests { use super::*; const GIGABIT: NonZeroU32 = NonZeroU32::new(1000).unwrap(); + const HUNDRED_MBPS: NonZeroU32 = NonZeroU32::new(100).unwrap(); /// The rates an I219 links at. const I219_RATES: [u32; 3] = [10, 100, 1000]; @@ -83,6 +86,7 @@ mod tests { /// 1,538 bytes on the wire for a 1,500-byte packet, 12,304 ns of a gigabit link. const GIGABIT_FRAME: Duration = Duration::from_nanos(12_304); + const HUNDRED_MBPS_FRAME: Duration = Duration::from_nanos(123_040); #[test] fn a_full_sized_frame_takes_12_304_ns_at_a_gigabit() { @@ -125,11 +129,11 @@ mod tests { } /// A batch that moved no stream with a round trip is held to the stretch, and to the floor - /// where the stretch is shorter. + /// where the stretch is shorter: at a gigabit, `STRETCH` frames are. #[test] fn a_batch_with_no_round_trip_is_held_to_the_stretch_above_the_floor() { - assert_eq!(cadence(None, ring(GIGABIT)), GIGABIT_FRAME * STRETCH as u32); - assert_eq!(cadence(None, ring(NonZeroU32::new(10_000).unwrap())), FLOOR); + assert_eq!(cadence(None, ring(HUNDRED_MBPS)), HUNDRED_MBPS_FRAME * STRETCH as u32); + assert_eq!(cadence(None, ring(GIGABIT)), FLOOR); } /// The bound is a `K`th of the least round trip between the floor and the stretch: a round @@ -141,7 +145,7 @@ mod tests { assert_eq!(cadence(Some(ms(16)), ten_mbps), Duration::from_micros(500)); assert_eq!(cadence(Some(ms(15)), ten_mbps), Duration::from_nanos(468_750)); assert_eq!(cadence(Some(FLOOR * K - Duration::from_nanos(K.into())), ten_mbps), FLOOR); - assert_eq!(cadence(Some(ms(15)), ring(GIGABIT)), GIGABIT_FRAME * STRETCH as u32); + assert_eq!(cadence(Some(ms(40)), ring(HUNDRED_MBPS)), HUNDRED_MBPS_FRAME * STRETCH as u32); } /// A short path's stream beside a long path's sets the interval: the batch reports its least From 443fb5d10d6516cf775c61d8c675fd8019951c8d Mon Sep 17 00:00:00 2001 From: japabu Date: Mon, 12 Oct 2026 08:50:08 +0200 Subject: [PATCH 8/9] =?UTF-8?q?netstack=20holds=20the=20I219's=20interrupt?= =?UTF-8?q?s=20a=20fixed=20131=20=C2=B5s=20apart:=20the=20pacing=20is=20de?= =?UTF-8?q?leted?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The owner ruled on pull request #861 to delete the pacing: the ACK clock, the stretch and the round-trip plumbing that served them. At the only link rate the T14 measured, a gigabit, none of them ever acted; at 100 and 10 Mb/s, where they would have decided the interval, nothing read them. With them goes the rest of the per-pass cadence: the ring bound, which no run reached either (a 1,000-descriptor ring holding half its descriptors in reserve against about ten frames an interval at a gigabit), the per-pass ITR write and its three-leading-bits rounding, and the driver's rx_free. What stays is what round 4 measured: the 1,000-descriptor ring that fills the grant, the tail moved once a batch, and the one interval. The interval is ITR = 512 units, 131.072 µs, written once at bring-up as main writes its 500. That is the value the part held in every one of round 4's ten B runs at STRETCH = 8 (itr_us 131.09 to 131.16): B asked 160 µs and the three-leading-bits rounding gave 512. Main's 128 µs is not what B ran, and 160 µs was never applied, so neither is taken. The gains round 4 read against D are the ring's, which absorbed D's MAC misses; the interval differs from main's by 2.4%. Removed with the code: cadence.rs and its tests, Tcp::take_least_rtt and its three tests, Rx::rtt, node's Batch (receive answers whether a pass is owed again), Card::pace, I219::throttle, throttle_units, rx_free and the throttle test. The bring-up test again reads the throttle the part holds. The sub-millisecond round-trip issue keeps the receive buffer's growth, which rests on main's code; the cadence paragraph and its K go. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...-trip-reads-no-finer-than-a-millisecond.md | 18 +- issues/the-lan-is-not-yet-production-grade.md | 4 +- system/netstack/i219/src/lib.rs | 62 ++---- system/netstack/i219/src/tests.rs | 39 +--- system/netstack/node/src/lease.rs | 4 - system/netstack/node/src/lib.rs | 19 +- system/netstack/node/tests/streams.rs | 7 +- system/netstack/shard/src/lib.rs | 6 - system/netstack/shard/tcp/src/rx.rs | 5 - system/netstack/shard/tcp/src/stack.rs | 22 +-- system/netstack/shard/tcp/tests/rtt.rs | 44 ----- system/netstack/src/cadence.rs | 186 ------------------ system/netstack/src/card.rs | 28 +-- system/netstack/src/i219.rs | 10 - system/netstack/src/main.rs | 12 +- 15 files changed, 38 insertions(+), 428 deletions(-) delete mode 100644 system/netstack/src/cadence.rs diff --git a/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md b/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md index aa6239e8a8a..ef6eb67f355 100644 --- a/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md +++ b/issues/netstacks-receiver-round-trip-reads-no-finer-than-a-millisecond.md @@ -12,15 +12,11 @@ ticks once a millisecond, so an echo's age is a whole number of milliseconds and never less than one tick. A path whose round trip is under a millisecond, a LAN's, reads as at least 1 ms. -The pass's cadence (`system/netstack/src/cadence.rs`) holds the card's -interrupt for a `K`th of the least round trip, never under its 160 µs floor, -so with `K = 32` every path under 5.12 ms is held to the floor and the tick -moves no interval. What it moves is the receive buffer's growth, twice what the -user read per round trip (`rx.rs`): a 200 µs path that reads 1 ms grows its -buffer to five times what it needs. These figures are derived from the floor, -`K`, the tick and the growth rule. No machine has measured a sub-millisecond -path. +What that moves is the receive buffer's growth, twice what the user read per +round trip (`rx.rs`): a 200 µs path that reads 1 ms grows its buffer to five +times what it needs. That figure is derived from the tick and the growth rule. +No machine has measured a sub-millisecond path. -**Exit**: a stream on a sub-millisecond path reports its round trip to within -a `K`th of the path's own, read by a host test that delivers echoes on such a -path. +**Exit**: a stream on a sub-millisecond path grows its receive buffer to twice +what the user reads in the path's own round trip, read by a host test that +delivers echoes on such a path. diff --git a/issues/the-lan-is-not-yet-production-grade.md b/issues/the-lan-is-not-yet-production-grade.md index a62432f400d..40b956a0d6f 100644 --- a/issues/the-lan-is-not-yet-production-grade.md +++ b/issues/the-lan-is-not-yet-production-grade.md @@ -39,8 +39,8 @@ why, and the boot still leases. and raise it under traffic. Nothing has one, and `issues/no-machine-has-read-an-intel-nic-across-a-link-change.md` waits on it. - **Throughput** is measured against Ubuntu on the same machine and cable, with - line rate (about 940 Mb/s of TCP) as the target: interrupt moderation (the - pass's cadence, `system/netstack/src/cadence.rs`), ring sizes, checksum and + line rate (about 940 Mb/s of TCP) as the target: interrupt moderation (a + fixed throttle today, not adaptive), ring sizes, checksum and segmentation offload. - **The part is handed back** in the state the next operating system and the management firmware that shares it expect. Ubuntu has had link after every diff --git a/system/netstack/i219/src/lib.rs b/system/netstack/i219/src/lib.rs index f284e2a3807..8569405645c 100644 --- a/system/netstack/i219/src/lib.rs +++ b/system/netstack/i219/src/lib.rs @@ -164,37 +164,23 @@ pub const RX_RING: usize = 1000; pub const TX_RING: usize = 16; /// Free receive descriptors at which the part reports itself short: /// `RCTL.RDMTS = 00b`, "1/2" of `RDLEN` (§10.2.5.1), raising `RXDMT0`. -pub const RX_SHORT: usize = RX_RING / 2; +const RX_SHORT: usize = RX_RING / 2; /// Receive buffers given back and not yet handed to the part at most: the /// tail moves once a batch ([`I219::rx_flush`]), and sooner once this many -/// wait. A quarter of the [`RX_SHORT`] descriptors a pacing caller keeps in -/// reserve, so what a batch holds back never brings the part to its report -/// alone. +/// wait. A quarter of [`RX_SHORT`], so what a batch holds back never brings +/// the part to its report alone. pub const HELD_MAX: usize = RX_SHORT / 4; /// Bytes per receive buffer — `RCTL.BSIZE = 00b` with `BSEX` clear (§10.2.5.1). pub const RX_BUF_BYTES: usize = 2048; /// Bytes per transmit buffer, and therefore the longest frame [`I219::tx_reserve`] /// takes. pub const TX_BUF_BYTES: usize = 2048; -/// `ITR` (§10.2.4.2) counts the least interval between two of the function's -/// interrupts in units of this many nanoseconds, in sixteen bits; zero is no -/// throttle at all. -pub const THROTTLE_UNIT_NANOS: u64 = 256; - -/// What [`I219::throttle`] writes `ITR` with for an interval of `nanos`: its -/// units at or below `nanos`, between one and the field's largest, kept to -/// their three leading bits. -/// -/// **Rounded down**, so the interval never exceeds what the caller bounded it -/// by, and **to three leading bits**, so an interval the caller recomputes -/// every pass moves the register only on a step of an eighth to a quarter, and -/// loses at most a fifth of what was asked. -pub const fn throttle_units(nanos: u64) -> u32 { - let units = nanos / THROTTLE_UNIT_NANOS; - let units = if units < 1 { 1 } else if units > u16::MAX as u64 { u16::MAX as u32 } else { units as u32 }; - let low = units.ilog2().saturating_sub(2); - units >> low << low -} +/// `ITR` (§10.2.4.2): the least interval between two of the function's +/// interrupts, in 256 ns units — 131 µs. It runs from the last interrupt, so a +/// frame after a quiet spell that long interrupts at once, and a gigabit +/// stream's full-sized frames, one each 12.3 µs, wake their reader about ten at +/// a time. +const THROTTLE: u32 = 512; const _: () = { // §7.1.8 and §7.2.4: both ring lengths are programmed in bytes and "must be @@ -805,8 +791,6 @@ pub struct I219 { rx_tail: usize, /// Buffers given back since the tail last moved. rx_held: usize, - /// What `ITR` holds, in its own units. - throttle: u32, /// Next transmit descriptor to hand out, which is also `TDT`. tx_next: usize, /// Next transmit descriptor to reclaim. @@ -956,7 +940,6 @@ impl I219 { rx_next: 0, rx_tail: RX_RING - 1, rx_held: 0, - throttle: 0, tx_next: 0, tx_clean: 0, tx_wake: false, @@ -974,10 +957,10 @@ impl I219 { nic.regs.write32(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO); nic.accepted(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO)?; - // The throttle alone, and the caller's to set ([`Self::throttle`]): - // the two receive timers hold back every frame's interrupt, the first - // after a quiet spell too, and the throttle none of those. - nic.regs.write32(regs::ITR, 0); + // The throttle alone: the two receive timers hold back every frame's + // interrupt, the first after a quiet spell too, and the throttle none + // of those. + nic.regs.write32(regs::ITR, THROTTLE); nic.regs.write32(regs::RDTR, 0); nic.regs.write32(regs::RADV, 0); nic.regs.write32(regs::TIDV, 0); @@ -1309,13 +1292,6 @@ impl I219 { self.move_tail(); } - /// Receive descriptors the device holds empty, every buffer given back - /// handed to it: what it can take before the next pass without running - /// short. Read after [`Self::rx_flush`], with nothing left to poll. - pub fn rx_free(&self) -> usize { - (self.rx_tail + RX_RING - self.rx_next) % RX_RING - } - /// Republish descriptor `index`, and move the tail once [`HELD_MAX`] /// buffers wait for it. /// @@ -1332,18 +1308,6 @@ impl I219 { } } - /// Hold the function's interrupts at least `nanos` apart (§10.2.4.2's - /// `ITR`), as [`throttle_units`] puts it; the register is written only - /// when that changes. The throttle runs from the last interrupt, so a frame - /// after a quiet spell that long interrupts at once. - pub fn throttle(&mut self, nanos: u64) { - let units = throttle_units(nanos); - if units != self.throttle { - self.throttle = units; - self.regs.write32(regs::ITR, units); - } - } - /// Move the tail over every ready descriptor from where it stands. /// /// **The tail moves over a run, never to an index.** §7.1.8's tail diff --git a/system/netstack/i219/src/tests.rs b/system/netstack/i219/src/tests.rs index ca72f9c8913..57e67d4fcd2 100644 --- a/system/netstack/i219/src/tests.rs +++ b/system/netstack/i219/src/tests.rs @@ -123,11 +123,12 @@ fn bring_up_programs_what_the_initialization_sections_name() { assert_eq!(nic.peek(regs::TDLEN) as usize, TX_RING * tx_desc::BYTES); assert_eq!(nic.peek(regs::TDT), 0); - // Both timers off, and the throttle the caller's to set: the first frame - // after a quiet spell is not held back. + // Both timers off and the throttle on: the first frame after a quiet + // spell is not held back, and no peer's frames wake the reader more often + // than the throttle. assert_eq!(nic.peek(regs::RDTR), 0); assert_eq!(nic.peek(regs::RADV), 0); - assert_eq!(nic.peek(regs::ITR), 0); + assert_eq!(u64::from(nic.peek(regs::ITR)) * 256, 131_072, "§10.2.4.2 counts in 256 ns"); assert_eq!(driver.mac(), NVM_MAC); } @@ -683,38 +684,6 @@ fn the_tail_moves_once_a_batch_and_sooner_only_once_held_max_wait() { ); driver.rx_flush(); assert_eq!(writes_of(&nic, regs::RDT), writes + 2, "{}", nic.because("a flush with nothing held wrote RDT")); - assert_eq!(driver.rx_free(), RX_RING - 1, "{}", nic.because("the part does not hold the whole ring")); -} - -/// §10.2.4.2's `ITR` takes its 256 ns units at or below what the caller -/// asked, inside its sixteen bits, to three leading bits, and is written only -/// when that changes: an interval recomputed every pass costs no write unless -/// it moves by a step of an eighth to a quarter. -#[test] -fn the_throttle_is_written_only_when_its_three_leading_bits_change() { - let nic = Nic::new(25); - let mut driver = open(&nic); - let opened = writes_of(&nic, regs::ITR); - for (nanos, units, written) in [ - // 390 units, 0b110000110: 0b110 << 6. - (100_000, 384, 1), - (99_000, 384, 1), - (98_303, 320, 2), - (65_536, 256, 3), - (65_535, 224, 4), - (7 * 256, 7, 5), - (0, 1, 6), - (255, 1, 6), - (u64::MAX, 0b111 << 13, 7), - ] { - driver.throttle(nanos); - assert_eq!( - (nic.peek(regs::ITR), writes_of(&nic, regs::ITR) - opened), - (units, written), - "{}", - nic.because(&format!("asked for {nanos} ns")) - ); - } } // --- transmit --- diff --git a/system/netstack/node/src/lease.rs b/system/netstack/node/src/lease.rs index 2e63ca4fb4f..9398ca4d322 100644 --- a/system/netstack/node/src/lease.rs +++ b/system/netstack/node/src/lease.rs @@ -107,10 +107,6 @@ impl Stack { self.shard.receive(now, frame); } - pub(crate) fn take_least_rtt(&mut self) -> Option { - self.shard.take_least_rtt() - } - pub(crate) fn transmit(&mut self, now: Instant, credit: usize, sink: impl FnMut(&[u8])) -> usize { self.shard.transmit(now, credit, sink) } diff --git a/system/netstack/node/src/lib.rs b/system/netstack/node/src/lib.rs index 014858f06f2..026abb3054d 100644 --- a/system/netstack/node/src/lib.rs +++ b/system/netstack/node/src/lib.rs @@ -69,18 +69,6 @@ toyos_net_wire::counters! { QueryFailed = "node.query-failed"; } -/// What one [`Node::receive`] took. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub struct Batch { - pub frames: usize, - /// The batch stopped at its bound, and another is owed without waiting for the device to - /// say so. - pub owed: bool, - /// The least round trip among the streams whose data the batch moved - /// ([`toyos_net_tcp::Tcp::take_least_rtt`]). - pub least_rtt: Option, -} - /// A line for the log. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Event { @@ -165,13 +153,14 @@ impl Node { /// The frames the device received since the last call, one by one and at most `budget`: /// `next` hands its next frame to the sink it is given and answers `false` once it had none. /// One pass over the streams follows the last frame, so what the batch brought reaches each - /// client in one write and not one a frame. + /// client in one write and not one a frame. `true` is a batch that stopped at the budget, and + /// another is owed without waiting for the device to say so. /// /// **The budget is the device's ring**: a device may refill it as fast as it is read, and a /// batch that waited for it to empty would keep the deadlines, the transmit opportunity and /// every stream's pass waiting with it, while a bound below what one interrupt's wait can /// bring would owe a second pass for every long wait. - pub fn receive(&mut self, now: Instant, budget: usize, mut next: impl FnMut(&mut dyn FnMut(&[u8])) -> bool, mut draw: impl FnMut() -> u32) -> Batch { + pub fn receive(&mut self, now: Instant, budget: usize, mut next: impl FnMut(&mut dyn FnMut(&[u8])) -> bool, mut draw: impl FnMut() -> u32) -> bool { let mut taken = 0usize; let owed = loop { if taken == budget { @@ -186,7 +175,7 @@ impl Node { taken = taken.saturating_add(1); }; self.pass(now, false); - Batch { frames: taken, owed, least_rtt: self.stack.take_least_rtt() } + owed } /// A transmit opportunity with room for `credit` frames, each handed to `sink` as it is built. diff --git a/system/netstack/node/tests/streams.rs b/system/netstack/node/tests/streams.rs index b18ce57bdb9..89bbc81b462 100644 --- a/system/netstack/node/tests/streams.rs +++ b/system/netstack/node/tests/streams.rs @@ -679,14 +679,13 @@ fn a_batch_stops_at_its_budget_and_says_a_pass_is_owed() { sink(&frame); true }; - let took = net.node.receive(net.now, BUDGET, endless, draw(&mut net.draws)); - assert_eq!((took.frames, took.owed, handed), (BUDGET, true, BUDGET), "the batch is the budget, and a pass is owed"); + let owed = net.node.receive(net.now, BUDGET, endless, draw(&mut net.draws)); + assert_eq!((owed, handed), (true, BUDGET), "the batch is the budget, and a pass is owed"); assert_eq!(client.borrow().inbox, b"again", "the rest of the batch was the same segment again"); let next = net.far.text(b"once"); let short: Vec<&[u8]> = vec![&next; BUDGET - 1]; - let took = net.node.receive(net.now, BUDGET, batch(&short), draw(&mut net.draws)); - assert_eq!((took.frames, took.owed), (BUDGET - 1, false), "the ring was emptied"); + assert!(!net.node.receive(net.now, BUDGET, batch(&short), draw(&mut net.draws)), "the ring was emptied"); assert_eq!(client.borrow().inbox, b"againonce"); } diff --git a/system/netstack/shard/src/lib.rs b/system/netstack/shard/src/lib.rs index e6592c4b633..9a3a7d90c66 100644 --- a/system/netstack/shard/src/lib.rs +++ b/system/netstack/shard/src/lib.rs @@ -242,12 +242,6 @@ impl Shard { // ---- the device ---- - /// The least round trip among the TCP connections whose data moved since the last call - /// ([`Tcp::take_least_rtt`]). - pub fn take_least_rtt(&mut self) -> Option { - self.tcp.take_least_rtt() - } - /// A frame the device received. pub fn receive(&mut self, now: Instant, frame: &[u8]) { match self.ip.receive(now, self.iface, frame) { diff --git a/system/netstack/shard/tcp/src/rx.rs b/system/netstack/shard/tcp/src/rx.rs index afa55416017..ec4873b08b8 100644 --- a/system/netstack/shard/tcp/src/rx.rs +++ b/system/netstack/shard/tcp/src/rx.rs @@ -150,11 +150,6 @@ impl Rx { self.buf.capacity() } - /// The receiver's round-trip estimate, once one has been sampled. - pub const fn rtt(&self) -> Option { - self.rtt - } - /// After in-order text of `len` bytes arrived at `now`, with its TSecr and that echo's age /// where it is one this end sent. pub fn sample_rtt(&mut self, len: u32, echo: Option<(u32, Option)>, now: Instant) { diff --git a/system/netstack/shard/tcp/src/stack.rs b/system/netstack/shard/tcp/src/stack.rs index 6ad47360e8b..6fdd2cf6a83 100644 --- a/system/netstack/shard/tcp/src/stack.rs +++ b/system/netstack/shard/tcp/src/stack.rs @@ -232,8 +232,6 @@ pub struct Tcp { port_table: [u16; 16], log: Log, scratch: Vec, - /// The least round trip of a connection whose data moved since [`Tcp::take_least_rtt`]. - least_rtt: Option, } fn slot(slots: &mut [Slot], index: u32, generation: u32) -> Option<&mut T> { @@ -391,7 +389,6 @@ impl Tcp { port_table, log: Log::default(), scratch: Vec::new(), - least_rtt: None, }) } @@ -399,18 +396,6 @@ impl Tcp { &self.log.counters } - /// The least round trip among the connections whose data moved since the last call: the - /// receiver's own estimate where in-order text arrived, the sender's SRTT where an - /// acknowledgment moved SND.UNA. `None` where none moved or none has an estimate yet. - /// - /// A caller that holds segments back by a fraction of it before they reach [`Self::receive`] - /// lengthens no such connection's round trip by more than that fraction, and holds back no - /// more than that fraction of a round trip's acknowledgments: the least wins, so a short path - /// beside a long one keeps its own clock. - pub fn take_least_rtt(&mut self) -> Option { - self.least_rtt.take() - } - /// Refusals and reachability advice since the last call. pub fn drain_events(&mut self) -> impl Iterator + '_ { self.log.drain() @@ -882,16 +867,11 @@ impl Tcp { } } Tcb::Sync(mut sync) => { - let (una, next) = (sync.tx.una, sync.rx.next); + let una = sync.tx.una; let verdict = sync.receive(seg, &mut ctx); if sync.tx.una != una { conn.soft = None; } - let received = (sync.rx.next != next).then(|| sync.rx.rtt()).flatten(); - let acknowledged = (sync.tx.una != una).then(|| sync.rtt.srtt()).flatten(); - for rtt in received.into_iter().chain(acknowledged) { - self.least_rtt = Some(self.least_rtt.map_or(rtt, |least| least.min(rtt))); - } return self.after_sync(index, tuple, sync, verdict, now); } ended @ Tcb::Ended(_) => conn.state = ended, diff --git a/system/netstack/shard/tcp/tests/rtt.rs b/system/netstack/shard/tcp/tests/rtt.rs index 08e62e278c8..4093801542c 100644 --- a/system/netstack/shard/tcp/tests/rtt.rs +++ b/system/netstack/shard/tcp/tests/rtt.rs @@ -121,47 +121,3 @@ fn s_rt_016_no_timer_when_all_is_acknowledged() { h.input(10, seg(5001).ack(1101)); assert_eq!(h.info().rtx_timer, None); } - -// ---- the least round trip of the connections whose data moved ---- - -/// Text that arrives in order reports the receiver's own estimate: an echo of the ACK sent at 0, -/// aged 15 ms. It is reported once, and the handshake that came before it moved no data. -#[test] -fn text_arriving_in_order_reports_the_receivers_round_trip_once() { - let mut h = fixture_ef(); - assert_eq!(h.tcp.take_least_rtt(), None, "the handshake moved no data"); - h.input(15, seg(5001).ack(1001).wnd(512).ts(50_015, 1000).len(1448)); - assert_eq!(h.tcp.take_least_rtt(), Some(ms(15))); - assert_eq!(h.tcp.take_least_rtt(), None, "taken"); -} - -/// Text above a hole moves nothing in order and reports nothing; the segment that fills the hole -/// does, with the estimate its own echo moved: the dup ACK of 20, aged 5 ms, an eighth of the way. -#[test] -fn text_above_a_hole_reports_nothing_until_the_hole_fills() { - let mut h = fixture_ef(); - h.input(15, seg(5001).ack(1001).wnd(512).ts(50_015, 1000).len(1448)); - h.tcp.take_least_rtt(); - h.input(20, seg(5001 + 2 * 1448).ack(1001).wnd(512).ts(50_020, 1015).len(1448)); - assert_eq!(h.tcp.take_least_rtt(), None, "nothing arrived in order"); - h.input(25, seg(5001 + 1448).ack(1001).wnd(512).ts(50_025, 1020).len(1448)); - assert_eq!(h.tcp.take_least_rtt(), Some(std::time::Duration::from_micros(13_750))); -} - -/// An acknowledgment that moves SND.UNA reports the sender's SRTT, and one segment that moves -/// both directions reports the lesser: text echoing 0 aged 30 ms, beside an SRTT of 12.5 ms. -#[test] -fn an_acknowledgment_reports_the_senders_round_trip_and_the_least_wins() { - let mut h = fixture_ef(); - h.send(0, 100); - h.input(30, seg(5001).ack(1101).wnd(512).ts(50_030, 1000)); - let srtt = h.info().srtt; - assert_eq!(srtt, Some(std::time::Duration::from_micros(12_500))); - assert_eq!(h.tcp.take_least_rtt(), srtt); - - h.send(30, 100); - h.input(60, seg(5001).ack(1201).wnd(512).ts(50_060, 1030).len(1448)); - let srtt = h.info().srtt.expect("an estimate"); - assert!(srtt < ms(30), "the sender's estimate is the lesser: {srtt:?}"); - assert_eq!(h.tcp.take_least_rtt(), Some(srtt)); -} diff --git a/system/netstack/src/cadence.rs b/system/netstack/src/cadence.rs deleted file mode 100644 index 1e5d47deafe..00000000000 --- a/system/netstack/src/cadence.rs +++ /dev/null @@ -1,186 +0,0 @@ -//! The pass's one cadence: how long the card holds its interrupt after a pass that took frames. -//! Every frame of the next batch waits up to that long, and so does every acknowledgment the -//! batch owes, every pipe write to a client and every wake of one: they follow the pass one for -//! one, so nothing else is timed. -//! -//! **The interval is the least of three bounds, never under [`FLOOR`], and the ring's wins over -//! the floor:** -//! - **The ACK clock**: the least round trip among the streams whose data the batch moved -//! (`toyos_net_tcp::Tcp::take_least_rtt`), over [`K`]. The interval is added to the delay of -//! every acknowledgment and to every round-trip sample the peer takes, so it keeps at least `K` -//! acknowledgments a round trip on the sender's clock and lengthens the round trip by at most -//! one `K`th. A receiver's estimate already holds the interval, so under that feedback it -//! settles at `rtt · K / (K − 1)` and does not climb. A batch that moved no stream with an -//! estimate has no clock to keep. -//! - **The stretch**: [`STRETCH`] full-sized frames at the link's rate. A sender that does not pace -//! answers an acknowledgment with a burst of what it acknowledges, and a queue on the path has -//! to hold that burst or lose its tail; this bounds it whatever the round trip, at a rate slow -//! enough that the stretch is longer than the floor. At a gigabit it is shorter, and the floor -//! holds. -//! - **The ring**: the descriptors the device holds empty above the count it reports itself short -//! at must take one interval and one interval of lateness of full-sized frames at the link's -//! rate. -//! - **[`FLOOR`]**: what any peer's frames can make of netstack's wakes, a flood for nobody's or a -//! stream's on a short path. The ring wins over it: a wake too many costs cycles, and a ring -//! that overruns loses frames. - -use std::num::NonZeroU32; -use std::time::Duration; - -/// Acknowledgments a round trip, at the least, and the inverse of the most the interval -/// lengthens a round trip by. -pub const K: u32 = 32; - -/// The most full-sized frames a batch holds at the link's rate above [`FLOOR`], and so the most -/// an acknowledgment releases from a sender that does not pace. -pub const STRETCH: u64 = 8; - -/// The least interval. A wake — the kernel's delivery of the interrupt and netstack's pass around -/// its batch — costs about 30 µs on the hardware ToyOS targets, so wakes this far apart take under -/// a fifth of a CPU; and the I219, which rounds an interval down by at most a fifth, still holds -/// them 128 µs apart. -pub const FLOOR: Duration = Duration::from_micros(160); - -/// A full-sized frame as the link carries it, in bits: the largest frame the stack sends or -/// takes (`toyos_net_ip::FRAME`), its FCS, the preamble and start delimiter, and the interframe -/// gap (IEEE 802.3 clause 4: 4, 8 and 12 bytes). -const WIRE_FRAME_BITS: u64 = (toyos_net_ip::FRAME as u64 + 4 + 8 + 12) * 8; - -/// The receive ring, as the pass leaves it. -#[derive(Clone, Copy, Debug)] -pub struct Ring { - /// Descriptors the device holds empty, every buffer the pass took given back, above the count - /// at which it reports itself short. - pub room: usize, - /// The link's rate. - pub mbps: NonZeroU32, -} - -/// How long the card holds its interrupt after a pass whose batch moved streams with -/// `least_rtt` as their least round trip, on `ring`. -pub fn cadence(least_rtt: Option, ring: Ring) -> Duration { - let frames = |n: u64| Duration::from_nanos(n * WIRE_FRAME_BITS * 1000 / u64::from(ring.mbps.get())); - let stretch = frames(STRETCH); - let clock = least_rtt.map_or(stretch, |rtt| (rtt / K).min(stretch)); - clock.max(FLOOR).min(frames(ring.room as u64) / 2) -} - -#[cfg(test)] -mod tests { - use super::*; - - const GIGABIT: NonZeroU32 = NonZeroU32::new(1000).unwrap(); - const HUNDRED_MBPS: NonZeroU32 = NonZeroU32::new(100).unwrap(); - - /// The rates an I219 links at. - const I219_RATES: [u32; 3] = [10, 100, 1000]; - - /// A ring of 1,000 that reports itself short at half, as a pass that took every frame leaves it. - fn ring(mbps: NonZeroU32) -> Ring { - Ring { room: 499, mbps } - } - - fn ms(n: u64) -> Duration { - Duration::from_millis(n) - } - - /// 1,538 bytes on the wire for a 1,500-byte packet, 12,304 ns of a gigabit link. - const GIGABIT_FRAME: Duration = Duration::from_nanos(12_304); - const HUNDRED_MBPS_FRAME: Duration = Duration::from_nanos(123_040); - - #[test] - fn a_full_sized_frame_takes_12_304_ns_at_a_gigabit() { - assert_eq!(WIRE_FRAME_BITS, 1538 * 8); - assert_eq!(cadence(Some(Duration::MAX), Ring { room: 2, mbps: GIGABIT }), GIGABIT_FRAME); - } - - /// At every link rate from 10 Mb/s to 400 Gb/s, every round trip and every ring, the full-sized - /// frames that arrive at line rate in two intervals — one and its lateness — fit in what the - /// ring holds above its reserve, the floor included. - #[test] - fn the_interval_never_exceeds_what_the_ring_holds_at_any_link_rate() { - for mbps in [10, 100, 1000, 2500, 10_000, 25_000, 100_000, 400_000] { - let mbps = NonZeroU32::new(mbps).unwrap(); - for room in [0, 1, 64, 255, 499, 4095, 65_535] { - let ring = Ring { room, mbps }; - for rtt in [None, Some(Duration::ZERO), Some(Duration::from_micros(200)), Some(ms(15)), Some(ms(300)), Some(Duration::MAX)] { - let interval = cadence(rtt, ring); - let bits = 2 * interval.as_nanos() * u128::from(mbps.get()) / 1000; - let arrive = bits / u128::from(WIRE_FRAME_BITS); - assert!(arrive <= room as u128, "{mbps} Mb/s, room {room}, rtt {rtt:?}: {interval:?} brings {arrive} frames into {room}"); - } - } - } - } - - /// Whatever a batch moved — nothing with a round trip, as a flood for nobody or a datagram flow - /// does, or a stream on any path — a pass that took every frame holds an I219 at any of its - /// rates at least 128 µs, as the part applies it: no peer wakes netstack more often. - #[test] - fn no_batch_holds_an_i219_under_128_us() { - for mbps in I219_RATES { - for rtt in [None, Some(Duration::ZERO), Some(Duration::from_micros(200)), Some(ms(1)), Some(ms(15)), Some(Duration::MAX)] { - let asked = cadence(rtt, ring(NonZeroU32::new(mbps).unwrap())); - let nanos = u64::try_from(asked.as_nanos()).unwrap(); - let applied = u64::from(toyos_i219::throttle_units(nanos)) * toyos_i219::THROTTLE_UNIT_NANOS; - assert!(applied >= 128_000, "{mbps} Mb/s, rtt {rtt:?}: asked {asked:?}, the part holds {applied} ns"); - } - } - } - - /// A batch that moved no stream with a round trip is held to the stretch, and to the floor - /// where the stretch is shorter: at a gigabit, `STRETCH` frames are. - #[test] - fn a_batch_with_no_round_trip_is_held_to_the_stretch_above_the_floor() { - assert_eq!(cadence(None, ring(HUNDRED_MBPS)), HUNDRED_MBPS_FRAME * STRETCH as u32); - assert_eq!(cadence(None, ring(GIGABIT)), FLOOR); - } - - /// The bound is a `K`th of the least round trip between the floor and the stretch: a round - /// trip short enough to put it under the floor gets the floor, and one long enough to put it - /// over the stretch gets the stretch. - #[test] - fn the_least_round_trip_over_k_is_the_interval_between_the_floor_and_the_stretch() { - let ten_mbps = ring(NonZeroU32::new(10).unwrap()); - assert_eq!(cadence(Some(ms(16)), ten_mbps), Duration::from_micros(500)); - assert_eq!(cadence(Some(ms(15)), ten_mbps), Duration::from_nanos(468_750)); - assert_eq!(cadence(Some(FLOOR * K - Duration::from_nanos(K.into())), ten_mbps), FLOOR); - assert_eq!(cadence(Some(ms(40)), ring(HUNDRED_MBPS)), HUNDRED_MBPS_FRAME * STRETCH as u32); - } - - /// A short path's stream beside a long path's sets the interval: the batch reports its least - /// round trip, and that is what bounds it. A long path alone is bounded by the stretch, and on - /// a ring with less room than the stretch, by the ring. - #[test] - fn a_short_round_trip_bounds_the_interval_and_a_long_one_meets_the_stretch_or_the_ring() { - let ten_mbps = ring(NonZeroU32::new(10).unwrap()); - assert_eq!(cadence(Some(ms(8)), ten_mbps), Duration::from_micros(250)); - assert_eq!(cadence(Some(ms(1000)), ten_mbps), Duration::from_nanos(1_230_400) * STRETCH as u32); - let short = Ring { room: 20, mbps: GIGABIT }; - assert_eq!(cadence(Some(ms(1000)), short), GIGABIT_FRAME * 10); - } - - /// The interval lengthens the round trip the receiver measures by up to itself, and the - /// next interval is taken from that estimate. From the path's own round trip and from an - /// estimate far above it alike, the estimate moves only toward `rtt · K / (K − 1)` and - /// settles there: it never ratchets past it. Each path's `K`th clears the floor and is - /// under the stretch of a 10 Mb/s link. - #[test] - fn under_its_own_feedback_the_round_trip_settles_and_does_not_ratchet() { - let ten_mbps = ring(NonZeroU32::new(10).unwrap()); - for path in [ms(6), ms(15), ms(40)] { - let settled = path * K / (K - 1); - for start in [path, ms(10_000)] { - let mut estimate = start; - for _ in 0..200 { - let next = path + cadence(Some(estimate), ten_mbps); - let toward = if start < settled { estimate <= next && next <= settled } else { settled <= next && next <= estimate }; - assert!(toward, "{path:?} from {start:?}: {estimate:?} moved to {next:?}, away from {settled:?}"); - estimate = next; - } - let off = settled.abs_diff(estimate); - assert!(off <= Duration::from_micros(1), "{path:?} from {start:?}: settled at {estimate:?}, not {settled:?}"); - } - } - } -} diff --git a/system/netstack/src/card.rs b/system/netstack/src/card.rs index 3e266577531..29cc9a8cbf9 100644 --- a/system/netstack/src/card.rs +++ b/system/netstack/src/card.rs @@ -1,10 +1,6 @@ -use std::num::NonZeroU32; -use std::time::Duration; - use toyos_i219::Part; use toyos_inspect::Snapshot; -use crate::cadence::{cadence, Ring, FLOOR}; use crate::i219; use crate::virtio_net::VirtioNet; @@ -25,14 +21,9 @@ impl Card { panic!("netstack: the NIC this program was given is not one it can drive — {why}") } - /// Paced from the start, before any batch has said what it moved. pub fn intel(claim: toyos::PciDev, part: Part) -> Self { match i219::Nic::open(claim, part) { - Ok(nic) => { - let card = Self::Intel(nic); - card.pace(None); - card - } + Ok(nic) => Self::Intel(nic), Err(why) => Self::undrivable(why), } } @@ -154,23 +145,6 @@ impl Card { took } - /// Hold the card's interrupts after a batch that moved streams whose least round trip is - /// `least_rtt`, by [`cadence`] over the ring the batch left. virtio's device has no throttle. - pub fn pace(&self, least_rtt: Option) { - let Self::Intel(nic) = self else { return }; - let interval = match nic.link() { - toyos_i219::Link::Up { speed, .. } => cadence( - least_rtt, - Ring { - room: nic.rx_free().saturating_sub(toyos_i219::RX_SHORT), - mbps: NonZeroU32::new(speed.mbps().into()).expect("a link's rate is not zero"), - }, - ), - toyos_i219::Link::Down => FLOOR, - }; - nic.throttle(u64::try_from(interval.as_nanos()).unwrap_or(u64::MAX)); - } - /// Hands the next received frame to `take` and gives its buffer back to /// the card once `take` returns; `false` when none waits. fn rx(&self, take: &mut dyn FnMut(&[u8])) -> bool { diff --git a/system/netstack/src/i219.rs b/system/netstack/src/i219.rs index 9dd79ac66c0..2cf5f50d47a 100644 --- a/system/netstack/src/i219.rs +++ b/system/netstack/src/i219.rs @@ -267,16 +267,6 @@ impl Nic { self.driver.borrow_mut().rx_flush(); } - /// Receive descriptors the part holds empty. - pub fn rx_free(&self) -> usize { - self.driver.borrow().rx_free() - } - - /// Hold the part's interrupts at least `nanos` apart. - pub fn throttle(&self, nanos: u64) { - self.driver.borrow_mut().throttle(nanos); - } - /// How many frames the transmit ring takes now. pub fn tx_room(&self) -> usize { self.driver.borrow_mut().tx_room() diff --git a/system/netstack/src/main.rs b/system/netstack/src/main.rs index dcbb29c8272..b61b1922a27 100644 --- a/system/netstack/src/main.rs +++ b/system/netstack/src/main.rs @@ -9,8 +9,7 @@ //! random source, the kernel's pipes (`pipes`), the clients' connections //! (`client`) and their requests (`serve`). //! -//! **One pass**: the card's link and its received frames, as one batch, go to the node, and -//! what the batch moved sets how long the card holds its next interrupt (`cadence`); then +//! **One pass**: the card's link and its received frames, as one batch, go to the node, then //! every deadline that is due, then the node is offered the card's transmit //! room until it has no frame left or the card no room; what the node has to //! say is written to the log and to the clients that waited for it; and the @@ -43,7 +42,6 @@ use toyos_net_shard::{Config, Secrets}; use toyos_net_wire::ethernet::{IndividualMac, MacAddr}; use toyos_net_wire::Instant; -mod cadence; mod card; mod client; mod i219; @@ -382,11 +380,7 @@ fn main() { node.link(clock(), link_up, draw); } } - let batch = card.receive(|budget, next| node.receive(clock(), budget, next, draw)); - // A pass that took no frame says nothing new about the streams the card's wait is for. - if batch.frames > 0 { - card.pace(batch.least_rtt); - } + let owed = card.receive(|budget, next| node.receive(clock(), budget, next, draw)); let now = clock(); if node.next_deadline().is_some_and(|at| at <= now) { node.fire(now, draw); @@ -428,7 +422,7 @@ fn main() { // were taken with this pass's. let nanos = |left: Duration| u64::try_from(left.as_nanos()).unwrap_or(u64::MAX); let mut timeout = node.next_deadline().map_or(u64::MAX, |at| nanos(at.since(clock()))); - if batch.owed { + if owed { timeout = 0; } if let Some(left) = leases.due_in() { From ec72322c81693ebcce22ded437628184fb9433b2 Mon Sep 17 00:00:00 2001 From: japabu Date: Mon, 12 Oct 2026 16:36:37 +0200 Subject: [PATCH 9/9] issues: netstack's latency under a receive batch of more than 64 frames is unmeasured The one finding of the owner's external review that held in triage (https://github.com/ToyOSOrg/ToyOS/pull/861#issuecomment-6126361353, item 4). Design B raised `Node::receive`'s budget from main's 64 to the card's ring, 999 on the I219, and a batch's frames share one `now` and run before the deadlines, the transmit opportunity and the clients' answers. That changes latency only for a pass that brings more than 64 frames, and round 4's LAN round trip was read at about 12 a pass. Filed with the LAN track's holder as owner, and as the exit a T14 reading at both budgets that is reported and, as #885 has it, decides nothing about the budget by itself. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy --- ...eive-batch-over-64-frames-is-unmeasured.md | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 issues/netstacks-latency-under-a-receive-batch-over-64-frames-is-unmeasured.md diff --git a/issues/netstacks-latency-under-a-receive-batch-over-64-frames-is-unmeasured.md b/issues/netstacks-latency-under-a-receive-batch-over-64-frames-is-unmeasured.md new file mode 100644 index 00000000000..1997ed22945 --- /dev/null +++ b/issues/netstacks-latency-under-a-receive-batch-over-64-frames-is-unmeasured.md @@ -0,0 +1,35 @@ +--- +status: open +kind: tooling +opened: 2026-10-12 +--- + +# netstack's latency under a receive batch of more than 64 frames is unmeasured + +`Node::receive` takes at most the card's ring in one batch (`Card::receive`, +`system/netstack/src/card.rs`): `toyos_i219::RX_RING - 1`, 999 frames, on the +I219, and the queue's 256 on virtio. Before pull request #861 it took 64. Every +frame of a batch reaches the stack under one `now`, read before the batch +(`system/netstack/src/main.rs`'s loop), and the deadlines that are due, the +transmit opportunity and the clients' answers run only after the batch's last +frame. So a backlog that was served as several 64-frame passes, with those +three between them, is now one pass: a frame late in it is stamped with the +batch's start, and a deadline due during it fires after it. + +Nothing moves while a pass brings 64 frames or fewer, and nothing has read a +pass that brings more. The T14's LAN round trip was read only under one +gigabit download of full-sized frames, at about 12 frames a pass (round 4 of +pull request #861: 59.7 passes per MB, and some 700 full-sized frames carry a +MB; derived). A pass brings more than 64 when netstack is about 0.79 ms behind +full-sized frames at a gigabit (64 frames of 12.3 µs on the wire, derived), or +under minimum-sized frames, of which a gigabit carries about 195 in one +131 µs throttle interval (672 ns a frame on the wire, derived). + +**Exit**: a T14 reading, in one session, of the LAN round trip and of how late +netstack fires a due deadline, under a load that brings more than 64 frames a +pass, with the ring's budget and with a budget of 64, recorded in the commit +that closes this. A gap between the two is investigated where it shows, and is +no verdict on the budget by itself. + +**Owner**: whoever holds `issues/the-lan-is-not-yet-production-grade.md`, whose +stage 2 sets the ring sizes and the interrupt moderation this batch follows.