[PATCH v2 0/4] net/gve: add flow steering support

[Jasper Tran O'Leary]

Thank you for these notes. I submitted a v3 with the following changes.

1. Fixed bit allocation in the flow rule bitmap using the suggestion given.
2. Changed rte_zmalloc/rte_free for struct gve_flow to calloc/free. In
making this change, I realized that the driver currently would not support
flow operations from secondary processes, so I made a note of that in
gve.rst.
3. Changed both verbs to "added" to match the notes from other drivers.

On Tue, Mar 3, 2026 at 7:21 AM Stephen Hemminger <stephen at networkplumber.org>
wrote:

> On Tue,  3 Mar 2026 00:58:00 +0000
> "Jasper Tran O'Leary" <jtranoleary at google.com> 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.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mails.dpdk.org/archives/dev/attachments/20260303/ede86574/attachment.htm>