Re: [PATCH v7 net-next 6/8] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support
From: Roger Quadros <rogerq@kernel.org>
Date: 2023-12-08 13:43:51
Subsystem:
kernel selftest framework, networking drivers, networking [general], networking [ipv4/ipv6], the rest · Maintainers:
Shuah Khan, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern, Ido Schimmel, Linus Torvalds
On 07/12/2023 17:24, Vladimir Oltean wrote:
On Fri, Dec 01, 2023 at 03:58:00PM +0200, Roger Quadros wrote:quoted
- now passes all ethtool_mm.sh kselftests (patch 8 required)There's another problem with your patch set, leading to the traffic_test() selftest passing with the "emac" and "pmac" argument when it shouldn't. Drivers which implement frame preemption are supposed to be prepared to distinguish, in their ethtool get_eth_mac_stats() method, between ETHTOOL_MAC_STATS_SRC_EMAC, ETHTOOL_MAC_STATS_SRC_PMAC and ETHTOOL_MAC_STATS_SRC_AGGREGATE. You don't, so you report the same thing everywhere, which is incorrect, and it also fools the selftest. I would like you to figure out which source are your stats coming from, reject everything else, and edit the selftest to do something sensible for your hardware. If it's as I suspect and the reported counters are eMAC+pMAC aggregates, then I guess the most sensible thing to do would be to probe the device with an ethtool -S --src pmac at the beginning of the test, see what it reports, and if we don't get pMAC counters from it, fall back to aggregate ones during calls to traffic_test().
Thanks for the catch.
How do the below 2 patches look to resolve this?
From af2a8503dc04c54d6eaf50954628009aba54e2c8 Mon Sep 17 00:00:00 2001
From: Roger Quadros <rogerq@kernel.org>
Date: Fri, 8 Dec 2023 15:11:06 +0200
Subject: [PATCH] net: ethernet: ti: am65-cpsw: Fix get_eth_mac_stats
We do not support individual stats for PMAC and EMAC so
report only aggregate stats.
Fixes: 67372d7a85fcd ("net: ethernet: am65-cpsw: Add standard Ethernet MAC stats to ethtool")
Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
drivers/net/ethernet/ti/am65-cpsw-ethtool.c | 37 ++++++++++++---------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
index 44e01d68db39..934b03382508 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c@@ -671,21 +671,28 @@ static void am65_cpsw_get_eth_mac_stats(struct net_device *ndev, stats = port->stat_base; - s->FramesTransmittedOK = readl_relaxed(&stats->tx_good_frames); - s->SingleCollisionFrames = readl_relaxed(&stats->tx_single_coll_frames); - s->MultipleCollisionFrames = readl_relaxed(&stats->tx_mult_coll_frames); - s->FramesReceivedOK = readl_relaxed(&stats->rx_good_frames); - s->FrameCheckSequenceErrors = readl_relaxed(&stats->rx_crc_errors); - s->AlignmentErrors = readl_relaxed(&stats->rx_align_code_errors); - s->OctetsTransmittedOK = readl_relaxed(&stats->tx_octets); - s->FramesWithDeferredXmissions = readl_relaxed(&stats->tx_deferred_frames); - s->LateCollisions = readl_relaxed(&stats->tx_late_collisions); - s->CarrierSenseErrors = readl_relaxed(&stats->tx_carrier_sense_errors); - s->OctetsReceivedOK = readl_relaxed(&stats->rx_octets); - s->MulticastFramesXmittedOK = readl_relaxed(&stats->tx_multicast_frames); - s->BroadcastFramesXmittedOK = readl_relaxed(&stats->tx_broadcast_frames); - s->MulticastFramesReceivedOK = readl_relaxed(&stats->rx_multicast_frames); - s->BroadcastFramesReceivedOK = readl_relaxed(&stats->rx_broadcast_frames); + switch (s->src) { + case ETHTOOL_MAC_STATS_SRC_AGGREGATE: + s->FramesTransmittedOK = readl_relaxed(&stats->tx_good_frames); + s->SingleCollisionFrames = readl_relaxed(&stats->tx_single_coll_frames); + s->MultipleCollisionFrames = readl_relaxed(&stats->tx_mult_coll_frames); + s->FramesReceivedOK = readl_relaxed(&stats->rx_good_frames); + s->FrameCheckSequenceErrors = readl_relaxed(&stats->rx_crc_errors); + s->AlignmentErrors = readl_relaxed(&stats->rx_align_code_errors); + s->OctetsTransmittedOK = readl_relaxed(&stats->tx_octets); + s->FramesWithDeferredXmissions = readl_relaxed(&stats->tx_deferred_frames); + s->LateCollisions = readl_relaxed(&stats->tx_late_collisions); + s->CarrierSenseErrors = readl_relaxed(&stats->tx_carrier_sense_errors); + s->OctetsReceivedOK = readl_relaxed(&stats->rx_octets); + s->MulticastFramesXmittedOK = readl_relaxed(&stats->tx_multicast_frames); + s->BroadcastFramesXmittedOK = readl_relaxed(&stats->tx_broadcast_frames); + s->MulticastFramesReceivedOK = readl_relaxed(&stats->rx_multicast_frames); + s->BroadcastFramesReceivedOK = readl_relaxed(&stats->rx_broadcast_frames); + break; + default: + break; + } }; static int am65_cpsw_get_ethtool_ts_info(struct net_device *ndev,
base-commit: a78e0a2c4353d6c100e45c5ef738113bf2d0fda5 -- 2.34.1 From 0b20d8b8ef110d886396ee2486f3a9e20170cc85 Mon Sep 17 00:00:00 2001 From: Roger Quadros <rogerq@kernel.org> Date: Fri, 8 Dec 2023 15:38:57 +0200 Subject: [PATCH] selftests: forwarding: ethtool_mm: support devices that don't support pmac stats Some devices do not support individual 'pmac' and 'emac' stats. For such devices, resort to 'aggregate' stats. Signed-off-by: Roger Quadros <rogerq@kernel.org> --- tools/testing/selftests/net/forwarding/ethtool_mm.sh | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
index 6212913f4ad1..e3f2e62029ca 100755
--- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
+++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh@@ -26,6 +26,13 @@ traffic_test() local delta= before=$(ethtool_std_stats_get $if "eth-mac" "FramesTransmittedOK" $src) + # some devices don't support individual pmac/emac stats, + # use aggregate stats for them. + if [ "$before" == null ]; then + src="aggregate" + before=$(ethtool_std_stats_get $if "eth-mac" "FramesTransmittedOO +K" $src) + fi $MZ $if -q -c $num_pkts -p 64 -b bcast -t ip -R $PREEMPTIBLE_PRIO
base-commit: af2a8503dc04c54d6eaf50954628009aba54e2c8 -- 2.34.1 -- cheers, -roger