From 764db4c831028a8d3aea65269a02cd37c2c4c282 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 9 Oct 2026 09:22:41 +0200 Subject: [PATCH 1/2] The pipe ABI gives a listener its TCP_NODELAY and an accept's answer its connection's A host gives a connection the option its listener held when the connection began. The pipe ABI could say neither half: netstack refused the option request for a listener's id, libc's accept sent a second request to hand its listener's stored value to each connection, and std built every accepted stream with the option off. toyos::net: MsgType::TcpListenerSetOption (14, the number the removed option read left) on SocketOptionRequest, and TcpAcceptPipedResponse ends in a TcpOptions, one word an option, which TcpAccepted carries as it is. netstack on smoltcp: a listener is one socket that becomes its connection, so the socket holds the listener's option from the moment it listens, a set reaches it only while it is still in Listen, and the socket that listens after an accept or after a reset is opened with what the listener holds then (Listening::open, Listening::set_nodelay). The accept answers what the socket holds. smoltcp's reset keeps the Nagle switch, which is why a re-listen sets it. libc: setsockopt on a bound listener sends the listener's request, bind hands over a value set before it, and accept takes the accepted socket's value from the answer and sends nothing after it, so no reset between the answer and a hand-over can fail it. std: an accepted stream's nodelay is the answer's, and a stream and its duplicates share one value locked across the request that changes it, as UdpSocket's broadcast is. std's TcpListener has no call for the option on any host and gets none here: a setter nothing calls is dead code in std's build. The guest case reads the libc half against lines this Mac prints for the same source, and a Rust job reads std's and, by toyos::net, the rule for a connection that waited when the option was set, ordered by the wake netstack writes for it. Closes issues/std-holds-a-streams-nodelay-per-duplicate.md: its exit is the duplicate lines of nodelay_accepted. The listener issue is renamed to what is left of it: libc's poll watches no socket, and no Rust program can set a listener's option, the socket2 and mio forks answering from guesses. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- ...c-program-cannot-order-the-waiting-case.md | 69 ++++++++++++ ...cepted-streams-is-answered-from-a-guess.md | 58 ---------- ...uses-the-option-requests-a-host-answers.md | 12 +- ...-a-streams-tcp-nodelay-reached-netstack.md | 20 ++-- ...d-holds-a-streams-nodelay-per-duplicate.md | 26 ----- issues/toyos-has-its-own-network-stack.md | 3 +- sdk/std/sys/net/connection.rs | 23 ++-- tests/netcase/nodelay_kept.c | 36 ++++-- .../src/bin/nodelay_accepted.rs | 103 ++++++++++++++++++ tests/toyos.rs | 97 +++++++++++------ toyos/src/net.rs | 64 ++++++++++- userland/libc/src/socket.rs | 49 +++++---- userland/netstack/src/listen.rs | 66 +++++++---- userland/netstack/src/listen/tests.rs | 62 +++++++++++ userland/netstack/src/main.rs | 41 +++++-- 15 files changed, 512 insertions(+), 217 deletions(-) create mode 100644 issues/a-listeners-tcp-nodelay-has-no-setter-in-std-or-the-forks-and-a-c-program-cannot-order-the-waiting-case.md delete mode 100644 issues/a-listening-sockets-tcp-nodelay-is-refused-and-an-accepted-streams-is-answered-from-a-guess.md delete mode 100644 issues/std-holds-a-streams-nodelay-per-duplicate.md create mode 100644 tests/toyos-rust-tests/src/bin/nodelay_accepted.rs diff --git a/issues/a-listeners-tcp-nodelay-has-no-setter-in-std-or-the-forks-and-a-c-program-cannot-order-the-waiting-case.md b/issues/a-listeners-tcp-nodelay-has-no-setter-in-std-or-the-forks-and-a-c-program-cannot-order-the-waiting-case.md new file mode 100644 index 00000000000..36608a68f60 --- /dev/null +++ b/issues/a-listeners-tcp-nodelay-has-no-setter-in-std-or-the-forks-and-a-c-program-cannot-order-the-waiting-case.md @@ -0,0 +1,69 @@ +--- +status: open +kind: defect +opened: 2026-10-09 +--- + +# A listener's TCP_NODELAY has no setter in std or the forks, and a C program cannot order the waiting case + +A host takes `TCP_NODELAY` on a listening socket and gives it to the +connections that begin afterwards. Read on macOS 27 on arm64, by libc's +`setsockopt` and `getsockopt` over loopback: set before the connection began, +the accepted socket has the option; set after the connection was established +and before the accept, it does not. `userland/netstack/node/tests/host.rs` +asks the same of every host the suite runs on, and sets the option in the +second case only once the listener is readable, which is the host saying the +connection waits to be accepted: a connect returns before the listener's end +is established, so the answer rests on that event and not on the order +loopback delivers in. A red there prints the host's name and both answers. + +The pipe ABI carries both halves (`toyos/src/net.rs`): +`MsgType::TcpListenerSetOption` sets a listener's option, and an accept's +answer says what its connection holds (`TcpOptions`). netstack answers the +rule for both (`userland/netstack/src/listen.rs`), and the `libc_sockets` +guest test reads it: `nodelay_accepted` by `toyos::net`, for a connection that +waited when the option was set, its wake read first, and for the next one +dialled; `tests/netcase/nodelay_kept.c` by libc's `setsockopt` and +`getsockopt`, for a listener's set before `bind`, its set and its clear after, +and the connections dialled after each. What no program reads yet: + +- A C program cannot set the option while a connection waits and know that + one does: libc's `poll` watches kernel handles and a socket descriptor is + none (`userland/libc/src/posix_io.rs`), so the only call that says a + connection waits is the `accept` that takes it. `nodelay_kept.c` therefore + holds the half of the rule a second dial orders, and not the first. +- No Rust program can set the option on a listener. `std::net::TcpListener` + has no call for it on any host, where a program reaches the socket through + socket2 by its descriptor. On ToyOS, read from the forks at the commits the + lock names and not run: socket2 (`1d90250`, `src/sys/toyos.rs`) keeps a + table of its own sockets, so a std listener's descriptor names none of + them; on a listener of its own its `setsockopt` of `TCP_NODELAY` answers + `Ok` and hands the option to nobody, and its `accept` records the accepted + socket's as off whatever the answer says. mio (`d1edcaa`, + `src/net/tcp/toyos_stream.rs`) answers `nodelay()` true for every stream + and its `set_nodelay` sends nothing. std's `TcpListener::accept` + (`sdk/std/sys/net/connection.rs`) takes the accepted stream's option from + the answer, which no test can turn red while no std listener can hold one. + +The node (`userland/netstack/node/src/listeners.rs`) has the listener's call, +`Node::set_listener_nodelay`, and gives an accepted stream what its connection +took from its listener; nothing ships it until the move. + +Not read: Linux. No Linux host was at hand when this was filed. The `host` +check runs `tests/host.rs` on Linux, and its first run on a pull request that +carries the file is that reading. + +**Exit condition**: `tests/host.rs` is green in a `host` check's log, which +is the Linux reading; and on ToyOS a program that sets `TCP_NODELAY` on a +listening socket and accepts one connection begun before and one begun after +reads the option on each as that test has a host answer, by libc's +`getsockopt`, which needs libc's `poll` to answer for a listening socket, and +by std's `nodelay()`, which needs a listener a Rust program can set the option +on: the socket2 fork's, with the lock naming the commit that hands its +listener's option to netstack and reads its accepted socket's from the answer, +and mio's that reads a stream's. + +**Owner**: whoever holds `issues/toyos-has-its-own-network-stack.md`; the +Linux reading is the orchestrator's, at the first `host` run of the pull +request that adds `tests/host.rs`; the fork commits are on socket2's and mio's +`toyos` branches. diff --git a/issues/a-listening-sockets-tcp-nodelay-is-refused-and-an-accepted-streams-is-answered-from-a-guess.md b/issues/a-listening-sockets-tcp-nodelay-is-refused-and-an-accepted-streams-is-answered-from-a-guess.md deleted file mode 100644 index ae9ae8036ff..00000000000 --- a/issues/a-listening-sockets-tcp-nodelay-is-refused-and-an-accepted-streams-is-answered-from-a-guess.md +++ /dev/null @@ -1,58 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-08 ---- - -# A listening socket's TCP_NODELAY is refused, and an accepted stream's is answered from a guess - -A host takes `TCP_NODELAY` on a listening socket and gives it to the -connections that begin afterwards. Read on macOS 27 on arm64, by libc's -`setsockopt` and `getsockopt` over loopback: set before the connection began, -the accepted socket has the option; set after the connection was established -and before the accept, it does not. `userland/netstack/node/tests/host.rs` -asks the same of every host the suite runs on, and sets the option in the -second case only once the listener is readable, which is the host saying the -connection waits to be accepted: a connect returns before the listener's end -is established, so the answer rests on that event and not on the order -loopback delivers in. A red there prints the host's name and both answers. - -ToyOS as it ships does neither half for an option set on a socket that -listens: - -- netstack answers `TcpSetOption` `ERR_NOT_CONNECTED` for - any id that is not a stream (`handle_tcp_set_option`, - `userland/netstack/src/main.rs`), and libc's `setsockopt` sends a listening - socket's id there (`userland/libc/src/socket.rs`), so a C program that sets - the option on its listener is refused, -1 and `ENOTCONN`, and the listener - keeps what it held. libc holds a listener from `bind`. One set before that - it keeps, and `accept` hands it to netstack for each connection, as - `connect` does: with every later set refused it is the option the listener - held when any connection began. `tests/netcase/nodelay_kept.c` holds the - refusal, and the accepted socket's read for a listener that held the option - at `bind` and for one that did not; its three lines on the refused set are - the ones this file's exit turns to the host's answers. -- std builds every `TcpStream`, an accepted one included, with - `nodelay: false` and answers `nodelay()` from that field without asking - netstack (`sdk/std/sys/net/connection.rs`). Nothing can make that false - today: std has no setter for a listener, and what libc's `accept` hands - over is for a socket of libc's. Once a listener's - option reaches the connections it accepts, it is false for each of them. - -The node (`userland/netstack/node/src/listeners.rs`) has the calls, -`Node::set_listener_nodelay` and `Node::listener_nodelay`, and gives an -accepted stream what its connection took from its listener; nothing ships it. - -Not read: Linux. No Linux host was at hand when this was filed. The `host` -check runs `tests/host.rs` on Linux, and its first run on a pull request that -carries the file is that reading. - -**Exit condition**: `tests/host.rs` is green in a `host` check's log, which -is the Linux reading; and on ToyOS a program that sets `TCP_NODELAY` on a -listening socket and accepts one connection begun before and one begun after -reads the option on each as that test has a host answer, by libc's -`getsockopt` and by std's `nodelay()`. - -**Owner**: whoever holds `issues/toyos-has-its-own-network-stack.md`; the -Linux reading is the orchestrator's, at the first `host` run of the pull -request that adds `tests/host.rs`. diff --git a/issues/a-stream-its-peer-reset-refuses-the-option-requests-a-host-answers.md b/issues/a-stream-its-peer-reset-refuses-the-option-requests-a-host-answers.md index 2bee6b0e5b1..34b15202366 100644 --- a/issues/a-stream-its-peer-reset-refuses-the-option-requests-a-host-answers.md +++ b/issues/a-stream-its-peer-reset-refuses-the-option-requests-a-host-answers.md @@ -30,16 +30,16 @@ request is not the fix: a client that dies sends none, and netstack, having closed its pipe ends, has nothing left that says the client is gone. libc's `connect` asks the same request of a connection it has just been -answered, to hand over a `TCP_NODELAY` set before it, and its `accept` of one -it has just been given, to hand over its listener's: a peer that resets -between netstack's answer and that request makes either fail `ENOTCONN`, with -the connection closed, which no host's `connect` or `accept` answers. +answered, to hand over a `TCP_NODELAY` set before it: a peer that resets +between netstack's answer and that request makes it fail `ENOTCONN`, with the +connection closed, which no host's `connect` answers. Its `accept` asks +nothing after its answer, which carries the connection's option. **Exit condition**: a guest test on `tests/netcase` whose peer resets a stream the client still holds, and `nodelay()` on it answers `Ok`, with `netstack_socket_churn` still green; and the two compromises above ended: the option request on a stream its peer reset is answered by netstack, and a libc -`connect` and a libc `accept` whose peer resets before the hand-over answer as -the host does, each held by a test that can be red. +`connect` whose peer resets before the hand-over answers as the host does, +held by a test that can be red. **Owner**: whoever holds `issues/toyos-has-its-own-network-stack.md`. diff --git a/issues/nothing-reads-whether-a-streams-tcp-nodelay-reached-netstack.md b/issues/nothing-reads-whether-a-streams-tcp-nodelay-reached-netstack.md index e203fc97284..7772b0d0732 100644 --- a/issues/nothing-reads-whether-a-streams-tcp-nodelay-reached-netstack.md +++ b/issues/nothing-reads-whether-a-streams-tcp-nodelay-reached-netstack.md @@ -7,23 +7,25 @@ opened: 2026-10-08 # Nothing reads whether a stream's TCP_NODELAY reached netstack netstack answers `MsgType::TcpSetOption` done or with an error, and nothing -answers what a stream holds: the pipe ABI has no option read -(`toyos/src/net.rs`), and std's `nodelay()` and libc's `getsockopt` answer the -value their own side stored. So a setter that stores the value and sends +answers what a stream holds once it is a client's: the pipe ABI says it once, +in the accept's answer (`TcpOptions`, `toyos/src/net.rs`), and has no option +read, and std's `nodelay()` and libc's `getsockopt` answer the value their own +side stored. So a setter of a stream's option that stores the value and sends nothing is seen by no test. Measured on libc's `connect`, which hands over a `TCP_NODELAY` set before the connection existed: with the hand-over deleted, the guest case that sets the option before `connect` and reads it after -(`tests/netcase/nodelay_kept.c`, the `libc_sockets` test) stays green. And on -its `accept`, which hands over the one its listener held at `bind`: with that -request deleted and the value still stored, the same case's read of the -accepted socket stays green. The +(`tests/netcase/nodelay_kept.c`, the `libc_sockets` test) stays green. The only effect of the option is when a small write leaves the machine, which a QEMU test may not time. +A listener's is read: what an accepted socket answers is what netstack said of +its connection, so libc's `bind` or its `setsockopt` on a listener storing the +value and sending nothing turns that case red. + **Exit**: a test reads what the stack holds for a stream's Nagle switch, on the node under a host test of its option call or through a guest's wire, and -turns red when libc's `connect` or `accept` or std's `set_nodelay` stores the -value and sends no request. +turns red when libc's `connect` or `setsockopt` on a connection, or std's +`set_nodelay`, stores the value and sends no request. **Owner**: whoever holds `issues/toyos-has-its-own-network-stack.md`, with the reset-stream option test of diff --git a/issues/std-holds-a-streams-nodelay-per-duplicate.md b/issues/std-holds-a-streams-nodelay-per-duplicate.md deleted file mode 100644 index f5ddbd738b2..00000000000 --- a/issues/std-holds-a-streams-nodelay-per-duplicate.md +++ /dev/null @@ -1,26 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-08 ---- - -# std holds a stream's nodelay per duplicate - -Read from the code, not run. `TcpStream::try_clone` -(`sdk/std/sys/net/connection.rs`, `duplicate`) shares the one netstack socket -between the stream and its duplicate and gives the duplicate its own copy of -`nodelay`. `set_nodelay` on either changes the socket both name and stores the -value only in the one it was called on, so `nodelay()` on the other answers -what was true before. A host's getter reads the kernel's value and cannot -differ. Two threads setting opposite values on one stream can also reach -netstack in one order and the store in the other. `UdpSocket`'s `broadcast` -is one value shared by every duplicate and locked across its request. - -**Exit**: `nodelay` is one value shared by every duplicate of a stream and -locked across the request that changes it, and a guest test sets it through a -duplicate and reads it from the original. - -**Owner**: whoever holds `issues/toyos-has-its-own-network-stack.md`, with the -reset-stream option test of -`issues/a-stream-its-peer-reset-refuses-the-option-requests-a-host-answers.md`, -which reads the same value. diff --git a/issues/toyos-has-its-own-network-stack.md b/issues/toyos-has-its-own-network-stack.md index 763e8e8f1cc..b78bfdff6bd 100644 --- a/issues/toyos-has-its-own-network-stack.md +++ b/issues/toyos-has-its-own-network-stack.md @@ -43,9 +43,10 @@ What the node does not yet meet: - A query [ip] refuses at the transmit opportunity that would have carried it, its next hop held failed or its table full, is reported to its socket inside `Node::transmit`, which carries no lookup on: the resolver reads the report at the node's next `receive`, `fire` or `link`, which at the latest is the end of the query's wait, where a query [udp] refuses in the resolver's own call, or one reported in a frame or at a deadline, ends its wait at once. `a_query_ip_refuses_at_its_transmit_opportunity_is_read_at_the_nodes_next_wake` holds it as it stands. It is no regression: what ships waits every such query out. Until that read, an ICMP error that quotes the query's addresses and ports takes [ip]'s word's place on the socket, [udp] holding one pending error a socket, the latest: the query is then waited out and not let go, which costs the early end and never an answer. Owner: the slice in which datagram senders wait on the hop (`Node::transmit` is the node's `lib.rs`). Exit: the node carries its lookups on after the stack's transmit, or [udp] refuses in the call a datagram whose next hop [ip] holds failed, so that no report waits on a socket to be displaced, and that test finds its lookup ended at the 4 s it started. - [ip]'s queue for a next hop it is resolving is every sender's: `PENDING_PER_NEIGHBOUR` is 16, a stopgap which the node's resolver asserts is no fewer than `toyos_dns::MAX_LOOKUPS`, so every held lookup's first query waits there whole, but a datagram a client sends to the same next hop in that window, which for everything off the link is the router, takes the oldest one's place (`nb.pending-overflow`, RFC 4861 §7.2.2), and a query lost so costs its lookup a `WAIT_MS`; smoltcp kept each datagram in its own socket until the neighbour answered. `a_clients_datagram_behind_the_burst_takes_the_oldest_querys_place_in_ips_queue` holds it as it stands. It is a regression against what ships for more than 16 datagrams at one next hop not yet resolved, and would ship at the move; and it sizes a limit of [ip]'s by a constant of the resolver's, which leaves four unresolved next hops filling `PENDING_TOTAL` where eight did. Owner: the slice in which datagram senders wait on the hop, where [udp]'s datagram calls already go through `Stack` and `Shard`. Its design: [udp]'s `serve` asks [ip] the next-hop question before it hands a datagram over, as [tcp]'s exit does with `Hop` (NUD-25, IP-D5); a socket whose oldest datagram's hop is pending keeps it in its own queue, which `TX_DATAGRAMS` and `TX_BYTES` already bound and refuse in the call, and is offered again when the hop resolves and told when it fails; [ip] then queues only its own datagrams. Exit, before the move: that test finds every query on the wire once the resolver answers ARP; `PENDING_PER_NEIGHBOUR` is `limits.ip.nud.pending_per_neighbour`'s 8 again; and the resolver's compile-time assertion, the change to `s_ip_nud_004_overflow_drops_the_oldest` and stage 3's departure line about it are deleted. - `toyos_dns::Lookup::on_datagram` still takes a reply's source address and port, which the node's connected sockets have already matched and the node hands it from its own record of the query: the two parameters are read only by netd's resolver on smoltcp. Exit, at the move: they go with that resolver. +- An accept's answer on the pipe ABI carries what its connection holds (`TcpOptions`, `toyos/src/net.rs`), and the node's `Accepted` does not: `Node::accept` has the connection's options in hand and files them with the stream, so netd reads them back with `Node::nodelay` of the id it was just answered, whose `None`, a stream the accept's own pass let go, the answer has no word for. Exit, at the move: `Accepted` carries its stream's options, or the move's body says which answer a `None` there gets and a test of the node finds it. - `node.address-refused` has no test: `toyos-dhcp` accepts no address or prefix [ip] refuses, by the same `toyos-net-wire` checks in both, so the refusal cannot be reached from the wire. Exit: the client hands [ip] a type that carries the check, and the counter goes. -The listener defects are this track's: `issues/a-handshake-nobody-finishes-holds-a-listeners-port-shut.md` and `issues/a-connect-between-two-accepts-is-reset.md`, on smoltcp until stage 5, and `issues/an-accept-that-never-reaches-netstack-strands-its-listener.md`, in std's accept; and `issues/a-listening-sockets-tcp-nodelay-is-refused-and-an-accepted-streams-is-answered-from-a-guess.md`, whose exit waits on the pipe ABI giving a listener its option, which the node has (`Node::set_listener_nodelay`) and nothing ships. +The listener defects are this track's: `issues/a-handshake-nobody-finishes-holds-a-listeners-port-shut.md` and `issues/a-connect-between-two-accepts-is-reset.md`, on smoltcp until stage 5, and `issues/an-accept-that-never-reaches-netstack-strands-its-listener.md`, in std's accept; and `issues/a-listeners-tcp-nodelay-has-no-setter-in-std-or-the-forks-and-a-c-program-cannot-order-the-waiting-case.md`: the pipe ABI gives a listener its option (`MsgType::TcpListenerSetOption`) and an accept's answer its connection's (`TcpOptions`, `toyos/src/net.rs`), netd on smoltcp answers both, and the move maps the first to `Node::set_listener_nodelay`; what is left of its exit is libc's `poll` and the socket2 and mio forks. Owed from the stage 3 specifications: by stage 5, the scenario for netd's mapping of UDP's refusals onto the pipe ABI, and `dhcp.renew-unroutable`, which netd counts where `toyos-net-udp` refuses the renewal `udp.no-route`; by IP hardening, the scenarios for fragment reassembly and path MTU discovery. diff --git a/sdk/std/sys/net/connection.rs b/sdk/std/sys/net/connection.rs index c593ca3d3d6..c0ab4d82cfe 100644 --- a/sdk/std/sys/net/connection.rs +++ b/sdk/std/sys/net/connection.rs @@ -129,12 +129,17 @@ pub struct TcpStream { local_port: u16, read_timeout_ms: AtomicU32, write_timeout_ms: AtomicU32, - nodelay: AtomicBool, + /// What netstack holds for the connection every duplicate names, locked + /// across the request that changes it so the two cannot come to differ. + nodelay: Arc>, nonblocking: AtomicBool, } impl TcpStream { - fn new(fd: OwnedFd, id: TcpSocketId, peer: SocketAddr, local_port: u16) -> TcpStream { + /// A stream on a connection netstack holds with `nodelay`: off for one a + /// connect made, and what its accept answered for one a listener was + /// handed. + fn new(fd: OwnedFd, id: TcpSocketId, peer: SocketAddr, local_port: u16, nodelay: bool) -> TcpStream { TcpStream { fd, socket: Arc::new(NetstackSocket::Tcp(id)), @@ -144,7 +149,7 @@ impl TcpStream { local_port, read_timeout_ms: AtomicU32::new(0), write_timeout_ms: AtomicU32::new(0), - nodelay: AtomicBool::new(false), + nodelay: Arc::new(crate::sync::Mutex::new(nodelay)), nonblocking: AtomicBool::new(false), } } @@ -165,7 +170,7 @@ impl TcpStream { let conn = toyos::net::tcp_connect(ip, port, duration_to_ms(Some(timeout))) .map_err(net_err_to_io)?; let fd = make_socket_fd(conn.rx, conn.tx)?; - Ok(TcpStream::new(fd, conn.socket_id, *addr, conn.local_port)) + Ok(TcpStream::new(fd, conn.socket_id, *addr, conn.local_port, false)) } fn raw_handle(&self) -> RawHandle { @@ -308,7 +313,7 @@ impl TcpStream { local_port: self.local_port, read_timeout_ms: AtomicU32::new(self.read_timeout_ms.load(Relaxed)), write_timeout_ms: AtomicU32::new(self.write_timeout_ms.load(Relaxed)), - nodelay: AtomicBool::new(self.nodelay.load(Relaxed)), + nodelay: Arc::clone(&self.nodelay), nonblocking: AtomicBool::new(self.nonblocking.load(Relaxed)), }) } @@ -322,14 +327,15 @@ impl TcpStream { } pub fn set_nodelay(&self, nodelay: bool) -> io::Result<()> { + let mut held = self.nodelay.lock().unwrap(); toyos::net::tcp_set_option(self.socket_id(), toyos::net::OPT_NODELAY, nodelay as u32) .map_err(net_err_to_io)?; - self.nodelay.store(nodelay, Relaxed); + *held = nodelay; Ok(()) } pub fn nodelay(&self) -> io::Result { - Ok(self.nodelay.load(Relaxed)) + Ok(*self.nodelay.lock().unwrap()) } pub fn set_keepalive(&self, _keepalive: bool) -> io::Result<()> { @@ -427,7 +433,8 @@ impl TcpListener { Ipv4Addr::from(accepted.remote_addr), accepted.remote_port, )); - Ok((TcpStream::new(fd, accepted.socket_id, peer, accepted.local_port), peer)) + let nodelay = accepted.options.nodelay(); + Ok((TcpStream::new(fd, accepted.socket_id, peer, accepted.local_port, nodelay), peer)) } pub fn duplicate(&self) -> io::Result { diff --git a/tests/netcase/nodelay_kept.c b/tests/netcase/nodelay_kept.c index da71e9f4090..c13226b40f2 100644 --- a/tests/netcase/nodelay_kept.c +++ b/tests/netcase/nodelay_kept.c @@ -1,13 +1,12 @@ /* A TCP_NODELAY set on a stream socket before connect is kept and holds for - the connection, and one cleared stays cleared. One set before bind is the - listener's, and each connection it accepts begins with it. A listener's set - after bind is refused: netstack has no option for a listener, where a host - takes the set and gives it to the connections that begin afterwards. argv: - the address and the port of a host that accepts and holds each connection, - then the two ports to listen on, which that host dials once WAITING is - said. */ + the connection, and one cleared stays cleared. A listener's is the option + of each connection that begins while it holds it, set before bind or after: + a connection accepted before a set or a clear keeps what it has, and the + next one dialled has what the listener holds then. argv: the address and + the port of a host that accepts and holds each connection, then the two + ports to listen on, which that host dials once at WAITING and once more at + AGAIN. */ #include -#include #include #include #include @@ -16,6 +15,7 @@ #include #define WAITING "nodelay_kept: both listeners wait for a peer" +#define AGAIN "nodelay_kept: both listeners wait for a second peer" static int wrong; @@ -72,6 +72,7 @@ int main(int argc, char **argv) { any.sin_port = htons((uint16_t)atoi(argv[4])); said("bind a fourth socket", bind(l, (struct sockaddr *)&any, sizeof any), 0); said("listen on it", listen(l, 1), 0); + said("read from that listener", nodelay(l), 0); printf("%s\n", WAITING); fflush(stdout); @@ -82,10 +83,21 @@ int main(int argc, char **argv) { said("accept from the listener that held none", without >= 0, 1); said("read from its connection", nodelay(without), 0); - errno = 0; - said("set on the listener", setsockopt(l, IPPROTO_TCP, TCP_NODELAY, &on, sizeof on), -1); - said("which is refused as not connected", errno == ENOTCONN, 1); - said("read from the listener", nodelay(l), 0); + said("set on the listener that held none", setsockopt(l, IPPROTO_TCP, TCP_NODELAY, &on, sizeof on), 0); + said("read from the listener", nodelay(l), 1); + said("read from the connection it accepted before", nodelay(without), 0); + said("cleared on the listener that held the option", setsockopt(held, IPPROTO_TCP, TCP_NODELAY, &off, sizeof off), 0); + said("read from the listener", nodelay(held), 0); + said("read from the connection it accepted before", nodelay(with), 1); + + printf("%s\n", AGAIN); + fflush(stdout); + int set = accept(l, NULL, NULL); + said("accept from the listener after its set", set >= 0, 1); + said("read from its connection", nodelay(set), 1); + int cleared = accept(held, NULL, NULL); + said("accept from the listener after its clear", cleared >= 0, 1); + said("read from its connection", nodelay(cleared), 0); if (wrong) { printf("nodelay_kept: %d wrong\n", wrong); diff --git a/tests/toyos-rust-tests/src/bin/nodelay_accepted.rs b/tests/toyos-rust-tests/src/bin/nodelay_accepted.rs new file mode 100644 index 00000000000..65fa32dba87 --- /dev/null +++ b/tests/toyos-rust-tests/src/bin/nodelay_accepted.rs @@ -0,0 +1,103 @@ +//! `TCP_NODELAY` as std answers it and as netstack's pipe ABI carries a +//! listener's. +//! +//! The first half is std's alone and prints what any host prints: a stream +//! and its duplicate are one socket, so a set on either is read on both, and +//! a stream accepted from a listener that holds no option has none. +//! +//! The second is ToyOS's: a listener made by `toyos::net`, whose option std +//! has no call for. A connection that waited when the option was set has not +//! got it, and the next one dialled has; the wake netstack writes the owner +//! is what says the first one waits. +//! +//! argv: the address and port of a host that accepts and holds each +//! connection, then the port std listens on and the port the pipe ABI's +//! listener does. That host dials each once at WAITING, and the second again +//! at AGAIN. + +use std::net::{TcpListener, TcpStream}; + +const WAITING: &str = "nodelay_accepted: both listeners wait for a peer"; +#[cfg(target_os = "toyos")] +const AGAIN: &str = "nodelay_accepted: the listener waits for a second peer"; + +struct Said(u32); + +impl Said { + fn said(&mut self, what: &str, got: T, want: T) { + let wrong = got != want; + println!("{what}: {got:?}{}", if wrong { " <-- WRONG" } else { "" }); + self.0 += u32::from(wrong); + } +} + +/// The listener `toyos::net` binds on `port`: one connection that waits before +/// the option is set, and one dialled after. +#[cfg(target_os = "toyos")] +fn the_pipe_abis_listener(port: u16, said: &mut Said) { + use toyos::net::{self, NetError, OPT_NODELAY}; + + let listener = net::tcp_bind([0; 4], port).unwrap_or_else(|e| panic!("a listener on {port}: {e:?}")); + println!("{WAITING}"); + let mut wake = [0u8; 1]; + assert_eq!(listener.notify.read(&mut wake), Ok(1), "the wake for the first connection"); + said.said("set on a listener a connection waits at", net::tcp_listener_set_option(listener.socket_id, OPT_NODELAY, 1), Ok(())); + let before = net::tcp_accept(listener.socket_id).unwrap_or_else(|e| panic!("the first accept: {e:?}")); + said.said("the connection that waited before the set", before.options.nodelay(), false); + said.said( + "a listener's request that names a stream", + net::tcp_listener_set_option(before.socket_id, OPT_NODELAY, 1), + Err(NetError::NotConnected), + ); + said.said( + "a stream's request that names a listener", + net::tcp_set_option(listener.socket_id, OPT_NODELAY, 1), + Err(NetError::NotConnected), + ); + said.said( + "an option a listener has none of", + net::tcp_listener_set_option(listener.socket_id, net::OPT_BROADCAST, 1), + Err(NetError::InvalidInput), + ); + println!("{AGAIN}"); + assert_eq!(listener.notify.read(&mut wake), Ok(1), "the wake for the second connection"); + let after = net::tcp_accept(listener.socket_id).unwrap_or_else(|e| panic!("the second accept: {e:?}")); + said.said("the connection dialled after the set", after.options.nodelay(), true); + for id in [before.socket_id, after.socket_id, listener.socket_id] { + net::tcp_close(id).unwrap_or_else(|e| panic!("closing {id:?}: {e:?}")); + } +} + +fn main() { + let args: Vec = std::env::args().collect(); + let [_, host, holding, std_port, pipe_port] = &args[..] else { + panic!("usage: nodelay_accepted ") + }; + let port = |text: &String| -> u16 { text.parse().unwrap_or_else(|e| panic!("the port {text}: {e}")) }; + let mut said = Said(0); + + let stream = TcpStream::connect((host.as_str(), port(holding))).expect("the holding server"); + let duplicate = stream.try_clone().expect("a duplicate"); + said.said("a fresh stream's nodelay", stream.nodelay().ok(), Some(false)); + said.said("set on the stream", stream.set_nodelay(true).is_ok(), true); + said.said("read from its duplicate", duplicate.nodelay().ok(), Some(true)); + said.said("cleared on the duplicate", duplicate.set_nodelay(false).is_ok(), true); + said.said("read from the stream", stream.nodelay().ok(), Some(false)); + + let listener = TcpListener::bind(("0.0.0.0", port(std_port))).expect("std's listener"); + #[cfg(target_os = "toyos")] + the_pipe_abis_listener(port(pipe_port), &mut said); + #[cfg(not(target_os = "toyos"))] + { + let _ = pipe_port; + println!("{WAITING}"); + } + let (accepted, _) = listener.accept().expect("std's accept"); + said.said("a stream accepted from a listener that holds none", accepted.nodelay().ok(), Some(false)); + + if said.0 != 0 { + println!("nodelay_accepted: {} wrong", said.0); + std::process::exit(1); + } + println!("nodelay_accepted: ok"); +} diff --git a/tests/toyos.rs b/tests/toyos.rs index e67da326128..7db775abb22 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -134,6 +134,9 @@ const RUST_SKIP: &[&str] = &[ // It needs a NIC in front of netstack and a host server behind it: // `netstack_socket_churn` runs it on `tests/netcase`. "netstack_socket_churn", + // It needs a host that dials its listeners when it says they wait: + // `libc_sockets` runs it on `tests/netcase`. + "nodelay_accepted", // It asserts nothing at all: it holds `dump_nmi_probe`'s boot open for // twenty seconds. On a shared boot it would be twenty seconds of nothing. "lan_hold", @@ -261,9 +264,9 @@ const MACHINE_TESTS: &[&str] = &[ // connections: netstack is one binary that owns its NIC, with no host // build, and the T14's peer is the bench's network. "netstack_socket_churn", - // What libc's socket calls ask of netstack, read back from a peer that - // answers: the calls are libc's requests on netstack's port, netstack has - // no host build, and the T14's peer is the bench's network. + // What libc's and std's socket calls ask of netstack, read back from a + // peer that answers: the calls are requests on netstack's port, netstack + // has no host build, and the T14's peer is the bench's network. "libc_sockets", // The nested-NMI report is a raw write to the 16550, which the T14 does not // have. @@ -2976,7 +2979,7 @@ fn boot_netcase( /// Have the guest's network carry what dials a loopback port of QEMU's /// choosing to each of `guest_ports`: those host ports, in that order. -fn forwards_into(qemu: &QemuInstance, guest_ports: [u16; 2]) -> Result<[u16; 2], String> { +fn forwards_into(qemu: &QemuInstance, guest_ports: [u16; N]) -> Result<[u16; N], String> { const FORWARD: &str = "TCP[HOST_FORWARD]"; let mut monitor = qemu::QmpMonitor::open(qemu.qmp_socket()); for port in guest_ports { @@ -2989,7 +2992,7 @@ fn forwards_into(qemu: &QemuInstance, guest_ports: [u16; 2]) -> Result<[u16; 2], // A row is the protocol, a descriptor, the host's address and port, then // the guest's. let table = monitor.human("info usernet"); - let mut host_ports = [0u16; 2]; + let mut host_ports = [0u16; N]; for (host, guest) in host_ports.iter_mut().zip(guest_ports) { *host = table .lines() @@ -3001,19 +3004,18 @@ fn forwards_into(qemu: &QemuInstance, guest_ports: [u16; 2]) -> Result<[u16; 2], Ok(host_ports) } -/// libc's sockets as a C program uses them, on one boot of `tests/netcase`: -/// each of its C cases dials a host server that holds what it accepts, or -/// sends to one that answers each datagram with itself, at the address the -/// guest's network gives the host; and once `nodelay_kept` says its two -/// listeners wait, the host dials each through a port QEMU forwards. A case's -/// own comparisons are its verdict. +/// libc's sockets as a C program uses them and std's as a Rust one does, on +/// one boot of `tests/netcase`: each job dials a host server that holds what +/// it accepts, or sends to one that answers each datagram with itself, at the +/// address the guest's network gives the host; and each time a job says its +/// listeners wait, the host dials them through the ports QEMU forwards. A +/// job's own comparisons are its verdict. fn libc_sockets() -> Result<(), String> { const HOST: &str = "10.0.2.2"; - /// The ports `nodelay_kept` listens on in the guest, which nothing else - /// on its boot binds. - const LISTENERS: [u16; 2] = [7001, 7002]; - /// `nodelay_kept.c`'s `WAITING`. - const WAITING: &str = "nodelay_kept: both listeners wait for a peer"; + /// The ports `nodelay_kept` and then `nodelay_accepted` listen on in the + /// guest, which nothing else on its boot binds. + const LISTENERS: [u16; 4] = [7001, 7002, 7003, 7004]; + const RUST_JOB: &str = "nodelay_accepted"; let holding = holding_server()?; let echo = std::net::UdpSocket::bind(("127.0.0.1", 0)).map_err(|e| format!("the answering server: {e}"))?; let answering = echo.local_addr().map_err(|e| format!("the answering server's port: {e}"))?.port(); @@ -3025,27 +3027,50 @@ fn libc_sockets() -> Result<(), String> { } }); - // The address first: every case names its peer by one. - let [held, clear] = LISTENERS; - let cases = [ - ("addr_order", holding.to_string()), - ("nodelay_kept", format!("{holding} {held} {clear}")), - ("sendto_unbound", answering.to_string()), - ]; let case = compile::repo_root().join("tests/netcase"); - let bins: Vec<(String, Vec)> = cases - .iter() - .map(|(name, _)| (name.to_string(), compile::link_toyos(&compile::compile_own_c(&case, name), name))) - .collect(); - let mut qemu = boot_netcase(&bins, &[], BootOptions { qmp: true, ..Default::default() })?; - let forwarded = forwards_into(&qemu, LISTENERS)?; - // Held to the test's end: a peer gone before the case's `accept` fails its hand-over. + let c_bins: Vec<(String, Vec)> = ["addr_order", "nodelay_kept", "sendto_unbound"] + .map(|name| (name.to_string(), compile::link_toyos(&compile::compile_own_c(&case, name), name))) + .into(); + let rust_bin = + qemu::build_toyos_bin(qemu::SUITE_ARCH, &compile::repo_root().join("tests/toyos-rust-tests"), RUST_JOB); + let mut qemu = + boot_netcase(&c_bins, &[(RUST_JOB.to_string(), rust_bin)], BootOptions { qmp: true, ..Default::default() })?; + let [held, clear, std_port, pipe_port] = LISTENERS; + let [to_held, to_clear, to_std, to_pipe] = forwards_into(&qemu, LISTENERS)?; + /// Each line a job says its listeners wait in, the constants of its + /// source, and the forwarded ports the host dials when it does. + type Waits<'a> = &'a [(&'a str, &'a [u16])]; + // The address first: every job names its peer by one. + let jobs: [(&str, String, Waits); 4] = [ + ("test_c_addr_order", holding.to_string(), &[]), + ( + "test_c_nodelay_kept", + format!("{holding} {held} {clear}"), + &[ + ("nodelay_kept: both listeners wait for a peer", &[to_held, to_clear]), + ("nodelay_kept: both listeners wait for a second peer", &[to_held, to_clear]), + ], + ), + ("test_c_sendto_unbound", answering.to_string(), &[]), + ( + "test_rs_nodelay_accepted", + format!("{holding} {std_port} {pipe_port}"), + &[ + ("nodelay_accepted: both listeners wait for a peer", &[to_std, to_pipe]), + ("nodelay_accepted: the listener waits for a second peer", &[to_pipe]), + ], + ), + ]; + // Held to the test's end: a peer gone before a job's `accept` is a + // connection its listener no longer holds. let mut dialled = Vec::new(); - for (name, ports) in cases { - let result = - qemu.run_test_hooked(&format!("test_c_{name} {HOST} {ports}"), Duration::from_secs(120), WAITING, |_| { - dialled.extend(forwarded.map(|port| std::net::TcpStream::connect(("127.0.0.1", port)))); - }); + for (job, ports, waits) in jobs { + let name = job.trim_start_matches("test_c_").trim_start_matches("test_rs_"); + let result = qemu.run_test_paced(&format!("{job} {HOST} {ports}"), Duration::from_secs(120), |_, line| { + for (_, dial) in waits.iter().filter(|(waiting, _)| line.trim_end().ends_with(waiting)) { + dialled.extend(dial.iter().map(|&port| std::net::TcpStream::connect(("127.0.0.1", port)))); + } + }); if let Some(Err(e)) = dialled.iter().find(|dial| dial.is_err()) { return Err(format!("{name}: the host could not dial a port QEMU forwards to the guest: {e}")); } @@ -3053,7 +3078,7 @@ fn libc_sockets() -> Result<(), String> { eprintln!(" [libc] {name} ended {:?} and said:\n{}", result.exit_code, result.stdout); } if let Some(why) = &result.error { - return Err(format!("{name}: {why}\nthe case said:\n{}", result.stdout)); + return Err(format!("{name}: {why}\nthe job said:\n{}", result.stdout)); } if result.exit_code != Some(0) { return Err(format!("{name} ended {:?}:\n{}", result.exit_code, result.stdout)); diff --git a/toyos/src/net.rs b/toyos/src/net.rs index b5d991d1533..4f3348f6ee8 100644 --- a/toyos/src/net.rs +++ b/toyos/src/net.rs @@ -19,6 +19,7 @@ pub enum MsgType { UdpClose = 11, DnsLookup = 12, TcpSetOption = 13, + TcpListenerSetOption = 14, UdpSetOption = 15, TcpConnectPiped = 20, TcpBindPiped = 21, @@ -36,6 +37,7 @@ impl MsgType { 11 => Some(Self::UdpClose), 12 => Some(Self::DnsLookup), 13 => Some(Self::TcpSetOption), + 14 => Some(Self::TcpListenerSetOption), 15 => Some(Self::UdpSetOption), 20 => Some(Self::TcpConnectPiped), 21 => Some(Self::TcpBindPiped), @@ -80,7 +82,11 @@ pub const ERR_RESOURCE_EXHAUSTED: u32 = 7; pub const ERR_PERMISSION_DENIED: u32 = 8; pub const ERR_OTHER: u32 = 255; -/// A stream's Nagle algorithm off: [`tcp_set_option`]. +/// Nagle's algorithm off. On a stream, [`tcp_set_option`]. On a listener, +/// [`tcp_listener_set_option`]: for every connection whose SYN arrives from +/// then on, and one that began before keeps what it has. A connection says +/// what it began with in its accept's answer ([`TcpOptions`]); one a connect +/// made begins with the algorithm on. pub const OPT_NODELAY: u32 = 1; /// A datagram socket may send to a broadcast address: [`udp_set_option`]. pub const OPT_BROADCAST: u32 = 2; @@ -185,11 +191,20 @@ ipc_payload! { pub socket_id: u32, } + /// The options a stream holds, a word each: zero is off and anything else + /// on. + #[derive(Debug, PartialEq, Eq)] + pub struct TcpOptions { + nodelay: u32, + } + pub struct TcpAcceptPipedResponse { pub socket_id: u32, pub remote_addr: [u8; 4], pub remote_port: u16, pub local_port: u16, + /// What the connection holds as it is handed over. + pub options: TcpOptions, } pub struct UdpBindRequest { @@ -222,8 +237,9 @@ ipc_payload! { pub len: u16, } - /// [`MsgType::TcpSetOption`] and [`MsgType::UdpSetOption`]: the request's - /// type says which kind of socket `socket_id` names. + /// [`MsgType::TcpSetOption`], [`MsgType::TcpListenerSetOption`] and + /// [`MsgType::UdpSetOption`]: the request's type says which kind of socket + /// `socket_id` names. pub struct SocketOptionRequest { pub socket_id: u32, pub option: u32, @@ -239,6 +255,17 @@ ipc_payload! { } } +impl TcpOptions { + pub fn new(nodelay: bool) -> Self { + Self { nodelay: nodelay as u32 } + } + + /// [`OPT_NODELAY`]. + pub fn nodelay(&self) -> bool { + self.nodelay != 0 + } +} + // Return types pub struct TcpConnection { @@ -261,6 +288,7 @@ pub struct TcpAccepted { pub remote_addr: [u8; 4], pub remote_port: u16, pub local_port: u16, + pub options: TcpOptions, } pub struct UdpBound { @@ -475,6 +503,7 @@ pub fn tcp_accept(socket_id: TcpSocketId) -> Result { remote_addr: resp.remote_addr, remote_port: resp.remote_port, local_port: resp.local_port, + options: resp.options, }) } @@ -496,6 +525,12 @@ pub fn tcp_set_option(socket_id: TcpSocketId, option: u32, value: u32) -> Result .status() } +pub fn tcp_listener_set_option(socket_id: TcpSocketId, option: u32, value: u32) -> Result<(), NetError> { + NetstackConn::connect()? + .request(MsgType::TcpListenerSetOption, &SocketOptionRequest { socket_id: socket_id.0, option, value })? + .status() +} + // UDP client functions pub fn udp_bind(addr: [u8; 4], port: u16) -> Result { @@ -655,4 +690,27 @@ mod tests { Err(IpcError::Malformed) )); } + + /// The accept's answer as a client decodes it: the options last, after + /// the ports, any non-zero word an option that is on, and an answer + /// without them refused. + #[test] + fn an_accepts_answer_ends_in_its_connections_options() { + let mut wire = [0x44, 0x33, 0x22, 0x11, 192, 0, 2, 7, 0x39, 0x30, 0x16, 0, 0, 0, 0, 0]; + let answer: TcpAcceptPipedResponse = crate::ipc::decode_payload(&wire).unwrap(); + assert_eq!(answer.socket_id, 0x1122_3344); + assert_eq!((answer.remote_addr, answer.remote_port, answer.local_port), ([192, 0, 2, 7], 12345, 22)); + assert!(!answer.options.nodelay()); + assert_eq!(answer.options, TcpOptions::new(false)); + wire[12] = 1; + let answer: TcpAcceptPipedResponse = crate::ipc::decode_payload(&wire).unwrap(); + assert_eq!(answer.options, TcpOptions::new(true)); + wire[12..].copy_from_slice(&[0, 0, 0, 0x80]); + let answer: TcpAcceptPipedResponse = crate::ipc::decode_payload(&wire).unwrap(); + assert!(answer.options.nodelay()); + assert!(matches!( + crate::ipc::decode_payload::(&wire[..12]), + Err(IpcError::Malformed) + )); + } } diff --git a/userland/libc/src/socket.rs b/userland/libc/src/socket.rs index 4cbc660eeec..f9f8f237379 100644 --- a/userland/libc/src/socket.rs +++ b/userland/libc/src/socket.rs @@ -61,10 +61,11 @@ struct SocketEntry { rx_fd: i32, // read end of rx pipe (netstack→client) tx_fd: i32, // write end of tx pipe (client→netstack) notify_fd: i32, // read end of listener notify pipe - // What netstack holds for the socket, which `getsockopt` answers from. A - // socket netstack does not hold yet keeps each for the call that makes it - // there to hand over: `broadcast` for a datagram socket's bind, `nodelay` - // for a stream's `connect` and for each connection a listener accepts. + // What netstack holds for the socket, which `getsockopt` answers from: set + // by a request it answered, or read from the accept's answer. A socket + // netstack does not hold yet keeps each for the call that makes it there + // to hand over: `broadcast` and a listener's `nodelay` for `bind`, a + // stream's `nodelay` for `connect`. nodelay: bool, broadcast: bool, } @@ -252,6 +253,14 @@ pub unsafe extern "C" fn bind(fd: i32, addr: *const Sockaddr, addrlen: SocklenT) Ok(b) => b, Err(e) => { set_errno(net_err_to_errno(e)); return -1; } }; + if entry.nodelay { + if let Err(e) = toyos::net::tcp_listener_set_option(bound.socket_id, OPT_NODELAY, 1) { + // `bound`'s pipe end closes where it drops. + let _ = toyos::net::tcp_close(bound.socket_id); + set_errno(net_err_to_errno(e)); + return -1; + } + } entry.netstack_id = bound.socket_id.0; entry.local_port = bound.bound_port; entry.bound = true; @@ -290,9 +299,6 @@ pub unsafe extern "C" fn accept( }; let listener_id = TcpSocketId(entry.netstack_id); let notify_fd = entry.notify_fd; - // A set after `bind` is refused, so this is the option the listener held - // when any connection to it began. - let nodelay = entry.nodelay; // Block until a connection arrives (read 1 byte from notify pipe) let mut notify_byte = [0u8; 1]; @@ -302,14 +308,6 @@ pub unsafe extern "C" fn accept( Ok(a) => a, Err(e) => { set_errno(net_err_to_errno(e)); return -1; } }; - if nodelay { - if let Err(e) = toyos::net::tcp_set_option(accepted.socket_id, OPT_NODELAY, 1) { - // `accepted`'s pipe ends close where it drops. - let _ = toyos::net::tcp_close(accepted.socket_id); - set_errno(net_err_to_errno(e)); - return -1; - } - } if !addr.is_null() && !addrlen.is_null() { fill_sockaddr(addr, addrlen, accepted.remote_addr, accepted.remote_port); @@ -326,7 +324,7 @@ pub unsafe extern "C" fn accept( rx_fd: accepted.rx.into_raw().0 as i32, tx_fd: accepted.tx.into_raw().0 as i32, notify_fd: 0, - nodelay, + nodelay: accepted.options.nodelay(), broadcast: false, }; let new_fd = alloc_socket(new_entry); @@ -596,14 +594,17 @@ pub unsafe extern "C" fn setsockopt( }; match option { Kept::NoDelay => { - // netstack holds a connection from `connect` or `accept`, and a - // listener from `bind`, whose id names no connection: it refuses - // a listener's set, and nothing is kept for one. - if entry.netstack_id != 0 { - if let Err(e) = toyos::net::tcp_set_option(TcpSocketId(entry.netstack_id), OPT_NODELAY, on as u32) { - set_errno(net_err_to_errno(e)); - return -1; - } + // netstack holds a listener from `bind` and a connection from + // `connect` or `accept`, and each has its own request. + let id = TcpSocketId(entry.netstack_id); + let asked = match (entry.connected, entry.bound) { + (true, _) => toyos::net::tcp_set_option(id, OPT_NODELAY, on as u32), + (false, true) => toyos::net::tcp_listener_set_option(id, OPT_NODELAY, on as u32), + (false, false) => Ok(()), + }; + if let Err(e) = asked { + set_errno(net_err_to_errno(e)); + return -1; } entry.nodelay = on; } diff --git a/userland/netstack/src/listen.rs b/userland/netstack/src/listen.rs index f4ba715cd8a..9d374ed08e2 100644 --- a/userland/netstack/src/listen.rs +++ b/userland/netstack/src/listen.rs @@ -1,4 +1,5 @@ -//! Whether a listener's owner is owed a wake, and what its accept finds. +//! Whether a listener's owner is owed a wake, what its accept finds, and the +//! option its connections begin with. //! //! **A listener is one smoltcp socket that becomes the connection it //! accepts**, so the port listens only while that socket is in `Listen`: one @@ -10,14 +11,21 @@ //! take.** An owner refused holds no wake, so the connection it left is //! announced again, and an owner refused for room is not woken until room //! returns. +//! +//! **A connection begins with the option its listener held when its SYN +//! arrived**, which is what a host gives it: the socket holds the listener's +//! option from the moment it listens, a set reaches it only while it still +//! does, and what it holds once it is a connection is that connection's. use smoltcp::socket::tcp; -/// A listener's port, and whether its owner holds a wake it has not spent on -/// an accept. +/// A listener's port, whether its owner holds a wake it has not spent on an +/// accept, and whether Nagle's algorithm is off for the connections that begin +/// from here on. pub struct Listening { port: u16, woken: bool, + nodelay: bool, } /// What an accept finds, handed the pipes `P` its request carried. @@ -35,19 +43,35 @@ pub enum Accept

{ impl Listening { pub fn new(port: u16) -> Self { - Self { port, woken: false } + Self { port, woken: false, nodelay: false } } pub fn port(&self) -> u16 { self.port } + /// Has a closed `socket` listen on the port, with the listener's option. + pub fn open(&self, socket: &mut tcp::Socket) { + let port = self.port; + socket.listen(port).unwrap_or_else(|e| panic!("netstack: a closed socket refused to listen on {port}: {e:?}")); + socket.set_nagle_enabled(!self.nodelay); + } + + /// The listener's option from here on. `socket` takes it only while no + /// SYN has reached it. + pub fn set_nodelay(&mut self, socket: &mut tcp::Socket, nodelay: bool) { + self.nodelay = nodelay; + if socket.state() == tcp::State::Listen { + socket.set_nagle_enabled(!nodelay); + } + } + /// The bytes to write the owner for `socket`: one wake if a connection /// waits, there is `room` to take it, and the owner holds no wake, and /// none otherwise. The wake is held from here on, so the caller ends the /// listener if the owner is not handed it. pub fn wake(&mut self, socket: &mut tcp::Socket, room: bool) -> &'static [u8] { - let owed = settle(socket, self.port) && room && !self.woken; + let owed = self.settle(socket) && room && !self.woken; self.woken |= owed; if owed { &[1] } else { &[] } } @@ -56,31 +80,29 @@ impl Listening { /// its request carried. pub fn accept

(&mut self, socket: &mut tcp::Socket, room: bool, pipes: Option

) -> Accept

{ self.woken = false; - match (settle(socket, self.port), room, pipes) { + match (self.settle(socket), room, pipes) { (_, _, None) => Accept::NoPipes, (false, _, Some(_)) => Accept::Nothing, (true, false, Some(_)) => Accept::NoRoom, (true, true, Some(pipes)) => Accept::Take(pipes), } } -} -/// Puts `socket` back to listening on `port` if its peer reset it before its -/// owner took it, and says whether it holds a connection: a handshake -/// finished, whatever the peer did since. Its FIN included, which can land in -/// the same pass as the handshake's last ACK, so no pass ever sees the socket -/// `Established`. -fn settle(socket: &mut tcp::Socket, port: u16) -> bool { - match socket.state() { - tcp::State::Listen | tcp::State::SynReceived => false, - tcp::State::Established | tcp::State::CloseWait => true, - tcp::State::Closed => { - socket - .listen(port) - .unwrap_or_else(|e| panic!("netstack: a closed socket refused to listen on {port}: {e:?}")); - false + /// Puts `socket` back to listening if its peer reset it before its owner + /// took it, and says whether it holds a connection: a handshake finished, + /// whatever the peer did since. Its FIN included, which can land in the + /// same pass as the handshake's last ACK, so no pass ever sees the socket + /// `Established`. + fn settle(&self, socket: &mut tcp::Socket) -> bool { + match socket.state() { + tcp::State::Listen | tcp::State::SynReceived => false, + tcp::State::Established | tcp::State::CloseWait => true, + tcp::State::Closed => { + self.open(socket); + false + } + other => panic!("netstack: a listener's socket is {other:?}, which only netstack closing or connecting it reaches"), } - other => panic!("netstack: a listener's socket is {other:?}, which only netstack closing or connecting it reaches"), } } diff --git a/userland/netstack/src/listen/tests.rs b/userland/netstack/src/listen/tests.rs index da0179aac32..7c12c482bb0 100644 --- a/userland/netstack/src/listen/tests.rs +++ b/userland/netstack/src/listen/tests.rs @@ -521,3 +521,65 @@ fn a_peer_that_answers_and_never_closes_is_reset_at_the_ceiling() { assert_eq!(at_the_ceiling(&mut net, answer), Ownerless::Cut, "the slot of a connection with no client never comes back"); the_cut_is_said(&mut net); } + +/// Whether the socket holds `TCP_NODELAY`, which is what its accept answers. +fn nodelay(net: &mut Net) -> bool { + !net.socket().nagle_enabled() +} + +/// The owner's set of `TCP_NODELAY` on its listener. +fn set_nodelay(net: &mut Net, on: bool) { + net.listening.set_nodelay(net.sockets.get_mut::(net.listener), on); +} + +/// **A connection has the option its listener held when its SYN arrived**: one +/// set while the socket still listens is the connection's, and one cleared +/// after its SYN, whether or not its handshake has finished, does not reach +/// it. +#[test] +fn a_connection_begins_with_the_option_its_listener_held() { + let mut net = Net::new(); + assert!(!nodelay(&mut net), "a fresh listener holds the option"); + set_nodelay(&mut net, true); + let isn = net.syn(5001); + set_nodelay(&mut net, false); + assert!(nodelay(&mut net), "a clear after the SYN reached a handshake in progress"); + net.send(5001, TcpControl::None, PEER_ISN + 1, Some(isn + 1)); + net.pass(); + set_nodelay(&mut net, false); + assert_eq!(net.accept(true, true), Accept::Take(())); + assert!(nodelay(&mut net), "a clear after the SYN reached a connection that waits"); +} + +/// **A set after a connection began is the next connection's**: the waiting +/// one keeps what it has, and the socket that listens in its place holds the +/// listener's option. +#[test] +fn a_set_after_a_connection_began_is_the_next_connections() { + let mut net = Net::new(); + established(&mut net, 5001); + set_nodelay(&mut net, true); + assert_eq!(net.accept(true, true), Accept::Take(())); + assert!(!nodelay(&mut net), "a set after the connection was established reached it"); + let mut next = tcp::Socket::new(tcp::SocketBuffer::new(vec![0; 4096]), tcp::SocketBuffer::new(vec![0; 4096])); + net.listening.open(&mut next); + assert_eq!(next.state(), tcp::State::Listen); + assert!(!next.nagle_enabled(), "the socket that listens next does not hold the listener's option"); +} + +/// **A socket its peer reset before its owner took it listens again with the +/// listener's option as it is then**, and not as it was when the reset +/// connection's SYN arrived. +#[test] +fn a_reset_connections_socket_listens_again_with_the_listeners_option() { + let mut net = Net::new(); + let isn = net.syn(5001); + set_nodelay(&mut net, true); + net.ack_and(5001, isn, TcpControl::Rst); + assert_eq!(net.socket().state(), tcp::State::Closed, "the premise: reset before it was taken"); + assert!(!nodelay(&mut net), "the premise: the reset connection began without the option"); + assert!(!net.wakes(true)); + established(&mut net, 5002); + assert_eq!(net.accept(true, true), Accept::Take(())); + assert!(nodelay(&mut net), "the connection after a reset one began without its listener's option"); +} diff --git a/userland/netstack/src/main.rs b/userland/netstack/src/main.rs index bb763e574e1..b8f90f73085 100644 --- a/userland/netstack/src/main.rs +++ b/userland/netstack/src/main.rs @@ -582,6 +582,7 @@ impl Netstack { Some(MsgType::UdpClose) => self.handle_udp_close(&req, socket_set), Some(MsgType::DnsLookup) => self.handle_dns_lookup(req, socket_set), Some(MsgType::TcpSetOption) => self.handle_tcp_set_option(&req, socket_set), + Some(MsgType::TcpListenerSetOption) => self.handle_tcp_listener_set_option(&req, socket_set), Some(MsgType::UdpSetOption) => self.handle_udp_set_option(&req), Some(MsgType::TcpConnectPiped) => self.handle_tcp_connect_piped(req, socket_set, iface), Some(MsgType::TcpBindPiped) => self.handle_tcp_bind_piped(&req, socket_set), @@ -870,6 +871,24 @@ impl Netstack { } } + fn handle_tcp_listener_set_option(&mut self, msg: &Request, socket_set: &mut SocketSet<'_>) { + let Ok(req) = ipc::decode_payload::(msg.payload()) else { + msg.client.error(ERR_INVALID_INPUT); + return; + }; + let Some(listener) = self.piped_listeners.get_mut(&req.socket_id) else { + msg.client.error(ERR_NOT_CONNECTED); + return; + }; + match req.option { + OPT_NODELAY => { + listener.listening.set_nodelay(socket_set.get_mut::(listener.handle), req.value != 0); + msg.client.done(); + } + _ => msg.client.error(ERR_INVALID_INPUT), + } + } + /// smoltcp sends to a broadcast address for every socket, so the /// permission has nothing here to switch. fn handle_udp_set_option(&self, msg: &Request) { @@ -1043,7 +1062,9 @@ impl Netstack { } }; - let remote = socket_set.get_mut::(old_handle).remote_endpoint().unwrap(); + let accepted = socket_set.get_mut::(old_handle); + let remote = accepted.remote_endpoint().unwrap(); + let options = TcpOptions::new(!accepted.nagle_enabled()); let remote_addr = match remote.addr { IpAddress::Ipv4(a) => a.octets(), }; @@ -1057,24 +1078,20 @@ impl Netstack { let rx_buf = tcp::SocketBuffer::new(vec![0u8; TCP_SOCKET_BUFFER]); let tx_buf = tcp::SocketBuffer::new(vec![0u8; TCP_SOCKET_BUFFER]); let mut new_listener = tcp::Socket::new(rx_buf, tx_buf); - // A fresh socket on the port an established one holds: neither refusal - // `listen` has (port 0, a socket not closed) can be this one. - new_listener - .listen(local_port) - .unwrap_or_else(|e| panic!("netstack: a fresh socket refused to listen on {local_port}: {e:?}")); - let new_handle = socket_set.add(new_listener); - self.sockets.insert(req.socket_id, SocketKind::TcpListener(new_handle)); - - self.piped_listeners + let listener = self + .piped_listeners .get_mut(&req.socket_id) - .expect("looked up above; nothing between there and here removes a piped_listeners entry") - .handle = new_handle; + .expect("looked up above; nothing between there and here removes a piped_listeners entry"); + listener.listening.open(&mut new_listener); + listener.handle = socket_set.add(new_listener); + self.sockets.insert(req.socket_id, SocketKind::TcpListener(listener.handle)); msg.client.result(&TcpAcceptPipedResponse { socket_id: stream_id, remote_addr, remote_port: remote.port, local_port, + options, }); } From 8bb866cb0a55e09f732676d9ac3d03505900e8a8 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 9 Oct 2026 10:18:59 +0200 Subject: [PATCH 2/2] A bind's request carries its listener's options, the node's listen takes them and its accept answers its stream's The round-1 review of the listener's-option stage found three defects and this answers them. A bind's request ends in the TcpOptions an accept's answer ends in, the shell's Listening is made with them and opens its first socket with them before the socket is in the set, and libc's bind names its socket's TCP_NODELAY there: the second request libc sent after the bind's answer, and its close-and-fail unwind, are gone, and with them the frame in which a SYN could begin a connection without an option set before bind. toyos::net::tcp_bind keeps its signature, which the socket2 and mio forks call, and stands on tcp_bind_with; the renamed issue's exit asks for one call once the forks pass their own. The node's listen takes the listener's nodelay, written to [tcp] in the call that made the passive open, and Accepted carries the option its stream was handed over with, which the node had in hand in take and dropped. Three tests of the node: a listener holds what its listen named from its first connection; a handshake reset before it ends leaves the next connection the option its listener holds when its own SYN arrives, both ways round; and an accept whose own pass lets its stream go still answers the stream's option, where Node::nodelay of its id answers None. The track's node line is closed by them. The three tests added on a smoltcp socket are deleted: no new test is built on the stack being replaced. The shell's rule differs from the host's for the connection after a handshake its peer reset, since smoltcp puts a socket reset in SynReceived straight back to Listen with the Nagle switch it had; that is filed, read from the two sources and not run, with the move as its exit, and the shell's module header says it. run_test_hooked lost its last caller with a hook and is deleted; nodelay_accepted's arm for other hosts, which nothing in the tree built, is deleted; libc's accept writing its caller's address before alloc_socket can fail is filed; the renamed issue's Linux paragraph is struck, the host check's log of #777 on ubuntu-24.04 being that reading. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- ...hands-its-option-to-the-next-connection.md | 48 ++++++++ ...c-program-cannot-order-the-waiting-case.md | 44 ++++---- ...allers-address-before-it-can-still-fail.md | 25 +++++ ...-a-streams-tcp-nodelay-reached-netstack.md | 5 +- ...s-ceiling-verdict-serve-only-debugs-run.md | 9 +- issues/toyos-has-its-own-network-stack.md | 3 +- tests/common/qemu.rs | 34 +----- .../src/bin/netstack_gone_mid_bind.rs | 4 +- .../src/bin/nodelay_accepted.rs | 14 +-- toyos/src/net.rs | 42 +++++-- userland/libc/src/socket.rs | 19 +--- userland/netstack/node/src/listeners.rs | 24 ++-- userland/netstack/node/tests/lease.rs | 2 +- userland/netstack/node/tests/listeners.rs | 104 ++++++++++++++++-- userland/netstack/src/listen.rs | 18 ++- userland/netstack/src/listen/tests.rs | 64 +---------- userland/netstack/src/main.rs | 15 +-- 17 files changed, 288 insertions(+), 186 deletions(-) create mode 100644 issues/a-handshake-reset-before-it-ends-hands-its-option-to-the-next-connection.md create mode 100644 issues/libc-accept-writes-its-callers-address-before-it-can-still-fail.md diff --git a/issues/a-handshake-reset-before-it-ends-hands-its-option-to-the-next-connection.md b/issues/a-handshake-reset-before-it-ends-hands-its-option-to-the-next-connection.md new file mode 100644 index 00000000000..17e83abab58 --- /dev/null +++ b/issues/a-handshake-reset-before-it-ends-hands-its-option-to-the-next-connection.md @@ -0,0 +1,48 @@ +--- +status: open +kind: defect +opened: 2026-10-09 +--- + +# A handshake reset before it ends hands its option to the next connection + +netstack on smoltcp gives a connection the `TCP_NODELAY` its listener held +when its SYN arrived (`userland/netstack/src/listen.rs`), which is a host's +rule (`userland/netstack/node/tests/host.rs`), except for the connection that +follows a handshake its peer reset before it ended. + +Read from the two sources, not run. A listener there is one smoltcp socket. A +set of the listener's option is written to that socket only while it is in +`Listen` (`Listening::set_nodelay`); once a SYN has arrived the set is kept in +`Listening` for the socket that listens next, which `Listening::open` gives it +after an accept, or after a reset that left the socket `Closed` +(`Listening::settle`). smoltcp 0.12.0 puts a socket that takes an RST in +`SynReceived` straight back to `Listen` (`src/socket/tcp.rs`, the +`(State::SynReceived, TcpControl::Rst)` arm: `self.tuple = None; +self.set_state(State::Listen)`), with the Nagle switch it had. No pass of +netstack sees that socket `Closed`, so nothing writes the listener's option to +it again. + +The sequence: the listener holds the option, a SYN arrives, the owner clears +the option, the peer resets, as a SYN scan does. The socket listens again with +Nagle's algorithm off, and the next connection begins with `TCP_NODELAY` at a +listener that held none when its SYN arrived; its accept answers the option +on. The mirror gives a connection without the option at a listener whose +`getsockopt` reads 1. A set that reaches the socket while it listens again, or +the accept that replaces it, ends the difference: one connection a reset +handshake is affected, if the owner changed the option during that handshake. + +No test holds it, and none is to be built: the owner's ruling is that no new +test is built on smoltcp. The node, which replaces this stack, has the rule +for this case: [tcp] keeps the options with the listener and copies them to a +connection when its SYN arrives (LS-10), a reset handshake leaves nothing +behind, and +`a_handshake_reset_before_it_ends_leaves_the_next_connection_its_listeners_option` +(`userland/netstack/node/tests/listeners.rs`) plays the sequence both ways +round and reads the next connection's option from the accept's answer. + +**Exit condition**: netd runs on the node and `userland/netstack/src/listen.rs` +and smoltcp are deleted, with that test of the node green. + +**Owner**: whoever holds `issues/toyos-has-its-own-network-stack.md`, at the +move. diff --git a/issues/a-listeners-tcp-nodelay-has-no-setter-in-std-or-the-forks-and-a-c-program-cannot-order-the-waiting-case.md b/issues/a-listeners-tcp-nodelay-has-no-setter-in-std-or-the-forks-and-a-c-program-cannot-order-the-waiting-case.md index 36608a68f60..b99e8c9cc8e 100644 --- a/issues/a-listeners-tcp-nodelay-has-no-setter-in-std-or-the-forks-and-a-c-program-cannot-order-the-waiting-case.md +++ b/issues/a-listeners-tcp-nodelay-has-no-setter-in-std-or-the-forks-and-a-c-program-cannot-order-the-waiting-case.md @@ -17,11 +17,18 @@ connection waits to be accepted: a connect returns before the listener's end is established, so the answer rests on that event and not on the order loopback delivers in. A red there prints the host's name and both answers. -The pipe ABI carries both halves (`toyos/src/net.rs`): -`MsgType::TcpListenerSetOption` sets a listener's option, and an accept's -answer says what its connection holds (`TcpOptions`). netstack answers the -rule for both (`userland/netstack/src/listen.rs`), and the `libc_sockets` -guest test reads it: `nodelay_accepted` by `toyos::net`, for a connection that +`tests/host.rs` is green on Linux too: the `host` check runs it on +`ubuntu-24.04`, whose log names +`a_host_gives_a_connection_the_nodelay_its_listener_had_when_it_began ... ok`. + +The pipe ABI carries both halves (`toyos/src/net.rs`): a bind's request +carries its listener's options and `MsgType::TcpListenerSetOption` sets one +afterwards, and an accept's answer says what its connection holds +(`TcpOptions`). netstack answers the rule for both +(`userland/netstack/src/listen.rs`), but for the connection after a reset +handshake +(`issues/a-handshake-reset-before-it-ends-hands-its-option-to-the-next-connection.md`), +and the `libc_sockets` guest test reads it: `nodelay_accepted` by `toyos::net`, for a connection that waited when the option was set, its wake read first, and for the next one dialled; `tests/netcase/nodelay_kept.c` by libc's `setsockopt` and `getsockopt`, for a listener's set before `bind`, its set and its clear after, @@ -39,31 +46,28 @@ and the connections dialled after each. What no program reads yet: table of its own sockets, so a std listener's descriptor names none of them; on a listener of its own its `setsockopt` of `TCP_NODELAY` answers `Ok` and hands the option to nobody, and its `accept` records the accepted - socket's as off whatever the answer says. mio (`d1edcaa`, + socket's as off whatever the answer says; its bind names no option + (`toyos::net::tcp_bind`, which `tcp_bind_with` stands beside only until + the fork's bind passes its own). mio (`d1edcaa`, `src/net/tcp/toyos_stream.rs`) answers `nodelay()` true for every stream and its `set_nodelay` sends nothing. std's `TcpListener::accept` (`sdk/std/sys/net/connection.rs`) takes the accepted stream's option from the answer, which no test can turn red while no std listener can hold one. -The node (`userland/netstack/node/src/listeners.rs`) has the listener's call, -`Node::set_listener_nodelay`, and gives an accepted stream what its connection -took from its listener; nothing ships it until the move. - -Not read: Linux. No Linux host was at hand when this was filed. The `host` -check runs `tests/host.rs` on Linux, and its first run on a pull request that -carries the file is that reading. +The node (`userland/netstack/node/src/listeners.rs`) takes a listener's +option in its `listen` and in `Node::set_listener_nodelay`, and answers an +accept with what its stream's connection took from its listener; nothing ships +it until the move. -**Exit condition**: `tests/host.rs` is green in a `host` check's log, which -is the Linux reading; and on ToyOS a program that sets `TCP_NODELAY` on a +**Exit condition**: on ToyOS a program that sets `TCP_NODELAY` on a listening socket and accepts one connection begun before and one begun after reads the option on each as that test has a host answer, by libc's `getsockopt`, which needs libc's `poll` to answer for a listening socket, and by std's `nodelay()`, which needs a listener a Rust program can set the option on: the socket2 fork's, with the lock naming the commit that hands its -listener's option to netstack and reads its accepted socket's from the answer, -and mio's that reads a stream's. +listener's option to netstack in its bind and reads its accepted socket's from +the answer, and mio's that reads a stream's; and `toyos::net` has one bind +call, which takes its options. **Owner**: whoever holds `issues/toyos-has-its-own-network-stack.md`; the -Linux reading is the orchestrator's, at the first `host` run of the pull -request that adds `tests/host.rs`; the fork commits are on socket2's and mio's -`toyos` branches. +fork commits are on socket2's and mio's `toyos` branches. diff --git a/issues/libc-accept-writes-its-callers-address-before-it-can-still-fail.md b/issues/libc-accept-writes-its-callers-address-before-it-can-still-fail.md new file mode 100644 index 00000000000..6362ac91690 --- /dev/null +++ b/issues/libc-accept-writes-its-callers-address-before-it-can-still-fail.md @@ -0,0 +1,25 @@ +--- +status: open +kind: defect +opened: 2026-10-09 +--- + +# libc's accept writes its caller's address before it can still fail + +`accept` (`userland/libc/src/socket.rs`) fills the caller's `addr` and +`addrlen` with the peer's address as soon as netstack answers, and only then +asks `alloc_socket` for a descriptor. With the table of `MAX_SOCKETS` full +that fails: `accept` closes the connection and returns -1 with `ENOMEM`, +having already written the caller's buffer and its length. Read from the +code, not run: no test fills the table, and no host was asked what it leaves +in the buffer of an `accept` it refuses for want of a descriptor. + +It is the one failure of `accept` left that touches the caller's buffer, now +that it asks netstack nothing after its answer. The word is not POSIX's +either, which names `EMFILE` for a process out of descriptors. + +**Exit condition**: `accept` takes its descriptor before it writes `addr`, +and a guest C case that fills the socket table reads the buffer and its +length unchanged after the refused `accept`, with `EMFILE`. + +**Owner**: libc, `userland/libc`; whoever next changes its `accept`. diff --git a/issues/nothing-reads-whether-a-streams-tcp-nodelay-reached-netstack.md b/issues/nothing-reads-whether-a-streams-tcp-nodelay-reached-netstack.md index 7772b0d0732..777bb780912 100644 --- a/issues/nothing-reads-whether-a-streams-tcp-nodelay-reached-netstack.md +++ b/issues/nothing-reads-whether-a-streams-tcp-nodelay-reached-netstack.md @@ -19,8 +19,9 @@ only effect of the option is when a small write leaves the machine, which a QEMU test may not time. A listener's is read: what an accepted socket answers is what netstack said of -its connection, so libc's `bind` or its `setsockopt` on a listener storing the -value and sending nothing turns that case red. +its connection, so libc's `bind` naming no option in its request, or its +`setsockopt` on a listener storing the value and sending nothing, turns that +case red. **Exit**: a test reads what the stack holds for a stream's Nagle switch, on the node under a host test of its option call or through a guest's wire, and diff --git a/issues/run-test-and-its-ceiling-verdict-serve-only-debugs-run.md b/issues/run-test-and-its-ceiling-verdict-serve-only-debugs-run.md index 94c6f2066a5..8a5bbf94f5e 100644 --- a/issues/run-test-and-its-ceiling-verdict-serve-only-debugs-run.md +++ b/issues/run-test-and-its-ceiling-verdict-serve-only-debugs-run.md @@ -6,7 +6,7 @@ opened: 2026-10-03 # `run_test` and its ceiling verdict serve only `--debug`'s `run` -`QemuInstance::run_test`, `run_test_hooked` and `run_test_paced` +`QemuInstance::run_test` and `run_test_paced` (`tests/common/qemu.rs`), and `ceiling_verdict`, the verdict their read loop ends a wait on, have one caller: interactive debug mode's `run ` (`tests/toyos.rs`, `qemu.run_test(test_name, Duration::from_secs(60))`). Every @@ -18,8 +18,9 @@ path. ## Measured -`git grep -c 'run_test(\|run_test_hooked(\|run_test_paced(' -- tests/toyos.rs tests/common` -counts 40 lines in `tests/toyos.rs` and 34 across sixteen other `tests/common` +`git grep -c 'run_test(\|run_test_hooked(\|run_test_paced(' -- tests/toyos.rs tests/common`, +over the three functions those revisions had (`run_test_hooked` is deleted +since), counts 40 lines in `tests/toyos.rs` and 34 across sixteen other `tests/common` modules at `06788146b^`, and one in `tests/toyos.rs`, the debug loop's, at `06788146b`, the commit that cut the guest suite; the five in `tests/common/qemu.rs` are the three functions' own. @@ -32,5 +33,5 @@ first cut left them that one caller. ## What would close it `--debug`'s `run` reads its test through a wait the suite uses, and -`run_test`, `run_test_hooked`, `run_test_paced`, `ceiling_verdict` and the +`run_test`, `run_test_paced`, `ceiling_verdict` and the checks and readers that exist for them are deleted. diff --git a/issues/toyos-has-its-own-network-stack.md b/issues/toyos-has-its-own-network-stack.md index d8a4a0e23ff..46af3557386 100644 --- a/issues/toyos-has-its-own-network-stack.md +++ b/issues/toyos-has-its-own-network-stack.md @@ -42,10 +42,9 @@ What the node does not yet meet: - A query [ip] refuses at the transmit opportunity that would have carried it, its next hop held failed or its table full, is reported to its socket inside `Node::transmit`, which carries no lookup on: the resolver reads the report at the node's next `receive`, `fire` or `link`, which at the latest is the end of the query's wait, where a query [udp] refuses in the resolver's own call, or one reported in a frame or at a deadline, ends its wait at once. `a_query_ip_refuses_at_its_transmit_opportunity_is_read_at_the_nodes_next_wake` holds it as it stands. It is no regression: what ships waits every such query out. Until that read, an ICMP error that quotes the query's addresses and ports takes [ip]'s word's place on the socket, [udp] holding one pending error a socket, the latest: the query is then waited out and not let go, which costs the early end and never an answer. Owner: the slice in which datagram senders wait on the hop (`Node::transmit` is the node's `lib.rs`). Exit: the node carries its lookups on after the stack's transmit, or [udp] refuses in the call a datagram whose next hop [ip] holds failed, so that no report waits on a socket to be displaced, and that test finds its lookup ended at the 4 s it started. - [ip]'s queue for a next hop it is resolving is every sender's: `PENDING_PER_NEIGHBOUR` is 16, a stopgap which the node's resolver asserts is no fewer than `toyos_dns::MAX_LOOKUPS`, so every held lookup's first query waits there whole, but a datagram a client sends to the same next hop in that window, which for everything off the link is the router, takes the oldest one's place (`nb.pending-overflow`, RFC 4861 §7.2.2), and a query lost so costs its lookup a `WAIT_MS`; smoltcp kept each datagram in its own socket until the neighbour answered. `a_clients_datagram_behind_the_burst_takes_the_oldest_querys_place_in_ips_queue` holds it as it stands. It is a regression against what ships for more than 16 datagrams at one next hop not yet resolved, and would ship at the move; and it sizes a limit of [ip]'s by a constant of the resolver's, which leaves four unresolved next hops filling `PENDING_TOTAL` where eight did. Owner: the slice in which datagram senders wait on the hop, where [udp]'s datagram calls already go through `Stack` and `Shard`. Its design: [udp]'s `serve` asks [ip] the next-hop question before it hands a datagram over, as [tcp]'s exit does with `Hop` (NUD-25, IP-D5); a socket whose oldest datagram's hop is pending keeps it in its own queue, which `TX_DATAGRAMS` and `TX_BYTES` already bound and refuse in the call, and is offered again when the hop resolves and told when it fails; [ip] then queues only its own datagrams. Exit, before the move: that test finds every query on the wire once the resolver answers ARP; `PENDING_PER_NEIGHBOUR` is `limits.ip.nud.pending_per_neighbour`'s 8 again; and the resolver's compile-time assertion, the change to `s_ip_nud_004_overflow_drops_the_oldest` and stage 3's departure line about it are deleted. - `toyos_dns::Lookup::on_datagram` still takes a reply's source address and port, which the node's connected sockets have already matched and the node hands it from its own record of the query: the two parameters are read only by netd's resolver on smoltcp. Exit, at the move: they go with that resolver. -- An accept's answer on the pipe ABI carries what its connection holds (`TcpOptions`, `toyos/src/net.rs`), and the node's `Accepted` does not: `Node::accept` has the connection's options in hand and files them with the stream, so netd reads them back with `Node::nodelay` of the id it was just answered, whose `None`, a stream the accept's own pass let go, the answer has no word for. Exit, at the move: `Accepted` carries its stream's options, or the move's body says which answer a `None` there gets and a test of the node finds it. - `node.address-refused` has no test: `toyos-dhcp` accepts no address or prefix [ip] refuses, by the same `toyos-net-wire` checks in both, so the refusal cannot be reached from the wire. Exit: the client hands [ip] a type that carries the check, and the counter goes. -The listener defects are this track's: `issues/a-handshake-nobody-finishes-holds-a-listeners-port-shut.md` and `issues/a-connect-between-two-accepts-is-reset.md`, on smoltcp until stage 5, and `issues/an-accept-that-never-reaches-netstack-strands-its-listener.md`, in std's accept; and `issues/a-listeners-tcp-nodelay-has-no-setter-in-std-or-the-forks-and-a-c-program-cannot-order-the-waiting-case.md`: the pipe ABI gives a listener its option (`MsgType::TcpListenerSetOption`) and an accept's answer its connection's (`TcpOptions`, `toyos/src/net.rs`), netd on smoltcp answers both, and the move maps the first to `Node::set_listener_nodelay`; what is left of its exit is libc's `poll` and the socket2 and mio forks. +The listener defects are this track's: `issues/a-handshake-nobody-finishes-holds-a-listeners-port-shut.md` and `issues/a-connect-between-two-accepts-is-reset.md`, on smoltcp until stage 5, and `issues/an-accept-that-never-reaches-netstack-strands-its-listener.md`, in std's accept; and `issues/a-listeners-tcp-nodelay-has-no-setter-in-std-or-the-forks-and-a-c-program-cannot-order-the-waiting-case.md`: the pipe ABI gives a listener its option in its bind's request and by `MsgType::TcpListenerSetOption` and an accept's answer its connection's (`TcpOptions`, `toyos/src/net.rs`), netd on smoltcp answers them but for `issues/a-handshake-reset-before-it-ends-hands-its-option-to-the-next-connection.md`, which the move ends with smoltcp, and the move maps the bind to `Node::listen`, whose `nodelay` the request's is, the set to `Node::set_listener_nodelay` and the answer's options to `Accepted`'s; what is left of its exit is libc's `poll` and the socket2 and mio forks. Owed from the stage 3 specifications: by stage 5, the scenario for netd's mapping of UDP's refusals onto the pipe ABI, and `dhcp.renew-unroutable`, which netd counts where `toyos-net-udp` refuses the renewal `udp.no-route`; by IP hardening, the scenarios for fragment reassembly and path MTU discovery. diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index 22ad5752d3e..ffd33add970 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -1490,40 +1490,14 @@ impl QemuInstance { } pub fn run_test(&mut self, name: &str, timeout: Duration) -> TestResult { - self.run_test_hooked(name, timeout, "", |_| {}) - } - - /// `run_test`, with `action` run once the guest prints `ready_line`. - /// - /// The hook is inside the read loop because that is the only place the - /// two facts meet: the guest is holding the keyboard claim, and the host has - /// not injected yet. A sleep would be a guess in both directions. - pub fn run_test_hooked( - &mut self, - name: &str, - timeout: Duration, - ready_line: &str, - action: impl FnOnce(&Path), - ) -> TestResult { - let mut action = Some(action); - self.run_test_paced(name, timeout, |socket, line| { - if ready_line.is_empty() || !line.contains(ready_line) { - return; - } - if let Some(action) = action.take() { - action(socket.expect("run_test_hooked needs BootOptions { qmp: true }")); - } - }) + self.run_test_paced(name, timeout, |_, _| {}) } /// `run_test`, with `step` run on every console line the guest prints. /// - /// [`Self::run_test_hooked`] injects a whole sequence in one call and holds - /// the reader while it does, so the host runs at its own speed and what - /// reaches the guest is whatever survived the queues in between — a packet - /// the guest was never given reads exactly like one it lost. A step driven - /// by the guest's own output can stay behind it, which is how an injection - /// test costs a slow guest wall-clock instead of a verdict. + /// A step driven by the guest's own output can stay behind it, which is + /// how an injection test costs a slow guest wall-clock instead of a + /// verdict. pub fn run_test_paced( &mut self, name: &str, diff --git a/tests/toyos-rust-tests/src/bin/netstack_gone_mid_bind.rs b/tests/toyos-rust-tests/src/bin/netstack_gone_mid_bind.rs index d57f1048a95..5e0686dd8ed 100644 --- a/tests/toyos-rust-tests/src/bin/netstack_gone_mid_bind.rs +++ b/tests/toyos-rust-tests/src/bin/netstack_gone_mid_bind.rs @@ -53,7 +53,7 @@ use std::process::{Child, ChildStdout, Command, Stdio}; use toyos::endow::EndowError; use toyos::net::{ - MsgType, NetError, NetstackConn, SocketCloseRequest, TcpBindPipedRequest, TcpBindResponse, + MsgType, NetError, NetstackConn, SocketCloseRequest, TcpBindPipedRequest, TcpBindResponse, TcpOptions, }; use toyos::{namespace, port, AsHandle}; use toyos_abi::syscall::{self, SVC_LABEL}; @@ -203,7 +203,7 @@ fn mid_flight(request: Request) -> ! { .request_with_handles( [netstack_notify.into()], MsgType::TcpBindPiped, - &TcpBindPipedRequest { addr: [0, 0, 0, 0], port: SSH_PORT, _pad: 0 }, + &TcpBindPipedRequest { addr: [0, 0, 0, 0], port: SSH_PORT, _pad: 0, options: TcpOptions::new(false) }, ) .and_then(|pending| pending.response::().map(|_| ())) .expect_err("a bind into a port whose acceptor is gone was answered"); diff --git a/tests/toyos-rust-tests/src/bin/nodelay_accepted.rs b/tests/toyos-rust-tests/src/bin/nodelay_accepted.rs index 65fa32dba87..ffe92985b23 100644 --- a/tests/toyos-rust-tests/src/bin/nodelay_accepted.rs +++ b/tests/toyos-rust-tests/src/bin/nodelay_accepted.rs @@ -1,9 +1,9 @@ //! `TCP_NODELAY` as std answers it and as netstack's pipe ABI carries a //! listener's. //! -//! The first half is std's alone and prints what any host prints: a stream -//! and its duplicate are one socket, so a set on either is read on both, and -//! a stream accepted from a listener that holds no option has none. +//! The first half is std's alone: a stream and its duplicate are one socket, +//! so a set on either is read on both, and a stream accepted from a listener +//! that holds no option has none. //! //! The second is ToyOS's: a listener made by `toyos::net`, whose option std //! has no call for. A connection that waited when the option was set has not @@ -18,7 +18,6 @@ use std::net::{TcpListener, TcpStream}; const WAITING: &str = "nodelay_accepted: both listeners wait for a peer"; -#[cfg(target_os = "toyos")] const AGAIN: &str = "nodelay_accepted: the listener waits for a second peer"; struct Said(u32); @@ -33,7 +32,6 @@ impl Said { /// The listener `toyos::net` binds on `port`: one connection that waits before /// the option is set, and one dialled after. -#[cfg(target_os = "toyos")] fn the_pipe_abis_listener(port: u16, said: &mut Said) { use toyos::net::{self, NetError, OPT_NODELAY}; @@ -85,13 +83,7 @@ fn main() { said.said("read from the stream", stream.nodelay().ok(), Some(false)); let listener = TcpListener::bind(("0.0.0.0", port(std_port))).expect("std's listener"); - #[cfg(target_os = "toyos")] the_pipe_abis_listener(port(pipe_port), &mut said); - #[cfg(not(target_os = "toyos"))] - { - let _ = pipe_port; - println!("{WAITING}"); - } let (accepted, _) = listener.accept().expect("std's accept"); said.said("a stream accepted from a listener that holds none", accepted.nodelay().ok(), Some(false)); diff --git a/toyos/src/net.rs b/toyos/src/net.rs index 4f3348f6ee8..e51359f5cc2 100644 --- a/toyos/src/net.rs +++ b/toyos/src/net.rs @@ -83,10 +83,10 @@ pub const ERR_PERMISSION_DENIED: u32 = 8; pub const ERR_OTHER: u32 = 255; /// Nagle's algorithm off. On a stream, [`tcp_set_option`]. On a listener, -/// [`tcp_listener_set_option`]: for every connection whose SYN arrives from -/// then on, and one that began before keeps what it has. A connection says -/// what it began with in its accept's answer ([`TcpOptions`]); one a connect -/// made begins with the algorithm on. +/// its bind's [`TcpOptions`] and then [`tcp_listener_set_option`]: for every +/// connection whose SYN arrives from then on, and one that began before keeps +/// what it has. A connection says what it began with in its accept's answer +/// ([`TcpOptions`]); one a connect made begins with the algorithm on. pub const OPT_NODELAY: u32 = 1; /// A datagram socket may send to a broadcast address: [`udp_set_option`]. pub const OPT_BROADCAST: u32 = 2; @@ -174,6 +174,8 @@ ipc_payload! { pub addr: [u8; 4], pub port: u16, pub _pad: u16, + /// What the listener holds from the moment its port listens. + pub options: TcpOptions, } pub struct TcpBindResponse { @@ -191,8 +193,8 @@ ipc_payload! { pub socket_id: u32, } - /// The options a stream holds, a word each: zero is off and anything else - /// on. + /// The options a listener or a stream holds, a word each: zero is off and + /// anything else on. #[derive(Debug, PartialEq, Eq)] pub struct TcpOptions { nodelay: u32, @@ -471,7 +473,13 @@ pub fn tcp_connect( Ok(TcpConnection { rx, tx, socket_id: TcpSocketId(resp.socket_id), local_port: resp.local_port }) } +/// A listener that holds no option. pub fn tcp_bind(addr: [u8; 4], port: u16) -> Result { + tcp_bind_with(addr, port, TcpOptions::new(false)) +} + +/// A listener that holds `options` before any SYN can reach its port. +pub fn tcp_bind_with(addr: [u8; 4], port: u16, options: TcpOptions) -> Result { let netstack = NetstackConn::connect()?; let (notify, netstack_notify) = crate::pipe_pair().map_err(|_| NetError::Io)?; @@ -479,7 +487,7 @@ pub fn tcp_bind(addr: [u8; 4], port: u16) -> Result { .request_with_handles( [netstack_notify.into()], MsgType::TcpBindPiped, - &TcpBindPipedRequest { addr, port, _pad: 0 }, + &TcpBindPipedRequest { addr, port, _pad: 0, options }, )? .response()?; @@ -691,6 +699,26 @@ mod tests { )); } + /// The bind's request as netstack decodes it: the options last, after + /// the address and the port, any non-zero word an option that is on, and + /// a request without them refused. + #[test] + fn a_binds_request_ends_in_its_listeners_options() { + let mut wire = [192, 0, 2, 7, 0x16, 0, 0, 0, 0, 0, 0, 0]; + let request: TcpBindPipedRequest = crate::ipc::decode_payload(&wire).unwrap(); + assert_eq!((request.addr, request.port, request.options), ([192, 0, 2, 7], 22, TcpOptions::new(false))); + wire[8] = 1; + let request: TcpBindPipedRequest = crate::ipc::decode_payload(&wire).unwrap(); + assert_eq!(request.options, TcpOptions::new(true)); + wire[8..].copy_from_slice(&[0, 0, 0, 0x80]); + let request: TcpBindPipedRequest = crate::ipc::decode_payload(&wire).unwrap(); + assert!(request.options.nodelay()); + assert!(matches!( + crate::ipc::decode_payload::(&wire[..8]), + Err(IpcError::Malformed) + )); + } + /// The accept's answer as a client decodes it: the options last, after /// the ports, any non-zero word an option that is on, and an answer /// without them refused. diff --git a/userland/libc/src/socket.rs b/userland/libc/src/socket.rs index f9f8f237379..1a5986a94da 100644 --- a/userland/libc/src/socket.rs +++ b/userland/libc/src/socket.rs @@ -5,7 +5,7 @@ use alloc::vec::Vec; use core::ptr; use toyos_abi::RawHandle; use toyos_abi::syscall; -use toyos::net::{NetError, TcpSocketId, UdpSocketId, OPT_BROADCAST, OPT_NODELAY}; +use toyos::net::{NetError, TcpOptions, TcpSocketId, UdpSocketId, OPT_BROADCAST, OPT_NODELAY}; use crate::errno::{ EACCES, EADDRINUSE, EAFNOSUPPORT, EBADF, ECONNREFUSED, ECONNRESET, EFAULT, EINVAL, EIO, ENOMEM, ENOPROTOOPT, ENOSPC, @@ -63,9 +63,10 @@ struct SocketEntry { notify_fd: i32, // read end of listener notify pipe // What netstack holds for the socket, which `getsockopt` answers from: set // by a request it answered, or read from the accept's answer. A socket - // netstack does not hold yet keeps each for the call that makes it there - // to hand over: `broadcast` and a listener's `nodelay` for `bind`, a - // stream's `nodelay` for `connect`. + // netstack does not hold yet keeps each for the call that makes it there: + // a listener's `nodelay` goes in its `bind`'s request, `broadcast` is + // handed over after a datagram socket's bind, and a stream's `nodelay` + // after its `connect`. nodelay: bool, broadcast: bool, } @@ -249,18 +250,10 @@ pub unsafe extern "C" fn bind(fd: i32, addr: *const Sockaddr, addrlen: SocklenT) match entry.kind { SocketKind::Tcp => { - let bound = match toyos::net::tcp_bind(ip, port) { + let bound = match toyos::net::tcp_bind_with(ip, port, TcpOptions::new(entry.nodelay)) { Ok(b) => b, Err(e) => { set_errno(net_err_to_errno(e)); return -1; } }; - if entry.nodelay { - if let Err(e) = toyos::net::tcp_listener_set_option(bound.socket_id, OPT_NODELAY, 1) { - // `bound`'s pipe end closes where it drops. - let _ = toyos::net::tcp_close(bound.socket_id); - set_errno(net_err_to_errno(e)); - return -1; - } - } entry.netstack_id = bound.socket_id.0; entry.local_port = bound.bound_port; entry.bound = true; diff --git a/userland/netstack/node/src/listeners.rs b/userland/netstack/node/src/listeners.rs index 552c962a6ac..008db818e93 100644 --- a/userland/netstack/node/src/listeners.rs +++ b/userland/netstack/node/src/listeners.rs @@ -25,8 +25,10 @@ //! //! **A stream starts with the options its connection has**, and those are its listener's as //! they were when its SYN arrived: [tcp] hands them over then (LS-10), the node reads them -//! back at the accept, and an option set on the listener afterwards reaches the connections -//! that begin afterwards. A host does the same with `TCP_NODELAY` set on a listening socket +//! back at the accept and answers them with the stream, and an option set on the listener +//! afterwards reaches the connections that begin afterwards. A listener holds what its listen +//! named before any SYN can reach its port: the node takes no frame between the passive open +//! and the options. A host does the same with `TCP_NODELAY` set on a listening socket //! (`tests/host.rs` asks the host the tests run on). use alloc::boxed::Box; @@ -68,6 +70,10 @@ pub struct Accepted { pub id: StreamId, pub remote: Endpoint, pub local: Port, + /// Whether Nagle's algorithm is off for the stream as it is handed over: what + /// [`Node::nodelay`] answers for as long as the node holds the stream, which may be no + /// longer than this accept's own pass. + pub nodelay: bool, } /// Why an accept took no connection. Whatever waits still waits, and the pipe ends are dropped. @@ -103,9 +109,11 @@ pub(crate) struct Listeners { impl Node { /// A passive open at `addr`, 0.0.0.0 meaning every address the interface holds or comes to - /// hold, on `port` or on a port chosen from `draw`'s candidates. Answers the listener and - /// its port. Refused, nothing was made and `owner` is dropped. - pub fn listen(&mut self, addr: Ipv4Addr, port: Option, owner: Box, mut draw: impl FnMut() -> u32) -> Result<(ListenerId, Port), ListenRefused> { + /// hold, on `port` or on a port chosen from `draw`'s candidates, with `nodelay` as + /// [`Node::set_listener_nodelay`] would set it, for every connection of the listener's + /// from its first. Answers the listener and its port. Refused, nothing was made and + /// `owner` is dropped. + pub fn listen(&mut self, addr: Ipv4Addr, port: Option, nodelay: bool, owner: Box, mut draw: impl FnMut() -> u32) -> Result<(ListenerId, Port), ListenRefused> { if self.room() == 0 { return Err(ListenRefused::Full); } @@ -125,7 +133,9 @@ impl Node { }; let id = ListenerId(self.listeners.next); self.listeners.next = self.listeners.next.saturating_add(1); - self.listeners.live.insert(id, Listener { bound, port, owner, unspent: 0, options: Options::default() }); + let options = Options { nodelay, ..Options::default() }; + self.stack.tcp_set_listener_options(bound, options); + self.listeners.live.insert(id, Listener { bound, port, owner, unspent: 0, options }); Ok((id, port)) } @@ -162,7 +172,7 @@ impl Node { let Some((conn, tuple, options)) = self.stack.tcp_accept(bound) else { return Err(AcceptRefused::Nothing) }; // The peer's address is what `streams` counts a stream its client can see no more by. let id = self.streams.accepted(conn, tuple.remote.addr, options, pipes); - Ok(Accepted { id, remote: tuple.remote, local: tuple.local.port }) + Ok(Accepted { id, remote: tuple.remote, local: tuple.local.port, nodelay: options.nodelay }) } /// The owner lets go of the listener: netstack calls it for a close, and when the kernel diff --git a/userland/netstack/node/tests/lease.rs b/userland/netstack/node/tests/lease.rs index 48ac5ee2275..c72c2dcb831 100644 --- a/userland/netstack/node/tests/lease.rs +++ b/userland/netstack/node/tests/lease.rs @@ -212,7 +212,7 @@ impl Drop for Owner { fn listen(wire: &mut Wire, addr: Ipv4Addr, port: u16) -> Result, ListenRefused> { let told = Rc::new(Told::default()); - wire.node.listen(addr, Port::new(port), Box::new(Owner(told.clone())), || panic!("a named port draws nothing")).map(|_| told) + wire.node.listen(addr, Port::new(port), false, Box::new(Owner(told.clone())), || panic!("a named port draws nothing")).map(|_| told) } /// Where the peer of the listeners is, at `MAC_B`. diff --git a/userland/netstack/node/tests/listeners.rs b/userland/netstack/node/tests/listeners.rs index a42c736e7cc..3557c135f62 100644 --- a/userland/netstack/node/tests/listeners.rs +++ b/userland/netstack/node/tests/listeners.rs @@ -364,8 +364,13 @@ impl Net { } fn listen(&mut self, port: u16) -> (ListenerId, Owner) { + self.listen_with(port, false) + } + + /// A listen that names the listener's `TCP_NODELAY`. + fn listen_with(&mut self, port: u16, nodelay: bool) -> (ListenerId, Owner) { let owner = Owner::default(); - let (id, bound) = self.node.listen(ANY, Port::new(port), Box::new(WakeEnd(owner.clone())), draw(&mut self.draws)).expect("a free port and a place"); + let (id, bound) = self.node.listen(ANY, Port::new(port), nodelay, Box::new(WakeEnd(owner.clone())), draw(&mut self.draws)).expect("a free port and a place"); assert_eq!(bound.get(), port); (id, owner) } @@ -378,10 +383,12 @@ impl Net { answer.map(|accepted| (accepted, client)) } - /// An accept that takes `peer`'s connection. + /// An accept that takes `peer`'s connection, which the answer and the node say the same + /// option of. fn accepts(&mut self, id: ListenerId, peer: Peer) -> (StreamId, Client) { let (accepted, client) = self.accept(id).expect("a connection and a place"); assert_eq!((accepted.remote, accepted.local), (peer.endpoint(), Port::new(SSH).unwrap())); + assert_eq!(Some(accepted.nodelay), self.node.nodelay(accepted.id), "the accept's answer and the stream it made"); (accepted.id, client) } @@ -655,7 +662,7 @@ fn a_listener_holds_a_place_and_a_listen_without_one_makes_nothing() { let mut net = with_places(1); let (id, _owner) = net.listen(SSH); let refused = Owner::default(); - let answer = net.node.listen(ANY, Port::new(TELNET), Box::new(WakeEnd(refused.clone())), draw(&mut net.draws)); + let answer = net.node.listen(ANY, Port::new(TELNET), false, Box::new(WakeEnd(refused.clone())), draw(&mut net.draws)); assert_eq!(answer.unwrap_err(), ListenRefused::Full); assert!(refused.borrow().dropped); net.syn_to(P1, TELNET); @@ -718,14 +725,14 @@ fn a_listen_on_a_taken_port_is_refused_and_a_drawn_port_listens() { let mut net = Net::new(); let (_, owner) = net.listen(SSH); let second = Owner::default(); - let answer = net.node.listen(ANY, Port::new(SSH), Box::new(WakeEnd(second.clone())), draw(&mut net.draws)); + let answer = net.node.listen(ANY, Port::new(SSH), false, Box::new(WakeEnd(second.clone())), draw(&mut net.draws)); assert_eq!(answer.unwrap_err(), ListenRefused::InUse); assert!(second.borrow().dropped); assert_eq!((net.node.listeners(), net.node.held()), (1, 1)); net.handshake(P1); assert_eq!(wakes(&owner), 1, "the listener that holds the port still listens"); - let (_, port) = net.node.listen(ANY, None, Box::new(WakeEnd(Owner::default())), || 0x0001_0005).unwrap(); + let (_, port) = net.node.listen(ANY, None, false, Box::new(WakeEnd(Owner::default())), || 0x0001_0005).unwrap(); assert_eq!(port.get(), 49_152 + 5); net.syn_to(P2, port.get()); let synack = net.last(P2); @@ -740,7 +747,7 @@ fn a_listen_on_a_taken_port_is_refused_and_a_drawn_port_listens() { fn a_listener_is_at_the_address_it_named_and_only_one_the_machine_holds() { let mut net = Net::new(); let refused = Owner::default(); - let answer = net.node.listen(ELSEWHERE, Port::new(SSH), Box::new(WakeEnd(refused.clone())), draw(&mut net.draws)); + let answer = net.node.listen(ELSEWHERE, Port::new(SSH), false, Box::new(WakeEnd(refused.clone())), draw(&mut net.draws)); assert_eq!(answer.unwrap_err(), ListenRefused::NotLocal); assert!(refused.borrow().dropped); assert_eq!((net.node.listeners(), net.node.held()), (0, 0)); @@ -749,7 +756,7 @@ fn a_listener_is_at_the_address_it_named_and_only_one_the_machine_holds() { assert!(reset.rst && (reset.seq, reset.ack) == (0, Some(ISS + 1)), "{reset:?}"); let named = Owner::default(); - let (_, port) = net.node.listen(A, Port::new(SSH), Box::new(WakeEnd(named.clone())), draw(&mut net.draws)).expect("the machine's address, and a port no listener holds"); + let (_, port) = net.node.listen(A, Port::new(SSH), false, Box::new(WakeEnd(named.clone())), draw(&mut net.draws)).expect("the machine's address, and a port no listener holds"); assert_eq!((port.get(), net.node.listeners()), (SSH, 1)); net.handshake(P2); assert_eq!(wakes(&named), 1); @@ -760,9 +767,9 @@ fn a_listener_is_at_the_address_it_named_and_only_one_the_machine_holds() { fn a_port_is_one_listeners(first: Ipv4Addr, second: Ipv4Addr) { let mut net = Net::new(); let holder = Owner::default(); - let (id, _) = net.node.listen(first, Port::new(SSH), Box::new(WakeEnd(holder.clone())), draw(&mut net.draws)).expect("a port no listener holds"); + let (id, _) = net.node.listen(first, Port::new(SSH), false, Box::new(WakeEnd(holder.clone())), draw(&mut net.draws)).expect("a port no listener holds"); let refused = Owner::default(); - let answer = net.node.listen(second, Port::new(SSH), Box::new(WakeEnd(refused.clone())), draw(&mut net.draws)); + let answer = net.node.listen(second, Port::new(SSH), false, Box::new(WakeEnd(refused.clone())), draw(&mut net.draws)); assert_eq!(answer.unwrap_err(), ListenRefused::InUse, "{second} beside {first}"); assert!(refused.borrow().dropped); assert_eq!((net.node.listeners(), net.node.held()), (1, 1)); @@ -882,6 +889,85 @@ fn a_stream_starts_with_the_options_its_connection_took_from_its_listener() { assert_eq!((net.node.set_listener_nodelay(id, true), net.node.listener_nodelay(id)), (false, None)); } +// A listen names its listener's option, so no SYN reaches the port between the passive open and +// a set: the first connection has it with no call between, as a host's has the option set +// before `listen` (`tests/host.rs`, its first answer). The wire is RFC 9293 §3.7.4's, as above. +#[test] +fn a_listener_holds_the_option_its_listen_named_from_its_first_connection() { + let mut net = Net::new(); + let (id, _owner) = net.listen_with(SSH, true); + assert_eq!(net.node.listener_nodelay(id), Some(true)); + net.handshake(P1); + let (accepted, client) = net.accept(id).unwrap(); + assert_eq!((accepted.nodelay, net.node.nodelay(accepted.id)), (true, Some(true))); + net.says(&client, b"a"); + net.says(&client, b"b"); + assert_eq!(net.texts(P1), [b"a".to_vec(), b"b".to_vec()], "the second write waits for no acknowledgment"); + + assert!(net.node.set_listener_nodelay(id, false)); + net.handshake(P2); + let (accepted, _client) = net.accept(id).unwrap(); + assert_eq!((accepted.nodelay, net.node.listener_nodelay(id)), (false, Some(false)), "a listen's option is the listener's to change"); +} + +// A handshake its peer resets before it ends (RFC 9293 §3.10.7.4, first check, in SYN-RECEIVED) +// leaves nothing of its options behind: the listener's option changed while it was in progress, +// and the next connection begins with what the listener holds when its own SYN arrives. Both +// ways round, since the stack this one replaces keeps the reset handshake's option for the next +// connection (`issues/a-handshake-reset-before-it-ends-hands-its-option-to-the-next-connection.md`). +#[test] +fn a_handshake_reset_before_it_ends_leaves_the_next_connection_its_listeners_option() { + for held in [true, false] { + let mut net = Net::new(); + let (id, owner) = net.listen_with(SSH, held); + net.syn(P1); + assert!(net.node.set_listener_nodelay(id, !held)); + net.rst(P1); + assert_eq!((wakes(&owner), net.accept(id).unwrap_err()), (0, AcceptRefused::Nothing), "the reset handshake is no connection"); + net.handshake(P2); + let (accepted, _client) = net.accept(id).unwrap(); + assert_eq!((accepted.remote, accepted.nodelay), (P2.endpoint(), !held), "the listener held {held} at the reset handshake's SYN"); + assert_eq!((net.node.nodelay(accepted.id), net.node.listener_nodelay(id)), (Some(!held), Some(!held))); + + // And a set while the reset handshake was in progress, taken back before the next SYN. + net.syn(P3); + assert!(net.node.set_listener_nodelay(id, held)); + net.rst(P3); + assert!(net.node.set_listener_nodelay(id, !held)); + let again = Peer { addr: C, port: 40_001 }; + net.handshake(again); + let (accepted, _client) = net.accept(id).unwrap(); + assert_eq!((accepted.remote, accepted.nodelay), (again.endpoint(), !held)); + } +} + +/// The write end of a client's pipe whose reader is gone for good: the node resets the stream. +struct BrokenEnd; + +impl ToClient for BrokenEnd { + fn write(&mut self, _: &[u8]) -> Result { + Err(WriteRefusal::Broken) + } +} + +// The accept's own pass moves what the connection already received, and a pipe that refuses it +// for good ends the stream there (RFC 9293 §3.10.7.4's reset is the peer's notice). The answer +// still says what the stream was handed over with, where `Node::nodelay` of its id has no stream +// left to answer for. +#[test] +fn an_accept_answers_the_option_of_a_stream_its_own_pass_let_go() { + let mut net = Net::new(); + let (id, _owner) = net.listen_with(SSH, true); + net.handshake(P1); + net.text(P1, b"hello"); + let (client, Pipes { from_client, .. }) = client(); + let accepted = net.node.accept(net.now, id, Some(Pipes { to_client: Box::new(BrokenEnd), from_client })).unwrap(); + net.pump(); + assert!(net.last(P1).rst, "{:?}", net.heard); + assert_eq!((accepted.remote, accepted.nodelay), (P1.endpoint(), true)); + assert_eq!((net.node.nodelay(accepted.id), net.node.streams(), client.borrow().dropped), (None, 0, 2)); +} + // `streams` counts a stream its client can see no more by its peer's address, and lets an // address keep sixteen alive at once. Seventeen are accepted from 192.0.2.2 and one from // 192.0.2.3, each client leaves bytes behind, and each peer takes a segment of them fifty diff --git a/userland/netstack/src/listen.rs b/userland/netstack/src/listen.rs index 9d374ed08e2..edf54605204 100644 --- a/userland/netstack/src/listen.rs +++ b/userland/netstack/src/listen.rs @@ -13,9 +13,14 @@ //! returns. //! //! **A connection begins with the option its listener held when its SYN -//! arrived**, which is what a host gives it: the socket holds the listener's -//! option from the moment it listens, a set reaches it only while it still -//! does, and what it holds once it is a connection is that connection's. +//! arrived**, which is what a host gives it, except the connection after a +//! handshake its peer reset: the socket holds the listener's option from the +//! moment it listens, its bind's from the first, a set reaches it only while +//! it still does, and what it holds once it is a connection is that +//! connection's. smoltcp puts a socket reset in `SynReceived` straight back to +//! `Listen` with what it held, which no pass here sees, so a set that arrived +//! during that handshake reaches no socket until the next accept or the next +//! set: `issues/a-handshake-reset-before-it-ends-hands-its-option-to-the-next-connection.md`. use smoltcp::socket::tcp; @@ -42,8 +47,9 @@ pub enum Accept

{ } impl Listening { - pub fn new(port: u16) -> Self { - Self { port, woken: false, nodelay: false } + /// A listener on `port` that holds `nodelay` from its first socket on. + pub fn new(port: u16, nodelay: bool) -> Self { + Self { port, woken: false, nodelay } } pub fn port(&self) -> u16 { @@ -53,7 +59,7 @@ impl Listening { /// Has a closed `socket` listen on the port, with the listener's option. pub fn open(&self, socket: &mut tcp::Socket) { let port = self.port; - socket.listen(port).unwrap_or_else(|e| panic!("netstack: a closed socket refused to listen on {port}: {e:?}")); + socket.listen(port).unwrap_or_else(|e| panic!("netstack: a socket refused to listen on {port}: {e:?}")); socket.set_nagle_enabled(!self.nodelay); } diff --git a/userland/netstack/src/listen/tests.rs b/userland/netstack/src/listen/tests.rs index 7c12c482bb0..ac210aaf03a 100644 --- a/userland/netstack/src/listen/tests.rs +++ b/userland/netstack/src/listen/tests.rs @@ -106,7 +106,7 @@ impl Net { wire, sockets, listener, - listening: Listening::new(PORT), + listening: Listening::new(PORT, false), sent: Vec::new(), now: Instant::from_millis(0), arp: true, @@ -521,65 +521,3 @@ fn a_peer_that_answers_and_never_closes_is_reset_at_the_ceiling() { assert_eq!(at_the_ceiling(&mut net, answer), Ownerless::Cut, "the slot of a connection with no client never comes back"); the_cut_is_said(&mut net); } - -/// Whether the socket holds `TCP_NODELAY`, which is what its accept answers. -fn nodelay(net: &mut Net) -> bool { - !net.socket().nagle_enabled() -} - -/// The owner's set of `TCP_NODELAY` on its listener. -fn set_nodelay(net: &mut Net, on: bool) { - net.listening.set_nodelay(net.sockets.get_mut::(net.listener), on); -} - -/// **A connection has the option its listener held when its SYN arrived**: one -/// set while the socket still listens is the connection's, and one cleared -/// after its SYN, whether or not its handshake has finished, does not reach -/// it. -#[test] -fn a_connection_begins_with_the_option_its_listener_held() { - let mut net = Net::new(); - assert!(!nodelay(&mut net), "a fresh listener holds the option"); - set_nodelay(&mut net, true); - let isn = net.syn(5001); - set_nodelay(&mut net, false); - assert!(nodelay(&mut net), "a clear after the SYN reached a handshake in progress"); - net.send(5001, TcpControl::None, PEER_ISN + 1, Some(isn + 1)); - net.pass(); - set_nodelay(&mut net, false); - assert_eq!(net.accept(true, true), Accept::Take(())); - assert!(nodelay(&mut net), "a clear after the SYN reached a connection that waits"); -} - -/// **A set after a connection began is the next connection's**: the waiting -/// one keeps what it has, and the socket that listens in its place holds the -/// listener's option. -#[test] -fn a_set_after_a_connection_began_is_the_next_connections() { - let mut net = Net::new(); - established(&mut net, 5001); - set_nodelay(&mut net, true); - assert_eq!(net.accept(true, true), Accept::Take(())); - assert!(!nodelay(&mut net), "a set after the connection was established reached it"); - let mut next = tcp::Socket::new(tcp::SocketBuffer::new(vec![0; 4096]), tcp::SocketBuffer::new(vec![0; 4096])); - net.listening.open(&mut next); - assert_eq!(next.state(), tcp::State::Listen); - assert!(!next.nagle_enabled(), "the socket that listens next does not hold the listener's option"); -} - -/// **A socket its peer reset before its owner took it listens again with the -/// listener's option as it is then**, and not as it was when the reset -/// connection's SYN arrived. -#[test] -fn a_reset_connections_socket_listens_again_with_the_listeners_option() { - let mut net = Net::new(); - let isn = net.syn(5001); - set_nodelay(&mut net, true); - net.ack_and(5001, isn, TcpControl::Rst); - assert_eq!(net.socket().state(), tcp::State::Closed, "the premise: reset before it was taken"); - assert!(!nodelay(&mut net), "the premise: the reset connection began without the option"); - assert!(!net.wakes(true)); - established(&mut net, 5002); - assert_eq!(net.accept(true, true), Accept::Take(())); - assert!(nodelay(&mut net), "the connection after a reset one began without its listener's option"); -} diff --git a/userland/netstack/src/main.rs b/userland/netstack/src/main.rs index b8f90f73085..3b63bfcae2d 100644 --- a/userland/netstack/src/main.rs +++ b/userland/netstack/src/main.rs @@ -1008,20 +1008,17 @@ impl Netstack { let rx_buf = tcp::SocketBuffer::new(vec![0u8; TCP_SOCKET_BUFFER]); let tx_buf = tcp::SocketBuffer::new(vec![0u8; TCP_SOCKET_BUFFER]); let mut socket = tcp::Socket::new(rx_buf, tx_buf); - if socket.listen(port).is_err() { - msg.client.error(ERR_ADDR_IN_USE); - return; - } + let listening = listen::Listening::new(port, req.options.nodelay()); + // Before the socket is in the set, so before a SYN can reach it. A + // fresh socket and a port that is not zero: neither refusal `listen` + // has can be this one. + listening.open(&mut socket); let handle = socket_set.add(socket); let socket_id = self.alloc_id(); self.sockets.insert(socket_id, SocketKind::TcpListener(handle)); - self.piped_listeners.insert(socket_id, PipedListener { - handle, - notify_write, - listening: listen::Listening::new(port), - }); + self.piped_listeners.insert(socket_id, PipedListener { handle, notify_write, listening }); msg.client.result(&TcpBindResponse { socket_id,