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 > [email protected] wrote: > > > From: Martin Spinler <[email protected]> > > > > 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" > >
