Re: [PATCH v4 0/8] net/nfb: rework to real multiport

Martin Spinler
2026-01-23T17:34:06Z

Thanks for the review, my v5 changes are described in the cover letter.

In contrast (what is not changed):
- no patch is suitable for backporting
- NULL checks was just imagination of AI I think
- 64 ports ought to be enough for anybody -> reworked,
imho much cleaner code now

My reaction to Claude:
- AI is quiet good for commit messages generated from context
when I'm stuck (1/8) as well for doc (8/8).
- Memory leak is in patch 4/8, not in 2/8. But yes,
it was here and it was my fault.
- RTE_EXPORT_INTERNAL_SYMBOL seems necessary (sym_chk fails without it)
and the __rte_internal was already there.
- Prefixing in the TAILQ_HEAD(pmd_internals_head, pmd_internals):
it is bit misleading to me as it just defines struct
inside .cfile, anyway, I added the nfb_ prefix.
- Went through the rte_mallocs and they are used in
process-shared contexts, changed to calloc just in one case.

My reaction to Grok:
- I think just the Code duplication hint was relevant,
- as well as the Port selection logic.

On Thu, 2026-01-22 at 17:14 -0800, Stephen Hemminger wrote:
> On Thu, 22 Jan 2026 08:27:11 +0100
> spinler@cesnet.cz wrote:
>
> > From: Martin Spinler <spinler@cesnet.cz>
> >
> > This series implements real multiport for better user experience.
> >
> > The existing driver creates one ethdev/port for one PCI device.
> > As the CESNET-NDK based cards aren't capable to represent each
> > Ethernet port by own PCI device, new driver implementation
> > processes real port configuration from firmware/card and switches
> > from rte_eth_dev_pci_generic_probe to multiple rte_eth_dev_create calls.
> >
> > ---
>
> Wading through all the AI review...
>
> Things you should fix:
> - make sure all calls to snprintf() have overflow checks
> - clarity about what should be backported (Fixes/stable)
> - make sure all calls to rte_malloc et al have checks for NULL
>
> Things you could fix:
> - using malloc vs rte_malloc for data structures not shared
> not a big issue; but feel free to change.
> - avoiding code duplication
> - check prefix of global variables
> - mark driver only API's as __rte_internal
>
> Things I can fix when merging:
> - am willing to reword commit messages for readability as needed
>
> Things I don't care about:
> - release notes only have to be right after series, no need for per-patch
> - looks like all new functions are internal only, not sure why review wants tests.
> - assume 64 bit port mask is a hardware limit, you won't go over in future.
> - driver internal structures do not need doxygen comments
> - don't care about any AI warnings like "if you change X in the future it will break"
>
>