Re: [PATCH v8 net-next 15/15] net: dsa: netc: add support for ethtool private statistics

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch duplicates standard MAC statistics into the driver-private `ethtool -S` interface, violating networking subsystem guidelines.
--

commit 69ac5ab61c392e1560b98f2d827a22869f473c36
Author: Wei Fang <wei.fang@nxp.com>

net: dsa: netc: add support for ethtool private statistics

This commit implements the ethtool private statistics interface to expose
additional port-level and MAC-level counters. It adds string-based counters
for both the eMAC and pMAC.

> Note that although rtnl_link_stats64 provides some standard statistics
> such as rx octets, rx frame errors, rx dropped packets, and tx packets,
> these are overall port statistics. The NETC switch supports preemption
> on each port, and each port has two MACs (eMAC and pMAC). The driver
> private statistics are used to obtain statistics for each MAC, allowing
> users to perform analysis and debugging.

Does the standard struct ethtool_eth_mac_stats interface already provide a
native mechanism to report these exact metrics individually for the eMAC
and pMAC? 

The ETHTOOL_MAC_STATS_SRC_EMAC and ETHTOOL_MAC_STATS_SRC_PMAC source fields
appear to be designed for this exact purpose.

> diff --git a/drivers/net/dsa/netc/netc_ethtool.c b/drivers/net/dsa/netc/netc_ethtool.c
> index ac8940b5a85c6..e6028851a6d09 100644
> --- a/drivers/net/dsa/netc/netc_ethtool.c
> +++ b/drivers/net/dsa/netc/netc_ethtool.c
[ ... ]
> @@ -19,6 +19,50 @@ static const struct ethtool_rmon_hist_range netc_rmon_ranges[] = {
[ ... ]
> +static const struct netc_port_stat netc_emac_counters[] = {
> +	{ NETC_PM_ROCT(0),	"eMAC rx octets" },
> +	{ NETC_PM_RVLAN(0),	"eMAC rx VLAN frames" },
> +	{ NETC_PM_RERR(0),	"eMAC rx frame errors" },
> +	{ NETC_PM_RUCA(0),	"eMAC rx unicast frames" },
> +	{ NETC_PM_RDRP(0),	"eMAC rx dropped packets" },
> +	{ NETC_PM_RPKT(0),	"eMAC rx packets" },
> +	{ NETC_PM_TOCT(0),	"eMAC tx octets" },
> +	{ NETC_PM_TVLAN(0),	"eMAC tx VLAN frames" },
> +	{ NETC_PM_TFCS(0),	"eMAC tx FCS errors" },
> +	{ NETC_PM_TUCA(0),	"eMAC tx unicast frames" },
> +	{ NETC_PM_TPKT(0),	"eMAC tx packets" },
> +	{ NETC_PM_TUND(0),	"eMAC tx undersized packets" },
> +	{ NETC_PM_TIOCT(0),	"eMAC tx invalid octets" },
> +};

Are these counters duplicating metrics that already have a standard uAPI?

The networking subsystem guidelines state that stats with a standard uAPI
should not be duplicated in the ethtool -S interface.

Since the driver already implements netc_port_get_eth_mac_stats() to handle
the eMAC and pMAC sources, could these standard MAC statistics be provided
through that standardized interface rather than adding them as private
string-based counters?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518082506.1318236-1-wei.fang@nxp.com?part=15