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

Stephen Hemminger
2026-02-03T01:50:26Z

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.