Conversation
This commit forces timeout check of all flows in the flow table at the shutdown stage of Suricata. Gathering of capture-bypassed flow statistics was left to the bypass capture method via BypassUpdate callback. Until now, capture-bypassed flows that did not timeout had their statistics unchecked in the period between last check and shutdown. This commit forces gathering of statistics from these flows. Ticket: 8440
This change prevents capture-bypassed flows to be removed from the flow table by a worker thread. If a flow like this is de-initialized any other way than how FlowManager handles de-initialization of capture-bypassed flows, the underlying bypass method is not aware that it should not filter-out the flow anymore (e.g. EBPF map is not updated). This can lead to a resource leak, such as EBPF map being fully filled out and bypass being incapable of filtering any new flows. Until now, this issue happened in a case when capture-bypassed flow has reached its timeout (as in suricata.yaml flow-timeouts.x.bypassed), but worker thread was the first who pre-emptively de-initialized the flow, before FlowManager could perform proper de-initialization. Ticket: 8442
Decreased default sleep of BypassManager thread from 10ms to 10us, but increased the number of sleep loops adequately to keep the same total sleep time of 10ms. Add configurable option to set number of sleep iterations.
This feature brings capture offload into the DPDK Suricata runmode. The offload is based on the DPDK's rte_flow rules, mainly the drop and count actions. The bypass utilizes the BypassManager thread and API. The number of flows offloaded to the NIC differs based on the used NIC, the current version was tested on only Mellanox ConnectX-6 and supports offload of around 2 millions flows. The real number of rte_flow rules inserted into the NIC is double of the offloaded flows (so around 4 millions rules in this case), because rte_flow needs 2 separate rules to match both directions of a flow. All of the flows that cannot be bypassed in the NIC are offloaded locally in Suricata. The process of applying the bypass is as follows: the workers enqueue a flow_key created from the bypassed flow into a rte_ring, the BypassManager constantly polls the ring, tries to create and insert the respective rte_flow rule into the NIC. The Flow Manager is in the meantime responsible for checking the activity of the flows, performed by reading the counters attached to the rte_flow rules, and potentially destroying the rules in the case the flow timeouts. The statistics from the flows are collected periodically (managed by Flow Manager) and in the shutdown stage and they are captured in eve.json and stats.log. The feature can be toggled on/off and the maximum numbers of capture-bypassed flows (up to NICs maximum) can be set in suricata.yaml. Ticket: 7871
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16203 +/- ##
==========================================
- Coverage 83.15% 82.94% -0.21%
==========================================
Files 1004 1007 +3
Lines 277688 278367 +679
==========================================
- Hits 230918 230904 -14
- Misses 46770 47463 +693
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
While it is still in your mind-cache, can you expand the "ci: remove dpdk headers from scan-build" commit message with an explanation and reasons of why you do that? Can you remind me if the issue found with the DPDK-patchable or not? |
There was a problem hiding this comment.
🟡 Changes recommended
Hardware-mode validation, rule lifecycle, accounting, concurrency, and statistics schema issues can cause crashes, traffic loss, or incorrect telemetry.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds DPDK hardware flow bypass using mlx5 rte_flow rules, integrated with Suricata’s bypass and flow managers.
Changes:
- Adds dynamic bidirectional hardware bypass and accounting.
- Adds configuration, lifecycle, statistics, and shutdown handling.
- Updates documentation, schema, build files, and CI.
File summaries
| File | Description |
|---|---|
suricata.yaml.in |
Adds bypass configuration examples. |
src/util-dpdk.h |
Defines shared DPDK resources. |
src/util-dpdk.c |
Manages DPDK resource lifecycle. |
src/util-dpdk-rte-flow.h |
Declares bypass APIs. |
src/util-dpdk-rte-flow.c |
Implements hardware bypass rules. |
src/util-dpdk-rte-flow-structs.h |
Defines bypass state and counters. |
src/util-dpdk-rss.h |
Adds RSS group selection. |
src/util-dpdk-rss.c |
Creates grouped RSS rules. |
src/util-dpdk-mlx5.h |
Exposes mlx5 post-start setup. |
src/util-dpdk-mlx5.c |
Configures jump and RSS rules. |
src/util-dpdk-ixgbe.c |
Uses the default RSS group. |
src/util-dpdk-common.h |
Exposes mempool cache sizing. |
src/util-dpdk-common.c |
Implements cache-size calculation. |
src/util-device.c |
Reports bypassed packet statistics. |
src/util-device-private.h |
Includes expanded DPDK resources. |
src/suricata.c |
Registers DPDK live-device extensions. |
src/source-dpdk.h |
Adds bypass configuration state. |
src/source-dpdk.c |
Integrates packet callbacks and metrics. |
src/runmode-dpdk.h |
Adds bypass configuration metadata. |
src/runmode-dpdk.c |
Initializes dynamic bypass. |
src/Makefile.am |
Builds the new implementation. |
src/flow-private.h |
Adds the shutdown flow flag. |
src/flow-manager.c |
Collects bypass statistics at shutdown. |
src/flow-hash.h |
Exposes existing-flow lookup. |
src/flow-hash.c |
Adjusts bypass timeout handling. |
src/flow-bypass.c |
Makes manager polling configurable. |
etc/schema.json |
Adds DPDK statistics schema entries. |
doc/userguide/capture-hardware/dpdk.rst |
Documents dynamic bypass. |
configure.ac |
Enables offload support for DPDK. |
.github/workflows/scan-build.yml |
Excludes DPDK headers from scan-build. |
Review details
Suppressed comments (2)
src/util-dpdk-rte-flow.c:520
- The configured dequeue size is stored as
uint32_tbut truncated to 16 bits here. For example, a documented-valid burst of 65536 with a larger ring becomes zero, so the bypass manager never dequeues any flow. Keep the configured width through the DPDK call.
uint16_t ring_dequeue_num = rte_flow_bypass_data->rte_ring_dequeue_burst_size;
src/source-dpdk.c:371
- The read-then-subtract reset is not atomic as a transaction. With multiple enabled interfaces, two queue-0 threads can read the same shared occupancy value and both subtract it, underflowing these unsigned counters and corrupting future averages. Drain each accumulator with an atomic exchange/CAS, or ensure only one thread owns the shared statistics.
SC_ATOMIC_SUB(rte_flow_bypass_data->rte_bypass_ring_occupancy, ring_occupancy);
SC_ATOMIC_SUB(rte_flow_bypass_data->rte_bypass_ring_ops, ring_ops);
- Files reviewed: 30/30 changed files
- Comments generated: 13
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (ldev->dpdk_vars->rte_flow_bypass_data != NULL && | ||
| ldev->dpdk_vars->rte_flow_bypass_data->bypass_mp != NULL) { | ||
| rte_mempool_free(ldev->dpdk_vars->rte_flow_bypass_data->bypass_mp); | ||
| ldev->dpdk_vars->rte_flow_bypass_data->bypass_mp = NULL; | ||
| } |
|
@adaki4 this needs a rebase now |
|
Continues in #16288 |
DPDK dynamic bypass with rte_flow rules
This feature brings capture offload into the DPDK Suricata
runmode. The offload is based on the DPDK's rte_flow hardware rules,
which can filter traffic directly in the NIC.
The bypass utilizes the BypassManager thread and API for
creating bypass rules and the FlowManager for collecting statistics and destroying
rules.
Related branches
These commits assure the proper working of the bypass implementation, but they originate in different branches, that have not been merged yet.
Changes
Link to ticket: #7871
Previous PR: #15877