Keep NAT tuples exclusive across overlapping pools

.gitignore

@@ -1,5 +1,8 @@
 **.profraw
 **/__fuzz__/**
+# libfuzzer writes one of these per worker into the working directory when
+# `just fuzz` is given -j; the corpus itself lives under __fuzz__.
+fuzz-*.log
 # qemu-user core dumps from SIGABRT under emulated tests.
 **/qemu_*.core
 result*

acl-filter/src/tests.rs

@@ -935,6 +935,10 @@ mod end_to_end {
             static_nat_writer.get_reader(),
         ));
 
+        // Share the allocator between port forwarding and masquerade.
+        let mut allocator = NatAllocatorWriter::new();
+        allocator.update_nat_allocator(MasqueradeConfig::new(overlay.vpc_table()), 1, &flow_table);
+
         // Port forwarding
         let mut portfw_writer = PortFwTableWriter::new();
         portfw_writer
@@ -944,11 +948,10 @@ mod end_to_end {
             "port-forwarder",
             portfw_writer.reader(),
             flow_table.clone(),
+            allocator.get_reader(),
         ));
 
         // Masquerade (creates the related flow pair used by 'flow'-scoped replies)
-        let mut allocator = NatAllocatorWriter::new();
-        allocator.update_nat_allocator(MasqueradeConfig::new(overlay.vpc_table()), 1, &flow_table);
         pipeline = pipeline.add_stage(Masquerade::new(
             "masquerade",
             flow_table.clone(),

concurrency-macros/src/lib.rs

@@ -11,19 +11,10 @@ use syn::{
     parse_macro_input,
 };
 
-/// Resolve a path prefix for `dataplane-concurrency` in the consumer's
-/// `Cargo.toml`. Returns a token stream that resolves to the crate root,
-/// so callers can append `::stress` or `::with_loom` etc.
+/// Resolve the consumer's name for `dataplane-concurrency`.
 ///
-/// * Workspace consumer with `concurrency = { package = "dataplane-concurrency", ... }`
-///   in its `Cargo.toml`: returns `::concurrency`.
-/// * External consumer with `dataplane-concurrency = "..."` directly:
-///   returns `::dataplane_concurrency`.
-/// * `dataplane-concurrency`'s own integration tests: returns
-///   `::dataplane_concurrency` (which requires the test file to do
-///   `extern crate dataplane_concurrency;` -- cargo doesn't let a crate
-///   list itself as a regular dev-dep, but `extern crate` works in the
-///   integration test).
+/// Workspace crates use `concurrency`; external users and this crate's integration tests use
+/// `dataplane_concurrency`.
 fn concurrency_crate_path() -> TokenStream2 {
     match crate_name("dataplane-concurrency") {
         Ok(FoundCrate::Itself) => {
@@ -103,9 +94,8 @@ pub fn concurrency_mode(attr: TokenStream, item: TokenStream) -> TokenStream {
 
 /// Mark a backend-routed concurrency test.
 ///
-/// The default backend emits a flat `#[test]`; loom/shuttle emit a
-/// nested `concurrency_model::{loom,shuttle}` leaf so nextest filters
-/// can select one backend cleanly.
+/// The default backend emits a flat test. Model-checker tests get a backend-named leaf so nextest
+/// can select them safely.
 ///
 /// # Example
 ///
@@ -116,10 +106,8 @@ pub fn concurrency_mode(attr: TokenStream, item: TokenStream) -> TokenStream {
 /// }
 /// ```
 ///
-/// The function must take no arguments and return `()`. The body is
-/// captured as a closure, so it must be `Fn() + Send + Sync + 'static`
-/// (no borrows of locals, no `FnOnce`-only constructs). This matches
-/// what `loom::model` and `shuttle::check_*` require.
+/// The function must take no arguments, return `()`, and work as an
+/// `Fn() + Send + Sync + 'static` closure.
 ///
 /// # Limitations
 ///
@@ -155,16 +143,7 @@ pub fn test(_attr: TokenStream, item: TokenStream) -> TokenStream {
     }
 
     let krate = concurrency_crate_path();
-    // Default backend: flat `#[test] fn <name>() { ... }`. No nested
-    // module wrapping -- the production code path runs the body once,
-    // and there is no second backend to disambiguate from.
-    //
-    // Model-checker backends: emit `mod <fn_name> { mod concurrency_model
-    // { #[test] fn <backend>() { ... } } }`. The leaf function name
-    // identifies the active backend, so a nextest report shows entries
-    // like `some_test::concurrency_model::loom` and a filter like
-    // `-E 'test(/concurrency_model::loom$/)'` picks them out
-    // unambiguously.
+    // Backend-named leaves let nextest isolate tests that use model-checker primitives.
     quote! {
         #[cfg(not(any(feature = "loom", feature = "shuttle")))]
         #[::core::prelude::v1::test]
@@ -193,6 +172,72 @@ pub fn test(_attr: TokenStream, item: TokenStream) -> TokenStream {
                 fn shuttle() {
                     #krate::stress(|| #block);
                 }
+
+            }
+        }
+    }
+    .into()
+}
+
+/// Give a test a backend-named leaf without wrapping its body in `stress`.
+///
+/// Use this when a generator is the outer loop and invokes `stress` per generated case:
+///
+/// ```ignore
+/// bolero::check!().with_type().cloned().for_each(|scenario: Scenario| {
+///     concurrency::stress(move || scenario.run());   // one exploration per generated shape
+/// });
+/// ```
+///
+/// The leaf is named `plain`, `loom`, or `shuttle`, allowing the same nextest filters used by
+/// [`macro@test`].
+#[proc_macro_attribute]
+pub fn model_test(_attr: TokenStream, item: TokenStream) -> TokenStream {
+    let func = parse_macro_input!(item as ItemFn);
+
+    let attrs = &func.attrs;
+    let sig = &func.sig;
+    let block = &func.block;
+    let fn_name = &sig.ident;
+
+    if let Some(asyncness) = sig.asyncness {
+        return syn::Error::new_spanned(
+            asyncness,
+            "#[concurrency::model_test] does not support async functions yet",
+        )
+        .to_compile_error()
+        .into();
+    }
+    if !sig.inputs.is_empty() {
+        return syn::Error::new_spanned(
+            &sig.inputs,
+            "#[concurrency::model_test] functions must take no arguments",
+        )
+        .to_compile_error()
+        .into();
+    }
+
+    quote! {
+        #[allow(non_snake_case)]
+        mod #fn_name {
+            use super::*;
+            mod concurrency_model {
+                use super::*;
+
+                #[cfg(feature = "loom")]
+                #[::core::prelude::v1::test]
+                #(#attrs)*
+                fn loom() #block
+
+                #[cfg(feature = "shuttle")]
+                #[::core::prelude::v1::test]
+                #(#attrs)*
+                fn shuttle() #block
+
+                #[cfg(not(any(feature = "loom", feature = "shuttle")))]
+                #[::core::prelude::v1::test]
+                #(#attrs)*
+                fn plain() #block
             }
         }
     }

concurrency/src/macros.rs

@@ -160,4 +160,4 @@ macro_rules! with_std {
     ($($item:item)*) => {};
 }
 
-pub use concurrency_macros::{concurrency_mode, test};
+pub use concurrency_macros::{concurrency_mode, model_test, test};

dataplane/src/packet_processor/mod.rs

@@ -122,6 +122,7 @@ pub(crate) fn start_router<Buf: PacketBufferMut>(
             "port-forwarder",
             portfw_factory.handle(),
             flow_table_clone.clone(),
+            natallocator_factory.handle(),
         );
         let pkt_stats_nf = PacketStatsNF::new(pkt_stats.clone());
 

development/code/running-tests.md

@@ -62,8 +62,75 @@ change this.
 The major downside is that these processes are very computationally intensive and can take a long time to run.
 In fact, the [afl] fuzzer runs until you terminate it.
 
+## Running a real fuzzing campaign
+
+To run a target under [libfuzzer], which is coverage guided and explores far deeper than the random
+driver the test suite uses, list the targets and pick one:
+
+```shell
+just fuzz-list -p dataplane-nat
+just fuzz 'masquerade::apalloc::region::bolero_tests::decompose_properties' 10min -p dataplane-nat
+```
+
+The duration defaults to `60s`; anything after it is forwarded to `cargo bolero test`. As a sense of
+the difference, a property that manages a few thousand cases per second under `just test` reaches
+several hundred thousand per minute here, because libfuzzer mutates towards inputs that reach new
+code rather than sampling blindly.
+
+Findings are written to a `__fuzz__` directory beside the test. That directory is gitignored: the
+corpus is a local artifact that seeds later runs on the same machine, not something to commit.
+
+Pass `-j` to spread the campaign over more cores, which is the cheapest way to reach deeper:
+
+```shell
+just fuzz 'some::module::tests::some_property' 10min -p some-package -j 60
+```
+
+Each worker then writes a `fuzz-<n>.log` into the directory you ran from, rather than into
+`__fuzz__`. Those are gitignored too, and are only worth reading when a run reports a crash.
+
+### Sanitizers
+
+`cargo bolero` builds with the `fuzz` profile and links [AddressSanitizer] unless told otherwise, so
+a plain `just fuzz` is already an asan campaign. To swap sanitizers, set the same `sanitize`
+variable the rest of the justfile uses:
+
+```shell
+just sanitize=thread fuzz 'some::module::tests::some_property' 5min -p some-package
+```
+
+[ThreadSanitizer] only reports on a target that actually spawns threads, so it is worth the extra
+cost on a concurrency suite and close to pointless on a single-threaded property. It also takes
+much longer to get going, because thread instrumentation changes the ABI: `just` therefore adds
+`--build-std` for it, since a std left uninstrumented fails the build on a mismatch against `core`.
+
+A sanitizer is not free. Instrumentation costs roughly a factor of four in executions per second,
+so it is worth spending some of a campaign with none at all, reaching deeper into the input space
+in exchange for only catching what the test's own assertions catch:
+
+```shell
+just sanitize=NONE fuzz 'some::module::tests::some_property' 30min -p some-package
+```
+
+The two are complementary: asan for memory errors the assertions cannot see, `NONE` for depth.
+
+The suite as a whole can also be run under either sanitizer with the standard runner, which is what
+CI's `sanitize/fuzz/*` jobs do:
+
+```shell
+just profile=fuzz sanitize=thread test
+just profile=fuzz sanitize=address test
+```
+
+That covers far more code than a single fuzz target, but only with the brief random driver rather
+than a real campaign. The two are complementary.
+
 > [!NOTE]
-> Dedicated `just` recipes for running full fuzz campaigns (with libfuzzer/afl) are planned for a future PR.
+> `just fuzz` passes `--rustc-bootstrap`, because libfuzzer wants a nightly compiler for its
+> sanitizer coverage flags while the pinned toolchain is stable. An [afl] recipe is still to come.
+
+[AddressSanitizer]: https://clang.llvm.org/docs/AddressSanitizer.html
+[ThreadSanitizer]: https://clang.llvm.org/docs/ThreadSanitizer.html
 
 [README.md]: ../../README.md
 [afl]: https://github.com/AFLplusplus/AFLplusplus

flow-entry/src/flow_table/concurrent_fuzz.rs

@@ -296,7 +296,7 @@ impl Scenario {
 /// Drive one bolero shape per iteration through [`concurrency::stress`]:
 /// a single direct run on the std backend (real OS threads — build with
 /// `just test sanitize=thread`), or the full portfolio under shuttle.
-#[test]
+#[concurrency::model_test]
 fn stress_test_concurrency_model() {
     // Single-threaded runtime is enough: we never need the timer task to
     // run, only a context for `insert`'s `tokio::task::spawn` to succeed.

justfile

@@ -170,6 +170,32 @@ test package="tests.all" *args: (build (if package == "tests.all" { "tests.all"
     declare -r target="{{ if package == "tests.all" { "tests.all" } else { "tests.pkg." + package } }}"
     cargo nextest run --archive-file results/${target}/*.tar.zst --workspace-remap $(pwd) {{ filter }}
 
+# List the bolero targets `just fuzz` can run. Args go to `cargo bolero list`
+[script]
+fuzz-list *args="":
+    {{ _just_debuggable_ }}
+    cargo bolero list {{ _cargo_feature_flags }} {{ args }}
+
+# Fuzz one bolero target under libfuzzer. See development/code/running-tests.md
+[script]
+fuzz target time="60s" *args="":
+    {{ _just_debuggable_ }}
+    # libfuzzer wants a nightly compiler for its sanitizer coverage flags, while the
+    # pinned toolchain is stable; --rustc-bootstrap bridges that. cargo-bolero already
+    # builds with the fuzz profile and links AddressSanitizer unless told otherwise, so
+    # a plain `just fuzz` is already an asan run. Findings land in a gitignored
+    # `__fuzz__` directory beside the test.
+    #
+    # `sanitize=thread` additionally rebuilds std: thread instrumentation changes the
+    # ABI, so a std left uninstrumented fails the build on a mismatch against `core`.
+    # asan does not need that, and skipping the std rebuild keeps it far quicker.
+    # `sanitize=NONE` drops instrumentation altogether, which buys roughly four times
+    # the executions per second in exchange for only catching what the test asserts.
+    cargo bolero test '{{ target }}' --rustc-bootstrap -T '{{ time }}' \
+        {{ if sanitize != "" { "--sanitizer " + sanitize } else { "" } }} \
+        {{ if sanitize == "thread" { "--build-std" } else { "" } }} \
+        {{ _cargo_feature_flags }} {{ args }}
+
 # Build and run the criterion benches. The rte_acl benches are gated behind the
 # `dpdk` feature, so run `just features=dpdk bench` to exercise them; a plain
 # `just bench` builds them as empty `main()` and only runs the reference benches.
@@ -444,24 +470,20 @@ doctest *args:
     {{ _just_debuggable_ }}
     cargo test --doc {{ _cargo_feature_flags }} {{ _cargo_profile_flag }} {{ args }}
 
-# Run tests with code coverage. Args will be forwarded to nextest
+# Run instrumented tests and report coverage. Args are forwarded to nextest; for example,
+# `just coverage -p dataplane-nat` scopes the run to this crate.
 [script]
-coverage target="tests.all" *args: (build (if target == "tests.all" { "tests.all" } else { "tests.pkg." + target }) args)
+coverage *args:
     {{ _just_debuggable_ }}
-    declare -r target="{{ if target == "tests.all" { "tests.all" } else { "tests.pkg." + target } }}"
     export LLVM_COV="$(pwd)/devroot/bin/llvm-cov"
     export LLVM_PROFDATA="$(pwd)/devroot/bin/llvm-profdata"
-    export CARGO_LLVM_COV_TARGET_DIR="$(pwd)/target/llvm-cov"
-    export CARGO_LLVM_COV_BUILD_DIR="$(pwd)"
-    cargo llvm-cov clean
-    cargo llvm-cov show-env
-    cargo llvm-cov --no-report --branch nextest --archive-file "./results/${target}/"*.tar.zst --workspace-remap . {{ args }}
-    # NOTE: --profile="" is intentional. When collecting coverage from a nextest archive, the
-    # profile path component that cargo-llvm-cov normally expects in the profdata directory is
-    # absent. Passing an empty profile string removes that component from the lookup path so
-    # the tool can find the profdata generated by the archive run above.
-    cargo llvm-cov report --html --profile="" --output-dir=./target/nextest/coverage
-    cargo llvm-cov --branch report --codecov --profile="" --output-path=./target/nextest/coverage/codecov.json
+    declare -r out="./target/nextest/coverage"
+    cargo llvm-cov clean --workspace
+    cargo llvm-cov --no-report --branch nextest {{ args }}
+    mkdir -p "${out}"
+    cargo llvm-cov report --branch --html --output-dir="${out}"
+    cargo llvm-cov report --branch --codecov --output-path="${out}/codecov.json"
+    cargo llvm-cov report --branch --summary-only
 
 # Regenerate the dependency graph for the project
 [script]

miri.just

@@ -39,8 +39,11 @@ test *args="":
       {{ _just_debuggable_ }}
       declare -ri START_SEED="$((10#$schedule_seed))"
       declare -ri END_SEED="$((START_SEED + ${seeds}))"
+      # Parallel Miri workers cannot share an incremental compilation session.
+      declare -rx CARGO_INCREMENTAL=0
       declare MIRIFLAGS=""
-      declare RUSTFLAGS=""
+      # Environment RUSTFLAGS replace the cargo-configured flags.
+      declare RUSTFLAGS="--cfg=tokio_unstable --check-cfg=cfg(emulated) "
       MIRIFLAGS+="-Zmiri-compare-exchange-weak-failure-rate=${weak_failure_rate} "
       MIRIFLAGS+="-Zmiri-disable-isolation "
       MIRIFLAGS+="-Zmiri-many-seeds=${START_SEED}..${END_SEED} "