The I219 throttles its interrupts to 128 µs, netstack reads a send pipe only when the kernel says it holds bytes, and one receive bound for every card owes the next pass at once

issues/the-lan-is-not-yet-production-grade.md

@@ -39,8 +39,9 @@ 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 (off
-  today), ring sizes, checksum and segmentation offload.
+  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
+  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
   ToyOS boot so far; that becomes a tested property.

toyos-i219/src/lib.rs

@@ -156,6 +156,12 @@ pub const RX_RING: usize = 256;
 /// 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;
 /// 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`]
@@ -191,13 +197,6 @@ const _: () = {
     assert!(OFF_RX_RING.is_multiple_of(16) && OFF_TX_RING.is_multiple_of(16));
 };
 
-/// Frames this driver hands up in one pass before it says there are no more.
-///
-/// **A ring that never empties is a ring the specification permits**: at line
-/// rate the device refills descriptors as fast as they are returned, and a
-/// caller that loops until the ring is empty never returns to its other work.
-pub const RX_BUDGET: u32 = 64;
-
 /// Why the function was not brought up. Each keeps its own word: a caller asks
 /// different things of a grant that is too small and of a part with no station
 /// address.
@@ -766,7 +765,6 @@ pub struct I219<R: Registers, C, D, I> {
     /// receive ring keeps in software's hands (§7.1.8: the queue is empty when
     /// head equals tail).
     rx_tail: usize,
-    rx_budget: u32,
     /// Next transmit descriptor to hand out, which is also `TDT`.
     tx_next: usize,
     /// Next transmit descriptor to reclaim.
@@ -915,7 +913,6 @@ impl<R: Registers, C: Clock, D: DmaBuffers, I: Interrupts> I219<R, C, D, I> {
             link_up_at: None,
             rx_next: 0,
             rx_tail: RX_RING - 1,
-            rx_budget: RX_BUDGET,
             tx_next: 0,
             tx_clean: 0,
             tx_wake: false,
@@ -933,10 +930,10 @@ impl<R: Registers, C: Clock, D: DmaBuffers, I: Interrupts> I219<R, C, D, I> {
         nic.regs.write32(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO);
         nic.accepted(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO)?;
 
-        // No moderation on either side: §10.2.4.2's throttle and the two
-        // receive timers all hold an interrupt back, and what this driver
-        // waits on is the frame that has already arrived.
-        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);
@@ -1085,8 +1082,7 @@ impl<R: Registers, C: Clock, D: DmaBuffers, I: Interrupts> I219<R, C, D, I> {
         self.link_up_at.map(|at| at.saturating_sub(self.opened_at))
     }
 
-    /// Take the interrupt, read and acknowledge its causes, and give the
-    /// receive budget back.
+    /// Take the interrupt, and read and acknowledge its causes.
     ///
     /// **The record has to be taken, not merely noticed**: a claim reads ready
     /// while it holds an undrained interrupt, so a caller that saw the token
@@ -1103,7 +1099,6 @@ impl<R: Registers, C: Clock, D: DmaBuffers, I: Interrupts> I219<R, C, D, I> {
     /// down, so a link that reads up on both sides of it starts the same
     /// deadline.
     pub fn begin_pass(&mut self) -> Result<Pass, PassRefused<I::Refused>> {
-        self.rx_budget = RX_BUDGET;
         let messages = match self.irq.taken() {
             Ok(count) => count,
             Err(why) if why == I::IDLE => 0,
@@ -1212,17 +1207,13 @@ impl<R: Registers, C: Clock, D: DmaBuffers, I: Interrupts> I219<R, C, D, I> {
         }
     }
 
-    /// The next received frame, or `None` — for an empty ring and for a budget
-    /// already spent alike.
+    /// The next received frame, or `None` for an empty ring.
     ///
     /// A refused descriptor is counted, its buffer given straight back, and
     /// the walk continues: one frame this driver will not act on may not hide
     /// the ones behind it.
     pub fn poll_rx(&mut self) -> Option<Frame> {
         loop {
-            if self.rx_budget == 0 {
-                return None;
-            }
             let index = self.rx_next;
             let at = OFF_RX_RING + index * rx_desc::BYTES;
             let word = self.dma.read64(at + 8);
@@ -1235,7 +1226,6 @@ impl<R: Registers, C: Clock, D: DmaBuffers, I: Interrupts> I219<R, C, D, I> {
             self.dma.observe();
             let word = self.dma.read64(at + 8);
             self.rx_next = (index + 1) % RX_RING;
-            self.rx_budget -= 1;
             match parse_rx(word) {
                 Ok(len) => {
                     self.counters.received = self.counters.received.saturating_add(1);

toyos-i219/src/tests.rs

@@ -122,11 +122,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: an interrupt this driver waits on may not be held back
-    // by one nothing else expires.
+    // 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.
     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, 128_000, "§10.2.4.2 counts in 256 ns");
 
     assert_eq!(driver.mac(), NVM_MAC);
 }
@@ -503,41 +503,6 @@ fn null_descriptor_padding_is_not_a_frame() {
     assert_eq!(got.len(), 1, "{}", nic.because("the real frame did not come up behind it"));
 }
 
-/// **A ring that never empties is what the wire does at line rate.** A pass
-/// has to end, or the caller's event loop never runs again — so the budget is
-/// what makes "no more this pass" something this driver can say.
-#[test]
-fn one_pass_hands_up_at_most_its_budget() {
-    let nic = Nic::new(8);
-    let mut driver = open(&nic);
-    nic.set_link(true);
-    // Twice the budget, arriving faster than they are taken: every buffer
-    // returned is another frame placed.
-    for i in 0..(RX_BUDGET as usize * 2) {
-        nic.deliver(&frame((i % 200) as u8 + 1, 128));
-    }
-    // Enough ticks for every one of them to be placed and written back, so
-    // what bounds the pass below is the budget and not the device.
-    for _ in 0..(RX_BUDGET * 4) {
-        nic.run();
-    }
-    one_pass(&mut driver);
-    let first = drain(&nic, &mut driver).len();
-    assert_eq!(
-        first, RX_BUDGET as usize,
-        "{}",
-        nic.because("a pass did not stop at its budget")
-    );
-    // And the next pass picks the rest up rather than losing them.
-    let mut rest = 0;
-    for _ in 0..8 {
-        nic.run();
-        one_pass(&mut driver);
-        rest += drain(&nic, &mut driver).len();
-    }
-    assert_eq!(first + rest, RX_BUDGET as usize * 2, "{}", nic.because("frames were lost"));
-}
-
 /// §7.1.8's tail "identifies the location beyond the last descriptor hardware
 /// can process", so it moves over a run of ready descriptors and never to the
 /// index that came back last. A buffer given back before an older one may not

userland/netstack/node/src/lib.rs

@@ -69,6 +69,11 @@ 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;
+
 /// A line for the log.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum Event {
@@ -150,16 +155,27 @@ impl Node {
 
     // ---- the device ----
 
-    /// The frames the device received since the last call, one by one: `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.
-    pub fn receive(&mut self, now: Instant, mut next: impl FnMut(&mut dyn FnMut(&[u8])) -> bool, mut draw: impl FnMut() -> u32) {
-        while next(&mut |frame| {
-            self.stack.receive(now, frame);
-            self.settle(now, &mut draw);
-        }) {}
-        self.bridge(now);
+    /// 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 {
+        let mut taken = 0usize;
+        let owed = loop {
+            if taken == RECEIVE_BUDGET {
+                break true;
+            }
+            if !next(&mut |frame| {
+                self.stack.receive(now, frame);
+                self.settle(now, &mut draw);
+            }) {
+                break false;
+            }
+            taken = taken.saturating_add(1);
+        };
+        self.pass(now, false);
+        owed
     }
 
     /// A transmit opportunity with room for `credit` frames, each handed to `sink` as it is built.
@@ -207,7 +223,7 @@ impl Node {
         let out = self.client.timer(now, &mut draw);
         self.carry_out(now, out, None, &mut draw);
         self.settle(now, &mut draw);
-        self.bridge(now);
+        self.pass(now, false);
     }
 
     /// Hands the client what the shard reported and what reached its socket, and carries out

userland/netstack/node/src/listeners.rs

@@ -156,7 +156,7 @@ impl Node {
         listener.unspent = listener.unspent.saturating_sub(1);
         let bound = listener.bound;
         let answer = self.take(bound, pipes);
-        self.bridge(now);
+        self.pass(now, false);
         answer
     }
 

userland/netstack/node/src/streams.rs

@@ -40,6 +40,12 @@
 //! Every call that can move a stream ends in a pass: a batch of frames, a deadline, and each call
 //! here. A transmit opportunity can only fail a connect, whose next hop it found to answer
 //! nobody, and ends in a pass over the connects.
+//!
+//! **A send pipe found empty is read again only once the kernel says it holds bytes**
+//! ([`Node::bridge`]'s `readable`), or once its client writes no more, when an empty pipe is the
+//! end. netstack watches such a pipe for bytes as long as [tcp] has room for them
+//! ([`Watch::readable`]), and the kernel answers a watch of a pipe that already holds some at
+//! once, so no byte waits on a pass that never reads.
 
 use alloc::boxed::Box;
 use alloc::collections::{BTreeMap, VecDeque};
@@ -178,6 +184,9 @@ struct Stream {
     held: bool,
     /// [tcp] had room for the client's bytes when the last pass ended.
     room: bool,
+    /// The last read of the from-client pipe found it empty, and the kernel has not said since
+    /// that it holds bytes.
+    drained: bool,
     /// The options last written to [tcp].
     options: Options,
 }
@@ -199,6 +208,7 @@ impl Stream {
             extended: false,
             held: false,
             room: false,
+            drained: false,
             options,
         }
     }
@@ -266,6 +276,9 @@ impl Stream {
 
         let mut gave_up = false;
         while let Some(pipe) = self.from_client.as_mut() {
+            if self.drained && !self.done_writing {
+                break;
+            }
             // Never more than [tcp] has room for, and so never a read of no bytes, whose answer
             // would be the end's; [tcp] has none once the FIN is queued.
             let room = stack.tcp_status(conn).writable.min(CHUNK);
@@ -278,7 +291,10 @@ impl Stream {
             let (read, writer_gone) = match pipe.read(space) {
                 Ok(read) => (read, read == 0),
                 Err(ReadRefusal::Empty) if self.done_writing => (0, false),
-                Err(ReadRefusal::Empty) => break,
+                Err(ReadRefusal::Empty) => {
+                    self.drained = true;
+                    break;
+                }
                 Err(ReadRefusal::Broken) => {
                     stack.tcp_abort(now, conn);
                     return false;
@@ -377,8 +393,15 @@ impl Node {
         Ok(self.streams.hold(Stream { connecting: true, deadline, ..Stream::established(conn, remote.addr, Options::default(), pipes) }))
     }
 
-    /// One pass over every stream: netstack calls it when a pipe it watches is ready.
-    pub fn bridge(&mut self, now: Instant) {
+    /// One pass over every stream: netstack calls it once the kernel has answered pipes it
+    /// watches ready as [`Self::watches`] asked, and `readable` are the streams whose from-client
+    /// pipe it said holds bytes.
+    pub fn bridge(&mut self, now: Instant, readable: impl IntoIterator<Item = StreamId>) {
+        for id in readable {
+            if let Some(stream) = self.streams.live.get_mut(&id) {
+                stream.drained = false;
+            }
+        }
         self.pass(now, false);
     }
 
@@ -409,15 +432,15 @@ impl Node {
         stream.to_client = None;
         stream.done_writing = true;
         stream.left = true;
-        self.bridge(now);
+        self.pass(now, false);
     }
 
     /// The client sends no more: what its pipe still holds is sent, then the FIN. `false` is an
     /// id that names no established stream.
     pub fn shutdown_write(&mut self, now: Instant, id: StreamId) -> bool {
         let Some(stream) = self.streams.live.get_mut(&id).filter(|stream| !stream.connecting) else { return false };
         stream.done_writing = true;
-        self.bridge(now);
+        self.pass(now, false);
         true
     }
 
@@ -442,7 +465,7 @@ impl Node {
             PipeEnd::FromClient if stream.fin_queued => stream.from_client = None,
             PipeEnd::FromClient => stream.done_writing = true,
         }
-        self.bridge(now);
+        self.pass(now, false);
     }
 
     /// The kernel refused netstack's watch of a pipe end the node still holds: the handle is no