Hi, > -----Original Message----- > From: Stephen Hemminger <[email protected]> > Sent: Tuesday, June 23, 2026 9:17 PM > To: Gagandeep Singh <[email protected]> > Cc: [email protected]; Hemant Agrawal <[email protected]> > Subject: Re: [PATCH v3 0/9] ENETC driver related changes series > > On Tue, 23 Jun 2026 11:29:55 +0530 > Gagandeep Singh <[email protected]> wrote: > > > V3 changes: > > - Added documentation for all devargs in enetc4.rst. > > - Fixed kvlist memory leak issue. > > > > V2 changes: > > - Fixed an un-used variable compilation issue reported on fedora:43-gcc- > minsize > > - Fixed various AI reported issues: > > - Release notes updated for all new devargs > > - enect4.ini features doc updated for scattered RX. > > - removed Not required RTE_PTYPE_UNKNOWN. > > - Fixed mid-frame mbuf leak in SG case. > > - Enabled SG for enetc4 PF also. > > - move to calloc from rte_zmalloc in parse_txq_prior(). > > - added vaidation checks on strdup, strtoul. > > - added NC devargs to use cacheable ops conditionally. > > - removed dead code like bd_base_p etc. > > - Fixed rte_cpu_to_le_16() conversion on flags and combined > > all flags related patches in one patch. > > - Fixed memory leak issue due to TXQ priority patch. > > - There were some false positives, I have ignored them: > > Race condition on flags field: > > clean_tx_ring only touches HW-completed BDs > (next_to_clean→hwci), > > never newly-submitted BDs; doorbell hasn't fired yet. > > Missing dcbf in clean_tx_ring: > > DPDK is single-threaded per queue; TX path always overwrites > > flags completely before dcbf. > > TX dcbf granularity with wrap: > > Safe (AI admits it). > > RX refill flush at wrap: > > In-loop dcbf at i & mask == 0 already flushes aligned groups; > > trailing flush only needed for partial groups. > > RX reading before invalidate: > > dccivac precedes the read for every group in the loop > > > > Gagandeep Singh (7): > > net/enetc: fix TX BD structure > > net/enetc: fix queue initialization > > net/enetc: support ESP packet type in packet parsing > > net/enetc: update random MAC generation code > > net/enetc: add option to disable VSI messaging > > net/enetc: add devargs to control VSI-PSI timeout and delay > > net/enetc4: add cacheable BD ring support with SW cache maintenance > > > > Vanshika Shukla (2): > > net/enetc: support scatter-gather > > net/enetc: set user configurable priority to TX rings > > > > doc/guides/nics/enetc4.rst | 62 +++- > > doc/guides/nics/features/enetc4.ini | 1 + > > doc/guides/rel_notes/release_26_07.rst | 10 + > > drivers/net/enetc/base/enetc_hw.h | 13 +- > > drivers/net/enetc/enetc.h | 31 +- > > drivers/net/enetc/enetc4_ethdev.c | 172 ++++++++-- > > drivers/net/enetc/enetc4_vf.c | 206 ++++++++++-- > > drivers/net/enetc/enetc_ethdev.c | 25 +- > > drivers/net/enetc/enetc_rxtx.c | 430 ++++++++++++++++++++++--- > > 9 files changed, 831 insertions(+), 119 deletions(-) > > > > Did followup AI review and it had some more things that need fixing: > > Error > ===== > > [PATCH v2 7/9] net/enetc: add devargs to control VSI-PSI timeout and delay > > drivers/net/enetc/enetc4_vf.c, enetc4_vf_dev_init() > > kvlist is leaked on the two invalid-value error paths. It is > allocated by rte_kvargs_parse() (line 1347) and only freed at > line 1385, but both > > return -1; /* invalid VSI Timeout, line 1367 */ > return -1; /* invalid VSI Delay, line 1380 */ > > return before that free. A malformed enetc4_vsi_timeout= or > enetc4_vsi_delay= leaks the kvargs structure on every probe. > > Free before returning, e.g.: > > if (errno != 0 || hw->vsi_timeout == 0) { > ENETC_PMD_ERR("Invalid VSI Timeout value = %u", > hw->vsi_timeout); > rte_kvargs_free(kvlist); > return -1; > } > > (same for the delay path), or restructure with a goto. > > > Warning > ======= > > Series (patches 6-9) > > The new runtime devargs - enetc4_vsi_disable, enetc4_vsi_timeout, > enetc4_vsi_delay, enetc4_txq_prior, and nc - are registered via > RTE_PMD_REGISTER_PARAM_STRING and noted in the release notes, but > doc/guides/nics/enetc4.rst has no Runtime Configuration section > describing them. Convention is to document devargs in the NIC guide > so users can find the syntax (e.g. the nc=1 / 'a|b|c' priority list > formats are non-obvious). > > Info > ==== > > [PATCH v2 5/9] and [PATCH v2 9/9] - RX multi-segment reassembly > > In enetc_clean_rx_ring_nc() and enetc_clean_rx_ring_cacheable(), > on the frame-last BD: > > first_seg->pkt_len -= rx_ring->crc_len; > > reduces pkt_len but leaves the final segment's data_len unchanged, > so pkt_len != sum(data_len) when crc_len is non-zero. The old > single-segment path kept them equal (pkt_len = data_len = buf_len > - crc_len). > > This is currently unreachable: enetc4 does not advertise > RTE_ETH_RX_OFFLOAD_KEEP_CRC, so crc_len is always 0 and the > subtraction is a no-op. Flagging only so the asymmetry is on record > if KEEP_CRC is ever added - at that point the last segment's > data_len would need the same adjustment (and the CRC may straddle > the last two segments).
It seems that AI reviewed the V2 series instead of the V3 series. I have manually verified that the first two issues are fixed in the V3 series. For the last issue, I will submit a separate patch as a new feature, introducing KEEP_CRC for the enetc4 driver. Additionally, AI has acknowledged that there will be no functional issue as of now, since crc_len will be only 0.
