On Fri, Jan 24, 2025 at 07:10:51PM +0530, Basharath Hussain Khaja wrote:

From: Roger Quadros <redacted>

Changes for enabling ethtool support for the newly added PRU Ethernet
interfaces. Extends the support for statistics collection from PRU internal
memory and displays it in the user space. Along with statistics,
enable/disable of features, configuring link speed etc.are now supported.

The firmware running on PRU maintains statistics in internal data memory.
When requested ethtool collects all the statistics for the specified
interface and displays it in the user space.

Makefile is updated to include ethtool support into PRUETH driver.

Signed-off-by: Roger Quadros <redacted>
Signed-off-by: Andrew F. Davis <redacted>
Signed-off-by: Parvathi Pudi <parvathi@couthit.com>
Signed-off-by: Basharath Hussain Khaja <redacted>

...

...

+static const struct {
+	char string[ETH_GSTRING_LEN];
+	u32 offset;
+} prueth_ethtool_stats[] = {
+	{"txBcast", PRUETH_STAT_OFFSET(tx_bcast)},
+	{"txMcast", PRUETH_STAT_OFFSET(tx_mcast)},
+	{"txUcast", PRUETH_STAT_OFFSET(tx_ucast)},
+	{"txOctets", PRUETH_STAT_OFFSET(tx_octets)},
+	{"rxBcast", PRUETH_STAT_OFFSET(rx_bcast)},
+	{"rxMcast", PRUETH_STAT_OFFSET(rx_mcast)},
+	{"rxUcast", PRUETH_STAT_OFFSET(rx_ucast)},
+	{"rxOctets", PRUETH_STAT_OFFSET(rx_octets)},

Hi Roger, Basharath, all,

There seems to be some overlap between the above and struct rtnl_link_stats64.

Please implement those stats which are present in struct rtnl_link_stats64
using ndo_get_stats64 and omit them from your implementation of
get_ethtool_stats.

IOW, get_ethtool_stats() is for extended stats, whereas is for standard
stats ndo_get_stats64().  And standard stats should not be presented to the
user as extended stats.

Link: https://docs.kernel.org/networking/statistics.html#notes-for-driver-authors

+	{"tx64byte", PRUETH_STAT_OFFSET(tx64byte)},
+	{"tx65_127byte", PRUETH_STAT_OFFSET(tx65_127byte)},
+	{"tx128_255byte", PRUETH_STAT_OFFSET(tx128_255byte)},
+	{"tx256_511byte", PRUETH_STAT_OFFSET(tx256_511byte)},
+	{"tx512_1023byte", PRUETH_STAT_OFFSET(tx512_1023byte)},
+	{"tx1024byte", PRUETH_STAT_OFFSET(tx1024byte)},
+	{"rx64byte", PRUETH_STAT_OFFSET(rx64byte)},
+	{"rx65_127byte", PRUETH_STAT_OFFSET(rx65_127byte)},
+	{"rx128_255byte", PRUETH_STAT_OFFSET(rx128_255byte)},
+	{"rx256_511byte", PRUETH_STAT_OFFSET(rx256_511byte)},
+	{"rx512_1023byte", PRUETH_STAT_OFFSET(rx512_1023byte)},
+	{"rx1024byte", PRUETH_STAT_OFFSET(rx1024byte)},

Similarly, the above, along with rxOverSizedFrames and rxUnderSizedFrames
below seem to be RMON (RFC 2819) statistics. So I think they should
be handled by implementing get_rmon_stats().

+
+	{"lateColl", PRUETH_STAT_OFFSET(late_coll)},
+	{"singleColl", PRUETH_STAT_OFFSET(single_coll)},
+	{"multiColl", PRUETH_STAT_OFFSET(multi_coll)},
+	{"excessColl", PRUETH_STAT_OFFSET(excess_coll)},

And likewise, the section above and below seem to overlap
with Basic IEEE 802.3 MAC statistics which I believe
should be handled by implementing get_eth_mac_stats()

+
+	{"rxMisAlignmentFrames", PRUETH_STAT_OFFSET(rx_misalignment_frames)},
+	{"stormPrevCounterBC", PRUETH_STAT_OFFSET(stormprev_counter_bc)},
+	{"stormPrevCounterMC", PRUETH_STAT_OFFSET(stormprev_counter_mc)},
+	{"stormPrevCounterUC", PRUETH_STAT_OFFSET(stormprev_counter_uc)},
+	{"macRxError", PRUETH_STAT_OFFSET(mac_rxerror)},
+	{"SFDError", PRUETH_STAT_OFFSET(sfd_error)},
+	{"defTx", PRUETH_STAT_OFFSET(def_tx)},
+	{"macTxError", PRUETH_STAT_OFFSET(mac_txerror)},
+	{"rxOverSizedFrames", PRUETH_STAT_OFFSET(rx_oversized_frames)},
+	{"rxUnderSizedFrames", PRUETH_STAT_OFFSET(rx_undersized_frames)},
+	{"rxCRCFrames", PRUETH_STAT_OFFSET(rx_crc_frames)},
+	{"droppedPackets", PRUETH_STAT_OFFSET(dropped_packets)},
+
+	{"txHWQOverFlow", PRUETH_STAT_OFFSET(tx_hwq_overflow)},
+	{"txHWQUnderFlow", PRUETH_STAT_OFFSET(tx_hwq_underflow)},
+	{"vlanDropped", PRUETH_STAT_OFFSET(vlan_dropped)},
+	{"multicastDropped", PRUETH_STAT_OFFSET(multicast_dropped)},
+};

...