> On Fri, Jan 24, 2025 at 07:10:51PM +0530, Basharath Hussain Khaja wrote:
>> From: Roger Quadros <rogerq@xxxxxx>
>>
>> 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 <rogerq@xxxxxx>
>> Signed-off-by: Andrew F. Davis <afd@xxxxxx>
>> Signed-off-by: Parvathi Pudi <parvathi@xxxxxxxxxxx>
>> Signed-off-by: Basharath Hussain Khaja <basharath@xxxxxxxxxxx>
>
> ...
>
>> diff --git a/drivers/net/ethernet/ti/icssm/icssm_ethtool.c
>> b/drivers/net/ethernet/ti/icssm/icssm_ethtool.c
>
> ...
>
>> +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.
>> + {"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.
>> +
>> + {"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.
>> +
>> + {"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
