I noticed that sashiko-nipa aslo reported some warnings, most of them are minor issues or false positives. I will fix/improve them in the next version. Only this patch reported a high warning, but I don't think it's a problem. See sashiko's comment and my reply below.
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260509102954.4116624-1-wei.fang%40nxp.com > +static const struct netc_port_stat netc_port_counters[] = { > + { NETC_PTGSLACR, "port gate late arrival frames" }, > + { NETC_PSDFTCR, "port SDF transmit frames" }, > + { NETC_PSDFDDCR, "port SDF drop duplicate frames" }, > + { NETC_PRXDCR, "port rx discard frames" }, > + { NETC_PRXDCRRR, "port rx discard read-reset" }, > + { NETC_PRXDCRR0, "port rx discard reason 0" }, > + { NETC_PRXDCRR1, "port rx discard reason 1" }, > + { NETC_PTXDCR, "port tx discard frames" }, > + { NETC_PTXDCRRR, "port tx discard read-reset" }, > + { NETC_PTXDCRR0, "port tx discard reason 0" }, > + { NETC_PTXDCRR1, "port tx discard reason 1" }, > + { NETC_BPDCR, "bridge port discard frames" }, > + { NETC_BPDCRRR, "bridge port discard read-reset" }, > + { NETC_BPDCRR0, "bridge port discard reason 0" }, > + { NETC_BPDCRR1, "bridge port discard reason 1" }, > +}; Should the port rx/tx discard frame counts and the bridge port discard frame count be exposed through ethtool -S at all? These look like values that map onto the standard rtnl_link_stats64 fields (rx_dropped / tx_dropped) reported via ndo_get_stats64 and visible through "ip -s link show" on the DSA user netdev. [Wei Fang] The port rx/tx discard frame counts and the bridge port discard frame count are used to count the number of packets lost due to certain reasons including buffer exhaustion. By querying the packet loss reason register, users can clearly understand why the frames are dropped. These counters are not the same concept as rx_dropped/tx_dropped in rtnl_link_stats64. For example, rx_dropped should not includes packets dropped by the device due to buffer exhaustion, but PRXDCR includes them. Documentation/networking/statistics.rst calls out that ethtool -S is for driver-private counters only, and the networking review guidance asks that stats which already have a standard uAPI not be duplicated under ethtool -S even when the driver is not currently feeding the standard interface. Could the discard frame totals be routed through ndo_get_stats64 instead, leaving only the truly private parts (the per-reason RR0/RR1 codes, the SDF counters, the gate late-arrival counter) under ethtool -S? [Wei Fang] As I explained above, I would like to keep them in the driver-private counters. Is including NETC_PRXDCRRR / NETC_PTXDCRRR / NETC_BPDCRRR in the routine ethtool -S read path intentional? The "RR" suffix on these registers [Wei Fang] Yes, for example, NETC_PRXDCR represents the total statistics, while NETC_PRXDCRRR represents the statistics between the last read and the current read. During user debugging, this register, along with the packet drop reason register, can be used to determine the cause of packet drop during this period. appears to denote read-reset semantics; the sibling ENETC4 driver documents the same naming convention, e.g. in drivers/net/ethernet/freescale/enetc/enetc4_hw.h: /* Port Rx discard count read-reset register */ ... ENETC4_PRXDCRRR ... netc_port_get_ethtool_stats() reads them on every invocation: for (i = 0; i < ARRAY_SIZE(netc_port_counters); i++) *data++ = netc_port_rd(np, netc_port_counters[i].reg); If reading PRXDCRRR/PTXDCRRR/BPDCRRR clears the underlying counter as a side effect, then "watch ethtool -S" or any periodic poll will see a delta-since-last-read value rather than a cumulative total, and two concurrent observers will perturb each other's view. Would it be safer to expose only the non-reset variants (PRXDCR/PTXDCR/BPDCR, which are already in this list as "port rx/tx discard frames" and "bridge port discard frames") here, and move the read-reset aliases to a one-shot debug interface such as devlink or debugfs? [Wei Fang] Currently, we do not support devlink and debugfs, which makes debugging packet drop issues quite cumbersome for users. This current approach can serve as a transitional solution; we can remove it from ethtool -S when debugfs or devlink are supported. > + > +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" }, > +}; > + > +static const struct netc_port_stat netc_pmac_counters[] = { > + { NETC_PM_ROCT(1), "pMAC rx octets" }, > + { NETC_PM_RVLAN(1), "pMAC rx VLAN frames" }, > + { NETC_PM_RERR(1), "pMAC rx frame errors" }, > + { NETC_PM_RUCA(1), "pMAC rx unicast frames" }, > + { NETC_PM_RDRP(1), "pMAC rx dropped packets" }, > + { NETC_PM_RPKT(1), "pMAC rx packets" }, > + { NETC_PM_TOCT(1), "pMAC tx octets" }, > + { NETC_PM_TVLAN(1), "pMAC tx VLAN frames" }, > + { NETC_PM_TFCS(1), "pMAC tx FCS errors" }, > + { NETC_PM_TUCA(1), "pMAC tx unicast frames" }, > + { NETC_PM_TPKT(1), "pMAC tx packets" }, > + { NETC_PM_TUND(1), "pMAC tx undersized packets" }, > + { NETC_PM_TIOCT(1), "pMAC tx invalid octets" }, > +}; Do most of these eMAC and pMAC strings duplicate counters that already have a standard uAPI? [Wei Fang] No, the standard uAPI is implemented in patch 14. This part of the statistics is not supported by the standard API. Several look like direct overlaps with struct ethtool_eth_mac_stats, which this driver already populates via .get_eth_mac_stats: eMAC/pMAC rx octets (NETC_PM_ROCT) vs OctetsReceivedOK (already exposed via NETC_PM_REOCT) eMAC/pMAC tx octets (NETC_PM_TOCT) vs OctetsTransmittedOK (already exposed via NETC_PM_TEOCT) eMAC/pMAC rx packets (NETC_PM_RPKT) vs FramesReceivedOK (NETC_PM_RFRM) eMAC/pMAC tx packets (NETC_PM_TPKT) vs FramesTransmittedOK (NETC_PM_TFRM) eMAC/pMAC tx FCS errors / rx frame errors vs FrameCheckSequenceErrors and the FramesLostDueToIntMAC*Error fields [Wei Fang] They are different registers and the statistical data they produce is different. In addition, FrameCheckSequenceErrors is used for RX not TX. Others overlap rtnl_link_stats64 (visible via "ip -s link show"): rx octets / tx octets -> rx_bytes / tx_bytes rx packets / tx packets -> rx_packets / tx_packets rx dropped packets (RDRP) -> rx_dropped rx/tx unicast frames -> derivable from rtnl_link_stats64's total/multicast/broadcast triplet [Wei Fang] As I explained in v5, rtnl_link_stats64 is used to collect total statistics for the network device. For NETC switch, its ports support preemption, so each port has two MACs, the netc_p/emac_counters is used to collect statistics for each MAC. Their usage are different. And tx undersized packets (NETC_PM_TUND) overlaps the rmon undersize buckets that .get_rmon_stats already reports. [Wei Fang] The undersize_pkts of struct ethtool_rmon_stats is used for RX not TX.
