[PATCH net-next 4/5] selftests: forwarding: extend ethtool_std_stats_get with pause statistics

Ioana Ciornei posted 5 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH net-next 4/5] selftests: forwarding: extend ethtool_std_stats_get with pause statistics
Posted by Ioana Ciornei 1 month, 1 week ago
Even though pause frame statistics are not exported through the same
ethtool command, there is no point in adding another helper just for
them. Extent the ethtool_std_stats_get() function so that we are able to
interrogate using the same helper all the standard statistics.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 tools/testing/selftests/net/forwarding/lib.sh | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index a9034f0bb58b..efd236ae1c28 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -831,8 +831,12 @@ ethtool_std_stats_get()
 	local name=$1; shift
 	local src=$1; shift
 
-	ethtool --json -S $dev --groups $grp -- --src $src | \
-		jq '.[]."'"$grp"'"."'$name'"'
+	if [[ "$grp" == "pause" ]]; then
+		ethtool -I --json -a $dev | jq '.[].statistics.'$name
+	else
+		ethtool --json -S $dev --groups $grp -- --src $src | \
+			jq '.[]."'"$grp"'"."'$name'"'
+	fi
 }
 
 qdisc_stats_get()
-- 
2.25.1
Re: [PATCH net-next 4/5] selftests: forwarding: extend ethtool_std_stats_get with pause statistics
Posted by Petr Machata 1 month, 1 week ago
Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> Even though pause frame statistics are not exported through the same
> ethtool command, there is no point in adding another helper just for
> them. Extent the ethtool_std_stats_get() function so that we are able to
> interrogate using the same helper all the standard statistics.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
>  tools/testing/selftests/net/forwarding/lib.sh | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index a9034f0bb58b..efd236ae1c28 100644
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -831,8 +831,12 @@ ethtool_std_stats_get()
>  	local name=$1; shift
>  	local src=$1; shift
>  
> -	ethtool --json -S $dev --groups $grp -- --src $src | \
> -		jq '.[]."'"$grp"'"."'$name'"'
> +	if [[ "$grp" == "pause" ]]; then
> +		ethtool -I --json -a $dev | jq '.[].statistics.'$name

I think name needs to be quoted here? In fact, unless the pause group is
highly unlikely to ever get a key that contains a dash, it should either
be quoted in the horrible way the else branch does it, or do this much
more readable thing instead:

        jq --arg name "$name" '.[].statistics[$name]'

> +	else
> +		ethtool --json -S $dev --groups $grp -- --src $src | \

Since you are touching this line -- can you fix the missing quoting,
please?

> +			jq '.[]."'"$grp"'"."'$name'"'
> +	fi
>  }
>  
>  qdisc_stats_get()
Re: [PATCH net-next 4/5] selftests: forwarding: extend ethtool_std_stats_get with pause statistics
Posted by Ioana Ciornei 1 month ago
On Fri, Feb 27, 2026 at 05:38:40PM +0100, Petr Machata wrote:
> 
> Ioana Ciornei <ioana.ciornei@nxp.com> writes:
> 
> > Even though pause frame statistics are not exported through the same
> > ethtool command, there is no point in adding another helper just for
> > them. Extent the ethtool_std_stats_get() function so that we are able to
> > interrogate using the same helper all the standard statistics.
> >
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> >  tools/testing/selftests/net/forwarding/lib.sh | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> > index a9034f0bb58b..efd236ae1c28 100644
> > --- a/tools/testing/selftests/net/forwarding/lib.sh
> > +++ b/tools/testing/selftests/net/forwarding/lib.sh
> > @@ -831,8 +831,12 @@ ethtool_std_stats_get()
> >  	local name=$1; shift
> >  	local src=$1; shift
> >  
> > -	ethtool --json -S $dev --groups $grp -- --src $src | \
> > -		jq '.[]."'"$grp"'"."'$name'"'
> > +	if [[ "$grp" == "pause" ]]; then
> > +		ethtool -I --json -a $dev | jq '.[].statistics.'$name
> 
> I think name needs to be quoted here? In fact, unless the pause group is
> highly unlikely to ever get a key that contains a dash,

I would expect that the pause group is pretty much set and will not get
new counters but, sure, I can add the quotes just to be on the safe
side.

> it should either
> be quoted in the horrible way the else branch does it, or do this much
> more readable thing instead:
> 
>         jq --arg name "$name" '.[].statistics[$name]'
> 

Thanks! Wasn't aware of this type of jq variable usage but indeed it
looks better.

> > +	else
> > +		ethtool --json -S $dev --groups $grp -- --src $src | \
> 
> Since you are touching this line -- can you fix the missing quoting,
> please?

Sure, I will add them.

> 
> > +			jq '.[]."'"$grp"'"."'$name'"'

And I think $name above needs double quoting as well.

Ioana
Re: [PATCH net-next 4/5] selftests: forwarding: extend ethtool_std_stats_get with pause statistics
Posted by Petr Machata 1 month ago
Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> On Fri, Feb 27, 2026 at 05:38:40PM +0100, Petr Machata wrote:
>> 
>> Ioana Ciornei <ioana.ciornei@nxp.com> writes:
>> 
>> > Even though pause frame statistics are not exported through the same
>> > ethtool command, there is no point in adding another helper just for
>> > them. Extent the ethtool_std_stats_get() function so that we are able to
>> > interrogate using the same helper all the standard statistics.
>> >
>> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
>> > ---
>> >  tools/testing/selftests/net/forwarding/lib.sh | 8 ++++++--
>> >  1 file changed, 6 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
>> > index a9034f0bb58b..efd236ae1c28 100644
>> > --- a/tools/testing/selftests/net/forwarding/lib.sh
>> > +++ b/tools/testing/selftests/net/forwarding/lib.sh
>> > @@ -831,8 +831,12 @@ ethtool_std_stats_get()
>> >  	local name=$1; shift
>> >  	local src=$1; shift
>> >  
>> > -	ethtool --json -S $dev --groups $grp -- --src $src | \
>> > -		jq '.[]."'"$grp"'"."'$name'"'
>> > +	if [[ "$grp" == "pause" ]]; then
>> > +		ethtool -I --json -a $dev | jq '.[].statistics.'$name
>> 
>> I think name needs to be quoted here? In fact, unless the pause group is
>> highly unlikely to ever get a key that contains a dash,
>
> I would expect that the pause group is pretty much set and will not get
> new counters but, sure, I can add the quotes just to be on the safe
> side.

Oh, don't get me wrong, I don't believe $name will contain whitespace.
But I would expect shellcheck to complain about the missing quotes.

>> it should either
>> be quoted in the horrible way the else branch does it, or do this much
>> more readable thing instead:
>> 
>>         jq --arg name "$name" '.[].statistics[$name]'
>> 
>
> Thanks! Wasn't aware of this type of jq variable usage but indeed it
> looks better.
>
>> > +	else
>> > +		ethtool --json -S $dev --groups $grp -- --src $src | \
>> 
>> Since you are touching this line -- can you fix the missing quoting,
>> please?
>
> Sure, I will add them.
>
>> 
>> > +			jq '.[]."'"$grp"'"."'$name'"'
>
> And I think $name above needs double quoting as well.

Oh yeah. That's just going to be an absolute confusion of quotes though.
Can you convert to the --arg form as well? I think it should be:

			jq --arg grp "$grp" --arg name "$name" \
				'.[][$grp][$name]'