All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Machata <petrm@nvidia.com>
To: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	<netdev@vger.kernel.org>
Cc: 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>,
	Petr Machata <petrm@nvidia.com>, <mlxsw@nvidia.com>
Subject: [PATCH net-next 02/14] selftests: forwarding.config.sample: Move overrides to lib.sh
Date: Tue, 26 Mar 2024 17:54:29 +0100	[thread overview]
Message-ID: <b9b8a11a22821a7aa532211ff461a34f596e26bf.1711464583.git.petrm@nvidia.com> (raw)
In-Reply-To: <cover.1711464583.git.petrm@nvidia.com>

forwarding.config.sample, net/lib.sh and net/forwarding/lib.sh contain
definitions and redefinitions of some of the same variables. The overlap
between net/forwarding/lib.sh and forwarding.config.sample is especially
large. This duplication is a potential source of confusion and problems.

It would be overall less error prone if each variable were defined in one
place only. In this patch set, that place is the library itself. Therefore
move all comments from forwarding.config.sample to net/forwarding/lib.sh.

Move over also a definition of TC_FLAG, which was missing from lib.sh
entirely.

Additionally, add to lib.sh a default definition of the topology variables.
The logic behind this is that forgetting to specify forwarding.config was a
frequent source of frustration for the selftest users. But really, most of
the time the default veth based topology is just fine. We considered just
sourcing forwarding.config.sample instead if forwarding.config is not
available, but this is a cleaner solution.

That means the syntax of the forwarding.config.sample override has to
change to an array assignment, so that the whole variable is overwritten,
not just individual keys, which could leave the value of some keys
unchanged. Do the same in lib.sh for any cut'n'pasters out there.

The config file is then given a sort of carte blanche to redefine whatever
variables it sees fit from the libraries. This is described in a comment in
the file. Only a handful of variables are left behind, to illustrate the
customization.

The fact that the variables are now missing from forwarding.config.sample,
and therefore would miss from forwarding.config derived from that file as
well, should not change anything. This is just the sample file. Users that
keep their own forwarding.config would retain it as before.

The only observable change is introduction of TC_FLAG to lib.sh, because
now the filters would not be attempted to install to HW datapath. For veth
pairs this does not change anything. For HW deployments, users presumably
have forwarding.config with this value overridden.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Benjamin Poirier <bpoirier@nvidia.com>
---
 .../net/forwarding/forwarding.config.sample   | 53 ++++----------
 tools/testing/selftests/net/forwarding/lib.sh | 69 ++++++++++++++++---
 2 files changed, 74 insertions(+), 48 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/forwarding.config.sample b/tools/testing/selftests/net/forwarding/forwarding.config.sample
index 1fc4f0242fc5..f1ca95e79a65 100644
--- a/tools/testing/selftests/net/forwarding/forwarding.config.sample
+++ b/tools/testing/selftests/net/forwarding/forwarding.config.sample
@@ -3,51 +3,28 @@
 
 ##############################################################################
 # Topology description. p1 looped back to p2, p3 to p4 and so on.
-declare -A NETIFS
 
-NETIFS[p1]=veth0
-NETIFS[p2]=veth1
-NETIFS[p3]=veth2
-NETIFS[p4]=veth3
-NETIFS[p5]=veth4
-NETIFS[p6]=veth5
-NETIFS[p7]=veth6
-NETIFS[p8]=veth7
-NETIFS[p9]=veth8
-NETIFS[p10]=veth9
+NETIFS=(
+	[p1]=veth0
+	[p2]=veth1
+	[p3]=veth2
+	[p4]=veth3
+	[p5]=veth4
+	[p6]=veth5
+	[p7]=veth6
+	[p8]=veth7
+	[p9]=veth8
+	[p10]=veth9
+)
 
 # Port that does not have a cable connected.
 NETIF_NO_CABLE=eth8
 
 ##############################################################################
-# Defines
+# In addition to the topology-related variables, it is also possible to override
+# in this file other variables that net/lib.sh, net/forwarding/lib.sh or other
+# libraries or selftests use. E.g.:
 
-# IPv4 ping utility name
-PING=ping
-# IPv6 ping utility name. Some distributions use 'ping' for IPv6.
 PING6=ping6
-# Packet generator. Some distributions use 'mz'.
 MZ=mausezahn
-# mausezahn delay between transmissions in microseconds.
-MZ_DELAY=0
-# Time to wait after interfaces participating in the test are all UP
 WAIT_TIME=5
-# Whether to pause on failure or not.
-PAUSE_ON_FAIL=no
-# Whether to pause on cleanup or not.
-PAUSE_ON_CLEANUP=no
-# Type of network interface to create
-NETIF_TYPE=veth
-# Whether to create virtual interfaces (veth) or not
-NETIF_CREATE=yes
-# Timeout (in seconds) before ping exits regardless of how many packets have
-# been sent or received
-PING_TIMEOUT=5
-# Minimum ageing_time (in centiseconds) supported by hardware
-LOW_AGEING_TIME=1000
-# Flag for tc match, supposed to be skip_sw/skip_hw which means do not process
-# filter by software/hardware
-TC_FLAG=skip_hw
-# IPv6 traceroute utility name.
-TROUTE6=traceroute6
-
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index ee48c4603ff0..dbd4348f85b8 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -1,34 +1,83 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
+##############################################################################
+# Topology description. p1 looped back to p2, p3 to p4 and so on.
+
+declare -A NETIFS=(
+    [p1]=veth0
+    [p2]=veth1
+    [p3]=veth2
+    [p4]=veth3
+    [p5]=veth4
+    [p6]=veth5
+    [p7]=veth6
+    [p8]=veth7
+    [p9]=veth8
+    [p10]=veth9
+)
+
+# Port that does not have a cable connected.
+: "${NETIF_NO_CABLE:=eth8}"
+
 ##############################################################################
 # Defines
 
-# Can be overridden by the configuration file.
+# Networking utilities.
 : "${PING:=ping}"
-: "${PING6:=ping6}"
-: "${MZ:=mausezahn}"
-: "${MZ_DELAY:=0}"
+: "${PING6:=ping6}"	# Some distros just use ping.
 : "${ARPING:=arping}"
+: "${TROUTE6:=traceroute6}"
+
+# Packet generator.
+: "${MZ:=mausezahn}"	# Some distributions use 'mz'.
+: "${MZ_DELAY:=0}"
+
+# Host configuration tools.
 : "${TEAMD:=teamd}"
+: "${MCD:=smcrouted}"
+: "${MC_CLI:=smcroutectl}"
+
+# Constants for netdevice bring-up:
+# Default time in seconds to wait for an interface to come up before giving up
+# and bailing out. Used during initial setup.
+: "${INTERFACE_TIMEOUT:=600}"
+# Like INTERFACE_TIMEOUT, but default for ad-hoc waiting in testing scripts.
+: "${WAIT_TIMEOUT:=20}"
+# Time to wait after interfaces participating in the test are all UP.
 : "${WAIT_TIME:=5}"
+
+# Whether to pause on, respectively, after a failure and before cleanup.
 : "${PAUSE_ON_FAIL:=no}"
 : "${PAUSE_ON_CLEANUP:=no}"
-: "${NETIF_TYPE:=veth}"
+
+# Whether to create virtual interfaces, and what netdevice type they should be.
 : "${NETIF_CREATE:=yes}"
-: "${MCD:=smcrouted}"
-: "${MC_CLI:=smcroutectl}"
+: "${NETIF_TYPE:=veth}"
+
+# Constants for ping tests:
+# How many packets should be sent.
 : "${PING_COUNT:=10}"
+# Timeout (in seconds) before ping exits regardless of how many packets have
+# been sent or received
 : "${PING_TIMEOUT:=5}"
-: "${WAIT_TIMEOUT:=20}"
-: "${INTERFACE_TIMEOUT:=600}"
+
+# Minimum ageing_time (in centiseconds) supported by hardware
 : "${LOW_AGEING_TIME:=1000}"
+
+# Whether to check for availability of certain tools.
 : "${REQUIRE_JQ:=yes}"
 : "${REQUIRE_MZ:=yes}"
 : "${REQUIRE_MTOOLS:=no}"
+
+# Whether to override MAC addresses on interfaces participating in the test.
 : "${STABLE_MAC_ADDRS:=no}"
+
+# Flags for tcpdump
 : "${TCPDUMP_EXTRA_FLAGS:=}"
-: "${TROUTE6:=traceroute6}"
+
+# Flags for TC filters.
+: "${TC_FLAG:=skip_hw}"
 
 net_forwarding_dir=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")")
 
-- 
2.43.0


  parent reply	other threads:[~2024-03-26 17:00 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-26 16:54 [PATCH net-next 00/14] selftests: Fixes for kernel CI Petr Machata
2024-03-26 16:54 ` [PATCH net-next 01/14] selftests: net: libs: Change variable fallback syntax Petr Machata
2024-03-26 16:54 ` Petr Machata [this message]
2024-03-26 16:54 ` [PATCH net-next 03/14] selftests: forwarding: README: Document customization Petr Machata
2024-03-26 16:54 ` [PATCH net-next 04/14] selftests: forwarding: ipip_lib: Do not import lib.sh Petr Machata
2024-03-26 16:54 ` [PATCH net-next 05/14] selftests: forwarding: Move several selftests Petr Machata
2024-03-26 16:54 ` [PATCH net-next 06/14] selftests: forwarding: Ditch skip_on_veth() Petr Machata
2024-03-26 16:54 ` [PATCH net-next 07/14] selftests: forwarding: Change inappropriate log_test_skip() calls Petr Machata
2024-03-26 16:54 ` [PATCH net-next 08/14] selftests: lib: Define more kselftest exit codes Petr Machata
2024-03-26 16:54 ` [PATCH net-next 09/14] selftests: forwarding: Have RET track kselftest framework constants Petr Machata
2024-03-26 16:54 ` [PATCH net-next 10/14] selftests: forwarding: Convert log_test() to recognize RET values Petr Machata
2024-03-26 16:54 ` [PATCH net-next 11/14] selftests: forwarding: Support for performance sensitive tests Petr Machata
2024-03-26 16:54 ` [PATCH net-next 12/14] selftests: forwarding: Mark performance-sensitive tests Petr Machata
2024-03-26 16:54 ` [PATCH net-next 13/14] selftests: forwarding: router_mpath_nh_lib: Don't skip, xfail on veth Petr Machata
2024-03-26 16:54 ` [PATCH net-next 14/14] selftests: forwarding: Add a test for testing lib.sh functionality Petr Machata
2024-03-29  1:11 ` [PATCH net-next 00/14] selftests: Fixes for kernel CI patchwork-bot+netdevbpf

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=b9b8a11a22821a7aa532211ff461a34f596e26bf.1711464583.git.petrm@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=mlxsw@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --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.