On Tue, 3 Mar 2026 00:58:00 +0000
"Jasper Tran O'Leary" <[email protected]> wrote:
> This patch series adds flow steering support to the Google Virtual
> Ethernet (gve) driver. This functionality allows traffic to be directed
> to specific receive queues based on user-specified flow patterns.
>
> The series includes foundational support for extended admin queue
> commands needed to handle flow rules, the specific adminqueue commands
> for flow rule management, and the integration with the DPDK rte_flow
> API. The series adds support flow matching on the following protocols:
> IPv4, IPv6, TCP, UDP, SCTP, ESP, and AH.
>
> Patch Overview:
>
> 1. "net/gve: add flow steering device option" checks for and enables
> the flow steering capability in the device options during
> initialization.
> 2. "net/gve: introduce extended adminq command" adds infrastructure
> for sending extended admin queue commands. These commands use a
> flexible buffer descriptor format required for flow rule management.
> 3. "net/gve: add adminq commands for flow steering" implements the
> specific admin queue commands to add and remove flow rules on the
> device, including handling of rule IDs and parameters.
> 4. "net/gve: add rte flow API integration" exposes the flow steering
> functionality via the DPDK rte_flow API. This includes strict
> pattern validation, rule parsing, and lifecycle management (create,
> destroy, flush). It ensures thread-safe access to the flow subsystem
> and proper resource cleanup during device reset.
>
> Jasper Tran O'Leary (2):
> net/gve: add adminq commands for flow steering
> net/gve: add rte flow API integration
>
> Vee Agarwal (2):
> net/gve: add flow steering device option
> net/gve: introduce extended adminq command
>
> doc/guides/nics/features/gve.ini | 12 +
> doc/guides/nics/gve.rst | 26 +
> doc/guides/rel_notes/release_26_03.rst | 1 +
> drivers/net/gve/base/gve.h | 3 +-
> drivers/net/gve/base/gve_adminq.c | 118 ++++-
> drivers/net/gve/base/gve_adminq.h | 57 +++
> drivers/net/gve/gve_ethdev.c | 83 +++-
> drivers/net/gve/gve_ethdev.h | 46 ++
> drivers/net/gve/gve_flow_rule.c | 656 +++++++++++++++++++++++++
> drivers/net/gve/gve_flow_rule.h | 65 +++
> drivers/net/gve/meson.build | 1 +
> 11 files changed, 1063 insertions(+), 5 deletions(-)
> create mode 100644 dpdk/drivers/net/gve/gve_flow_rule.c
> create mode 100644 dpdk/drivers/net/gve/gve_flow_rule.h
>
Automated review spotted a few things:
1. rte_bitmap_scan usage is incorrect.
2. Using rte_malloc where not necessary
3. Grammar in the release note.
I can fix the last one when merging, the second one is not a big issue
but would be good to have.
---
Error: Incorrect rte_bitmap_scan usage — wrong rule ID allocation (~85%
confidence)
In gve_create_flow_rule():
c
uint64_t bmp_slab __rte_unused;
...
if (rte_bitmap_scan(priv->avail_flow_rule_bmp, &flow->rule_id,
&bmp_slab) == 1) {
rte_bitmap_clear(priv->avail_flow_rule_bmp, flow->rule_id);
}
rte_bitmap_scan() writes the slab base position to pos and the slab bit pattern
to slab. The actual bit position of the first available rule is pos +
__builtin_ctzll(slab), not pos alone. The __rte_unused annotation on bmp_slab
confirms the slab value is being ignored entirely.
After the first rule allocation clears bit 0, a subsequent scan returns the
same slab base (pos=0) with bit 0 now clear in the slab. The code would again
set rule_id=0 and attempt to clear an already-clear bit, then try to create a
duplicate rule on the device.
Suggested fix:
c
uint64_t bmp_slab;
uint32_t pos;
...
if (rte_bitmap_scan(priv->avail_flow_rule_bmp, &pos,
&bmp_slab) == 1) {
flow->rule_id = pos + __builtin_ctzll(bmp_slab);
rte_bitmap_clear(priv->avail_flow_rule_bmp, flow->rule_id);
}
Warning: rte_zmalloc used for ordinary control structures
Both avail_flow_rule_bmp_mem and individual gve_flow structures are allocated
with rte_zmalloc, but they don't require DMA access, NUMA placement, or
multi-process shared memory. Standard calloc/malloc would be more appropriate
and wouldn't consume limited hugepage resources.
Warning: Release notes tense inconsistency
rst
* Added application-initiated device reset.
+ * Add support for receive flow steering.
"Added" (past tense) vs "Add" (imperative). Both entries should use the same
tense for consistency. The DPDK convention is imperative mood.
