All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Machata <petrm@nvidia.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Petr Machata <petrm@nvidia.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>, <nbu-linux-internal@nvidia.com>,
	Shuah Khan <shuah@kernel.org>,
	"Nikolay Aleksandrov" <razor@blackwall.org>,
	Hangbin Liu <liuhangbin@gmail.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Benjamin Poirier <bpoirier@nvidia.com>,
	Ido Schimmel <idosch@nvidia.com>, Jiri Pirko <jiri@nvidia.com>,
	<linux-kselftest@vger.kernel.org>
Subject: Re: [RFC PATCH net-next mlxsw 03/14] selftests: forwarding: README: Document customization
Date: Tue, 26 Mar 2024 11:31:31 +0100	[thread overview]
Message-ID: <87cyrhjim4.fsf@nvidia.com> (raw)
In-Reply-To: <20240325173417.1a79b631@kernel.org>


Jakub Kicinski <kuba@kernel.org> writes:

> On Mon, 25 Mar 2024 18:29:10 +0100 Petr Machata wrote:
>> +The forwarding selftests framework uses a number of variables that
>> +influence its behavior and tools it invokes, and how it invokes them, in
>> +various ways. A number of these variables can be overridden. The way these
>> +overridable variables are specified is typically one of the following two
>> +syntaxes:
>> +
>> +	: "${VARIABLE:=default_value}"
>> +	VARIABLE=${VARIABLE:=default_value}
>> +
>> +Any of these variables can be overridden. Notably net/forwarding/lib.sh and
>> +net/lib.sh contain a number of overridable variables.
>> +
>> +One way of overriding these variables is through the environment:
>> +
>> +	PAUSE_ON_FAIL=yes ./some_test.sh
>
> I like this conversion a lot. Makes me want to propose that we make this

Convention you mean?
Nothing was converted, this has always worked.

> a standard feature of kselftest. If "env" file exists in the test
> directory kselftest would load its contents before running every test.
>
> That's more of a broader question to anyone reading on linux-kselftest@
> if there's no interest more than happy to merge as is :)
>
>> +The variable NETIFS is special. Since it is an array variable, there is no
>> +way to pass it through the environment. Its value can instead be given as
>> +consecutive arguments to the selftest:
>> +
>> +	./some_test.sh swp{1..8}
>
> Did you consider allowing them to be defined as NETIF_0, NETIF_1 etc.?
> We can have lib.sh convert that into an array with a ugly-but-short
> loop, it's a bit tempting to get rid of the exception.

The exception is a bit annoying, yeah. But it works today, should stay,
and therefore should be documented, so the paragraph won't go away. I
use it all the time, too. I basically don't use the config file, I just
use the env overrides and the argv interface names. It's very handy.

The alternative is also very verbose:

	NETIF_1=swp1 NETIF_2=swp2 NETIF_3=swp3 [...] ./some_test.sh.

Maybe we could do this though?

	NETIFS="swp1 swp2 swp3 swp4 swp5 swp6 swp7 swp8" ./some_test.sh

And like this it won't make you want to pull your hair from all the
repetition:

	NETIFS=$(echo swp{1..8}) ./some_test.sh

But NETIFS is going to be a special case one way or another. That you
need to specify it through several variables, or a variable with a
special value, means you need to explain it as a special case in the
documentation. At which point you have two exceptions, and an
interaction between them, to describe.

  reply	other threads:[~2024-03-26 12:14 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-25 17:29 [RFC PATCH net-next mlxsw 00/14] selftests: Fixes for kernel CI Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 01/14] selftests: net: libs: Change variable fallback syntax Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 02/14] selftests: forwarding.config.sample: Move overrides to lib.sh Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 03/14] selftests: forwarding: README: Document customization Petr Machata
2024-03-26  0:34   ` Jakub Kicinski
2024-03-26 10:31     ` Petr Machata [this message]
2024-03-26 14:13       ` Jakub Kicinski
2024-03-26 17:32         ` Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 04/14] selftests: forwarding: ipip_lib: Do not import lib.sh Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 05/14] selftests: forwarding: Move several selftests Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 06/14] selftests: forwarding: Ditch skip_on_veth() Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 07/14] selftests: forwarding: Change inappropriate log_test_skip() calls Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 08/14] selftests: lib: Define more kselftest exit codes Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 09/14] selftests: forwarding: Have RET track kselftest framework constants Petr Machata
2024-03-26  0:43   ` Jakub Kicinski
2024-03-26 11:12     ` Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 10/14] selftests: forwarding: Convert log_test() to recognize RET values Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 11/14] selftests: forwarding: Support for performance sensitive tests Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 12/14] selftests: forwarding: Mark performance-sensitive tests Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 13/14] selftests: forwarding: router_mpath_nh_lib: Don't skip, xfail on veth Petr Machata
2024-03-25 17:29 ` [RFC PATCH net-next mlxsw 14/14] selftests: forwarding: Add a test for testing lib.sh functionality Petr Machata
2024-03-26  0:48 ` [RFC PATCH net-next mlxsw 00/14] selftests: Fixes for kernel CI Jakub Kicinski
2024-03-26 11:13   ` 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=87cyrhjim4.fsf@nvidia.com \
    --to=petrm@nvidia.com \
    --cc=bpoirier@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=idosch@nvidia.com \
    --cc=jiri@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=liuhangbin@gmail.com \
    --cc=nbu-linux-internal@nvidia.com \
    --cc=pabeni@redhat.com \
    --cc=razor@blackwall.org \
    --cc=shuah@kernel.org \
    --cc=vladimir.oltean@nxp.com \
    /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.