On Tue, 11 Aug 2026 13:17:31 +0530
Gagandeep Singh <[email protected]> wrote:
> 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 | 136 ++++-
> drivers/net/enetc/enetc4_ethdev.c | 428 +++++++++++--
> drivers/net/enetc/enetc4_vf.c | 807 ++++++++++++++++++++++---
> drivers/net/enetc/enetc_rxtx.c | 530 +++++++++++++++-
> 9 files changed, 2011 insertions(+), 153 deletions(-)
>
Still lots of issues found by AI review
ENETC4 v7 series review - 14 patches
No Reviewed-by; one error and two warnings remain.
All 14 commits build clean individually on top of c1a46b9 with gcc 13.3
and -Dwerror=true. check-git-log.sh: 14/14 valid. No trailing
whitespace. Consecutive-blank-line counts are back to the upstream
values in all four touched files.
Confirmed fixed from v5: the free()/rte_free() mismatch is gone --
enetc4_dev_uninit() no longer touches txq_prior at all and both arrays
are released in enetc4_dev_close(); rte_intr_vec_list_free() and
rte_intr_efd_disable() added at the intr_enable_fail: label so both the
teardown and the error path are covered; enetc4_alloc_rxbdr() now
validates ring_desc against MAX_BD_COUNT the same way the Tx side does;
tx_pause_active is RTE_ATOMIC with acquire/release accessors; the
register-dump version field is cast before the shift in both
enetc4_get_regs() and enetc4_vf_get_regs(); fw_version_get() computes
the fw_size == 0 length from the real values; release note reads "Tx
PAUSE ... Rx congestion mode"; parse_txq_prior() and parse_txq_wrr()
both return -ENOMEM consistently.
Patch 08
Error: pthread mutex in shared memory without PTHREAD_PROCESS_SHARED.
The new vsi_lock lives in struct enetc_eth_hw, which sits in
eth_dev->data->dev_private. ethdev allocates that with
rte_zmalloc_socket(), so it is hugepage memory visible to secondary
processes:
drivers/net/enetc/enetc.h:145
pthread_mutex_t vsi_lock; /* serializes all VSI-PSI mailbox ... */
drivers/net/enetc/enetc4_vf.c:1985
pthread_mutex_init(&hw->vsi_lock, NULL);
Two problems follow. A mutex initialized with NULL attributes is
undefined behaviour if it is ever locked from a second process. And
enetc4_vf_dev_init() has no primary-process guard -- enetc4_dev_close()
is the only place in the driver that checks rte_eal_process_type() --
so a secondary attaching to the same VF re-runs pthread_mutex_init() on
a mutex the primary may be holding.
pthread_mutexattr_t attr;
pthread_mutexattr_init(&attr);
pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED);
pthread_mutex_init(&hw->vsi_lock, &attr);
pthread_mutexattr_destroy(&attr);
rte_spinlock_t would sidestep the initialization problem but is the
wrong primitive here, since the poll loop under the lock waits up to
vsi_timeout iterations.
Warning: the reply status is still read outside the lock.
enetc4_msg_vsi_send() now holds vsi_lock across the write, the MB poll
and the completion-status check, which closes most of the window. But
every caller re-reads ENETC4_VSIMSGSR after the function has returned
and dropped the lock:
err = enetc4_msg_vsi_send(hw, msg);
if (err) {
ENETC_PMD_ERR("VSI message send error");
goto end;
}
enetc4_msg_vsi_reply_msg(enetc_hw, reply_msg); /* lock released */
Six call sites go through enetc4_msg_vsi_reply_msg() (lines 727, 973,
1019, 1404, 1504, 1637) and enetc4_vf_get_ip_minor_revision() reads the
register directly at line 1081. A second thread -- in particular the
interrupt thread doing the patch 08 speed re-query -- can take the lock
and start a new transaction between the unlock and that read, so the
caller parses someone else's reply.
enetc4_msg_vsi_send() already has the value in its local vsimsgsr.
Returning it through an out-parameter, and having callers parse from
that instead of re-reading the register, removes the window entirely.
Patch 13
Warning: RBMR read-modify-write still races between threads.
The atomic on tx_pause_active covers the flag but not the register.
enetc4_vf_set_congestion_mode() runs on the interrupt thread:
rbmr = enetc4_rxbdr_rd(enetc_hw, i, ENETC_RBMR);
if (enable)
rbmr |= ENETC_RBMR_CM;
else
rbmr &= ~(uint32_t)ENETC_RBMR_CM;
enetc4_rxbdr_wr(enetc_hw, i, ENETC_RBMR, rbmr);
enetc4_rx_queue_start() and enetc4_rx_queue_stop() perform the same
unguarded read-modify-write on ENETC_RBMR_EN from the control thread.
A link-status interrupt landing mid-sequence still loses one of the two
updates, leaving a ring enabled without congestion mode or stopped with
it set. Serializing the RBMR accesses -- vsi_lock would do, or a
dedicated per-device lock -- is what is missing.
Info: the release store on tx_pause_active is placed before the RBMR
writes it is described as publishing, so the ordering does not do what
the comment implies. Nothing else is published through this flag, so
rte_memory_order_relaxed on both sides would be correct and clearer.
Patch 06
Info: enetc4_txbdr_regs[] and enetc4_rxbdr_regs[] are still static const
arrays defined in enetc.h, so every translation unit including the
header gets a private copy. Moving them into one .c file with an extern
declaration avoids the duplication.
