Message-ID: <b9b8a11a22821a7aa532211ff461a34f596e26bf.1711464583.git.petrm@nvidia.com>
Date: Tue, 26 Mar 2024 17:54:29 +0100
From: Petr Machata <petrm@...dia.com>
To: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
	<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, <netdev@...r.kernel.org>
CC: Shuah Khan <shuah@...nel.org>, Nikolay Aleksandrov <razor@...ckwall.org>,
	Hangbin Liu <liuhangbin@...il.com>, Vladimir Oltean
	<vladimir.oltean@....com>, Benjamin Poirier <bpoirier@...dia.com>, "Ido
 Schimmel" <idosch@...dia.com>, Jiri Pirko <jiri@...dia.com>,
	<linux-kselftest@...r.kernel.org>, Petr Machata <petrm@...dia.com>,
	<mlxsw@...dia.com>
Subject: [PATCH net-next 02/14] selftests: forwarding.config.sample: Move overrides to lib.sh

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@...dia.com>
Reviewed-by: Benjamin Poirier <bpoirier@...dia.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

Powered by blists - more mailing lists