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

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 12 Aug 2026 17:03:03 +0530
Gagandeep Singh <g.singh@nxp.com> wrote:

> V8-changes:
>  - Fixed Warning RBMR RMW races between threads by adding lock
>  - Fixed `static const` arrays in `enetc.h`
>  - Proper pthread_mutexattr_t intialization added as suggested by AI
>  - VSIMSGSR stale re-read race is eliminated
> 
> V7-changes:
>  - added missing 14th patch.
> 
> V6-changes:
>  - fixed free() on memory allocated with rte_zmalloc().
>  - fixed eventfd and vector-list leak on interrupt teardown
>  - fixed Rx ring doubling still bypasses
>  - fixed VSI-PSI transaction issued from the interrupt handler
>  - fixed the free-and-reallocate of msg is unnecessary churn
>  - fixed unsynchronized access to tx_pause_active and RBMR
>  - fixed signed shift overflow in the register-dump version field
>  - fixed the fw_size == 0 early return reports the length of "0.0"
>  - unwanted blank lines removed.
>  - parse_txq_prior() returns updated.
> 
> V5-changes:
>  - 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.
> 
> 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 Rx offload for ENETC4
>   net/enetc: add TSO support for ENETC4 VF
>   net/enetc: add RSC (hardware LRO) support for ENETC4
>   net/enetc: extend PF-VF link speed field to 8 bits
>   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             | 100 +++
>  doc/guides/nics/features/enetc4.ini    |   8 +
>  doc/guides/rel_notes/release_26_11.rst |  21 +
>  drivers/net/enetc/base/enetc4_hw.h     | 128 +++-
>  drivers/net/enetc/base/enetc_hw.h      |   6 +
>  drivers/net/enetc/enetc.h              | 132 +++-
>  drivers/net/enetc/enetc4_ethdev.c      | 454 +++++++++++--
>  drivers/net/enetc/enetc4_vf.c          | 860 +++++++++++++++++++++----
>  drivers/net/enetc/enetc_rxtx.c         | 530 ++++++++++++++-
>  9 files changed, 2062 insertions(+), 177 deletions(-)
> 

AI review still discovered some things.
ENETC4 v8 series review - 14 patches

No Reviewed-by; two errors remain.

Fixed from v7: vsi_lock now uses PTHREAD_PROCESS_SHARED;
enetc4_msg_vsi_send() returns the latched VSIMSGSR via out-parameter and
all call sites parse from it instead of re-reading after the unlock;
RBMR read-modify-write serialized under vsi_lock in
set_congestion_mode(), rx_queue_setup(), rx_queue_start() and
rx_queue_stop() (VF only, PF correctly skips); tx_pause_active dropped
to relaxed with the comment updated; the BDR register arrays moved out
of the header into enetc4_ethdev.c with extern declarations.

I checked for lock-ordering problems from the new RBMR locking.
set_congestion_mode() releases vsi_lock before the subsequent
get_link_speed() reacquires it, so no recursive acquisition.


Errors

Patch 11 - VF vlan_pvid_set fails on every successful command.
The reply check compares reply_msg->class_id against
ENETC_MSG_CLASS_ID_CMD_SUCCESS (0x1), but class_id carries the command
class from VSIMSGSR[15:8], which for this command is
ENETC_CLASS_ID_SI_VLAN_ISO (0x24).  Never equal, so the function returns
-EINVAL even when the PSI accepted the command.  enetc4_msg_vsi_send()
already validated the class; every other mailbox op matches the class
first and then switches on status.

Patch 09 - VF stats_reset baseline underflows on counter wrap.
SIROCT0/SIRFRM0/SITOCT0/SITFRM0 are 64-bit counters on 8-byte stride,
but enetc4_rd() is rte_read32(), so only the low word is read.  The new
delta stores that in a uint64_t baseline and subtracts, so once the low
word wraps the result is ~1.8e19 instead of a small wrapped value.  At
10G a 32-bit octet counter wraps in about three seconds.  Either read
the full 64-bit counter, or mask the delta to 32 bits.  SITDFCR is a
genuine 32-bit register and only needs the mask.


Warnings

Patch 13 - "Flow control = Y" added to enetc4.ini with no implementing
ops.  features.rst requires flow_ctrl_get/flow_ctrl_set; none of the
three ops tables provide them, so rte_eth_dev_flow_ctrl_set() returns
-ENOTSUP.  The congestion mode this patch adds is driven by PF link
state and is not reachable through the flow-control API.  The other new
entries (Rx interrupt, LRO, TSO, FW version, Registers dump, CRC
offload) all check out.

Patch 08 - no primary-process guard in enetc4_vf_dev_init().
rte_eth_dev_pci_generic_probe() calls dev_init in secondaries too, and
the secondary attaches to the primary's dev_private.  A secondary
re-runs pthread_mutex_init() on a mutex the primary may hold, which is
undefined behaviour; PROCESS_SHARED makes cross-process locking valid
but not double initialization.  The same path re-runs dev_hw_init() and
the devarg parsing, overwriting the primary's txq_prior/txq_wrr
pointers.  An early return for RTE_PROC_SECONDARY in both the VF and PF
dev_init covers it.


Info

Patch 12 - the pre-existing reply_msg leak on the "Wrong reply message"
path sits in the if/else this patch rewrites; cheap to fold in.

Patch 10 - "ret = 0;" in the efd_enable failure branch is a dead store,
overwritten by the following rte_intr_enable().
rte_intr_callback_unregister() can return -EAGAIN; the return value is
discarded on teardown.

Series-wide - six functions declare int vsimsgsr_* mid-body after a
block of top-of-function declarations, and enetc4_vf_dev_init()
declares pthread_mutexattr_t attr after several statements.  C99 mixing
is allowed, but each of these functions is otherwise declarations-first.