Skip to content

Commit

Permalink
Merge branch 'selftests-fixes-for-kernel-ci'
Browse files Browse the repository at this point in the history
Petr Machata says:

====================
selftests: Fixes for kernel CI

As discussed on the bi-weekly call on Jan 30, and in mailing around
kernel CI effort, some changes are desirable in the suite of forwarding
selftests the better to work with the CI tooling. Namely:

- The forwarding selftests use a configuration file where names of
  interfaces are defined and various variables can be overridden. There
  is also forwarding.config.sample that users can use as a template to
  refer to when creating the config file. What happens a fair bit is
  that users either do not know about this at all, or simply forget, and
  are confused by cryptic failures about interfaces that cannot be
  created.

  In patches #1 - #3 have lib.sh just be the single source of truth with
  regards to which variables exist. That includes the topology variables
  which were previously only in the sample file, and any "tweak
  variables", such as what tools to use, sleep times, etc.

  forwarding.config.sample then becomes just a placeholder with a couple
  examples. Unless specific HW should be exercised, or specific tools
  used, the defaults are usually just fine.

- Several net/forwarding/ selftests (and one net/ one) cannot be run on
  veth pairs, they need an actual HW interface to run on. They are
  generic in the sense that any capable HW should pass them, which is
  why they have been put to net/forwarding/ as opposed to drivers/net/,
  but they do not generalize to veth. The fact that these tests are in
  net/forwarding/, but still complaining when run, is confusing.

  In patches #4 - #6 move these tests to a new directory
  drivers/net/hw.

- The following patches extend the codebase to handle well test results
  other than pass and fail.

  Patch #7 is preparatory. It converts several log_test_skip to XFAIL,
  so that tests do not spuriously end up returning non-0 when they
  are not supposed to.

  In patches #8 - #10, introduce some missing ksft constants, then support
  having those constants in RET, and then finally in EXIT_STATUS.

- The traffic scheduler tests generate a large amount of network traffic
  to test the behavior of the scheduler. This demands a relatively
  high-performance computer. On slow machines, such as with a debugging
  kernel, the test would spuriously fail.

  It can still be useful to "go through the motions" though, to possibly
  catch bugs in setup of the scheduler graph and passing packets around.
  Thus we still want to run the tests, just with lowered demands.

  To that end, in patches #11 - #12, introduce an environment variable
  KSFT_MACHINE_SLOW, with obvious meaning. Tests can then make checks
  more lenient, such as mark failures as XFAIL. A helper, xfail_on_slow,
  is provided to mark performance-sensitive parts of the selftest.

- In patch #13, use a similar mechanism to mark a NH group stats
  selftest to XFAIL HW stats tests when run on VETH pairs.

- All these changes complicate the hitherto straightforward logging and
  checking logic, so in patch #14, add a selftest that checks this
  functionality in lib.sh.

v1 (vs. an RFC circulated through linux-kselftest):
- Patch #9:
    - Clarify intended usage by s/set_ret/ret_set_ksft_status/,
      s/nret/ksft_status/
====================

Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jakub Kicinski <[email protected]>
  • Loading branch information
kuba-moo committed Mar 29, 2024
2 parents 5e47fbe + 8ff2d7a commit 51cf49f
Show file tree
Hide file tree
Showing 28 changed files with 565 additions and 171 deletions.
25 changes: 25 additions & 0 deletions tools/testing/selftests/drivers/net/hw/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# SPDX-License-Identifier: GPL-2.0+ OR MIT

TEST_PROGS = \
devlink_port_split.py \
ethtool.sh \
ethtool_extended_state.sh \
ethtool_mm.sh \
ethtool_rmon.sh \
hw_stats_l3.sh \
hw_stats_l3_gre.sh \
loopback.sh \
#

TEST_FILES := \
ethtool_lib.sh \
#

TEST_INCLUDES := \
../../../net/lib.sh \
../../../net/forwarding/lib.sh \
../../../net/forwarding/ipip_lib.sh \
../../../net/forwarding/tc_common.sh \
#

include ../../../lib.mk
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ ALL_TESTS="
different_speeds_autoneg_on
"
NUM_NETIFS=2
source lib.sh
lib_dir=$(dirname "$0")
source "$lib_dir"/../../../net/forwarding/lib.sh
source ethtool_lib.sh

h1_create()
Expand Down Expand Up @@ -286,8 +287,6 @@ different_speeds_autoneg_on()
ethtool -s $h1 autoneg on
}

skip_on_veth

trap cleanup EXIT

setup_prepare
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ ALL_TESTS="
"

NUM_NETIFS=2
source lib.sh
lib_dir=$(dirname "$0")
source "$lib_dir"/../../../net/forwarding/lib.sh
source ethtool_lib.sh

TIMEOUT=$((WAIT_TIMEOUT * 1000)) # ms
Expand Down Expand Up @@ -108,8 +109,6 @@ no_cable()
ip link set dev $swp3 down
}

skip_on_veth

setup_prepare

tests_run
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ ALL_TESTS="
NUM_NETIFS=2
REQUIRE_MZ=no
PREEMPTIBLE_PRIO=0
source lib.sh
lib_dir=$(dirname "$0")
source "$lib_dir"/../../../net/forwarding/lib.sh

traffic_test()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ ALL_TESTS="
"

NUM_NETIFS=2
source lib.sh
lib_dir=$(dirname "$0")
source "$lib_dir"/../../../net/forwarding/lib.sh

ETH_FCS_LEN=4
ETH_HLEN=$((6+6+2))
Expand Down Expand Up @@ -78,7 +79,7 @@ rmon_histogram()

for if in $iface $neigh; do
if ! ensure_mtu $if ${bucket[0]}; then
log_test_skip "$if does not support the required MTU for $step"
log_test_xfail "$if does not support the required MTU for $step"
return
fi
done
Expand All @@ -93,7 +94,7 @@ rmon_histogram()
jq -r ".[0].rmon[\"${set}-pktsNtoM\"][]|[.low, .high]|@tsv" 2>/dev/null)

if [ $nbuckets -eq 0 ]; then
log_test_skip "$iface does not support $set histogram counters"
log_test_xfail "$iface does not support $set histogram counters"
return
fi
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ ALL_TESTS="
test_double_enable
"
NUM_NETIFS=4
source lib.sh
lib_dir=$(dirname "$0")
source "$lib_dir"/../../../net/forwarding/lib.sh

h1_create()
{
Expand Down Expand Up @@ -324,17 +325,9 @@ setup_wait

used=$(ip -j stats show dev $rp1.200 group offload subgroup hw_stats_info |
jq '.[].info.l3_stats.used')
kind=$(ip -j -d link show dev $rp1 |
jq -r '.[].linkinfo.info_kind')
if [[ $used != true ]]; then
if [[ $kind == veth ]]; then
log_test_skip "l3_stats not offloaded on veth interface"
EXIT_STATUS=$ksft_skip
else
RET=1 log_test "l3_stats not offloaded"
fi
else
tests_run
fi
[[ $used = true ]]
check_err $? "hw_stats_info.used=$used"
log_test "l3_stats offloaded"
tests_run

exit $EXIT_STATUS
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ ALL_TESTS="
test_stats_tx
"
NUM_NETIFS=6
source lib.sh
source ipip_lib.sh
lib_dir=$(dirname "$0")
source "$lib_dir"/../../../net/forwarding/lib.sh
source "$lib_dir"/../../../net/forwarding/ipip_lib.sh

setup_prepare()
{
Expand Down Expand Up @@ -99,8 +100,6 @@ test_stats_rx()
test_stats g2a rx
}

skip_on_veth

trap cleanup EXIT

setup_prepare
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ ksft_skip=4

ALL_TESTS="loopback_test"
NUM_NETIFS=2
source tc_common.sh
source lib.sh
lib_dir=$(dirname "$0")
source "$lib_dir"/../../../net/forwarding/tc_common.sh
source "$lib_dir"/../../../net/forwarding/lib.sh

h1_create()
{
Expand Down
1 change: 1 addition & 0 deletions tools/testing/selftests/drivers/net/hw/settings
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
timeout=0
2 changes: 1 addition & 1 deletion tools/testing/selftests/drivers/net/mlxsw/mlxsw_lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ __mlxsw_only_on_spectrum()
local src=$1; shift

if ! mlxsw_on_spectrum "$rev"; then
log_test_skip $src:$caller "(Spectrum-$rev only)"
log_test_xfail $src:$caller "(Spectrum-$rev only)"
return 1
fi
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ for current_test in ${TESTS:-$ALL_TESTS}; do
RET=0
target=$(${current_test}_get_target "$should_fail")
if ((target == 0)); then
log_test_skip "'$current_test' should_fail=$should_fail test"
continue
fi

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ for current_test in ${TESTS:-$ALL_TESTS}; do
RET=0
target=$(${current_test}_get_target "$should_fail")
if ((target == 0)); then
log_test_skip "'$current_test' [$profile] should_fail=$should_fail test"
continue
fi
${current_test}_setup_prepare
Expand Down
1 change: 0 additions & 1 deletion tools/testing/selftests/net/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ TEST_PROGS += reuseaddr_ports_exhausted.sh
TEST_PROGS += txtimestamp.sh
TEST_PROGS += vrf-xfrm-tests.sh
TEST_PROGS += rxtimestamp.sh
TEST_PROGS += devlink_port_split.py
TEST_PROGS += drop_monitor_tests.sh
TEST_PROGS += vrf_route_leaking.sh
TEST_PROGS += bareudp.sh
Expand Down
9 changes: 1 addition & 8 deletions tools/testing/selftests/net/forwarding/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,12 @@ TEST_PROGS = bridge_fdb_learning_limit.sh \
bridge_vlan_unaware.sh \
custom_multipath_hash.sh \
dual_vxlan_bridge.sh \
ethtool_extended_state.sh \
ethtool_mm.sh \
ethtool_rmon.sh \
ethtool.sh \
gre_custom_multipath_hash.sh \
gre_inner_v4_multipath.sh \
gre_inner_v6_multipath.sh \
gre_multipath_nh_res.sh \
gre_multipath_nh.sh \
gre_multipath.sh \
hw_stats_l3.sh \
hw_stats_l3_gre.sh \
ip6_forward_instats_vrf.sh \
ip6gre_custom_multipath_hash.sh \
ip6gre_flat_key.sh \
Expand All @@ -43,8 +37,8 @@ TEST_PROGS = bridge_fdb_learning_limit.sh \
ipip_hier_gre_key.sh \
ipip_hier_gre_keys.sh \
ipip_hier_gre.sh \
lib_sh_test.sh \
local_termination.sh \
loopback.sh \
mirror_gre_bound.sh \
mirror_gre_bridge_1d.sh \
mirror_gre_bridge_1d_vlan.sh \
Expand Down Expand Up @@ -113,7 +107,6 @@ TEST_PROGS = bridge_fdb_learning_limit.sh \
vxlan_symmetric.sh

TEST_FILES := devlink_lib.sh \
ethtool_lib.sh \
fib_offload_lib.sh \
forwarding.config.sample \
ip6gre_lib.sh \
Expand Down
33 changes: 33 additions & 0 deletions tools/testing/selftests/net/forwarding/README
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,36 @@ o Checks shall be added to lib.sh for any external dependencies.
o Code shall be checked using ShellCheck [1] prior to submission.

1. https://www.shellcheck.net/

Customization
=============

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

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}

A way to customize variables in a persistent fashion is to create a file
named forwarding.config in this directory. lib.sh sources the file if
present, so it can contain any shell code. Typically it will contain
assignments of variables whose value should be overridden.

forwarding.config.sample is available in the directory as an example of
how forwarding.config might look.
53 changes: 15 additions & 38 deletions tools/testing/selftests/net/forwarding/forwarding.config.sample
Original file line number Diff line number Diff line change
Expand Up @@ -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

1 change: 0 additions & 1 deletion tools/testing/selftests/net/forwarding/ipip_lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@
# | $h2 + |
# | 192.0.2.18/28 |
# +---------------------------+
source lib.sh

h1_create()
{
Expand Down
Loading

0 comments on commit 51cf49f

Please sign in to comment.