Hi
> -----Original Message-----
> From: Stephen Hemminger <[email protected]>
> Sent: Monday, August 10, 2026 2:30 AM
> To: Gagandeep Singh <[email protected]>
> Cc: [email protected]; Hemant Agrawal <[email protected]>
> Subject: Re: [PATCH v4 00/14] net/enetc: add new features for ENETC4 on i.MX95
>
> On Fri, 7 Aug 2026 16:56:09 +0530
> Gagandeep Singh <[email protected]> wrote:
>
> > V4-changes:
> > - fix doc build issue: WARNING: undefined label: pmd_build_and_test
> >
> > v3-changes:
> > - fix doc build issue.
> > - fix compilation issue on fedore:43-gcc-minsize
> >
> > V2-changes:
> > - compilation fixes.
> >
> > V1-changes:
> > This series adds new PMD features to the ENETC4 driver targeting the
> > NXP i.MX95 NETC IP.
> >
> > The series covers:
> >
> > - KEEP_CRC Rx offload: preserve the Ethernet FCS in the receive buffer.
> > - TSO: TCP Segmentation Offload for the VF Tx path.
> > - RSC/LRO: hardware Receive Segment Coalesce for PF and VF Rx paths.
> > - Link speed code: extend the PF-to-VF mailbox field from 4-bit to
> > 8-bit to support speeds beyond 10G.
> > - Firmware version: report the NETC IP version via fw_version_get.
> > - Register dump: dump SI, port (PF) and BD ring registers.
> > - Ring parameters: implement rxq_info_get / txq_info_get for the VF.
> > - Link-up interrupt: refresh the cached link speed on each VF link-up
> > interrupt so that link_update returns the current speed immediately.
> > - Stats reset: software snapshot/delta approach for VF counter reset.
> > - Per-queue Rx interrupt: MSI-X per-queue Rx interrupts for the VF,
> > enabling interrupt-driven receive with l3fwd-power.
> > - SI VLAN: hardware port VLAN insertion/removal for PF and VF.
> > - VF link status bitmask: switch VF link status to bitmask encoding
> > to align with the PF and newer kernel driver conventions.
> > - TX PAUSE: VF sets Rx congestion mode when the PF signals TX PAUSE
> > negotiated on the wire; adds Flow control = Y to enetc4.ini.
> > - WRR Tx scheduler: per-ring WRR weights via enetc4_txq_wrr devarg.
> >
> > Gagandeep Singh (14):
> > net/enetc: add KEEP_CRC offload support for ENETC4
> > net/enetc: add TSO support for ENETC4 VF
> > net/enetc: add RSC (hardware LRO) support for ENETC4
> > net/enetc: extend link speed code field to 8-bit for PF-to-VF message
> > net/enetc: support firmware version get for VF
> > net/enetc: support registers dump
> > net/enetc: support ethtool ring parameters
> > net/enetc: refresh link speed on VF link-up interrupt
> > net/enetc: support stats reset for VF
> > net/enetc4: add per-queue Rx interrupt support for VF
> > net/enetc4: add SI-based port VLAN insertion and removal
> > net/enetc4: update VF link status to bitmask encoding
> > net/enetc4: enable TX PAUSE via VF RX congestion mode
> > net/enetc4: add WRR Tx scheduler devarg for VF rings
> >
> > doc/guides/nics/enetc4.rst | 77 +++
> > doc/guides/nics/features/enetc4.ini | 8 +
> > doc/guides/rel_notes/release_26_11.rst | 19 +
> > drivers/net/enetc/base/enetc4_hw.h | 129 +++-
> > drivers/net/enetc/base/enetc_hw.h | 6 +
> > drivers/net/enetc/enetc.h | 135 ++++-
> > drivers/net/enetc/enetc4_ethdev.c | 402 +++++++++++--
> > drivers/net/enetc/enetc4_vf.c | 779 ++++++++++++++++++++++---
> > drivers/net/enetc/enetc_rxtx.c | 529 ++++++++++++++++-
> > 9 files changed, 1939 insertions(+), 145 deletions(-)
> >
>
> Still lots of issues, the biggest one is that not each patch builds; not
> bisectable.
>
> More extensive AI review.
>
> ENETC4 v4 series review - 14 patches
>
> No Reviewed-by. The headline finding from the previous round is
> unchanged: 13 of the 14 commits fail to build. Only the final commit is
> clean, so
> git bisect is broken across essentially the whole series.
>
> Build (applied on c1a46b9, gcc 13.3, -Dwerror=true):
>
> [1] 8a0a793 01/14 add KEEP_CRC offload support for ENETC4
> [1] 63cae7e 02/14 add TSO support for ENETC4 VF
> [1] b28e1a8 03/14 add RSC (hardware LRO) support
> ... all fail ...
> [1] e334f6c 13/14 enable TX PAUSE via VF RX congestion mode
> [0] b5e71d6 14/14 add WRR Tx scheduler devarg for VF rings
>
> Three distinct causes, all introduced in patch 01.
>
>
> Patch 01
>
> Error: prev_seg used but never declared (fails commits 01-02).
>
> enetc_rxtx.c:597:25: error: 'prev_seg' undeclared
> enetc_rxtx.c:844:25: error: 'prev_seg' undeclared
>
> Patch 01 assigns prev_seg and passes it to enetc_rx_crc_trim() in both
> enetc_clean_rx_ring_nc() and enetc_clean_rx_ring_cacheable(), but the
> declaration hunk only changes first_seg/cur_seg to = NULL. The *prev_seg =
> NULL declaration does not arrive until patch 03. Move it to patch 01.
>
> Error: duplicate offload arrays (fails commits 01-13).
>
> enetc4_ethdev.c:110:17: error: redefinition of 'dev_rx_offloads_sup'
> enetc4_ethdev.c:117:17: error: redefinition of 'dev_tx_offloads_sup'
>
> Patch 01 adds a second copy of both arrays at the top of enetc4_ethdev.c; the
> pre-existing ones near line 97 are not deleted until patch 14. The deletion
> hunk
> belongs in patch 01.
>
> Error: duplicate rx_enable declaration (fails commits 01-13).
>
> enetc4_ethdev.c:552:18: error: redeclaration of 'rx_enable' with no
> linkage
>
> enetc4_rx_queue_setup() already declares uint32_t rx_enable upstream.
> Patch 01 adds a second one; the removal is in patch 14.
>
> This is the signature of building only the squashed series. Please run
> devtools/test-meson-builds.sh, or at minimum a -Dwerror=true build at each
> commit, before v5.
>
> Error: PF loses Scattered Rx and Multi-segment Tx.
>
> The old arrays deleted in patch 14 contained RTE_ETH_RX_OFFLOAD_SCATTER
> and RTE_ETH_TX_OFFLOAD_MULTI_SEGS. The replacements added in patch 01
> do not. Net result at the tip:
>
> upstream after the series
> RX: IPV4|UDP|TCP|SCATTER RX: IPV4|UDP|TCP|KEEP_CRC|TCP_LRO
> TX: IPV4|UDP|TCP|MULTI_SEGS TX: IPV4|UDP|TCP|TCP_TSO|UDP_TSO
>
> Nothing in any commit message says this is intentional. enetc4.ini still
> declares
> "Scattered Rx = Y", and the new RSC path builds multi-segment clusters, so the
> capability is still needed. The VF arrays keep both - only the PF regresses.
>
>
> Patch 02
>
> Error: mbuf leak in enetc_xmit_pkts_lso().
>
> Two validation paths drop a TSO frame with "start++; continue;":
>
> - hdr_len >= pkt_len || hdr_len > data_len
> - tso_segsz == 0 || data_unit > ENETC4_LSO_MAX_DATA_UNIT ||
> hdr_len + tso_segsz > ENETC4_LSO_MAX_FRAME
>
> Neither writes q_swbd[i].buffer_addr, and neither frees the mbuf, but both
> count
> the packet in the burst return value. The application considers it
> transmitted;
> enetc_clean_tx_ring() never sees it.
> Free-and-continue is the right shape here - add rte_pktmbuf_free(seg) before
> each start++.
>
> Error: 16-bit byte swap on a uint8_t field.
>
> In the non-TSO path of the same function:
>
> txbd->flags |= rte_cpu_to_le_16(ENETC4_TXBD_FLAGS_F);
>
> struct enetc_tx_bd.flags is uint8_t. On big-endian this evaluates to
> 0x8000 and truncates to 0, so the frame-last flag is never set and Tx stalls.
> Upstream and the LSO path in this very function both use plain "txbd->flags |=
> ENETC4_TXBD_FLAGS_F;". Drop the conversion.
>
> Warning: ring doubling bypasses MAX_BD_COUNT.
>
> Both enetc4_alloc_txbdr() (LSO, patch 02) and enetc4_alloc_rxbdr() (RSC, patch
> 03) double nb_desc, but the nb_desc > MAX_BD_COUNT check runs before the
> doubling. With MAX_BD_COUNT at 64000 the effective ring reaches 128000
> descriptors, and ENETC_RTBLENR_LEN(n) is just
> ((n) & ~0x7) with no upper clamp. Validate the post-doubling count, or halve
> the
> accepted nb_desc when LSO/RSC is on.
>
>
> Patch 04
>
> Warning: link speed decode has no upper bound.
>
> link.link_speed = (reply_msg->status - ENETC_SPEED_5000) * 1000 + 5000;
>
> status is uint8_t and the default arm now catches everything from 8 to 255,
> so a
> garbage or unrecognized code yields a fabricated speed (0xFF -> 253000 Mbps)
> rather than RTE_ETH_SPEED_NUM_UNKNOWN. Note also that 0xF, previously
> ENETC_SPEED_NOT_SUPPORTED, now decodes as 13000 Mbps in non-legacy
> mode. Bound the result, or reject codes that do not map to a defined
> RTE_ETH_SPEED_NUM_* value.
>
> Warning: flipping the reply-status extraction from 4-bit to 8-bit by default
> is a
> wire-protocol compatibility break. Users on a PF kernel older than 6.18.37
> must
> add vf_link_legacy=1 or link reporting breaks silently. That deserves an
> explicit
> line in the release notes, not just "extended the field to 8-bit".
>
> Info: adds a second #include <rte_kvargs.h> immediately below the existing
> one.
>
>
> Series-wide
>
> Warning: mailbox ops added to the no-VSI ops table.
>
> enetc4_vf_ops_no_vsi_m is selected when the user passes enetc4_vsi_disable,
> that is, explicitly asks for no VSI-PSI messaging.
> The series adds fw_version_get, mac_addr_set, mac_addr_add, promiscuous_*,
> allmulticast_*, vlan_filter_set, vlan_offload_set, vlan_pvid_set and
> stats_reset to
> it. Most of those reach
> enetc4_msg_vsi_send() directly or through a helper, so they will now attempt a
> transaction on the disabled mailbox and stall until vsi_timeout. Previously
> ethdev
> returned -ENOTSUP immediately. Only the genuinely register-local ops
> (get_reg,
> rxq_info_get, txq_info_get, and stats_reset if it stays direct-register)
> belong in that
> table.
>
> Warning: new devargs undocumented.
>
> vf_link_legacy (patch 04) and enetc4_txq_wrr (patch 14) are both in
> RTE_PMD_REGISTER_PARAM_STRING but absent from
> doc/guides/nics/enetc4.rst, which documents every pre-existing devarg.
> Same gap flagged in the previous round.
>
> Info: patch 01 adds two stray blank lines in enetc4_ethdev.c (one before
> enetc4_rx_queue_release, later removed in patch 03); patch 02 adds three more
> around enetc4_alloc_txbdr/enetc4_free_bdr and in struct enetc_bdr.
>
> Info: enetc_recv_pkts_rsc is assigned to dev->rx_pkt_burst from
> enetc4_rx_queue_setup(), which will silently override the nc=1 burst selection
> made in dev_init. Worth a guard or at least a comment on precedence.
>
> I stopped short of a full read of patches 05-14 since the build breakage has
> to be
> resolved first and several of these findings will shift hunks around. I can
> go back
> through 05-14 in detail once there is a v5, or now if you would rather have
> the
> complete list before the author respins.
V5 sent. I missed to mention V5-changes in the cover letter.
The changes are:
- Patch 1 Build fixes which includes:
'prev_seg' undeclared fixed.
redefinition of 'dev_rx_offloads_sup' fixed.
Error: duplicate rx_enable declaration fixed.
PF loses Scattered Rx and Multi-segment Tx
- mbuf leak in enetc_xmit_pkts_lso() fixed.
- fixed conversion on ENETC4_TXBD_FLAGS_F issue.
- fixed link speed decode has no upper bound.
- fixed mailbox ops added to the no-VSI ops table.
- new devargs documented.
Regards,
Gagan
