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

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

[Ivan Malov]

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>
---
 drivers/common/sfc_efx/base/ef10_nic.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
index 9af4259d38..954409ec90 100644
--- a/drivers/common/sfc_efx/base/ef10_nic.c
+++ b/drivers/common/sfc_efx/base/ef10_nic.c
@@ -2868,10 +2868,30 @@ ef10_nic_init(
 		if ((rc = ef10_upstream_port_vadaptor_alloc(enp)) != 0)
 			goto fail5;
 	}
+
+#if EFSYS_OPT_MAC_STATS
+	/*
+	 * Clear MAC statistics for the freshly allocated VADAPTER.
+	 * The probe-time wipe in 'ef10_nic_probe' predates the
+	 * allocation and cannot reach vadaptor-scoped counters;
+	 * do it here while 'en_vport_id' holds a valid value.
+	 */
+	rc = efx_mcdi_mac_stats_clear(enp);
+	if (rc != 0)
+		goto fail6;
+#endif
 	enp->en_nic_cfg.enc_mcdi_max_payload_length = MCDI_CTL_SDU_LEN_MAX_V2;
 
 	return (0);
 
+#if EFSYS_OPT_MAC_STATS
+fail6:
+	EFSYS_PROBE(fail6);
+	if (alloc_vadaptor != B_FALSE) {
+		(void) efx_mcdi_vadaptor_free(enp, enp->en_vport_id);
+		enp->en_vport_id = EVB_PORT_ID_NULL;
+	}
+#endif
 fail5:
 	EFSYS_PROBE(fail5);
 fail4:
-- 
2.47.3

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

[Stephen Hemminger]

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.

Re: [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@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.
>

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

[Stephen Hemminger]

On Mon, 24 Aug 2026 20:44:18 +0400 (+04)
Ivan Malov <ivan.malov@arknetworks.am> wrote:

> 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.

Agreed, I just do this to make sure everything gets looked at.