Re: [PATCH v4 00/14] net/enetc: add new features for ENETC4 on i.MX95

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri,  7 Aug 2026 16:56:09 +0530
Gagandeep Singh <g.singh@nxp.com> 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.