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

quoted

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>

...

quoted

diff --git a/drivers/net/ethernet/ti/icssm/icssm_ethtool.c

b/drivers/net/ethernet/ti/icssm/icssm_ethtool.c

...

quoted

+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

We will address this along with the changes that will be done by using 
rtnl_link_stats64 instead of legacy net_device_stats. 

quoted

+	{"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().

Sure. We will add get_rmon_stats() function and update necessary statistics
in that function.

quoted

+
+	{"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()

For now we will remove these stats in the next version
and address this in the next series of patches.

quoted

+
+	{"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)},
+};

...

Thanks & Best Regards,
Basharath