Re: [PATCH 0/4] Wangxun new feature

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 17 Jun 2026 18:59:55 +0800
Zaiyu Wang <zaiyuwang@trustnetic.com> wrote:

> This patchset introduces three new features and critical fixes for our
> recent release cycle.
> 
> Patch 1/2 adds support for UDP Segmentation Offload (USO) to improve
> large-packet transmission performance for UDP workloads.
> 
> Patch 3 enables VFs to sense PF ifconfig down/up events, allowing
> better fault tolerance and fast recovery in virtualized environments.
> 
> Patch 4 adds the missing VF support for the Amber-Lite 40G NICs, which
> was previously omitted in the initial integration.
> 
> Zaiyu Wang (4):
>   net/ngbe: add USO support
>   net/txgbe: add USO support
>   net/txgbe: add support for VF sensing PF down
>   net/txgbe: add VF support for Amber-Lite 40G NIC
> 
>  drivers/net/ngbe/ngbe_rxtx.c          | 13 +++---
>  drivers/net/txgbe/base/txgbe_devids.h |  2 +
>  drivers/net/txgbe/base/txgbe_hw.c     |  7 ++++
>  drivers/net/txgbe/base/txgbe_regs.h   |  7 +++-
>  drivers/net/txgbe/base/txgbe_type.h   |  2 +
>  drivers/net/txgbe/base/txgbe_vf.c     |  7 ++--
>  drivers/net/txgbe/txgbe_ethdev.c      |  4 +-
>  drivers/net/txgbe/txgbe_ethdev_vf.c   | 60 +++++++++++++++++++++++----
>  drivers/net/txgbe/txgbe_rxtx.c        | 13 +++---
>  9 files changed, 92 insertions(+), 23 deletions(-)
> 

Preliminary AI review found some bugs.

Review of [PATCH v2 0/4] net/{txgbe,ngbe}: USO, VF PF-down sensing,
                                              AML 40G VF support

Patch 3/4 (net/txgbe: add support for VF sensing PF down)

Error: dead store in txgbevf_get_pf_link_status() leaves the link
struct status field unmodified when the PF is down.

  +	if (!hw->pf_running) {
  +		link_up = false;
  +		link_speed = TXGBE_LINK_SPEED_UNKNOWN;
  +		link.link_duplex = RTE_ETH_LINK_HALF_DUPLEX;
  +		return rte_eth_linkstatus_set(dev, &link);
  +	}

The local variables link_up and link_speed are written here and never
read before the function returns, so they have no effect.  The actual
fields that need to change are link.link_status and link.link_speed,
neither of which is touched -- the function then publishes a link
struct that still has the previous link.link_status (whatever
rte_eth_linkstatus_get returned a few lines earlier) and only the
duplex updated.

Compare with the parallel code added in the same patch in
txgbevf_check_link_for_intr(), which gets it right:

  +		new_link.link_status = RTE_ETH_LINK_DOWN;
  +		new_link.link_speed = RTE_ETH_SPEED_NUM_NONE;
  +		new_link.link_duplex = RTE_ETH_LINK_HALF_DUPLEX;

In txgbevf_get_pf_link_status, assign directly to the link fields:

	if (!hw->pf_running) {
		link.link_status = RTE_ETH_LINK_DOWN;
		link.link_speed = RTE_ETH_SPEED_NUM_NONE;
		link.link_duplex = RTE_ETH_LINK_HALF_DUPLEX;
		return rte_eth_linkstatus_set(dev, &link);
	}

Warning: the commit message describes a flag that is not what the code
checks.  The message says "Detect PF ifconfig down when
TXGBE_VT_MSGTYPE_SPEC is present in mailbox commands", but the
implementation in txgbevf_mbx_process does:

  +		if (!(in_msg & TXGBE_VT_MSGTYPE_CTS)) {

TXGBE_VT_MSGTYPE_SPEC is defined in base/txgbe_mbx.h with a different
bit (0x10000000) and is only used in the 5-tuple-filter request path.
The detection here is "CTS is absent", not "SPEC is present".
Please rewrite the commit message to match the code.

Warning: no release notes entry.  This is new VF behavior visible to
applications (a new INTR_RESET callback is dispatched when the PF
comes back up); doc/guides/rel_notes/release_26_07.rst should
mention it.

Info: msgbuf is declared as
u32 msgbuf[TXGBE_VF_PERMADDR_MSG_LEN] = {0}; but only msgbuf[0] is
ever used.  Either size it to one element or drop the zero-init since
only the first element is touched before write_posted.

Patch 4/4 (net/txgbe: add VF support for Amber-Lite 40G NIC)

Warning: stray blank line between the if() and its body in
txgbe_reset_hw_vf:

  +	if (hw->mac.type == txgbe_mac_aml_vf ||
  +	    hw->mac.type == txgbe_mac_aml40_vf)
  +
   		wr32(hw, TXGBE_BME_AML, 0x1);

The blank line is a no-op for C but reads as a typo and checkpatch will
flag it.  Remove the blank '+' line.

Warning: no release notes entry.  New device IDs are user-visible (the
VF binds devices that previously did not work); please add a brief
note to release_26_07.rst.

Info: in txgbe_check_mac_link_vf the patch drops the explicit
sp_vf/aml_vf check before the wait loop:

  -	if ((mac->type == txgbe_mac_sp_vf ||
  -	     mac->type == txgbe_mac_aml_vf) && wait_to_complete) {
  +	if (wait_to_complete) {

The simplification is fine since txgbe_check_mac_link_vf is only ever
called with a VF mac type, but it is an unrelated cleanup that could be
mentioned in the commit message (or split into its own patch).

Patches 1/4 and 2/4 (net/{ngbe,txgbe}: add USO support)

Info: it would help readers if the commit message noted that
RTE_ETH_TX_OFFLOAD_UDP_TSO was already advertised in tx_offload_capa
(see ngbe_get_tx_port_offloads / txgbe_get_tx_port_offloads), so this
patch fills in the data path for a capability that was previously
claimed but unimplemented.  Without that context the change looks like
a new feature addition; with it, it is clearly a fix and could carry a
Fixes: tag plus Cc: stable.

Info: a brief features-list or feature-matrix note in release_26_07.rst
would still be welcome since UDP_SEG offload now actually works.

Stephen Hemminger <stephen@networkplumber.org>