Re: [PATCH 0/3] common/sfc_efx/base: add VADAPTER statistics for Medford4

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 11 Aug 2026 21:49:46 +0400
Ivan Malov <ivan.malov@arknetworks.am> wrote:

> This series adds support for per-VF (VADAPTER) MAC
> statistics on Medford4 in the libefx base layer.
> 
> The first patch selectively updates the MCDI headers, adding
> new definitions needed by the subsequent patches.
> 
> The second patch introduces netport VADAPTER statistics IDs,
> which identify the per-VF counter set returned by the NIC.
> 
> The third patch switches the netport statistics call to use
> GET_NETPORT_STATISTICS_V2, passing the EVB port handle so
> that the correct per-VF counters are retrieved.
> 
> Andy Moreton (2):
>   common/sfc_efx/base: add support for VADAPTER statistics IDs
>   common/sfc_efx/base: switch netport stats to use EVB port ID
> 
> Ivan Malov (1):
>   common/sfc_efx/base: update MCDI headers
> 
>  drivers/common/sfc_efx/base/efx_impl.h      |   3 +-
>  drivers/common/sfc_efx/base/efx_mcdi.c      |   2 +-
>  drivers/common/sfc_efx/base/efx_np.c        |  60 +++-
>  drivers/common/sfc_efx/base/efx_regs_mcdi.h | 313 +++++++++++++++++++-
>  drivers/common/sfc_efx/base/medford4_mac.c  |  12 +-
>  5 files changed, 373 insertions(+), 17 deletions(-)
> 

Detailed AI review had info level comments only.

Reviewed the series against upstream (applied cleanly with git am).

Patch 1 is generated MCDI header import, no review comments.

Patch 2/3 verification notes:

- All five efx_np_mac_stats() call sites are updated for the new
  argument order (efx_mcdi.c:2251, medford4_mac.c:175/198/201/205).
  The parameter count changed from 5 to 6, so a missed caller would
  be a compile error rather than a silent type-compatible swap.

- EFX_MCDI_DECLARE_BUF() was correctly bumped to V2_IN_LEN alongside
  emr_in_length, so the new PORT_ID write at offset 20 is in bounds.

- MC_CMD_GET_NETPORT_STATISTICS_V2_IN is a strict superset of V1:
  PORT_HANDLE/CMD/DMA_ADDR/DMA_LEN are at identical offsets, only
  PORT_ID at 20 is new.

- The new VADAPTER entries in efx_np_mac_stat_map[] land before the
  FEC entries in efx_mac_stat_t, so the array size is unchanged, and
  the map is only consulted to translate descriptors the FW actually
  reports, so the addition is inert on FW without vadapter stats.
  efx_mac.c already has the matching name strings, and
  medford4_mac_stats_get_mask() derives the mask from the LUT, so the
  counters surface automatically.

Info

Patch 2:

- EFX_NP_HW_STAT_ID() shifts the index by
  MC_CMD_STAT_ID_MAC_STAT_ID_LBN regardless of the source argument.
  This is correct today only because MAC/PHY/VADAPTER_STAT_ID_LBN are
  all 16. Using MC_CMD_STAT_ID_##_src##_STAT_ID_LBN would make the
  macro self-consistent. Pre-existing (the PHY variant has the same
  shape), so no need to fix here unless you are touching it anyway.

Patch 3:

- The request mixes V1 and V2 field names: PORT_HANDLE, CMD,
  DMA_ADDR_LO/HI and DMA_LEN still use the
  GET_NETPORT_STATISTICS_IN_* names while the length and PORT_ID use
  GET_NETPORT_STATISTICS_V2_IN_*. The offsets are identical so this
  is functionally correct, but naming all fields V2 would make it
  obvious at a glance which msgrequest is being sent.

- The V2 request length is now used unconditionally, with no
  capability check or V1 fallback. If any shipping Medford4 firmware
  implements only the 20-byte GET_NETPORT_STATISTICS_IN msgrequest,
  it will reject the 24-byte request and MAC stats break entirely,
  not just the vadapter counters. Is V2 guaranteed present in every
  FW build that advertises netport support? efx_np_mac_ctrl() already
  makes the same unconditional assumption with MAC_CTRL_IN_V2, so I
  suspect the answer is yes and there is a documented FW baseline --
  worth a line in the commit message either way.