Skip to content

toyos-net-node: the resolver on ToyOS's own UDP, host-tested and shipped in nothing - #774

Merged
Japabu merged 5 commits into
mainfrom
wt/toyos-ownstack-c
Oct 8, 2026
Merged

Japabu merged 5 commits into
mainfrom
wt/toyos-ownstack-c

Conversation

@Japabu

@Japabu Japabu commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Stage C of the cut of issues/toyos-has-its-own-network-stack.md's stage 5: the resolver on the node, host-tested and shipped in nothing. Both of its bases have landed (#771, #772); this head, 0e113baee, is the reviewed head ff4e4ce0d with main at 35d35859e merged in. The merge stopped nowhere and nothing was resolved by hand. Of the eleven files this stage touches, nine are byte-identical to ff4e4ce0d; Cargo.lock and the track each differ by one line that is main's own (a toyos-i219 dependency line main dropped; the track's T14 outbound-rows bullet), merged by git, and the stage's own hunks in both are unchanged. Nothing was built or run at this head: the gates below are measured at ff4e4ce0d and 442ecd55e, and the merged lock has not been through cargo metadata --locked.

442ecd55e is the fix round for the review of 025640bf6; ff4e4ce0d adds the round-2 review's notes to it and touches only tests/resolve.rs and the track.

What differs from what ships today

  1. A query's socket is connected to its resolver's port 53. [udp] delivers it only what that address and port sent to the address the query left from (RFC 5452 §9.1): nothing from another source, nothing sent to a broadcast or a joined group. netstack's sockets on smoltcp are bound and unconnected, and its lookup is handed the wire's source address and port.
  2. A lookup ends when its lease's resolvers change or the lease is lost (Ended::LeaseChanged), at once, its sockets closed. netstack's lookups keep the servers they started with.
  3. An id is drawn only for a query that is sent. toyos_dns::Lookup takes the draw, not the id; netstack on main drew one per reply read and per wait looked at. This changes what ships: netstack's resolver now draws less from the kernel's source, through the same three calls.
  4. No "unsent query" rule. On smoltcp a datagram stayed at the head of its socket while its neighbour was unresolved; here [udp] hands it to [ip] at the next opportunity and [ip]'s neighbour machine bounds its own requests. A query [udp] refuses outright is counted node.query-unsent, holds no socket and is waited out; one the network reports back (resolver unreachable, port refused) is counted node.query-failed and waited out. Recorded in the track, exit before the move.
  5. A burst at a cold resolver. Lookups started together at a resolver whose link address is not yet known lose all but the newest PENDING_PER_NEIGHBOUR (8) queries to [ip]'s queue (RFC 4861 §7.2.2); each lost one costs its lookup a WAIT_MS. A regression against what ships, held as it stands by a_burst_at_a_resolver_not_yet_resolved_..., recorded in the track, exit before the move.
  6. An ended lookup holds its place until its answer is taken (Node::take_resolved), so the list is bounded whatever the shell does.
  7. The resolver does not filter the lease's resolvers: toyos-dhcp already keeps only hosts.
  8. Unchanged: a truncated reply ends the lookup (issues/dns-resolver-refuses-a-truncated-answer.md), A records only, no cache, the first resolver is asked first on every lookup.

The review of 025640bf6, finding by finding

BLOCKER, --ci host. Not run by me and not claimed: it closes on CI's host job for the head that lands, as the review says.

BLOCKER, a reply is read whatever address it was sent to. Fixed where the rule lives: the query's socket is connected (item 1). lease.rs gains Stack::connect, [udp]'s call passed on, in the datagram sockets' section: the shard is private to that file, so it is the only way. The node no longer hands the lookup the wire's source: it hands its own record of the resolver the query went to, and port 53, which the connected socket has matched. The header's "an any-address bind that names no port is refused only for want of a free one" still holds: the bind is unchanged and the connect follows it. A connected socket can carry Error::Failed; datagram.rs's unreachable! stays unreachable because no client socket is connected, and the resolver's own receive counts it (node.query-failed) and reads past it.
The measurement: a_reply_is_read_only_at_the_address_its_query_left_from, added by patch to the reviewed tree 025640bf6, exits 101 there (a reply sent to 255.255.255.255 was read: the lookup ended with the forged address) and 0 at this head. Its premise, asserted per destination, is that the datagram reached [udp] (udp.rx moves).

BLOCKER, the re-reading cursor. Gone. The loop reads the first listed socket that holds a datagram, hands it over, and repeats until none does or the lookup ended.

NOTE, draws. Removed, not recorded: toyos_dns::Lookup::start, on_time and on_datagram take id: impl FnOnce() -> u16 and call it once for a query they send. toyos-dns test rfc5452_9_2_an_id_is_drawn_for_a_query_that_is_sent_and_for_nothing_else; mutations c4 and c5.

NOTE, a lookup and its lease. Decided: it ends (item 2). Tests for a renewal naming the same resolvers (the lookup goes on and is answered), one naming another (ends, port free), and a NAK (ends); mutation c2 on the last two.

NOTE, the third-party reply. a_recorded_reply_of_a_public_resolver_answers_the_nodes_query replays toyos-dns's REAL_GOOGLE through the node; the track bullet is dropped.

NOTE, a_resolver_that_answers_no_arp_holds_up_no_other. Deleted. toyos-net-ip/tests/nud.rs has no scenario for the rule it held; the track says so, with its exit.

NOTE, the two bullets' "when". Both say before the move; the burst bullet says it is a regression.

NOTE, the port-0 arm. The arm no longer exists (the node reads no source port). Port 0 is its own test, a_reply_from_port_0_is_not_read, with the premise that udp.rx-src-port-zero moved, and c1 is red on it by its own assertion.

NOTE, Node::resolve's type. NotStarted { NotConnected, ResourceExhausted }.

NOTE, the body's dead-primary sentence. Gone with the test.

The review of 442ecd55e: its notes

  • a_lookup_outlives_a_renewal_that_names_the_same_resolvers asserts its premise: the held lease's expiry moved across the ACK.
  • a_query_the_network_reports_unreachable_is_counted_and_waited_out asserts its premise beside the count: [ip] refused exactly one query on a failed neighbour (nb.failed-refused is 1).
  • The track: the [ip] bullet's exit says before the move; the bullet for a query that does not reach its resolver names the node.query-failed test and the 3 s it then ends at.

What moved outside the stage's fence, and why

  • toyos-dns/src/lib.rs and its tests: the draw (the review's NOTE, by the coordinator's instruction). Every one of its 43 call sites in tests.rs now passes a closure.
  • userland/netstack/src/resolve.rs, three call sites: (self.draw)() became &mut self.draw, because the signature they call changed. Nothing else of netstack's changed; its host tests ran in the request for that reason.
  • userland/netstack/node/src/lease.rs: Stack::connect, six lines.
  • userland/netstack/node/src/name.rs: millis is pub(crate), so the resolver keeps the same clock rather than a copy.

What stays recorded for the move

In issues/toyos-has-its-own-network-stack.md, each with its exit:

  • a query that does not reach its resolver is waited out (node.query-unsent, node.query-failed): before the move;
  • the burst at a cold resolver: before the move;
  • Lookup::on_datagram still takes a source address and port, read only by netd's resolver on smoltcp: they go with it, at the move;
  • toyos-net-ip has no test that a silent next hop holds up no datagram to another.

Checks (a trust boundary)

  • The node has no reader of its own: clippy::indexing_slicing is forbidden in the crate and every byte of a reply goes to toyos_dns::Lookup::on_datagram, whose rules (id, question, pointers that must point back, lengths) are toyos-dns's tests. What the node adds, and tests: the connected socket (source, port, destination), and which query's socket a reply reached.
  • Independent oracle: every DNS message in tests/resolve.rs but one is spelled by hand from RFC 1035 §4.1 and never by toyos-dns; that one is a public resolver's recorded reply. etherparse reads every emitted frame. No outside DNS parser: none is in the lockfile and one would be a new fetch.
  • No differential arm: netstack's resolver tests on the stack that is going are not ported, by the owner's ruling for the track.
  • Negative control: 21 named mutations, posted as a comment below; r18, c4 and c5 are in toyos-dns.

Gates

Run by the orchestrator from build-request-4.sh at 442ecd55e and, for the two rows marked so, again from build-request-5.sh at ff4e4ce0d; each line is the command's own exit, and the tree ended clean both times. Between the two heads only tests/resolve.rs (+8) and the track changed, so the other rows, the red-first run and the mutations are measured at 442ecd55e.

step command exit
lock cargo metadata --locked --format-version 1 0
node tests cargo test --locked --manifest-path userland/netstack/node/Cargo.toml 0, and 0 again after the mutations; 0 at ff4e4ce0d
toyos-dns tests cargo test --locked -p toyos-dns 0, and 0 again after the mutations
toyos-net-udp tests cargo test --locked -p toyos-net-udp 0
netstack tests cargo test --locked --manifest-path userland/netstack/Cargo.toml 0
gates cargo test --locked --lib -- hostws:: userlandhost:: sourcegate:: licence:: 0, running 45 tests
node clippy cargo clippy --locked --manifest-path userland/netstack/node/Cargo.toml --all-targets -- <the adopted lints> -D warnings 0; 0 at ff4e4ce0d
toyos-dns clippy the same, -p toyos-dns 0
red first the tree of 025640bf6 plus the destination test, built (0), that test run 101
21 mutations, 25 runs each applied, built (0), its named test run, reverted 101 each

Not run: cargo run -- --ci host; any guest test (the node ships in nothing; netstack's change is three call sites its host tests cover).

Each new mutation's red is its own assertion, read from its log: c1 on four tests, the lookup ended with the forged address (another host, port 5353, port 0, a reply sent to 255.255.255.255); c2 on two, no lookup ended where Err(LeaseChanged) was due; c3, 0 counted for 3; c4 and c5, the draw that must not be made was made. r1, r2 and r5 to r18 are the round-1 mutations regenerated against the new code and red on the same tests as before; r3 and r4 are replaced by c1.

Size against stage B (git diff --shortstat 288484ba4 ff4e4ce0d): the node's source, manifest and lock +276 −8; the node's tests +752; toyos-dns source +21 −10 and tests +73 −43; netstack +3 −3; the track +5 −1 (two of its lines reworded at this head).

Unsure of

  • a_query_the_network_reports_unreachable_is_counted_and_waited_out asserts exactly ROUNDS reports, which rests on [ip] holding a failed neighbour down for more than one second: the third query, a second after the failure, is then refused at once. The test asserts that premise (nb.failed-refused is 1).

🤖 Generated with Claude Code

https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A

Stage C of the cut of the own-stack track's stage 5: the socket half of
netstack's resolve module, as node calls over the node's own datagram
sockets. Host-tested, shipped in nothing; netstack's modules and
manifest are untouched.

resolve: Node::resolve starts a lookup of a name at the held lease's
resolvers and answers a LookupId; Node::take_resolved hands over the
lookups that ended; Node::let_go ends one nobody waits for. Every
decision about a question and every byte of a reply stays
toyos_dns::Lookup's. The node gives it one socket a query, on an
ephemeral port of one draw with an id of another (RFC 5452 §9.2), the
source address and port each reply arrived from, and the query whose
socket it reached. A wait is a deadline of the node's, and the lookups
are carried on in the node's pass over the stack, after the DHCP client
and the name.

A refusal is one of the pipe ABI's words datagram.rs already has:
NotConnected with no lease or no resolver, ResourceExhausted at
MAX_LOOKUPS or with no ephemeral port. An ended lookup holds its place
until its answer is taken, so the list is bounded whatever the shell
does.

What netstack's resolver does on smoltcp and this does not: it does not
close the socket of a query "whose wait ended before it left". On
smoltcp a datagram stayed at the head of its socket while its neighbour
was unresolved and kept the interface asking for that neighbour; here
[udp] hands the datagram to [ip] at the next opportunity and [ip]'s
neighbour machine bounds its own requests, so a socket cannot say
whether its query left and there is nothing to let go of. A query [udp]
refuses outright is counted node.query-unsent and holds no socket.

name::millis becomes pub(crate): the resolver keeps the same clock.

Tests are tests/resolve.rs, on stage B's Lan. Every DNS message in them
is spelled by hand from RFC 1035 §4.1; every frame the node emits is
read by etherparse. netstack's own resolver tests are not carried
across as a differential arm: they go with the flip.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A
@Japabu

Japabu commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Stage C's mutation patches, against 0b13818a4 (git apply --check clean on each). Each was applied, built, run against its named test of tests/resolve.rs, found red (exit 101) and reverted by build-request.sh / build-request-2.sh; the tree ended clean.

r1-id-not-drawn.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..2d2c638dd 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -71,7 +71,7 @@ pub(crate) struct Resolver {
 /// A query's id: the low half of one draw.
 fn id(draw: &mut impl FnMut() -> u32) -> u16 {
     let [_, _, high, low] = draw().to_be_bytes();
-    u16::from_be_bytes([high, low])
+    u16::from_be_bytes([high, low]) & 0
 }
 
 fn close(stack: &mut Stack, now: Instant, socket: SocketId) {
r2-port-not-drawn.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..5cf0a0f10 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -85,7 +85,7 @@ fn close(stack: &mut Stack, now: Instant, socket: SocketId) {
 fn act(asking: &mut Asking, step: Step, now: Instant, stack: &mut Stack, counters: &mut Counters, draw: &mut impl FnMut() -> u32) -> Option<Result<Vec<[u8; 4]>, Ended>> {
     let done = match step {
         // An any-address bind that names no port is refused only for want of a free one.
-        Step::Ask { asked, to, query } => match stack.bind(Ipv4Addr::UNSPECIFIED, None, &mut *draw) {
+        Step::Ask { asked, to, query } => match stack.bind(Ipv4Addr::UNSPECIFIED, None, || draw() & 0) {
             Ok((socket, _)) => {
                 if stack.send_to(now, socket, Ipv4Addr::from(to), PORT, &query).is_ok() {
                     asking.queries.push((asked, socket));
r3-every-source-is-the-resolver.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..ebecd20cb 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -164,7 +164,7 @@ impl Resolver {
                 let Some(message) = reply.get(..received.len) else { continue };
                 // Port 0 is not port 53: the lookup drops it.
                 let port = received.source_port.map_or(0, Port::get);
-                let step = asking.lookup.on_datagram(asked, received.source.octets(), port, message, ms, id(&mut *draw));
+                let step = asking.lookup.on_datagram(asked, [192, 0, 2, 53], port, message, ms, id(&mut *draw));
                 done = act(asking, step, now, stack, counters, &mut *draw);
                 // The step may have let this query go: its socket is read again from wherever it
                 // now is, or every socket from the first.
r4-every-port-is-53.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..a2eab40b7 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -163,7 +163,7 @@ impl Resolver {
                 };
                 let Some(message) = reply.get(..received.len) else { continue };
                 // Port 0 is not port 53: the lookup drops it.
-                let port = received.source_port.map_or(0, Port::get);
+                let port = received.source_port.map_or(PORT, |_| PORT);
                 let step = asking.lookup.on_datagram(asked, received.source.octets(), port, message, ms, id(&mut *draw));
                 done = act(asking, step, now, stack, counters, &mut *draw);
                 // The step may have let this query go: its socket is read again from wherever it
r5-any-port-of-the-lookup.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..239d086f3 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -164,7 +164,8 @@ impl Resolver {
                 let Some(message) = reply.get(..received.len) else { continue };
                 // Port 0 is not port 53: the lookup drops it.
                 let port = received.source_port.map_or(0, Port::get);
-                let step = asking.lookup.on_datagram(asked, received.source.octets(), port, message, ms, id(&mut *draw));
+                let oldest = asking.lookup.waiting().next().unwrap_or(asked);
+                let step = asking.lookup.on_datagram(oldest, received.source.octets(), port, message, ms, id(&mut *draw));
                 done = act(asking, step, now, stack, counters, &mut *draw);
                 // The step may have let this query go: its socket is read again from wherever it
                 // now is, or every socket from the first.
r6-unsent-not-counted.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..a9c514c1e 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -90,7 +90,7 @@ fn act(asking: &mut Asking, step: Step, now: Instant, stack: &mut Stack, counter
                 if stack.send_to(now, socket, Ipv4Addr::from(to), PORT, &query).is_ok() {
                     asking.queries.push((asked, socket));
                 } else {
-                    counters.add(Counter::QueryUnsent, 1);
+                    counters.add(Counter::QueryUnsent, 0);
                     close(stack, now, socket);
                 }
                 None
r7-refused-query-keeps-its-socket.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..7dd00cf8f 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -91,7 +91,6 @@ fn act(asking: &mut Asking, step: Step, now: Instant, stack: &mut Stack, counter
                     asking.queries.push((asked, socket));
                 } else {
                     counters.add(Counter::QueryUnsent, 1);
-                    close(stack, now, socket);
                 }
                 None
             }
r8-queries-never-let-go.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..57a6ff6e3 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -102,7 +102,7 @@ fn act(asking: &mut Asking, step: Step, now: Instant, stack: &mut Stack, counter
     };
     let lookup = &asking.lookup;
     asking.queries.retain(|&(asked, socket)| {
-        let read = lookup.waiting().any(|waiting| waiting == asked);
+        let read = lookup.waiting().any(|waiting| waiting == asked) || true;
         if !read {
             close(stack, now, socket);
         }
r9-cap-not-kept.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..a84db1cbb 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -126,7 +126,7 @@ impl Resolver {
         if servers.is_empty() {
             return Err(Refused::NotConnected);
         }
-        if self.asking.len().saturating_add(self.ended.len()) >= MAX_LOOKUPS {
+        if self.asking.len().saturating_add(self.ended.len()) >= usize::MAX {
             return Err(Refused::ResourceExhausted);
         }
         let Some((lookup, step)) = Lookup::start(name, &servers, millis(now), id(&mut *draw)) else { unreachable!("the lease names a resolver") };
r10-cap-forgets-untaken-answers.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..ee7201519 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -126,7 +126,7 @@ impl Resolver {
         if servers.is_empty() {
             return Err(Refused::NotConnected);
         }
-        if self.asking.len().saturating_add(self.ended.len()) >= MAX_LOOKUPS {
+        if self.asking.len() >= MAX_LOOKUPS {
             return Err(Refused::ResourceExhausted);
         }
         let Some((lookup, step)) = Lookup::start(name, &servers, millis(now), id(&mut *draw)) else { unreachable!("the lease names a resolver") };
r11-no-resolver-answered-exhausted.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..cc7db9925 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -124,7 +124,7 @@ impl Resolver {
     fn start(&mut self, now: Instant, name: Name, stack: &mut Stack, counters: &mut Counters, draw: &mut impl FnMut() -> u32) -> Result<LookupId, Refused> {
         let servers: Vec<[u8; 4]> = stack.lease().map(|lease| lease.dns.iter().map(Ipv4Addr::octets).collect()).unwrap_or_default();
         if servers.is_empty() {
-            return Err(Refused::NotConnected);
+            return Err(Refused::ResourceExhausted);
         }
         if self.asking.len().saturating_add(self.ended.len()) >= MAX_LOOKUPS {
             return Err(Refused::ResourceExhausted);
r12-let-go-keeps-its-sockets.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..50e02b090 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -188,9 +188,7 @@ impl Resolver {
             if asking.id != id {
                 return true;
             }
-            for &(_, socket) in &asking.queries {
-                close(stack, now, socket);
-            }
+            let _ = (&stack, now);
             false
         });
         self.ended.retain(|resolved| resolved.id != id);
r13-let-go-keeps-the-answer.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..8befe6702 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -193,7 +193,7 @@ impl Resolver {
             }
             false
         });
-        self.ended.retain(|resolved| resolved.id != id);
+        self.ended.retain(|resolved| resolved.id != id || true);
     }
 }
 
r14-ended-lookup-keeps-its-sockets.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..41b6d1934 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -175,9 +175,6 @@ impl Resolver {
                 done = act(asking, step, now, stack, counters, &mut *draw);
             }
             let Some(result) = done else { return true };
-            for &(_, socket) in &asking.queries {
-                close(stack, now, socket);
-            }
             ended.push(Resolved { id: asking.id, result });
             false
         });
r15-no-port-is-no-end.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..cb533b15f 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -95,7 +95,7 @@ fn act(asking: &mut Asking, step: Step, now: Instant, stack: &mut Stack, counter
                 }
                 None
             }
-            Err(_) => Some(Err(Ended::NoPort)),
+            Err(_) => None,
         },
         Step::Wait => None,
         Step::Done(result) => Some(result.map_err(Ended::Failed)),
r16-deadline-without-the-lookups.patch
diff --git a/userland/netstack/node/src/lib.rs b/userland/netstack/node/src/lib.rs
index 1518769b7..44cf1dffa 100644
--- a/userland/netstack/node/src/lib.rs
+++ b/userland/netstack/node/src/lib.rs
@@ -148,7 +148,7 @@ impl Node {
 
     pub fn next_deadline(&self) -> Option<Instant> {
         let name = self.name.as_ref().and_then(name::Name::next_deadline);
-        self.stack.next_deadline().into_iter().chain(self.client.next_deadline()).chain(name).chain(self.resolver.next_deadline()).min()
+        self.stack.next_deadline().into_iter().chain(self.client.next_deadline()).chain(name).min()
     }
 
     /// Every deadline at or before `now`; the frames they make due wait for [`Self::transmit`].
r17-deadline-is-the-latest.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index 02cd8de48..47f737449 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -118,7 +118,7 @@ impl Resolver {
 
     /// When the soonest lookup's wait is over.
     pub(crate) fn next_deadline(&self) -> Option<Instant> {
-        self.asking.iter().map(|asking| Instant::from_millis(asking.lookup.due())).min()
+        self.asking.iter().map(|asking| Instant::from_millis(asking.lookup.due())).max()
     }
 
     fn start(&mut self, now: Instant, name: Name, stack: &mut Stack, counters: &mut Counters, draw: &mut impl FnMut() -> u32) -> Result<LookupId, Refused> {
r18-dns-id-not-compared.patch
diff --git a/toyos-dns/src/lib.rs b/toyos-dns/src/lib.rs
index 288232af7..07bb42296 100644
--- a/toyos-dns/src/lib.rs
+++ b/toyos-dns/src/lib.rs
@@ -562,7 +562,7 @@ impl Lookup {
             return Step::Wait;
         };
         let Some(which) =
-            self.sent.iter().position(|s| s.asked == asked && port == PORT && s.to == from && s.id == carried)
+            self.sent.iter().position(|s| s.asked == asked && port == PORT && s.to == from)
         else {
             return Step::Wait;
         };

settle ends in the name's pass (serve_name) and then the resolver's; the
track keeps stage B's five bullets, then the resolver's three.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A
@Japabu

Japabu commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Review of wt/toyos-ownstack-c at 025640bf6, round 1, against .claude/agents/reviewer.md. Scope: git diff origin/wt/toyos-ownstack-b...025640bf6 (stage B's reviewed head 288484ba4 is the merge's second parent) and the two hand-resolved spots. Read, not run.

Net lines over stage B (git diff --shortstat): 7 files, +879 −9. Production (src/resolve.rs 220 new, src/lib.rs, src/name.rs, manifest, lock): +238 −8. Tests: +637. Track: +4 −1.

BLOCKER

  • PR body, "Not run" — cargo run -- --ci host has no measurement at 025640bf6 — the Evidence rule names it; the host check on this draft reads skipping. It closes on CI's host job for the head that lands: the step cargo run -- --ci host exits 0, and its log shows toyos-net-node's resolve binary run with every test this stage names passing, toyos-dns's 43 passing, and the clippy step clean under CI's toolchain. The orchestrator may close this one BLOCKER on reading that log only if the head CI ran is a reviewed head, or a rebase of one that resolved no conflict in the seven files this stage touches; a hand-resolved hunk is new code and comes back. It does not close the two below.
  • userland/netstack/node/src/resolve.rs:88 and :156-167 — a reply is read whatever address it was sent to — RFC 5452 §9.1, which the tests cite for the source address and port, also says a resolver MUST match the reply's destination address against the query's source address. The query's socket is bound to 0.0.0.0, and toyos-net-udp's receive hands an any-bound socket limited-broadcast, subnet-broadcast and joined-group datagrams ((Cast::LimitedBroadcast | Cast::SubnetBroadcast | Cast::Multicast(_), binding) => binding == Binding::Any); Received::destination and link_broadcast reach pass and are dropped unread. A datagram to 255.255.255.255, to 192.0.2.255 or, once answer_as has joined it, to 224.0.0.251, at the query's port from the resolver's address and port 53 with the query's id ends the lookup with its address. No test sends one. Fix where the rule lives: the query leaves from a socket [udp] delivers only unicast for the address it left from (bound to the lease's address, or connected to the resolver, which is [udp]'s whole 4-tuple filter and would stop the node passing source address and port to the lookup by hand), and the header's "an any-address bind is refused only for want of a free port" is rechecked against whichever is chosen. A new test a_reply_is_read_only_at_the_address_its_query_left_from delivers the true answer to each of the three destinations and finds no lookup ended; it is red on this head, which is the measurement, and green after.
  • userland/netstack/node/src/resolve.rs:153-172 — the re-reading rule ("its socket is read again from wherever it now is, or every socket from the first") has no test that fails without it, and none can be written — exact patch: replace line 171, at = asking.queries.iter().position(|&(listed, _)| listed == asked).unwrap_or(0);, with at = at.saturating_add(1);. I expect all 17 tests of tests/resolve.rs to stay green; the implementer runs it. The reason is structural: Node::receive is the only way a datagram reaches a socket and it ends in settle, so pass runs after every frame and finds at most one datagram in all the query sockets together (an_alias_answered_late_... delivers its three held frames one receive each). The rule was needed where smoltcp's poll took in many frames before one pass; here it is ported code for a state the node cannot reach. Either a test through Node's own calls turns red under that patch, or the cursor goes: read the first listed socket that holds a datagram, hand it over, repeat until none does or the lookup ended. That is smaller and has no position to keep right.

NOTE

  • userland/netstack/node/src/resolve.rs:167,174 — every pass draws one id per lookup in flight for a wait that is not due, and one per datagram read, used or not; settle runs once per received frame and twice per fire — at the move the draw is the kernel's random source, so up to 16 draws per frame for as long as a lookup is in flight, where netstack today pays them once per loop turn. The body lists it under "Unsure of" and nowhere else. A compromise found is removed (toyos_dns::Lookup takes the draw and calls it only for a query it sends) or recorded in the track with its exit; the pull request body is not the record.
  • userland/netstack/node/src/resolve.rs:125,132 — a lookup copies the lease's resolvers at its start and keeps asking them, and reading their answers, after that lease is renewed with other resolvers or lost and replaced: for the rest of its life (up to ROUNDS × resolvers × WAIT_MS, again per alias followed) a name goes to an address the held lease does not name. lease.rs's third rule says the resolvers come and go with the lease. netstack today says so of itself (set_servers: "a lookup already in flight keeps the servers it started with"); the node neither decides it nor tests it. Decide: such a lookup ends when the lease's resolvers change, or the behaviour is held by a test that renews the lease under a lookup and names where its next query goes, and the track records it.
  • issues/toyos-has-its-own-network-stack.md, the bullet "No reply a third party wrote has reached the node's resolver" — its exit waits for a guest where a host test reaches it now: toyos-dns/src/tests.rs holds four recorded replies, and REAL_GOOGLE (60 octets, id 0x1001, dns.google) replays through the node as tests/slirp.rs replays the OFFER and ACK, with the lookup's id drawn as 0x1001. Root CLAUDE.md puts a behaviour on the cheapest tier that reaches it, and that reply is the one oracle here the author of the code under test did not write. Add the test and drop the bullet, or say in the bullet why a host replay does not count.
  • userland/netstack/node/tests/resolve.rs:424 (a_resolver_that_answers_no_arp_holds_up_no_other) — the brief's question: it is not a test of this stage. What it holds beyond a_resolver_that_never_answers_... and a_query_udp_refuses_... is that [ip] queues per neighbour, which toyos-net-ip/tests/nud.rs owns and where a control for it belongs; no line of resolve.rs can block a second query behind a first, so no node mutation distinguishes it. Round 2 of this branch deleted a_query_leaves_from_no_port_a_client_holds on exactly that ground. Keep it only as the named regression holder for the smoltcp defect, with the body's "Checks" no longer counting it, or delete it. The burst test (:591) is the same kind and stands because a track bullet's exit reads it.
  • issues/toyos-has-its-own-network-stack.md, the burst bullet and the node.query-unsent bullet — neither exit says when. The burst is a regression against what ships (of 16 lookups started together at a resolver with no neighbour entry, 8 pay a WAIT_MS), and it ships at the move; its neighbours in the list say "before the move" or "at the move" where that is so. Say it, or say why it may ship.
  • userland/netstack/node/tests/resolve.rs:350 — the port-0 iteration asserts no premise that its datagram reached the query's socket, and r4's red is the 5353 iteration (the resolver's port 5353 answered): the arm map_or(0, ...) is unmeasured. Assert udp.rx-src-port-zero moved, and measure received.source_port.map_or(PORT, Port::get) against this test.
  • userland/netstack/node/src/resolve.rs:206 — Node::resolve answers in datagram::Refused, two of whose four words (AddrInUse, InvalidInput) it can never say; netstack's resolver today has a two-word type of its own. The shell at the move matches arms that cannot happen. A type of the two words it has.
  • PR body, "Checks" — "It is the host test the dead-primary issue's exit asks for" is false of the record: issues/netstack-resolver-asks-a-dead-primary-first-on-every-lookup.md asks for a stream against [SILENT, ANSWERS] answered in less than one WAIT_MS with none refused; this test shows one lookup answered at exactly WAIT_MS. (Prose.)

What the brief asked, where no finding came of it

  • What an answer's acceptance rests on: the id and the question section on toyos_dns::Lookup (its own 43 tests; r18 shows the node's test sees the id rule go; no node-level control removes the question check); the source address, the source port and which query's socket on the node handing the wire's values through (r3, r4, r5, each red on the forged address); nothing on [udp]'s connected-socket filter, because no query socket is connected. The destination address rests on nothing: the second BLOCKER.
  • Hostile bytes: read toyos-dns/src/lib.rs whole. Every read is slice-checked; a pointer must lower run, so a name's walk ends; a name stops at 255; counts are walked record by record and a short message is Stray; an alias loop ends at MAX_ALIASES; MAX_ALIASES - self.aliases cannot underflow because a chain is cut at the budget. resolve.rs's four unreachable!s: close cannot double-close (every close removes its entry, and SocketId carries a generation a client's stale id cannot match); recv_from errs only for a missing socket or a pending error, and [udp] sets one only on a connected socket; a first step is always Ask. A datagram is cut to 1,472 by the buffer. None reachable from the wire.
  • Bounds: 16 lookups, asking and ended together; per lookup at most ROUNDS × the lease's resolvers (toyos_dhcp::limits::MAX_DNS) sockets, each emptied every frame; an answer's addresses are bounded by one datagram.
  • Id and port: the low 16 bits of one draw and, from a second draw, an offset into 16,384 ports (RFC 6056 algorithm 1): 30 bits, the same as ships. The strength is the closure the shell passes; nothing in this stage can hold it to the kernel's source, and the move must.
  • Time: no flat wait. Lookup::ask always sets due past now, millis floors and Instant::from_millis(due) <= now implies millis(now) >= due, so no deadline is left at or before now after a pass and none is missed (r16, r17). Lease lost under a lookup: by reading, [udp] refuses for want of a source (route.no-source-address), the arm r6 and r7 control, and the lookup times out on the node's deadlines; link down: the lease stays and the same deadlines end it. Neither has a test; neither has an arm of its own.
  • The oracle, checked against RFC 1035 by hand: the query in a_query_is_rfc_1035s_octets_... (id 0x1234 as the draw's low half, flags 0x0100, counts 1/0/0/0, 3www7example0, type 1, class 1, port 49152 + 7) is §4.1.1 and §4.1.2 and is spelled without toyos-dns; reply (0x8180, the question echoed, owner 0xC00C, type, class, TTL, RDLENGTH, RDATA) is §4.1.1 to §4.1.4. Independent of the code under test: that query test, and every test in so far as its replies are hand-built; not independent of the author.
  • The 18 controls: src/resolve.rs and toyos-dns are byte-identical at 889cdbf27, 0b13818a4 and 025640bf6 (git diff --stat empty); between the first two heads tests/resolve.rs lost one test and renamed one helper used only by the tests of r3, r4, r5 and r18, which were re-run; the merge brought name.rs, tests/name.rs, a lib.rs hunk (r16, re-run) and toyos-net-shard's join, which no resolver test reaches. All 18 patches pass git apply --check at this head. Each log's red is the assertion the body names. The thirteen measured only at the first head stand.
  • The merge: settle ends self.serve_name(now); then self.resolver.pass(...); against stage B the track gains three bullets and one sentence and loses nothing.
  • CI's newer clippy, by reading resolve.rs and tests/resolve.rs for redundant_clone, unchecked_time_subtraction, manual_is_multiple_of, manual_isolate_lowest_one, drain_collect, wrong_self_convention: none found.
  • "No unsent query to let go of": right. [udp] hands the datagram to [ip], whose neighbour machine bounds its own requests, and the socket stays only to read a late answer.

SEND BACK

… ends with its lease

A query's socket is connected to its resolver's port 53, so [udp]'s
4-tuple filter decides what reaches it: nothing from another source or
port, nothing sent to a broadcast or a joined group (RFC 5452 §9.1's
destination match, which the any-bound socket did not make). The node
hands the lookup its own record of where the query went, not the
wire's. lease.rs gains Stack::connect, [udp]'s call passed on, in the
datagram sockets' section. An error the network reports against a
connected socket (its resolver unreachable, its port refused) is
counted node.query-failed and read past.

The reading loop has no cursor: the first listed socket that holds a
datagram is read, until none does or the lookup ended.

toyos_dns::Lookup takes the draw of a query's id, not the id, and calls
it only for a query it sends. netstack's resolver passes its draw at
its three call sites; nothing else of it changes.

A lookup ends LeaseChanged when the held lease names other resolvers
or none is held. Node::resolve answers in NotStarted, the two words it
can say.

Tests: the destination test, a recorded reply of a public resolver
(toyos-dns's REAL_GOOGLE) through the node, the lease's three cases,
the unreachable count; the port-0 case is its own test with its
premise. a_resolver_that_answers_no_arp_holds_up_no_other is deleted:
it held [ip]'s rule, and the track says [ip] has no scenario for it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A
@Japabu

Japabu commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Stage C's 21 mutation patches against 442ecd55e, and the red-first patch against the reviewed tree 025640bf6. Each mutation was applied, built, run against its named test, found red (exit 101) and reverted by build-request-4.sh; the tree ended clean. They replace the round-1 set above.

red-first.patch
diff --git a/userland/netstack/node/tests/resolve.rs b/userland/netstack/node/tests/resolve.rs
index c016562c1..d40cd2b69 100644
--- a/userland/netstack/node/tests/resolve.rs
+++ b/userland/netstack/node/tests/resolve.rs
@@ -635,3 +635,24 @@ fn a_lookup_is_not_carried_by_a_later_ones_schedule() {
     assert_eq!(net.run(second, 10 * WAIT_MS), Some(Err(Ended::Failed(Failure::TimedOut))));
     assert_eq!(net.ms(), WAIT_MS / 2 + rounds * WAIT_MS, "the second lookup waited on the first's schedule");
 }
+
+// RFC 5452 §9.1: a reply is matched on the address it was sent to as well, which is the one its
+// query left from. The limited broadcast, the subnet's broadcast and a group the node has joined
+// each reach [udp] at the query's port, which is the premise, and none is the query's answer.
+#[test]
+fn a_reply_is_read_only_at_the_address_its_query_left_from() {
+    let mut net = Net::leased(&[ANSWERS], Some(R));
+    net.lan.node.answer_as(net.lan.now, toyos_mdns::Host::new("toyos").unwrap()).unwrap();
+    let id = net.resolve("www.example").unwrap();
+    net.pass();
+    let asked = net.queried[0].clone();
+    let group = (Ipv4Addr::new(224, 0, 0, 251), [0x01, 0x00, 0x5e, 0x00, 0x00, 0xfb]);
+    for (destination, mac) in [(Ipv4Addr::BROADCAST, common::BROADCAST), (Ipv4Addr::new(192, 0, 2, 255), common::BROADCAST), group] {
+        let reached = net.lan.counted(Rule::Rx);
+        net.lan.deliver(&udp(mac, MAC_S, (ANSWERS, 53), (destination, asked.udp.source_port), &gives(&asked.udp.payload, FORGED)));
+        assert_eq!(net.lan.counted(Rule::Rx), reached + 1, "the premise: the datagram to {destination} reached [udp]");
+        assert_eq!(net.lan.node.take_resolved(), NONE, "a reply sent to {destination} was read");
+    }
+    net.resolver_says(asked.udp.source_port, &gives(&asked.udp.payload, ADDRESS));
+    assert_eq!(net.lan.node.take_resolved(), [Resolved { id, result: Ok(vec![ADDRESS]) }]);
+}
r1-id-not-drawn.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..77b87b927 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -96,7 +96,7 @@ pub(crate) struct Resolver {
 /// A query's id: the low half of one draw.
 fn id(draw: &mut impl FnMut() -> u32) -> u16 {
     let [_, _, high, low] = draw().to_be_bytes();
-    u16::from_be_bytes([high, low])
+    u16::from_be_bytes([high, low]) & 0
 }
 
 fn close(stack: &mut Stack, now: Instant, socket: SocketId) {
r2-port-not-drawn.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..c5111c7e1 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -126,7 +126,7 @@ fn waiting(queries: &[Query], stack: &mut Stack, reply: &mut [u8], counters: &mu
 fn act(asking: &mut Asking, step: Step, now: Instant, stack: &mut Stack, counters: &mut Counters, draw: &mut impl FnMut() -> u32) -> Option<Result<Vec<[u8; 4]>, Ended>> {
     let done = match step {
         // An any-address bind that names no port is refused only for want of a free one.
-        Step::Ask { asked, to, query } => match stack.bind(Ipv4Addr::UNSPECIFIED, None, &mut *draw) {
+        Step::Ask { asked, to, query } => match stack.bind(Ipv4Addr::UNSPECIFIED, None, || draw() & 0) {
             Ok((socket, _)) => {
                 let resolver = Ipv4Addr::from(to);
                 if stack.connect(now, socket, resolver, PORT).is_ok() && stack.send_to(now, socket, resolver, PORT, &query).is_ok() {
r5-any-port-of-the-lookup.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..72adf84d7 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -200,7 +200,8 @@ impl Resolver {
                 let message = reply.get(..len).unwrap_or_default();
                 // The socket is connected: [udp] delivered this from port 53 of the resolver the
                 // query went to, or not at all.
-                let step = asking.lookup.on_datagram(query.asked, query.to, PORT, message, ms, || id(&mut *draw));
+                let oldest = asking.lookup.waiting().next().unwrap_or(query.asked);
+                let step = asking.lookup.on_datagram(oldest, query.to, PORT, message, ms, || id(&mut *draw));
                 done = act(asking, step, now, stack, counters, &mut *draw);
             }
             if done.is_none() {
r6-unsent-not-counted.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..e72974e6d 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -132,7 +132,7 @@ fn act(asking: &mut Asking, step: Step, now: Instant, stack: &mut Stack, counter
                 if stack.connect(now, socket, resolver, PORT).is_ok() && stack.send_to(now, socket, resolver, PORT, &query).is_ok() {
                     asking.queries.push(Query { asked, socket, to });
                 } else {
-                    counters.add(Counter::QueryUnsent, 1);
+                    counters.add(Counter::QueryUnsent, 0);
                     close(stack, now, socket);
                 }
                 None
r7-refused-query-keeps-its-socket.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..792245dac 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -133,7 +133,6 @@ fn act(asking: &mut Asking, step: Step, now: Instant, stack: &mut Stack, counter
                     asking.queries.push(Query { asked, socket, to });
                 } else {
                     counters.add(Counter::QueryUnsent, 1);
-                    close(stack, now, socket);
                 }
                 None
             }
r8-queries-never-let-go.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..1c09b0000 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -144,7 +144,7 @@ fn act(asking: &mut Asking, step: Step, now: Instant, stack: &mut Stack, counter
     };
     let lookup = &asking.lookup;
     asking.queries.retain(|query| {
-        let read = lookup.waiting().any(|waiting| waiting == query.asked);
+        let read = lookup.waiting().any(|waiting| waiting == query.asked) || true;
         if !read {
             close(stack, now, query.socket);
         }
r9-cap-not-kept.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..f80af8cd2 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -168,7 +168,7 @@ impl Resolver {
         if resolvers.is_empty() {
             return Err(NotStarted::NotConnected);
         }
-        if self.asking.len().saturating_add(self.ended.len()) >= MAX_LOOKUPS {
+        if self.asking.len().saturating_add(self.ended.len()) >= usize::MAX {
             return Err(NotStarted::ResourceExhausted);
         }
         let servers: Vec<[u8; 4]> = resolvers.iter().map(Ipv4Addr::octets).collect();
r10-cap-forgets-untaken-answers.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..dd8d0af1f 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -168,7 +168,7 @@ impl Resolver {
         if resolvers.is_empty() {
             return Err(NotStarted::NotConnected);
         }
-        if self.asking.len().saturating_add(self.ended.len()) >= MAX_LOOKUPS {
+        if self.asking.len() >= MAX_LOOKUPS {
             return Err(NotStarted::ResourceExhausted);
         }
         let servers: Vec<[u8; 4]> = resolvers.iter().map(Ipv4Addr::octets).collect();
r11-no-resolver-answered-exhausted.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..7f71c112c 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -166,7 +166,7 @@ impl Resolver {
     fn start(&mut self, now: Instant, name: Name, stack: &mut Stack, counters: &mut Counters, draw: &mut impl FnMut() -> u32) -> Result<LookupId, NotStarted> {
         let resolvers = stack.lease().map(|lease| lease.dns.clone()).unwrap_or_default();
         if resolvers.is_empty() {
-            return Err(NotStarted::NotConnected);
+            return Err(NotStarted::ResourceExhausted);
         }
         if self.asking.len().saturating_add(self.ended.len()) >= MAX_LOOKUPS {
             return Err(NotStarted::ResourceExhausted);
r12-let-go-keeps-its-sockets.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..bc6d8cd93 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -221,9 +221,7 @@ impl Resolver {
             if asking.id != id {
                 return true;
             }
-            for query in &asking.queries {
-                close(stack, now, query.socket);
-            }
+            let _ = (&stack, now);
             false
         });
         self.ended.retain(|resolved| resolved.id != id);
r13-let-go-keeps-the-answer.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..466dbddd5 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -226,7 +226,7 @@ impl Resolver {
             }
             false
         });
-        self.ended.retain(|resolved| resolved.id != id);
+        self.ended.retain(|resolved| resolved.id != id || true);
     }
 }
 
r14-ended-lookup-keeps-its-sockets.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..52048f231 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -208,9 +208,6 @@ impl Resolver {
                 done = act(asking, step, now, stack, counters, &mut *draw);
             }
             let Some(result) = done else { return true };
-            for query in &asking.queries {
-                close(stack, now, query.socket);
-            }
             ended.push(Resolved { id: asking.id, result });
             false
         });
r15-no-port-is-no-end.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..4055a0ad0 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -137,7 +137,7 @@ fn act(asking: &mut Asking, step: Step, now: Instant, stack: &mut Stack, counter
                 }
                 None
             }
-            Err(_) => Some(Err(Ended::NoPort)),
+            Err(_) => None,
         },
         Step::Wait => None,
         Step::Done(result) => Some(result.map_err(Ended::Failed)),
r16-deadline-without-the-lookups.patch
diff --git a/userland/netstack/node/src/lib.rs b/userland/netstack/node/src/lib.rs
index ae7ef358a..25043f0be 100644
--- a/userland/netstack/node/src/lib.rs
+++ b/userland/netstack/node/src/lib.rs
@@ -149,7 +149,7 @@ impl Node {
 
     pub fn next_deadline(&self) -> Option<Instant> {
         let name = self.name.as_ref().and_then(name::Name::next_deadline);
-        self.stack.next_deadline().into_iter().chain(self.client.next_deadline()).chain(name).chain(self.resolver.next_deadline()).min()
+        self.stack.next_deadline().into_iter().chain(self.client.next_deadline()).chain(name).min()
     }
 
     /// Every deadline at or before `now`; the frames they make due wait for [`Self::transmit`].
r17-deadline-is-the-latest.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..9a25eba01 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -160,7 +160,7 @@ impl Resolver {
 
     /// When the soonest lookup's wait is over.
     pub(crate) fn next_deadline(&self) -> Option<Instant> {
-        self.asking.iter().map(|asking| Instant::from_millis(asking.lookup.due())).min()
+        self.asking.iter().map(|asking| Instant::from_millis(asking.lookup.due())).max()
     }
 
     fn start(&mut self, now: Instant, name: Name, stack: &mut Stack, counters: &mut Counters, draw: &mut impl FnMut() -> u32) -> Result<LookupId, NotStarted> {
r18-dns-id-not-compared.patch
diff --git a/toyos-dns/src/lib.rs b/toyos-dns/src/lib.rs
index 1f46cdaa9..58d25a96f 100644
--- a/toyos-dns/src/lib.rs
+++ b/toyos-dns/src/lib.rs
@@ -573,7 +573,7 @@ impl Lookup {
             return Step::Wait;
         };
         let Some(which) =
-            self.sent.iter().position(|s| s.asked == asked && port == PORT && s.to == from && s.id == carried)
+            self.sent.iter().position(|s| s.asked == asked && port == PORT && s.to == from)
         else {
             return Step::Wait;
         };
c1-query-socket-not-connected.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..1224b3e22 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -129,7 +129,7 @@ fn act(asking: &mut Asking, step: Step, now: Instant, stack: &mut Stack, counter
         Step::Ask { asked, to, query } => match stack.bind(Ipv4Addr::UNSPECIFIED, None, &mut *draw) {
             Ok((socket, _)) => {
                 let resolver = Ipv4Addr::from(to);
-                if stack.connect(now, socket, resolver, PORT).is_ok() && stack.send_to(now, socket, resolver, PORT, &query).is_ok() {
+                if stack.send_to(now, socket, resolver, PORT, &query).is_ok() {
                     asking.queries.push(Query { asked, socket, to });
                 } else {
                     counters.add(Counter::QueryUnsent, 1);
c2-lease-not-read.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..d2f499cc3 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -193,7 +193,7 @@ impl Resolver {
         let Self { asking: lookups, ended, reply, .. } = self;
         let ms = millis(now);
         lookups.retain_mut(|asking| {
-            let named = stack.lease().is_some_and(|lease| lease.dns == asking.resolvers);
+            let named = stack.lease().is_none_or(|lease| lease.dns == asking.resolvers || true);
             let mut done = if named { None } else { Some(Err(Ended::LeaseChanged)) };
             while done.is_none() {
                 let Some((query, len)) = waiting(&asking.queries, stack, reply.as_mut_slice(), counters) else { break };
c3-failed-not-counted.patch
diff --git a/userland/netstack/node/src/resolve.rs b/userland/netstack/node/src/resolve.rs
index d8a5cfc23..add47f210 100644
--- a/userland/netstack/node/src/resolve.rs
+++ b/userland/netstack/node/src/resolve.rs
@@ -113,7 +113,7 @@ fn waiting(queries: &[Query], stack: &mut Stack, reply: &mut [u8], counters: &mu
             match stack.recv_from(query.socket, reply) {
                 Ok(Some(received)) => return Some((*query, received.len)),
                 Ok(None) => break,
-                Err(Error::Failed(_)) => counters.add(Counter::QueryFailed, 1),
+                Err(Error::Failed(_)) => counters.add(Counter::QueryFailed, 0),
                 Err(Error::NoSuchSocket | Error::Refused(_)) => unreachable!("a query's socket is open while it is listed, and no rule refuses a receive"),
             }
         }
c4-dns-wait-draws-early.patch
diff --git a/toyos-dns/src/lib.rs b/toyos-dns/src/lib.rs
index 1f46cdaa9..12a4fba20 100644
--- a/toyos-dns/src/lib.rs
+++ b/toyos-dns/src/lib.rs
@@ -552,10 +552,11 @@ impl Lookup {
     /// The time is `now`: the newest query has had its [`WAIT_MS`] where it
     /// is due. `id` draws the ID of the query this sends.
     pub fn on_time(&mut self, now: u64, id: impl FnOnce() -> u16) -> Step {
+        let drawn = id();
         if now < self.due {
             return Step::Wait;
         }
-        self.ask(now, id)
+        self.ask(now, move || drawn)
     }
 
     /// `msg` arrived at `now` from `port` of `from`, on the port `asked` was
c5-dns-end-draws.patch
diff --git a/toyos-dns/src/lib.rs b/toyos-dns/src/lib.rs
index 1f46cdaa9..ee34867c0 100644
--- a/toyos-dns/src/lib.rs
+++ b/toyos-dns/src/lib.rs
@@ -533,6 +533,8 @@ impl Lookup {
 
     /// The next query for `name`, or the end where every one has been sent.
     fn ask(&mut self, now: u64, id: impl FnOnce() -> u16) -> Step {
+        let drawn = id();
+        let id = move || drawn;
         if self.asked == ROUNDS * self.servers.len() {
             return Step::Done(Err(match self.failed {
                 Some(rcode) => Failure::ServerFailed(rcode),

@Japabu

Japabu commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Review of wt/toyos-ownstack-c at 442ecd55e, round 2, against .claude/agents/reviewer.md. Scope: git diff 025640bf6 442ecd55e (8 files, +346 −159), the changed files whole, and toyos-net-udp/src/lib.rs and toyos-net-ip/src/nud.rs where the fix rests on them. Read, not run.

Net lines: against origin/main the stack of three is 21 files, +3723 −57. This stage over stage B (git diff --numstat 288484ba4 442ecd55e): production +300 −21 (the node's source, manifest and lock +276 −8, toyos-dns/src/lib.rs +21 −10, netstack +3 −3), tests +817 −43, track +5 −1. Since round 1: production +120 −71, tests +222 −85, track +4 −3. The growth since round 1 is the connect, the lease check, the failed-query arm and NotStarted, each asked for; the cursor's deletion is in it.

Round 1's BLOCKERs

  1. cargo run -- --ci host — OPEN. No measurement at 442ecd55e; the pull request's host, guest and toolchain checks read skipping (draft). The fix round widened what it must show, because a shipped crate's source changed. See "What closes it" below.
  2. A reply is read whatever address it was sent to — CLOSED. Measurement: ownstack-c-r4.log at 442ecd55e, red-first-a_reply_is_read_only_at_the_address_its_query_left_from EXIT=101 on the tree of 025640bf6 plus the test (its log: a reply sent to 255.255.255.255 was read, the lookup ended with 203.0.113.66), node-tests EXIT=0 at the head with that test ok, and mutation c1 (the connect removed) EXIT=101 on it and on the source, port-5353 and port-0 tests. By [udp]'s code and not only by the test: toyos-net-udp/src/lib.rs:592-598 is one exhaustive match, in which a Binding::Connected socket is handed Cast::Unicast only when destination == local && source == peer && source_port == Some(peer_port), and Cast::LimitedBroadcast | Cast::SubnetBroadcast | Cast::Multicast(_) only when binding == Binding::Any, which a connected socket is not; Cast::Acquisition only to the acquisition socket. connect (:427-432) fixes local to the route's source, the lease's address. All three destinations fall in the one arm. The red-first and c1 runs stop at the loop's first destination, so the subnet-broadcast and group iterations have no red of their own; they rest on that arm and on [udp]'s own s_udp_us_055_prop_the_demultiplexer, which I accept by reading.
  3. The re-reading cursor — CLOSED. It is deleted: resolve.rs:110-122 reads the first listed socket that holds a datagram and pass repeats until none does or the lookup ended. There is no position left to mutate; node-tests EXIT=0, 23 of tests/resolve.rs ok.

Round 1's NOTEs

  • Draws: closed. Lookup::ask is the only caller of the draw (toyos-dns/src/lib.rs:546), after the exhausted-rounds return; c4 and c5 EXIT=101 on rfc5452_9_2_an_id_is_drawn_for_a_query_that_is_sent_and_for_nothing_else.
  • A lookup and its lease: closed, decided as ending. resolve.rs:196-197; c2 EXIT=101 on both ending tests. settle runs pass after every client step, so no lease change passes between two looks.
  • The third-party reply: closed. The 60 octets in tests/resolve.rs are REAL_GOOGLE's; the premise asserts the node's own id and question against them; the bullet is gone.
  • a_resolver_that_answers_no_arp_holds_up_no_other: closed by deletion, with the rule's missing home recorded (see the NOTE on that bullet below).
  • The two bullets' "when": closed, both "Exit, before the move".
  • The port-0 arm: closed. The arm is gone; a_reply_from_port_0_is_not_read asserts udp.rx-src-port-zero moved and c1 is red on it.
  • Node::resolve's type: closed, NotStarted.
  • The body's dead-primary sentence: closed, gone.

BLOCKER

  • PR body, "Not run" — cargo run -- --ci host and guest / suite have no measurement at this head — the Evidence rule; and userland/netstack/src/resolve.rs ships in the image, so "the node ships in nothing" no longer covers the diff.

NOTE

  • userland/netstack/node/tests/resolve.rs (a_lookup_outlives_a_renewal_that_names_the_same_resolvers) — no premise that the renewal was taken — the test passes unchanged if the ACK is dropped, and it alone holds "a renewal that names the same resolvers leaves the lookup be". Its sibling asserts the premise through lease().dns; here nothing moves that is asserted. Assert that the held lease's base (or timers' expiry) moved across the deliver. This answers the implementer's second "unsure": it is not a mutation gap. pass reads the lease through one comparison, lease.dns == asking.resolvers, and the resolver holds nothing else of the lease that a renewal could change, so "ends on any renewal" is not one patch away and a reader of that line sees it; c2 holds the other side. What is missing is the premise, not a mutation.
  • userland/netstack/node/tests/resolve.rs (a_query_the_network_reports_unreachable_is_counted_and_waited_out) — the count of exactly ROUNDS rests on two of [ip]'s constants the test does not name — by toyos-net-ip/src/nud.rs: the queries of 0 s and 2 s are both pending when the neighbour fails at 3 × RETRANS = 3 s (two reports), and the query of 4 s is refused at once only because FAILED_HOLD (20 s) still holds the entry (the third). With a hold under one second the third query would start a new resolution that fails at 7 s, after the lookup ended at 6 s, and the count would be 2. It is deterministic and correct today, so the assertion stays; assert the premise beside it, that [ip] refused exactly one query on a failed neighbour (nb.failed-refused), so a red after a change to [ip]'s timing names its cause. The implementer's first "unsure" is therefore real but narrower than the body says: the hold has to outlast one second, not the lookup's schedule.
  • issues/toyos-has-its-own-network-stack.md, the new bullet "No test of toyos-net-ip holds that a next hop which never answers ARP holds up no datagram to another" — its exit says no when — round 1 asked the same of its two neighbours. The node's resolver rests on the rule from the move on, and this branch deleted the one test that reached it: "Exit, before the move".
  • issues/toyos-has-its-own-network-stack.md, the bullet "A query that does not reach its resolver" — its exit reads only the node.query-unsent test — the node.query-failed half has a_query_the_network_reports_unreachable_is_counted_and_waited_out, which the exit should name with the time it then ends at.
  • PR body, "Unsure of", first item — "longer than a lookup's schedule" is not what the count rests on (the NOTE above). (Prose.)

What the brief asked, where no finding came of it

  • toyos-net-node: clients' datagram sockets and the machine's mDNS name on ToyOS's own UDP, host-tested and shipped in nothing #772's warning. socket.pending is set in two places, icmp_error and unreachable (toyos-net-udp/src/lib.rs:646-674), both through connected(flow), which matches only Binding::Connected with the whole 4-tuple. A client's socket (datagram.rs), the responder's (name.rs) and the DHCP client's (lease.rs) are never connected, so datagram.rs:56, name.rs:79 and lease.rs:128 stay unreachable. Port reuse does not reach them: an error for a closed query's datagram that finds a client on the same port finds it unconnected and is counted udp.icmp-error-unconnected or dropped. In resolve.rs: waiting takes Error::Failed by name and its unreachable! covers only NoSuchSocket | Refused, which recv cannot return for a listed socket; connect and send_to are read with is_ok(), so a pending error taken by submit (possible only if a new query's socket reuses the port and resolver of a closed one whose datagram [ip] still held) is counted node.query-unsent and the socket closed; close errs only for a missing socket. A forged ICMP port-unreachable quoting the 4-tuple costs one count and ends nothing.
  • The shipped netstack. It draws at fewer moments: main drew one id per reply read and one per wait looked at; now Lookup::ask draws one per query sent. The first query's two draws (id in start, port in act) are in the order they were. The source is random_u16, the kernel's, so nothing on the wire can tell beyond fewer random syscalls. Host: netstack-tests EXIT=0, 27 ok, 9 of them resolve::tests::; their draw is a xorshift counter, and the one test whose premise names a drawn value (a_query_leaves_from_no_port_a_client_holds) reads only the first two draws and asserts its premise. Guest: git grep finds no guest test that drives a lookup, so no guest test sees the draws; the suite sees that netstack builds for the guest target and still serves what it served.
  • Lookup::on_datagram's source address and port. The node hands it query.to and PORT, a parameter with one value from this caller; the shipped resolver hands it the wire's, which its unconnected smoltcp sockets need. Recorded in the track with an exit at the move. Accepted as recorded.
  • CI's newer clippy, by reading the delta for redundant_clone, unchecked_time_subtraction, manual_is_multiple_of, manual_isolate_lowest_one, drain_collect, wrong_self_convention: none found. lease.dns.clone() is stored; the tests' queried[0].clone() precedes a &mut use of its owner.
  • The body's numbers against ownstack-c-r4.log: 21 mutations in 25 runs, each EXIT=101; 23, 44 and 27 tests; clean=yes. They agree.

What closes BLOCKER 1

On CI, once #771 and #772 have landed and this is ready, for the head that lands:

  • host: the step cargo run -- --ci host exits 0, and its log shows toyos-net-node's tests/resolve.rs with 23 passed, among them a_reply_is_read_only_at_the_address_its_query_left_from, a_reply_from_port_0_is_not_read, a_recorded_reply_of_a_public_resolver_answers_the_nodes_query, a_query_the_network_reports_unreachable_is_counted_and_waited_out and the three lease tests; toyos-dns with 44 passed, among them rfc5452_9_2_an_id_is_drawn_for_a_query_that_is_sent_and_for_nothing_else; netstack's unit tests with 27 passed, 9 of them resolve::tests::; and the clippy step clean under CI's toolchain over the node, toyos-dns and netstack for the guest target.
  • guest / suite: concluded success and not skipped, with toolchain having built this head's sources. No row in it is a lookup; what it measures of this change is that the image's netstack builds and every test that boots it stays green.

The orchestrator may close it on reading those two logs, and land, if the head CI ran is 442ecd55e plus the NOTE changes above and nothing else (two premise assertions in tests/resolve.rs, two phrases in the track), or a rebase of that which resolved no conflict in the eleven files this stage touches. A hunk outside that, or a hand-resolved one, is new code and comes back. A count in the log other than 23, 44 and 27 is read against the diff before anything lands.

SEND BACK

The same-resolvers renewal test asserts the lease's expiry moved across
the ACK; the unreachable test asserts [ip] refused exactly one query on
a failed neighbour. The track's [ip] bullet exits before the move, and
the unreachable bullet names the node.query-failed test and the time it
then ends at.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A
No conflict: Cargo.lock and the track merged by git alone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A
@Japabu

Japabu commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

CI at 0e113baee, read against round 2's list (orchestrator): run 37834280548, event pull_request, conclusion success; host, toolchain / build and guest / suite each success. The head is ff4e4ce0d with main merged by git alone (nothing resolved by hand; nine of the stage's eleven files identical, the other two differing by one line of main's each).

  • host: [ci] Host: 79 step(s), all green. toyos-net-node's tests/resolve.rs: 23 passed, among them a_reply_is_read_only_at_the_address_its_query_left_from, a_reply_from_port_0_is_not_read, a_recorded_reply_of_a_public_resolver_answers_the_nodes_query, a_query_the_network_reports_unreachable_is_counted_and_waited_out. toyos-dns: 44 passed, rfc5452_9_2_an_id_is_drawn_for_a_query_that_is_sent_and_for_nothing_else among them. netstack's unit tests: 27 passed.
  • guest / suite: success, not skipped; [ci] the suite: test result: ok. 36 passed, 36 total; [ci] Guest: 5 step(s), all green.

Round 2's open BLOCKER closes on this.

@Japabu
Japabu added this pull request to the merge queue Oct 8, 2026
Merged via the queue into main with commit a4416fe Oct 8, 2026
6 checks passed
@Japabu
Japabu deleted the wt/toyos-ownstack-c branch October 8, 2026 20:20
Japabu added a commit that referenced this pull request Oct 8, 2026
Resolved by hand, each to carry both stages:

- `userland/netstack/node/src/lib.rs`, the modules: `mod resolve;` of
  main's and `mod streams;` with its `pub use` of this branch's, both kept.
- `userland/netstack/node/src/lib.rs`, `Node::next_deadline`: one chain of
  five, the stack's, the DHCP client's, the name's, the resolver's and the
  streams' deadlines.
- `issues/toyos-has-its-own-network-stack.md`, the stage 5 paragraph: in
  the tree are the lease, the datagram sockets, the mDNS name, the resolver
  and streams; still to build are listeners.

Edited by hand where git saw no conflict: the node manifest's
`description`, which main left without the resolver, names it beside the
rest.

Merged by git and read: `settle`'s loop ends in `break` and its tail is
`serve_name`, then the resolver's pass; `receive` and `fire` run the
stream pass after `settle`; `transmit` ends in the pass over the connects,
main's having no tail of its own; `Node` and `Node::new` hold `name`,
`resolver` and `streams` once each; `lease.rs` has `mod tcp;`, and the
resolver's `Stack::connect` stands among the datagram forwards beside
`tcp_connect`; the track's list has this branch's six lines and all of
main's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A
Japabu added a commit that referenced this pull request Oct 8, 2026
…ge E

By hand: the node's module list and its header's paragraph on who owns what
(both sides whole), the manifest's description (what both sides put in the
package), and the track's stage 5 paragraph (in the tree: the resolver and
listeners both; still to build: the move). Everything else merged by itself.

places.rs says what the merged tree does: the responder's socket and each
query's are bound past Node::udp_bind and hold no place, so clients at the
bound refuse no lookup. The resolver's test that binds every dynamic port but
one now gives its node that many places, since a client's socket holds one,
and asserts that with every place a client's socket the lookup still starts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant