Re: [PATCH 1/1] common/sfc_efx/base: clear VADAPTER stats upon allocation

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 24 Aug 2026 15:26:47 +0400
Ivan Malov <ivan.malov@arknetworks.am> wrote:

> The probe-time clear in 'ef10_nic_probe' runs before the VADAPTER is
> allocated and so cannot zero its counters. Add a complementary clear
> to the point at which the VADAPTER is already in place, so that
> VADAPTER statistics begin at zero for both PFs and VFs.
> 
> This change affects only Medford4 NICs within the 26.11 release, where
> Medford4 VADAPTER statistics have been added in the first place.
> 
> Signed-off-by: Ivan Malov <ivan.malov@arknetworks.am>
> Reviewed-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
> Reviewed-by: Andy Moreton <andy.moreton@amd.com>

Looks correct but Opus AI review spotted something.

Review of [PATCH 1/1] common/sfc_efx/base: clear VADAPTER stats upon
allocation

The new failure path is correct: fail6 frees the vAdaptor only when
alloc_vadaptor is set (so the EVB case, where the vPort belongs to the
vSwitch, is left alone), resets en_vport_id, and falls through to the
existing fail5 unwinding.  efx_np_attach() runs in ef10_nic_probe()
before ef10_nic_init(), so ep_np_handle is valid at the new call site.

Warning: the code comment and the commit message do not match what the
code does on the family they name.  efx_mcdi_mac_stats_clear() splits
on efx_np_supported(), which is true for EFX_FAMILY_MEDFORD4 and
later.  On Medford4 it therefore takes the netport branch,

	efx_np_mac_stats(enp, epp->ep_np_handle, EFX_STATS_CLEAR, NULL, 0)

which never reads en_vport_id, so the comment's "do it here while
'en_vport_id' holds a valid value" does not describe the Medford4
behaviour.  Conversely, it is the pre-Medford4 families that use the
vPort-scoped branch, efx_mcdi_mac_stats(enp, enp->en_vport_id, ...);
the probe-time clear there runs with en_vport_id still EVB_PORT_ID_NULL
(0) and the new one runs with EVB_PORT_ID_ASSIGNED or the vSwitch
vPort, so Huntington/Medford/Medford2 do see a behaviour change as
well, contrary to "This change affects only Medford4 NICs".

Suggested fix: reword both to state the actual reason -- the vAdaptor
counters do not exist until the vAdaptor has been allocated, so a
clear issued at probe time cannot zero them -- and either drop the
Medford4-only claim or explain why the extra vPort-scoped clear is a
no-op on the older families.