If I may, the premise that the netport branch never reads 'en_vport_id' does not hold
water, as 'efx_np_mac_stats' clearly accepts the vPort ID as its third argument and
correctly places it into 'GET_NETPORT_STATISTICS_V2_IN_PORT_ID' unless the action is
'EFX_STATS_DISABLE'. The comment is therefore accurate and will stay. As for the
"affects only Medford4" claim, that refers to observable counter behaviour for
the specific NIC family in question.
Concerns expressed by the AI do not meet the threshold of an actual defect, and
thus no change to the patch is warranted.
Thank you.
On Mon, 24 Aug 2026, Stephen Hemminger wrote:
On Mon, 24 Aug 2026 15:26:47 +0400
Ivan Malov <[email protected]> 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 <[email protected]>
Reviewed-by: Viacheslav Galaktionov <[email protected]>
Reviewed-by: Andy Moreton <[email protected]>
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.
