All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Machata <me@pmachata.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Petr Machata <petrm@nvidia.com>,
	Ioana Ciornei <ioana.ciornei@nxp.com>,
	netdev@vger.kernel.org, Andrew  Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>, Shuah Khan <shuah@kernel.org>,
	Simon Horman <horms@kernel.org>,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH net-next 5/5] selftests: drivers: hw: add tests for the ethtool standard counters
Date: Sat, 28 Feb 2026 10:11:26 +0100	[thread overview]
Message-ID: <877brxf272.fsf@nvidia.com> (raw)
In-Reply-To: <20260227164348.63afcc43@kernel.org>


Jakub Kicinski <kuba@kernel.org> writes:

> On Fri, 27 Feb 2026 14:53:06 +0100 Petr Machata wrote:
>> Jakub Kicinski <kuba@kernel.org> writes:
>> > On Wed, 25 Feb 2026 17:06:48 +0200 Ioana Ciornei wrote:  
>> >> +ALL_TESTS="
>> >> +	test_eth_ctrl_stats
>> >> +	test_eth_mac_stats
>> >> +	test_pause_stats
>> >> +"
>> >> +STABLE_MAC_ADDRS=yes
>> >> +NUM_NETIFS=2
>> >> +lib_dir=$(dirname "$0")
>> >> +# shellcheck source=./../../../net/forwarding/lib.sh
>> >> +source "$lib_dir"/../../../net/forwarding/lib.sh  
>> >
>> > Argh, at some point we should probably decide whether we have 
>> > a preference on which "library" / set of env vars we use under 
>> > drivers/net/hw. Adding Petr to CC.  
>> 
>> I think the expectation is that by default, tests written in Bash are
>> run on one machine without remotes.
>
> I think we need to define the guidance by properties of the test, not
> the machine? Of course _tests_ which don't need a traffic source can

Oh yeah, I'm just stating that's how it currently is and how we got here.

> simply consume the NETIF evn variable, so its trivial to write them in
> any language without much library support. But for tests which need
> traffic input we need a different distinction than "does the author
> care about single-interface machines", so to speak?

I agree that adherence to the drivers/net/README protocol is valuable to
some users and would be good to uphold if reasonable in a given tests.
If that's what you have in mind.

There are going to be tests where it's not a great fit, but I think that
of those NUM_NETIFS=2 tests that we currently have, maybe
ethtool_extended_state has a good reason to be obstinate, because it
sets up negotiations and needs an extra unplugged netdevice.

>> 
>> > The existing tests under drivers/net/hw which source forwarding/lib.sh
>> > pre-date the "NIC setup" described in 
>> > tools/testing/selftests/drivers/net/README.rst
>> >
>> > Should we ask "NIC setup" to be used for all tests which only need
>> > NUM_NETIFS=2 ? These are basically simple tests where one netif is 
>> > a traffic generator and the other is the DUT. And IIUC the "forwarding
>> > setup" can't be used when the traffic generator is controlled over SSH?  
>> 
>> In principle nothing prevents lib.sh from growing brains to support
>> these remote shenanigans. I think it's just that so far nobody cared
>> enough to actually do it.
>> 
>> I think that a helper that in effect does "run this on a machine where
>> $swp1 is" is mostly what is needed. That and "make sure $swp1 and $swp2
>> are on the same machine". It's going to be annoying to work with though,
>> because you need to annotate every single command. I bet there's a nice
>> syntax to make it not activelly annoying.
>> 
>> If we have this, it might make sense to require tests to make use of it.
>> (With an explicit opt-out for special cases.) But I do not want every
>> test to have to reinvent this wheel and cargo-cult snippets from other
>> tests.
>> 
>> BTW, my guess is that even many multi-port tests that I wrote boil down
>> to just a bunch of fairly independent loopbacks whose far ends could be
>> on remote machines. It's not a priori nonsense to me that one would run
>> a test like this, or whatever magic we'd use:
>> 
>>     ./test.sh ssh://petr@10.1.2.3:eth1 swp1 veth1 ns://foo:veth2
>> 
>> And it just works, because only swp1 and swp2 need to be bridged, the
>> rest can be remote, and the traffic generation helper knows that to pump
>> traffic to ssh://10.1.2.3:eth1, obviously you need to ssh there first.
>> But the library would need to have helpers for this, and the tests would
>> need to use them.
>
> Right, to be clear I primarily care about being able to run these tests
> with traffic generator on a remote system. Otherwise someone who cares
> about single-port systems will have to add a separate test I guess?
> And we will have to maintain two tests? :S
>
> A nice to have is for all drivers/net/hw tests to use the "NIC" env
> variables (move tests which really only make sense on multi-port
> devices to drivers/net/forwarding?) But I think "translating"
> forwarding variables to NIC if NUMIF=2 could easily be done
> automatically by the test / lib so that's lower priority.

The reason for my diatribe above was like, if forwarding/lib.sh is
clever enough to handle these things transparently, then the NIC case
might be supported through a wrapper that just does the right thing, and
the test can be overtly just a run of the mill forwarding test.

The requirement for the remote netdevice to be up and preconfigured
throws a wrench in this idea though, so dunno.

One way or another, _some_ brains need to reside somewhere in a library.
I'm not sure forwarding.sh is the place. But I don't want to end up in
the same situation as like lib/half_the_tests.sh that reinvent logging
from first principles again and again.

  reply	other threads:[~2026-02-28 13:05 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-25 15:06 [PATCH net-next 0/5] net: dpaa2-mac: export standard statistics Ioana Ciornei
2026-02-25 15:06 ` [PATCH net-next 1/5] net: dpaa2-mac: extend APIs related to statistics Ioana Ciornei
2026-02-25 15:06 ` [PATCH net-next 2/5] net: dpaa2-mac: retrieve MAC statistics in one firmware command Ioana Ciornei
2026-02-27  2:26   ` [net-next,2/5] " Jakub Kicinski
2026-02-27 10:37     ` Ioana Ciornei
2026-03-01 16:09   ` [PATCH net-next 2/5] " Simon Horman
2026-03-02 12:51     ` Ioana Ciornei
2026-02-25 15:06 ` [PATCH net-next 3/5] net: dpaa2-mac: export standard statistics Ioana Ciornei
2026-02-25 15:06 ` [PATCH net-next 4/5] selftests: forwarding: extend ethtool_std_stats_get with pause statistics Ioana Ciornei
2026-02-27 16:38   ` Petr Machata
2026-03-02 13:57     ` Ioana Ciornei
2026-03-03 13:06       ` Petr Machata
2026-02-25 15:06 ` [PATCH net-next 5/5] selftests: drivers: hw: add tests for the ethtool standard counters Ioana Ciornei
2026-02-25 23:38   ` Andrew Lunn
2026-02-26  7:03     ` Ioana Ciornei
2026-02-26 12:19       ` Ioana Ciornei
2026-02-26 13:31         ` Andrew Lunn
2026-02-26 14:18           ` Ioana Ciornei
2026-02-27  2:25             ` Jakub Kicinski
2026-02-27  7:34               ` Ioana Ciornei
2026-02-27 14:17                 ` Andrew Lunn
2026-02-28  0:24                   ` Jakub Kicinski
2026-02-28  0:23                 ` Jakub Kicinski
2026-02-27  2:22   ` Jakub Kicinski
2026-02-27 13:53     ` Petr Machata
2026-02-28  0:43       ` Jakub Kicinski
2026-02-28  9:11         ` Petr Machata [this message]
2026-03-02 12:11           ` Ioana Ciornei
2026-03-03  0:07             ` Jakub Kicinski
2026-03-03 13:53               ` Ioana Ciornei
2026-03-03 16:43                 ` Jakub Kicinski
2026-02-27 15:45   ` Petr Machata
2026-03-02 14:15     ` Ioana Ciornei
2026-03-03 13:30       ` Petr Machata

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877brxf272.fsf@nvidia.com \
    --to=me@pmachata.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=ioana.ciornei@nxp.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=petrm@nvidia.com \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.