diff --git a/.github/workflows/package-freebsd.yml b/.github/workflows/package-freebsd.yml index 34d95302..e3c258ee 100644 --- a/.github/workflows/package-freebsd.yml +++ b/.github/workflows/package-freebsd.yml @@ -99,7 +99,7 @@ jobs: cargo build --release # The only place the FreeBSD cfg arms' unit tests ever run in - # CI — the main CI matrix is Linux-only, and a release build + # CI — the main CI matrix runs no FreeBSD job, and a release build # compiles no #[cfg(test)] code (AF-prefix strip round-trips, # platform module, config path gates). # @@ -116,7 +116,18 @@ jobs: # This bounds the damage; it does not fix anything. A test that can # block for ever is a defect at the test, and the ones this suite # has are bounded where they are written. - if ! timeout -s KILL 900 cargo test; then + # + # The tests that need a TCP address whose SYNs go unanswered + # (`testutil::Blackhole`) run in a second pass. FreeBSD resets a + # connect to a full accept queue, so it gets that address only + # from the kernel's blackhole settings, and those also stop every + # refused connect on the host, which other tests need. So the + # first pass skips them and the second runs only them, with the + # settings on. + BLACKHOLE_TESTS="node::tests::rx_stall:: transport::tcp::tests::background_connect_timeout_is_counted" + skips="" + for t in $BLACKHOLE_TESTS; do skips="$skips --skip $t"; done + if ! timeout -s KILL 900 cargo test -- $skips; then echo "FAIL: cargo test failed, or did not finish within its 900s bound." >&2 echo " If the output above stops mid-run, look for libtest's" >&2 echo " 'has been running for over' lines: they name the test" >&2 @@ -124,6 +135,16 @@ jobs: echo " are the rest of the answer." >&2 exit 1 fi + bh_was=$(sysctl -n net.inet.tcp.blackhole) + bh_local_was=$(sysctl -n net.inet.tcp.blackhole_local) + sysctl net.inet.tcp.blackhole=2 net.inet.tcp.blackhole_local=1 + bh_rc=0 + timeout -s KILL 300 cargo test --lib -- $BLACKHOLE_TESTS || bh_rc=$? + sysctl net.inet.tcp.blackhole="$bh_was" net.inet.tcp.blackhole_local="$bh_local_was" + if [ "$bh_rc" -ne 0 ]; then + echo "FAIL: the blackhole pass failed, or did not finish within its 300s bound." >&2 + exit 1 + fi packaging/freebsd/build-pkg.sh \ --version "$FREEBSD_PACKAGE_VERSION" \ diff --git a/src/node/tests/rx_stall.rs b/src/node/tests/rx_stall.rs index b0833b87..32a041ee 100644 --- a/src/node/tests/rx_stall.rs +++ b/src/node/tests/rx_stall.rs @@ -24,12 +24,13 @@ //! frames off the socket and answers them with the XX messages a peer would //! send. //! -//! The unanswered SYN is constructed locally: a listener with a backlog of -//! zero whose single accept slot is already taken. Linux drops further SYNs to -//! a listener whose accept queue is full, so a connect to it times out rather -//! than being refused. `Blackhole::silent()` checks that before any test -//! relies on it, which is what lets a regression show up at its real size: -//! one connect timeout per reply, counted in `connect_timeouts`. +//! The unanswered SYN is constructed locally by `Blackhole`: a listener whose +//! accept queue is full, which Linux, macOS and Windows leave unanswered, or +//! on FreeBSD a bound port with no listener and the kernel's blackhole +//! settings on (see `Blackhole`). A connect to it times out rather than being +//! refused. `Blackhole` checks that before any test relies on it, which is +//! what lets a regression show up at its real size: one connect timeout per +//! reply, counted in `connect_timeouts`. //! //! The tests print their measurements; run with `--nocapture` to see them. @@ -238,8 +239,8 @@ async fn prime_link(node: &Node, bh: &Blackhole) -> TcpStream { accept_end(bh) } -/// Close the node's connection to `bh` from the far end, after taking the -/// listener's accept slot so that any later dial to the address hangs. +/// Close the node's connection to `bh` from the far end, after filling `bh` +/// so that any later dial to the address hangs. async fn kill_link(node: &Node, bh: &mut Blackhole, far_end: TcpStream) { bh.fill(); drop(far_end); @@ -695,8 +696,8 @@ async fn msg1_resend_to_dead_outbound_leg_recovers_after_background_connect() { "no background connect toward the dial address" ); - // The address starts answering: empty the accept queue, and the - // background connect's retransmitted SYN completes. + // The address starts answering (`Blackhole::drain`), and the background + // connect's retransmitted SYN completes. let _filler_ends = bh.drain(); let start = Instant::now(); let mut tick = 1; diff --git a/src/testutil.rs b/src/testutil.rs index 442890d7..eda43a41 100644 --- a/src/testutil.rs +++ b/src/testutil.rs @@ -99,12 +99,22 @@ pub(crate) async fn wait_until bool>(mut f: F, limit: std::time::D /// A local TCP address whose SYNs go unanswered once filled. /// /// A listener with a backlog of one whose accept queue is filled: Linux, -/// macOS and the BSDs drop further SYNs to a listener whose accept queue is +/// macOS and Windows drop further SYNs to a listener whose accept queue is /// full, so a connect to it times out rather than completing or being /// refused. How many connects the queue takes before it is full differs by /// kernel (one on Linux, more on macOS), so filling stops at the first /// connect that times out. The listener and the fillers must be kept alive /// for as long as that is relied on. +/// +/// FreeBSD answers a SYN to a full queue from its syncache and then resets +/// the connection, so its queue never goes silent. There, filling replaces +/// the listener with a socket bound to the same address that does not +/// listen, and the kernel must be set not to reset a SYN to a port nobody +/// listens on: `net.inet.tcp.blackhole=2` and `net.inet.tcp.blackhole_local=1` +/// (root). Those settings also stop every refused connect on the host, so a +/// test that needs one cannot run in the same pass; the FreeBSD package +/// workflow runs the tests that use a filled `Blackhole` in a pass of their +/// own. pub(crate) struct Blackhole { pub(crate) listener: socket2::Socket, fillers: Vec, @@ -143,6 +153,7 @@ impl Blackhole { /// Fill the listener's accept queue, stopping at the first connect that /// times out, which shows a further connect now times out instead of /// completing or being refused. + #[cfg(not(target_os = "freebsd"))] pub(crate) fn fill(&mut self) { const MAX_FILLERS: usize = 64; for _ in 0..=MAX_FILLERS { @@ -163,6 +174,7 @@ impl Blackhole { /// answers again: the next connect to it, or the next retransmitted SYN /// of one already waiting, completes. Returns the accepted far ends, /// which the caller keeps alive while it relies on that. + #[cfg(not(target_os = "freebsd"))] pub(crate) fn drain(&mut self) -> Vec { self.fillers .iter() @@ -170,6 +182,37 @@ impl Blackhole { .collect() } + /// Close the listener and bind a socket that does not listen to the same + /// address, then check that a connect to it now times out. Connections + /// the listener already accepted are left as they are. + #[cfg(target_os = "freebsd")] + pub(crate) fn fill(&mut self) { + use socket2::{Domain, Socket, Type}; + let quiet = Socket::new(Domain::IPV4, Type::STREAM, None).unwrap(); + quiet.set_reuse_address(true).unwrap(); + drop(std::mem::replace(&mut self.listener, quiet)); + self.listener.bind(&self.addr.into()).unwrap(); + let probe = + std::net::TcpStream::connect_timeout(&self.addr, std::time::Duration::from_millis(200)); + match probe { + Err(e) if e.kind() == std::io::ErrorKind::TimedOut => {} + other => panic!( + "blackhole is not silent: probe connect returned {other:?}; on FreeBSD \ + this needs net.inet.tcp.blackhole=2 and net.inet.tcp.blackhole_local=1, \ + and a test using it belongs in package-freebsd.yml's blackhole pass" + ), + } + } + + /// Start listening on the bound socket, so the address answers again: the + /// next connect to it, or the next retransmitted SYN of one already + /// waiting, completes. Nothing was queued, so there are no far ends. + #[cfg(target_os = "freebsd")] + pub(crate) fn drain(&mut self) -> Vec { + self.listener.listen(1).unwrap(); + Vec::new() + } + /// The address in the transport form. pub(crate) fn transport_addr(&self) -> crate::transport::TransportAddr { crate::transport::TransportAddr::from_string(&self.addr.to_string())