Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -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*
Expand Down
83 changes: 83 additions & 0 deletions concurrency-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,89 @@ pub fn test(_attr: TokenStream, item: TokenStream) -> TokenStream {
fn shuttle() {
#krate::stress(|| #block);
}

}
}
}
.into()
}

/// Give a test the backend-named module shape of [`macro@test`] without touching its body.
///
/// [`macro@test`] wraps the whole body in [`stress`](../dataplane_concurrency/fn.stress.html),
/// which is what you want when the body *is* the thing being model-checked. It is the wrong shape
/// when a generator has to be the outer loop:
///
/// ```ignore
/// bolero::check!().with_type().cloned().for_each(|scenario: Scenario| {
/// concurrency::stress(move || scenario.run()); // one exploration per generated shape
/// });
/// ```
///
/// Wrapping *that* in `stress` would put the whole generator campaign inside a single
/// model-checking execution, making the generator's own choices part of the explored state space.
/// So such tests call `stress` themselves, and until now paid for it by losing the backend-named
/// leaf that `just features=shuttle test` filters on: the suite compiled under the model checker
/// and was never selected to run.
///
/// This attribute emits the module shape and nothing else, leaving the body verbatim:
///
/// ```ignore
/// #[concurrency::model_test]
/// fn stress_it() { /* ... calls stress itself ... */ }
/// ```
///
/// becomes `mod stress_it { mod concurrency_model { #[test] fn <backend>() { /* body */ } } }`,
/// where `<backend>` is `loom`, `shuttle` or `plain`. Unlike [`macro@test`], the wrapper is emitted
/// on every backend, so the name does not change shape between them; there is no existing flat
/// name to keep compatible here.
#[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(
Comment on lines +237 to +250

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Find model_test functions whose signatures may lose return types or generics.
printf '%s\n' 'Return-bearing model_test functions:'
rg -nUP --glob '*.rs' \
  '#\[[^]]*model_test[^]]*\][\s\S]{0,200}\bfn\s+\w+\s*(?:<[^>{}]*>)?\s*\([^)]*\)\s*->' . || true

printf '%s\n' 'Generic model_test functions:'
rg -nUP --glob '*.rs' \
  '#\[[^]]*model_test[^]]*\][\s\S]{0,200}\bfn\s+\w+\s*<' . || true

Repository: githedgehog/dataplane

Length of output: 226


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' 'Macro implementation:'
sed -n '200,310p' concurrency-macros/src/lib.rs

printf '%s\n' 'All model_test declarations and generated leaf names:'
rg -n -C 4 'model_test|fn (loom|shuttle|plain)' --glob '*.rs' .

printf '%s\n' 'Manifest and relevant test configuration:'
rg -n -C 3 'concurrency-macros|trybuild|model_test|harness|Result' --glob 'Cargo.toml' --glob '*.rs' .

Repository: githedgehog/dataplane

Length of output: 50378


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' 'concurrency-macros/src/lib.rs lines 220-300:'
sed -n '220,300p' concurrency-macros/src/lib.rs

printf '%s\n' 'Files containing model_test:'
rg -l --glob '*.rs' 'model_test' .

printf '%s\n' 'Focused model_test contexts:'
python3 - <<'PY'
from pathlib import Path

for path in Path(".").rglob("*.rs"):
    text = path.read_text(errors="replace")
    lines = text.splitlines()
    hits = [i for i, line in enumerate(lines) if "model_test" in line]
    if hits:
        print(f"\n--- {path} ---")
        for i in hits:
            start = max(0, i - 5)
            end = min(len(lines), i + 35)
            for n in range(start, end):
                print(f"{n + 1}:{lines[n]}")
PY

printf '%s\n' 'Macro declarations and generated functions:'
rg -n -C 8 'proc_macro_attribute|pub fn model_test|fn loom|fn shuttle|fn plain|sig\.generics|sig\.output|func\.sig' concurrency-macros/src/lib.rs

Repository: githedgehog/dataplane

Length of output: 20377


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' 'concurrency-macros/src/lib.rs lines 220-300:'
sed -n '220,300p' concurrency-macros/src/lib.rs

printf '%s\n' 'Files containing model_test:'
rg -l --glob '*.rs' 'model_test' .

printf '%s\n' 'Focused model_test contexts:'
python3 - <<'PY'
from pathlib import Path

for path in Path(".").rglob("*.rs"):
    text = path.read_text(errors="replace")
    lines = text.splitlines()
    hits = [i for i, line in enumerate(lines) if "model_test" in line]
    if hits:
        print(f"\n--- {path} ---")
        for i in hits:
            for n in range(max(0, i - 5), min(len(lines), i + 35)):
                print(f"{n + 1}:{lines[n]}")
PY

printf '%s\n' 'Macro declarations and generated functions:'
rg -n -C 8 'proc_macro_attribute|pub fn model_test|fn loom|fn shuttle|fn plain|sig\.generics|sig\.output|func\.sig' concurrency-macros/src/lib.rs

Repository: githedgehog/dataplane

Length of output: 20377


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

macro = Path("concurrency-macros/src/lib.rs").read_text()
required = [
    "fn loom() `#block`",
    "fn shuttle() `#block`",
    "fn plain() `#block`",
]
for fragment in required:
    assert fragment in macro, f"missing generated leaf: {fragment}"

assert "sig.output" not in macro
assert "sig.generics" not in macro

print("Generated leaves omit sig.output and sig.generics.")

for path in Path(".").rglob("*.rs"):
    lines = path.read_text(errors="replace").splitlines()
    for i, line in enumerate(lines):
        if "#[concurrency::model_test]" in line:
            window = "\n".join(lines[i : min(len(lines), i + 8)])
            match = re.search(r"\bfn\s+\w+\s*(?:<[^>\n]*>)?\s*\([^)]*\)\s*(->[^{]+)?\s*\{", window)
            if match:
                print(f"{path}:{i + 1}: return={bool(match.group(1))}, signature={match.group(0).strip()}")
PY

Repository: githedgehog/dataplane

Length of output: 645


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

macro = Path("concurrency-macros/src/lib.rs").read_text()
for fragment in ("fn loom() `#block`", "fn shuttle() `#block`", "fn plain() `#block`"):
    assert fragment in macro, f"missing generated leaf: {fragment}"

assert "sig.output" not in macro
assert "sig.generics" not in macro

print("Generated leaves omit sig.output and sig.generics.")

for path in Path(".").rglob("*.rs"):
    lines = path.read_text(errors="replace").splitlines()
    for i, line in enumerate(lines):
        if "#[concurrency::model_test]" in line:
            window = "\n".join(lines[i : min(len(lines), i + 8)])
            match = re.search(
                r"\bfn\s+\w+\s*(?:<[^>\n]*>)?\s*\([^)]*\)\s*(->[^{]+)?\s*\{",
                window,
            )
            if match:
                print(
                    f"{path}:{i + 1}: return={bool(match.group(1))}, "
                    f"signature={match.group(0).strip()}"
                )
PY

Repository: githedgehog/dataplane

Length of output: 645


Preserve supported return types or reject them.

model_test accepts zero-argument functions but generates unit-returning leaves. A fn test() -> Result<(), E> body therefore fails when it uses ? or returns Result. Preserve sig.output for supported result tests. Otherwise reject non-default outputs, generics, and other discarded signature fields.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@concurrency-macros/src/lib.rs` around lines 237 - 250, Update the model_test
expansion around sig, block, and fn_name to preserve sig.output when generating
test leaves, including supported Result-returning functions so ? and Result
returns compile. Explicitly reject unsupported non-default return types,
generics, and other signature fields that the generated function would discard,
using targeted syn errors consistent with the existing asyncness and input
validation.

Source: Coding guidelines

&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
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion concurrency/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
69 changes: 68 additions & 1 deletion development/code/running-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion flow-entry/src/flow_table/concurrent_fuzz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
26 changes: 26 additions & 0 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,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.
Expand Down
26 changes: 26 additions & 0 deletions nat/src/masquerade/allocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,32 @@ pub enum AllocatorError {
NoPoolFound,
}

impl AllocatorError {
/// Whether this error says the space simply ran out, rather than that something is wrong.
///
/// A caller holding several allocators over disjoint space may move on to the next one when
/// this holds, and only then: any other error is about the allocator rather than about how
/// full it is, and would be buried by a later success. The classification is the one
/// [`DoneReason`] already draws, where exactly these become `NatOutOfResources`.
/// The match is exhaustive on purpose: a new error has to be classified here rather than
/// silently defaulting to one side of it.
#[must_use]
pub fn is_exhaustion(&self) -> bool {
match self {
AllocatorError::NoFreeIp
| AllocatorError::NoPortBlock
| AllocatorError::NoFreePort(_) => true,
AllocatorError::PortAllocationFailed(_)
| AllocatorError::PortReservationFailed(_)
| AllocatorError::UnsupportedProtocol(_)
| AllocatorError::MissingDiscriminant
| AllocatorError::InternalIssue(_)
| AllocatorError::Denied
| AllocatorError::NoPoolFound => false,
}
}
}

impl From<&AllocatorError> for DoneReason {
fn from(error: &AllocatorError) -> Self {
match error {
Expand Down
Loading
Loading