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

[Ivan Malov]

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 <ivan.malov at 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 at arknetworks.am>
>> Reviewed-by: Viacheslav Galaktionov <viacheslav.galaktionov at arknetworks.am>
>> Reviewed-by: Andy Moreton <andy.moreton at 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.
>