Re: [PATCH v5 0/6] net/nfb: code cleanup

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon,  2 Feb 2026 20:33:24 +0100
spinler@cesnet.cz wrote:

> From: Martin Spinler <spinler@cesnet.cz>
> 
> This patchset mainly cleans up the code and prepare it for another
> quite large rework. Also it resolves some unpleasant behavior.
> 
> ---
> v5:
> * Rebased to next-net-main
> 
> Martin Spinler (6):
>   net/nfb: use constant values for max Rx/Tx queues count
>   net/nfb: fix bad pointer access in queue stats
>   net/nfb: update timestamp calculation to meaningful value
>   net/nfb: use process private variable for internal data
>   net/nfb: release allocated resources correctly
>   net/nfb: stop only started queues in fail path
> 
>  doc/guides/nics/nfb.rst      |   6 +-
>  drivers/net/nfb/nfb.h        |  14 ++-
>  drivers/net/nfb/nfb_ethdev.c | 166 +++++++++++++++++++++--------------
>  drivers/net/nfb/nfb_rx.c     |   2 +-
>  drivers/net/nfb/nfb_rx.h     |  13 +--
>  drivers/net/nfb/nfb_rxmode.c |  12 +--
>  drivers/net/nfb/nfb_stats.c  |  46 +++++-----
>  drivers/net/nfb/nfb_tx.c     |   2 +-
>  8 files changed, 152 insertions(+), 109 deletions(-)
> 

AI patch review summary.  My comments afterward,

> Here's the review. The series is in good shape overall — no errors found across all six patches. The main items worth raising with Martin:
> Patch 2: The new queue iteration still doesn't NULL-check the per-queue pointer before dereferencing. Given the commit message specifically calls out the pointer validity problem, this seems like an oversight.
> Patch 4: rte_zmalloc_socket for a small control structure of file handles is using limited hugepage memory unnecessarily — plain calloc would suffice.
> Patch 5: The removal of dev_stop from close relies on the ethdev layer calling stop first. That's fine for modern DPDK but worth a sanity check. Similarly, the dev_started = 0 removal is correct if the minimum supported DPDK version manages that flag in the ethdev layer.
> Everything else — commit messages, tags, formatting, error paths, the timestamp arithmetic, the start error unwinding — looks clean.

Ignore the comment on 5.

Patch 4 raises a good point, file handles are per-process so doesn't need to be in hugepages.

Not sure about comment about Patch #2. The queues are set to null on close. So there might be small race with another process getting stats.

I needed to do some manual application to workaround patch fuzz.
Then queued to next-net.