* [PATCH 0/2] selftests: net: ethtool_mm: Support devices with higher rx-min-frag-size
@ 2023-12-11 12:01 Roger Quadros
2023-12-11 12:01 ` [PATCH 1/2] selftests: forwarding: ethtool_mm: support " Roger Quadros
2023-12-11 12:01 ` [PATCH 2/2] selftests: forwarding: ethtool_mm: support devices that don't support pmac stats Roger Quadros
0 siblings, 2 replies; 8+ messages in thread
From: Roger Quadros @ 2023-12-11 12:01 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, shuah, vladimir.oltean
Cc: s-vadapalli, r-gunasekaran, vigneshr, srk, horms, p-varis, netdev,
rogerq
Hi,
Some devices e.g. TI SoCs have an errata due to which they cannot support
rx-min-frag-size of ETH_ZLEN (60). Also some devices cannot report
individual pmac/emac stats.
Modify the test so it can pass for such devices.
cheers,
-roger
Roger Quadros (1):
selftests: forwarding: ethtool_mm: support devices that don't support
pmac stats
Vladimir Oltean (1):
selftests: forwarding: ethtool_mm: support devices with higher
rx-min-frag-size
.../selftests/net/forwarding/ethtool_mm.sh | 44 ++++++++++++++++++-
1 file changed, 42 insertions(+), 2 deletions(-)
base-commit: 70028b2e51c61d8dda0a31985978f4745da6a11b
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] selftests: forwarding: ethtool_mm: support devices with higher rx-min-frag-size 2023-12-11 12:01 [PATCH 0/2] selftests: net: ethtool_mm: Support devices with higher rx-min-frag-size Roger Quadros @ 2023-12-11 12:01 ` Roger Quadros 2023-12-11 12:01 ` [PATCH 2/2] selftests: forwarding: ethtool_mm: support devices that don't support pmac stats Roger Quadros 1 sibling, 0 replies; 8+ messages in thread From: Roger Quadros @ 2023-12-11 12:01 UTC (permalink / raw) To: davem, edumazet, kuba, pabeni, shuah, vladimir.oltean Cc: s-vadapalli, r-gunasekaran, vigneshr, srk, horms, p-varis, netdev, rogerq From: Vladimir Oltean <vladimir.oltean@nxp.com> Some devices have errata due to which they cannot report ETH_ZLEN (60) in the rx-min-frag-size. This was foreseen of course, and lldpad has logic that when we request it to advertise addFragSize 0, it will round it up to the lowest value that is _actually_ supported by the hardware. The problem is that the selftest expects lldpad to report back to us the same value as we requested. Make the selftest smarter by figuring out on its own what is a reasonable value to expect. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Tested-by: Roger Quadros <rogerq@kernel.org> Signed-off-by: Roger Quadros <rogerq@kernel.org> --- .../selftests/net/forwarding/ethtool_mm.sh | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh index 39e736f30322..6212913f4ad1 100755 --- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh +++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh @@ -155,15 +155,48 @@ manual_failed_verification_h2_to_h1() manual_failed_verification $h2 $h1 } +smallest_supported_add_frag_size() +{ + local iface=$1 + local rx_min_frag_size= + + rx_min_frag_size=$(ethtool --json --show-mm $iface | \ + jq '.[]."rx-min-frag-size"') + + if [ $rx_min_frag_size -le 60 ]; then + echo 0 + elif [ $rx_min_frag_size -le 124 ]; then + echo 1 + elif [ $rx_min_frag_size -le 188 ]; then + echo 2 + elif [ $rx_min_frag_size -le 252 ]; then + echo 3 + else + echo "$iface: RX min frag size $rx_min_frag_size cannot be advertised over LLDP" + exit 1 + fi +} + +expected_add_frag_size() +{ + local iface=$1 + local requested=$2 + local min=$(smallest_supported_add_frag_size $iface) + + [ $requested -le $min ] && echo $min || echo $requested +} + lldp_change_add_frag_size() { local add_frag_size=$1 + local pattern= lldptool -T -i $h1 -V addEthCaps addFragSize=$add_frag_size >/dev/null # Wait for TLVs to be received sleep 2 - lldptool -i $h2 -t -n -V addEthCaps | \ - grep -q "Additional fragment size: $add_frag_size" + pattern=$(printf "Additional fragment size: %d" \ + $(expected_add_frag_size $h1 $add_frag_size)) + lldptool -i $h2 -t -n -V addEthCaps | grep -q "$pattern" } lldp() -- 2.34.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] selftests: forwarding: ethtool_mm: support devices that don't support pmac stats 2023-12-11 12:01 [PATCH 0/2] selftests: net: ethtool_mm: Support devices with higher rx-min-frag-size Roger Quadros 2023-12-11 12:01 ` [PATCH 1/2] selftests: forwarding: ethtool_mm: support " Roger Quadros @ 2023-12-11 12:01 ` Roger Quadros 2023-12-11 13:24 ` Vladimir Oltean 1 sibling, 1 reply; 8+ messages in thread From: Roger Quadros @ 2023-12-11 12:01 UTC (permalink / raw) To: davem, edumazet, kuba, pabeni, shuah, vladimir.oltean Cc: s-vadapalli, r-gunasekaran, vigneshr, srk, horms, p-varis, netdev, rogerq 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 -- 2.34.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] selftests: forwarding: ethtool_mm: support devices that don't support pmac stats 2023-12-11 12:01 ` [PATCH 2/2] selftests: forwarding: ethtool_mm: support devices that don't support pmac stats Roger Quadros @ 2023-12-11 13:24 ` Vladimir Oltean 2023-12-11 13:53 ` Roger Quadros 2023-12-12 14:07 ` Roger Quadros 0 siblings, 2 replies; 8+ messages in thread From: Vladimir Oltean @ 2023-12-11 13:24 UTC (permalink / raw) To: Roger Quadros Cc: davem, edumazet, kuba, pabeni, shuah, s-vadapalli, r-gunasekaran, vigneshr, srk, horms, p-varis, netdev On Mon, Dec 11, 2023 at 02:01:38PM +0200, Roger Quadros wrote: > 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 1. please follow the existing indentation scheme, don't mix tabs with spaces 2. someone mangled your patch into invalid bash syntax, splitting a line into 2 3. "FramesTransmittedOOK" has an extra "O" 4. it would be preferable if you could evaluate only once whether pMAC counters are reported, set a global variable, and in traffic_test(), if that variable is true, override $src with "aggregate". 5. why did you split the selftest patches out of the am65-cpsw patch set? It is for the am65-cpsw that they are needed. > > $MZ $if -q -c $num_pkts -p 64 -b bcast -t ip -R $PREEMPTIBLE_PRIO > > -- > 2.34.1 > Something like this? From ef5688a78908d99b607909fd7c93829c6a018b61 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: Mon, 11 Dec 2023 15:21:25 +0200 Subject: [PATCH] selftests: forwarding: ethtool_mm: fall back to aggregate if device does not report pMAC stats Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- tools/testing/selftests/net/forwarding/ethtool_mm.sh | 11 +++++++++++ tools/testing/selftests/net/forwarding/lib.sh | 8 ++++++++ 2 files changed, 19 insertions(+) diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh index 39e736f30322..2740133f95ec 100755 --- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh +++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh @@ -25,6 +25,10 @@ traffic_test() local after= local delta= + if [ has_pmac_stats[$netif] = false ]; then + src="aggregate" + fi + before=$(ethtool_std_stats_get $if "eth-mac" "FramesTransmittedOK" $src) $MZ $if -q -c $num_pkts -p 64 -b bcast -t ip -R $PREEMPTIBLE_PRIO @@ -284,6 +288,13 @@ for netif in ${NETIFS[@]}; do echo "SKIP: $netif does not support MAC Merge" exit $ksft_skip fi + + if check_ethtool_pmac_std_stats_support $netif; then + has_pmac_stats[$netif]=true + else + has_pmac_stats[$netif]=false + echo "$netif does not report pMAC statistics, falling back to aggregate" + fi done trap cleanup EXIT diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index 8f6ca458af9a..82ac6a066729 100755 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -146,6 +146,14 @@ check_ethtool_mm_support() fi } +check_ethtool_pmac_std_stats_support() +{ + local dev=$1; shift + + [ -n "$(ethtool --json -S $dev --all-groups --src pmac 2>/dev/null | \ + jq '.[]')" ] +} + check_locked_port_support() { if ! bridge -d link show | grep -q " locked"; then -- 2.34.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] selftests: forwarding: ethtool_mm: support devices that don't support pmac stats 2023-12-11 13:24 ` Vladimir Oltean @ 2023-12-11 13:53 ` Roger Quadros 2023-12-12 14:07 ` Roger Quadros 1 sibling, 0 replies; 8+ messages in thread From: Roger Quadros @ 2023-12-11 13:53 UTC (permalink / raw) To: Vladimir Oltean Cc: davem, edumazet, kuba, pabeni, shuah, s-vadapalli, r-gunasekaran, vigneshr, srk, horms, p-varis, netdev On 11/12/2023 15:24, Vladimir Oltean wrote: > On Mon, Dec 11, 2023 at 02:01:38PM +0200, Roger Quadros wrote: >> 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 > > 1. please follow the existing indentation scheme, don't mix tabs with spaces Sure. Will fix. > 2. someone mangled your patch into invalid bash syntax, splitting a line > into 2 > 3. "FramesTransmittedOOK" has an extra "O" Will fix. The mangling happened at my end. > 4. it would be preferable if you could evaluate only once whether pMAC > counters are reported, set a global variable, and in traffic_test(), > if that variable is true, override $src with "aggregate". > 5. why did you split the selftest patches out of the am65-cpsw patch > set? It is for the am65-cpsw that they are needed. Needed for the tests to pass, yes. I'll add them back at the beginning of the series. > >> >> $MZ $if -q -c $num_pkts -p 64 -b bcast -t ip -R $PREEMPTIBLE_PRIO >> >> -- >> 2.34.1 >> > > Something like this? Thanks for the hint! > > From ef5688a78908d99b607909fd7c93829c6a018b61 Mon Sep 17 00:00:00 2001 > From: Vladimir Oltean <vladimir.oltean@nxp.com> > Date: Mon, 11 Dec 2023 15:21:25 +0200 > Subject: [PATCH] selftests: forwarding: ethtool_mm: fall back to aggregate if > device does not report pMAC stats > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > tools/testing/selftests/net/forwarding/ethtool_mm.sh | 11 +++++++++++ > tools/testing/selftests/net/forwarding/lib.sh | 8 ++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh > index 39e736f30322..2740133f95ec 100755 > --- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh > +++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh > @@ -25,6 +25,10 @@ traffic_test() > local after= > local delta= > > + if [ has_pmac_stats[$netif] = false ]; then > + src="aggregate" > + fi > + > before=$(ethtool_std_stats_get $if "eth-mac" "FramesTransmittedOK" $src) > > $MZ $if -q -c $num_pkts -p 64 -b bcast -t ip -R $PREEMPTIBLE_PRIO > @@ -284,6 +288,13 @@ for netif in ${NETIFS[@]}; do > echo "SKIP: $netif does not support MAC Merge" > exit $ksft_skip > fi > + > + if check_ethtool_pmac_std_stats_support $netif; then > + has_pmac_stats[$netif]=true > + else > + has_pmac_stats[$netif]=false > + echo "$netif does not report pMAC statistics, falling back to aggregate" > + fi > done > > trap cleanup EXIT > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh > index 8f6ca458af9a..82ac6a066729 100755 > --- a/tools/testing/selftests/net/forwarding/lib.sh > +++ b/tools/testing/selftests/net/forwarding/lib.sh > @@ -146,6 +146,14 @@ check_ethtool_mm_support() > fi > } > > +check_ethtool_pmac_std_stats_support() > +{ > + local dev=$1; shift > + > + [ -n "$(ethtool --json -S $dev --all-groups --src pmac 2>/dev/null | \ > + jq '.[]')" ] > +} > + > check_locked_port_support() > { > if ! bridge -d link show | grep -q " locked"; then -- cheers, -roger ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] selftests: forwarding: ethtool_mm: support devices that don't support pmac stats 2023-12-11 13:24 ` Vladimir Oltean 2023-12-11 13:53 ` Roger Quadros @ 2023-12-12 14:07 ` Roger Quadros 2023-12-12 14:57 ` Vladimir Oltean 1 sibling, 1 reply; 8+ messages in thread From: Roger Quadros @ 2023-12-12 14:07 UTC (permalink / raw) To: Vladimir Oltean Cc: davem, edumazet, kuba, pabeni, shuah, s-vadapalli, r-gunasekaran, vigneshr, srk, horms, p-varis, netdev Hi Vladimir, On 11/12/2023 15:24, Vladimir Oltean wrote: > On Mon, Dec 11, 2023 at 02:01:38PM +0200, Roger Quadros wrote: >> 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 > > 1. please follow the existing indentation scheme, don't mix tabs with spaces > 2. someone mangled your patch into invalid bash syntax, splitting a line > into 2 > 3. "FramesTransmittedOOK" has an extra "O" > 4. it would be preferable if you could evaluate only once whether pMAC > counters are reported, set a global variable, and in traffic_test(), > if that variable is true, override $src with "aggregate". > 5. why did you split the selftest patches out of the am65-cpsw patch > set? It is for the am65-cpsw that they are needed. > >> >> $MZ $if -q -c $num_pkts -p 64 -b bcast -t ip -R $PREEMPTIBLE_PRIO >> >> -- >> 2.34.1 >> > > Something like this? > > From ef5688a78908d99b607909fd7c93829c6a018b61 Mon Sep 17 00:00:00 2001 > From: Vladimir Oltean <vladimir.oltean@nxp.com> > Date: Mon, 11 Dec 2023 15:21:25 +0200 > Subject: [PATCH] selftests: forwarding: ethtool_mm: fall back to aggregate if > device does not report pMAC stats > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > tools/testing/selftests/net/forwarding/ethtool_mm.sh | 11 +++++++++++ > tools/testing/selftests/net/forwarding/lib.sh | 8 ++++++++ > 2 files changed, 19 insertions(+) What is the proper way to run the script? I've been hardcoding the following in the script. NETIFS=( "eth0" "eth1" ) in setup_prepare() h1=eth0 h2=eth1 and run the script like so ./run_kselftest.sh -t net/forwarding:ethtool_mm.sh > > diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh > index 39e736f30322..2740133f95ec 100755 > --- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh > +++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh > @@ -25,6 +25,10 @@ traffic_test() > local after= > local delta= > > + if [ has_pmac_stats[$netif] = false ]; then This should be if [ ${has_pmac_stats[$if]} = false ]; then otherwise it doesn't work. > + src="aggregate" > + fi > + > before=$(ethtool_std_stats_get $if "eth-mac" "FramesTransmittedOK" $src) > > $MZ $if -q -c $num_pkts -p 64 -b bcast -t ip -R $PREEMPTIBLE_PRIO > @@ -284,6 +288,13 @@ for netif in ${NETIFS[@]}; do > echo "SKIP: $netif does not support MAC Merge" > exit $ksft_skip > fi > + > + if check_ethtool_pmac_std_stats_support $netif; then > + has_pmac_stats[$netif]=true > + else > + has_pmac_stats[$netif]=false > + echo "$netif does not report pMAC statistics, falling back to aggregate" > + fi > done > > trap cleanup EXIT > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh > index 8f6ca458af9a..82ac6a066729 100755 > --- a/tools/testing/selftests/net/forwarding/lib.sh > +++ b/tools/testing/selftests/net/forwarding/lib.sh > @@ -146,6 +146,14 @@ check_ethtool_mm_support() > fi > } > > +check_ethtool_pmac_std_stats_support() > +{ > + local dev=$1; shift > + > + [ -n "$(ethtool --json -S $dev --all-groups --src pmac 2>/dev/null | \ > + jq '.[]')" ] This is evaluating to true instead of false on my platform so something needs to be fixed here. Below is the output of "ethtool --json -S eth0 --all-groups --src pmac" [ { "ifname": "eth0", "eth-phy": {}, "eth-mac": {}, "eth-ctrl": {}, "rmon": {} } ] I suppose we want to check if eth-mac has anything or not. Something like this works [ 0 -ne $(ethtool --json -S $dev --all-groups --src pmac 2>/dev/null \ | jq '.[]."eth-mac" | length') ] OK? > +} > + > check_locked_port_support() > { > if ! bridge -d link show | grep -q " locked"; then also I had to revert a recent commit 25ae948b4478 ("selftests/net: add lib.sh") else i get an error message syaing ../lib.sh not found. Looks like that is not getting deployed on kselftest-install I will report this in the original patch thread as well. -- cheers, -roger ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] selftests: forwarding: ethtool_mm: support devices that don't support pmac stats 2023-12-12 14:07 ` Roger Quadros @ 2023-12-12 14:57 ` Vladimir Oltean 2023-12-12 19:48 ` Roger Quadros 0 siblings, 1 reply; 8+ messages in thread From: Vladimir Oltean @ 2023-12-12 14:57 UTC (permalink / raw) To: Roger Quadros Cc: davem, edumazet, kuba, pabeni, shuah, s-vadapalli, r-gunasekaran, vigneshr, srk, horms, p-varis, netdev On Tue, Dec 12, 2023 at 04:07:18PM +0200, Roger Quadros wrote: > What is the proper way to run the script? > > I've been hardcoding the following in the script. > > NETIFS=( "eth0" "eth1" ) > > in setup_prepare() > h1=eth0 > h2=eth1 > > and run the script like so > > ./run_kselftest.sh -t net/forwarding:ethtool_mm.sh IDK. I rsync the selftest dir to my board and do: $ cd selftests/net/forwarding $ ./ethtool.mm eth0 eth1 Running through run_kselftest.sh is probably better. I think that also supports passing the network interfaces as arguments, no need to hack up the script. > > diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh > > index 39e736f30322..2740133f95ec 100755 > > --- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh > > +++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh > > @@ -25,6 +25,10 @@ traffic_test() > > local after= > > local delta= > > > > + if [ has_pmac_stats[$netif] = false ]; then > > This should be > if [ ${has_pmac_stats[$if]} = false ]; then > > otherwise it doesn't work. Makes sense. > > + src="aggregate" > > + fi > > + > > before=$(ethtool_std_stats_get $if "eth-mac" "FramesTransmittedOK" $src) > > > > $MZ $if -q -c $num_pkts -p 64 -b bcast -t ip -R $PREEMPTIBLE_PRIO > > @@ -284,6 +288,13 @@ for netif in ${NETIFS[@]}; do > > echo "SKIP: $netif does not support MAC Merge" > > exit $ksft_skip > > fi > > + > > + if check_ethtool_pmac_std_stats_support $netif; then > > + has_pmac_stats[$netif]=true > > > > + else > > + has_pmac_stats[$netif]=false > > + echo "$netif does not report pMAC statistics, falling back to aggregate" > > + fi > > done > > > > trap cleanup EXIT > > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh > > index 8f6ca458af9a..82ac6a066729 100755 > > --- a/tools/testing/selftests/net/forwarding/lib.sh > > +++ b/tools/testing/selftests/net/forwarding/lib.sh > > @@ -146,6 +146,14 @@ check_ethtool_mm_support() > > fi > > } > > > > +check_ethtool_pmac_std_stats_support() > > +{ > > + local dev=$1; shift > > + > > + [ -n "$(ethtool --json -S $dev --all-groups --src pmac 2>/dev/null | \ > > + jq '.[]')" ] > > This is evaluating to true instead of false on my platform so something needs to be fixed here. > > Below is the output of "ethtool --json -S eth0 --all-groups --src pmac" > > [ { > "ifname": "eth0", > "eth-phy": {}, > "eth-mac": {}, > "eth-ctrl": {}, > "rmon": {} > } ] > > I suppose we want to check if eth-mac has anything or not. > > Something like this works > > [ 0 -ne $(ethtool --json -S $dev --all-groups --src pmac 2>/dev/null \ > | jq '.[]."eth-mac" | length') ] > > OK? Maybe giving the stats group as argument instead of hardcoding "eth-mac" would make sense. I hoped we could avoid hardcoding one particular group of counters in check_ethtool_pmac_std_stats_support(). > > +} > > + > > check_locked_port_support() > > { > > if ! bridge -d link show | grep -q " locked"; then > > also I had to revert a recent commit > > 25ae948b4478 ("selftests/net: add lib.sh") > > else i get an error message syaing ../lib.sh not found. > Looks like that is not getting deployed on kselftest-install > > I will report this in the original patch thread as well. I'm not using that part, so I didn't notice it :) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] selftests: forwarding: ethtool_mm: support devices that don't support pmac stats 2023-12-12 14:57 ` Vladimir Oltean @ 2023-12-12 19:48 ` Roger Quadros 0 siblings, 0 replies; 8+ messages in thread From: Roger Quadros @ 2023-12-12 19:48 UTC (permalink / raw) To: Vladimir Oltean Cc: davem, edumazet, kuba, pabeni, shuah, s-vadapalli, r-gunasekaran, vigneshr, srk, horms, p-varis, netdev On 12/12/2023 16:57, Vladimir Oltean wrote: > On Tue, Dec 12, 2023 at 04:07:18PM +0200, Roger Quadros wrote: >> What is the proper way to run the script? >> >> I've been hardcoding the following in the script. >> >> NETIFS=( "eth0" "eth1" ) >> >> in setup_prepare() >> h1=eth0 >> h2=eth1 >> >> and run the script like so >> >> ./run_kselftest.sh -t net/forwarding:ethtool_mm.sh > > IDK. I rsync the selftest dir to my board and do: > > $ cd selftests/net/forwarding > $ ./ethtool.mm eth0 eth1 > > Running through run_kselftest.sh is probably better. I think that also > supports passing the network interfaces as arguments, no need to hack up > the script. > >>> diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh >>> index 39e736f30322..2740133f95ec 100755 >>> --- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh >>> +++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh >>> @@ -25,6 +25,10 @@ traffic_test() >>> local after= >>> local delta= >>> >>> + if [ has_pmac_stats[$netif] = false ]; then >> >> This should be >> if [ ${has_pmac_stats[$if]} = false ]; then >> >> otherwise it doesn't work. > > Makes sense. > >>> + src="aggregate" >>> + fi >>> + >>> before=$(ethtool_std_stats_get $if "eth-mac" "FramesTransmittedOK" $src) >>> >>> $MZ $if -q -c $num_pkts -p 64 -b bcast -t ip -R $PREEMPTIBLE_PRIO >>> @@ -284,6 +288,13 @@ for netif in ${NETIFS[@]}; do >>> echo "SKIP: $netif does not support MAC Merge" >>> exit $ksft_skip >>> fi >>> + >>> + if check_ethtool_pmac_std_stats_support $netif; then >>> + has_pmac_stats[$netif]=true >> >> >>> + else >>> + has_pmac_stats[$netif]=false >>> + echo "$netif does not report pMAC statistics, falling back to aggregate" >>> + fi >>> done >>> >>> trap cleanup EXIT >>> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh >>> index 8f6ca458af9a..82ac6a066729 100755 >>> --- a/tools/testing/selftests/net/forwarding/lib.sh >>> +++ b/tools/testing/selftests/net/forwarding/lib.sh >>> @@ -146,6 +146,14 @@ check_ethtool_mm_support() >>> fi >>> } >>> >>> +check_ethtool_pmac_std_stats_support() >>> +{ >>> + local dev=$1; shift >>> + >>> + [ -n "$(ethtool --json -S $dev --all-groups --src pmac 2>/dev/null | \ >>> + jq '.[]')" ] >> >> This is evaluating to true instead of false on my platform so something needs to be fixed here. >> >> Below is the output of "ethtool --json -S eth0 --all-groups --src pmac" >> >> [ { >> "ifname": "eth0", >> "eth-phy": {}, >> "eth-mac": {}, >> "eth-ctrl": {}, >> "rmon": {} >> } ] >> >> I suppose we want to check if eth-mac has anything or not. >> >> Something like this works >> >> [ 0 -ne $(ethtool --json -S $dev --all-groups --src pmac 2>/dev/null \ >> | jq '.[]."eth-mac" | length') ] >> >> OK? > > Maybe giving the stats group as argument instead of hardcoding "eth-mac" > would make sense. I hoped we could avoid hardcoding one particular group > of counters in check_ethtool_pmac_std_stats_support(). You mean like this? check_ethtool_pmac_std_stats_support() { local dev=$1; shift local grp=$1; shift [ 0 -ne $(ethtool --json -S $dev --all-groups --src pmac 2>/dev/null \ | jq '.[]."$grp" | length') ] } Caller will call like so check_ethtool_pmac_std_stats_support $netif eth-mac -- cheers, -roger ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-12-12 19:48 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-12-11 12:01 [PATCH 0/2] selftests: net: ethtool_mm: Support devices with higher rx-min-frag-size Roger Quadros 2023-12-11 12:01 ` [PATCH 1/2] selftests: forwarding: ethtool_mm: support " Roger Quadros 2023-12-11 12:01 ` [PATCH 2/2] selftests: forwarding: ethtool_mm: support devices that don't support pmac stats Roger Quadros 2023-12-11 13:24 ` Vladimir Oltean 2023-12-11 13:53 ` Roger Quadros 2023-12-12 14:07 ` Roger Quadros 2023-12-12 14:57 ` Vladimir Oltean 2023-12-12 19:48 ` Roger Quadros
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).