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 new file mode 100644 index 00000000000..b99e8c9cc8e --- /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,73 @@ +--- +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. + +`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, +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; 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`) 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**: 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 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 +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/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 e203fc97284..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 @@ -7,23 +7,26 @@ 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` 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 -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/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/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 6af94f946c7..46af3557386 100644 --- a/issues/toyos-has-its-own-network-stack.md +++ b/issues/toyos-has-its-own-network-stack.md @@ -44,7 +44,7 @@ What the node does not yet meet: - `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. - `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 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/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/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/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/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 new file mode 100644 index 00000000000..ffe92985b23 --- /dev/null +++ b/tests/toyos-rust-tests/src/bin/nodelay_accepted.rs @@ -0,0 +1,95 @@ +//! `TCP_NODELAY` as std answers it and as netstack's pipe ABI carries a +//! listener's. +//! +//! 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 +//! 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"; +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. +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"); + the_pipe_abis_listener(port(pipe_port), &mut said); + 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..e51359f5cc2 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, +/// 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; @@ -168,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 { @@ -185,11 +193,20 @@ ipc_payload! { pub socket_id: u32, } + /// 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, + } + 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 +239,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 +257,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 +290,7 @@ pub struct TcpAccepted { pub remote_addr: [u8; 4], pub remote_port: u16, pub local_port: u16, + pub options: TcpOptions, } pub struct UdpBound { @@ -443,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)?; @@ -451,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()?; @@ -475,6 +511,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 +533,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 +698,47 @@ mod tests { Err(IpcError::Malformed) )); } + + /// 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. + #[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..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, @@ -61,10 +61,12 @@ 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: + // 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, } @@ -248,7 +250,7 @@ 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; } }; @@ -290,9 +292,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 +301,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 +317,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 +587,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/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 f4ba715cd8a..edf54605204 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,26 @@ //! 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, 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; -/// 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. @@ -34,20 +47,37 @@ pub enum Accept

{ } impl Listening { - pub fn new(port: u16) -> Self { - Self { port, woken: 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 { 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 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 +86,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..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, diff --git a/userland/netstack/src/main.rs b/userland/netstack/src/main.rs index bb763e574e1..3b63bfcae2d 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) { @@ -989,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, @@ -1043,7 +1059,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 +1075,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, }); }