Re: [PATCH net-next v5 3/3] net: dpaa2-mac: export standard statistics

[Ioana Ciornei <ioana.ciornei@nxp.com>]

On Mon, Mar 23, 2026 at 01:50:39PM +0200, Ioana Ciornei wrote:
> Take advantage of all the newly added MAC counters available through the
> MC API and export them through the standard statistics structures -
> rmon, eth-ctrl, eth-mac and pause.
> 
> A new version based feature is added into dpaa2-mac -
> DPAA2_MAC_FEATURE_STANDARD_STATS - and based on the two memory zones
> needed for gathering the MAC counters are setup for each statistics
> group.
> 
> The dpmac_counter structure is extended with a new field - size_t offset
> - which is being used to instruct the dpaa2_mac_transfer_stats()
> function where exactly to store a counter value inside the standard
> statistics structure.
> 
> The newly added support is used both in the dpaa2-eth driver as well as
> the dpaa2-switch one.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> Changes in v5:
> - use DPMAC_CNT_ING_UNDERSIZED for the rmon etherStatsUndersizePkts
>   counter
> Changes in v4:
> - add dma_sync_single_for_device before calling the MC API
> Changes in v3:
> - reduce the number of lines that have more than 80 chars
> Changes in v2:
> - none
> 
>  .../ethernet/freescale/dpaa2/dpaa2-ethtool.c  |  61 ++++-
>  .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  | 214 ++++++++++++++++++
>  .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |  17 ++
>  .../freescale/dpaa2/dpaa2-switch-ethtool.c    |  48 +++-
>  4 files changed, 338 insertions(+), 2 deletions(-)
> 

(...)

> +static const struct dpmac_counter dpaa2_mac_rmon_stats[] = {
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_ING_FRAME_64, hist[0]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_ING_FRAME_127, hist[1]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_ING_FRAME_255, hist[2]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_ING_FRAME_511, hist[3]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_ING_FRAME_1023, hist[4]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_ING_FRAME_1518, hist[5]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_ING_FRAME_1519_MAX, hist[6]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_EGR_FRAME_64, hist_tx[0]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_EGR_FRAME_127, hist_tx[1]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_EGR_FRAME_255, hist_tx[2]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_EGR_FRAME_511, hist_tx[3]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_EGR_FRAME_1023, hist_tx[4]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_EGR_FRAME_1518, hist_tx[5]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_EGR_FRAME_1519_MAX, hist_tx[6]),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_ING_UNDERSIZED, undersize_pkts),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_ING_OVERSIZED, oversize_pkts),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_ING_FRAG, fragments),
> +	DPMAC_RMON_COUNTER(DPMAC_CNT_ING_JABBER, jabbers),
> +};

The AI still gives here the following feedback:

	According to RFC 2819, the etherStatsUndersizePkts counter tracks the
	number of undersized packets received. Should this map to an ingress
	counter rather than the egress counter DPMAC_CNT_EGR_UNDERSIZED used
	here?

	Jakub Kicinski raised this concern in v4 review (Fri, 20 Mar 2026):
	https://lore.kernel.org/netdev/20260320191943.2325cf38@kernel.org/

As it can be seen above, in v5 I changed the counter used to be the one
for the ingress side. This is not an issue, from my point of view.

(...)

>  const struct ethtool_ops dpaa2_switch_port_ethtool_ops = {
>  	.get_drvinfo		= dpaa2_switch_get_drvinfo,
>  	.get_link		= ethtool_op_get_link,
> @@ -218,4 +261,7 @@ const struct ethtool_ops dpaa2_switch_port_ethtool_ops = {
>  	.get_strings		= dpaa2_switch_ethtool_get_strings,
>  	.get_ethtool_stats	= dpaa2_switch_ethtool_get_stats,
>  	.get_sset_count		= dpaa2_switch_ethtool_get_sset_count,
> +	.get_rmon_stats		= dpaa2_switch_get_rmon_stats,
> +	.get_eth_ctrl_stats	= dpaa2_switch_get_ctrl_stats,
> +	.get_eth_mac_stats	= dpaa2_switch_get_eth_mac_stats,
>  };

I know that the AI complains about the lack of the .get_pause_stats()
callback in the dpaa2-switch driver and the fact that memory is
alocated for the pause statistics is not actually used.

Since the dpaa2-switch does not currently expose pause configuration, I
also didn't expose the pause statistics. I also didn't want to pollute
this set by adding more unrelated changes to the dpaa2-switch. Hope
that's ok.

Ioana