Flowtable concurrent fuzz - #1581
Conversation
0ba4a4f to
714addf
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughExposes concurrency::stress (with module docs) and a shuttle_config helper; updates shuttle tests to use explicit shuttle::Runner; adds bolero generators and FlowInfo test constructor; introduces a bolero-driven concurrent FlowTable fuzz test; removes FlowTable::drain_stale. ChangesConcurrency Testing & FlowTable Fuzz Coverage
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
a311161 to
d24fc00
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds bolero-driven concurrent fuzz coverage for FlowTable and refactors shuttle runner configuration so bolero+shuttle suites can share the same “standard” shuttle Config (notably the increased stack size).
Changes:
- Add a new concurrent bolero stress test module for
FlowTable(flow-entry/src/flow_table/concurrent_fuzz.rs) and wire it into the test build. - Add
bolero::TypeGeneratorsupport fornet::flows::FlowStatusbehindcfg(test)/feature = "bolero". - Introduce and re-export
concurrency::shuttle_config()and update an existing shuttle fuzz test to use it.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
net/src/flows/flow_info.rs |
Adds a bolero TypeGenerator impl for FlowStatus under test/feature gating. |
net/Cargo.toml |
Changes dataplane-net default features to include bolero. |
flow-entry/src/flow_table/table.rs |
Removes drain_stale and tweaks assertions in a concurrency test. |
flow-entry/src/flow_table/mod.rs |
Registers the new concurrent_fuzz test module. |
flow-entry/src/flow_table/concurrent_fuzz.rs |
Adds a new bolero + concurrency::stress concurrent fuzz/stress test for FlowTable. |
concurrency/tests/quiescent_shuttle.rs |
Switches shuttle fuzz tests to use an explicit Runner with shared config. |
concurrency/src/stress.rs |
Adds shuttle_config() and reuses it inside stress()’s shuttle backend. |
concurrency/src/lib.rs |
Re-exports stress as supported API and exposes shuttle_config() under shuttle cfg. |
Cargo.lock |
Dependency lockfile updates (crate version bumps). |
Comments suppressed due to low confidence (1)
flow-entry/src/flow_table/table.rs:262
confidence: 8
tags: [docs]
`drain_stale` was removed, but `insert_common` still documents a safety argument in terms of “drain_stale’s stale condition …” (see the comment around `table.insert(...)`). That reference is now misleading and should be updated to describe the behavior without mentioning the deleted method.
/// Lookup a flow in the table.
///
/// # Panics
///
/// Panics if this thread already holds the read lock on the table.
pub fn lookup<Q>(&self, flow_key: &Q) -> Option<Arc<FlowInfo>>
</details>
sergeymatov
left a comment
There was a problem hiding this comment.
For me LGTM, we need to address [bolero] as non-dev dependency
80f0e39 to
155ff5f
Compare
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Sergey Matov <sergey.matov@githedgehog.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
So bolero x model-checker suites can drive the same scheduler portfolio and 4 MiB-stack config as #[concurrency::test] instead of hand-rolling a Runner with shuttle's defaults. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
One source of truth for the stack size; the per-shape Runner was silently using shuttle's ~60 KiB default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
A single harness covers both the thread sanitizer and the shuttle portfolio. Normalizing at generation keeps two threads inserting, so PCT always sees real concurrency and no shape is skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Only the fuzz harness ever called it, and it cannot run under shuttle (DashMap::retain holds a real shard lock across a yield point). Also switches two reshard-test asserts to assert_eq! for clippy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
new_with_status delegates to new instead of duplicating fields; the FlowStatus generator rejects out-of-range bytes rather than aliasing them to Detached. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
155ff5f to
d291dc6
Compare
yup, that was just me committing by accident. I only enabled it to help my IDE and never intended that for merge |
Keep state machine to change NAT status for flows for `FlowTable` scenario. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergey Matov <sergey.matov@githedgehog.com>
c13f66f to
97eeb5f
Compare
This is a temp placeholder while I discuss with @sergeymatov. Ignore for now if you aren't @sergeymatov
This PR is a temporary placeholder while the author discusses changes with
@sergeymatov; reviewers other than@sergeymatovare asked to ignore it for now. It adds a FlowTable concurrency fuzz test and supporting concurrency-harness adjustments, and removes a FlowTable API incompatible with shuttle-based testing.Summary
New bolero-driven concurrent fuzz test for FlowTable (cfg(test) and cfg(not(feature = "loom"))):
Concurrency harness changes and exports:
FlowTable API and tests:
Bolero test utilities for FlowInfo:
Notes, testing, and metadata
Conventional commits