selftests: forwarding: Updates to sysctl handling

PATCH In review Petr Machata 2 participants 5 messages May 3, 2018

Subsystems (4)

Networking Network drivers Kselftest IPv4/IPv6

lore

Share & tools

lore

Petr Machata

author

2018-05-03 10:36 UTC #0 lore raw

cover selftests: forwarding: Updates to sysctl handling

Some selftests need to adjust sysctl settings. In order to be neutral to the system that the test is run on, it is a good practice to change back to the original setting after the test ends. That involves some boilerplate that can be abstracted away.

In patch #1, introduce two functions, sysctl_set() and sysctl_restore(). The former stores the current value of a given setting, and sets a new value. The latter restores the setting to the previously-stored value.

In patch #2, use these wrappers in a number of tests.

Additionally in patch #3, fix a problem in mirror_gre_nh.sh, which neglected to set a sysctl that's crucial for the test to work.

Petr Machata (3):
selftests: forwarding: lib: Add sysctl_set(), sysctl_restore()
selftests: forwarding: Use sysctl_set(), sysctl_restore()
selftests: forwarding: mirror_gre_nh: Unset RP filter

 tools/testing/selftests/net/forwarding/lib.sh      | 28 ++++++++++++++++------
 .../selftests/net/forwarding/mirror_gre_changes.sh |  7 ++----
 .../selftests/net/forwarding/mirror_gre_nh.sh      |  6 +++++
 .../selftests/net/forwarding/router_multipath.sh   | 12 ++++------
 4 files changed, 33 insertions(+), 20 deletions(-)

Signature · 6 lines

2.4.11

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Petr Machata

author

2018-05-03 10:36 UTC #1 lore raw

PATCH 1/3 selftests: forwarding: lib: Add sysctl_set(), sysctl_restore()

Add two helper functions: sysctl_set() to change the value of a given sysctl setting, and sysctl_restore() to change it back to what it was.

Signed-off-by Petr Machata <petrm at mellanox.com>

Reviewed-by Ido Schimmel <idosch at mellanox.com>

---
 tools/testing/selftests/net/forwarding/lib.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

tools/testing/selftests/net/forwarding/lib.sh +17 -0

index 7fe6d27..426b294 100644
@@ -401,6 +401,23 @@ bridge_ageing_time_get()
 	echo $((ageing_time / 100))
 }
 
+declare -A SYSCTL_ORIG
+sysctl_set()
+{
+	local key=$1; shift
+	local value=$1; shift
+
+	SYSCTL_ORIG[$key]=$(sysctl -n $key)
+	sysctl -qw $key=$value
+}
+
+sysctl_restore()
+{
+	local key=$1; shift
+
+	sysctl -qw $key=${SYSCTL_ORIG["$key"]}
+}
+
 forwarding_enable()
 {
        ipv4_fwd=$(sysctl -n net.ipv4.conf.all.forwarding)

Signature · 6 lines

2.4.11

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Petr Machata

author

2018-05-03 10:37 UTC #2 lore raw

PATCH 2/3 selftests: forwarding: Use sysctl_set(), sysctl_restore()

In reply to Petr Machata

Instead of hand-managing the sysctl set and restore, use the wrappers sysctl_set() and sysctl_restore() to do the bookkeeping automatically.

Signed-off-by Petr Machata <petrm at mellanox.com>

Reviewed-by Ido Schimmel <idosch at mellanox.com>

---
 tools/testing/selftests/net/forwarding/lib.sh                | 11 ++++-------
 tools/testing/selftests/net/forwarding/mirror_gre_changes.sh |  7 ++-----
 tools/testing/selftests/net/forwarding/router_multipath.sh   | 12 ++++--------
 3 files changed, 10 insertions(+), 20 deletions(-)

tools/testing/selftests/net/forwarding/lib.sh +4 -7

index 426b294..a7a6750 100644
@@ -420,17 +420,14 @@ sysctl_restore()
 
 forwarding_enable()
 {
-       ipv4_fwd=$(sysctl -n net.ipv4.conf.all.forwarding)
-       ipv6_fwd=$(sysctl -n net.ipv6.conf.all.forwarding)
-
-       sysctl -q -w net.ipv4.conf.all.forwarding=1
-       sysctl -q -w net.ipv6.conf.all.forwarding=1
+	sysctl_set net.ipv4.conf.all.forwarding 1
+	sysctl_set net.ipv6.conf.all.forwarding 1
 }
 
 forwarding_restore()
 {
-       sysctl -q -w net.ipv6.conf.all.forwarding=$ipv6_fwd
-       sysctl -q -w net.ipv4.conf.all.forwarding=$ipv4_fwd
+	sysctl_restore net.ipv6.conf.all.forwarding
+	sysctl_restore net.ipv4.conf.all.forwarding
 }
 
 tc_offload_check()

tools/testing/selftests/net/forwarding/mirror_gre_changes.sh +2 -5

index fdb612f..50ab346 100755
@@ -36,9 +36,7 @@ setup_prepare()
 
 	# This test downs $swp3, which deletes the configured IPv6 address
 	# unless this sysctl is set.
-	local key=net.ipv6.conf.$swp3.keep_addr_on_down
-	SWP3_KEEP_ADDR_ON_DOWN=$(sysctl -n $key)
-	sysctl -qw $key=1
+	sysctl_set net.ipv6.conf.$swp3.keep_addr_on_down 1
 
 	ip address add dev $swp3 192.0.2.129/28
 	ip address add dev $h3 192.0.2.130/28

@@ -57,8 +55,7 @@ cleanup()
 	ip address del dev $h3 192.0.2.130/28
 	ip address del dev $swp3 192.0.2.129/28
 
-	local key=net.ipv6.conf.$swp3.keep_addr_on_down
-	sysctl -qw $key=$SWP3_KEEP_ADDR_ON_DOWN
+	sysctl_restore net.ipv6.conf.$swp3.keep_addr_on_down
 
 	mirror_gre_topo_destroy
 	vrf_cleanup

tools/testing/selftests/net/forwarding/router_multipath.sh +4 -8

index 6c43762..8b6d0fb 100755
@@ -205,13 +205,11 @@ multipath4_test()
        local weight_rp13=$3
        local t0_rp12 t0_rp13 t1_rp12 t1_rp13
        local packets_rp12 packets_rp13
-       local hash_policy
 
        # Transmit multiple flows from h1 to h2 and make sure they are
        # distributed between both multipath links (rp12 and rp13)
        # according to the configured weights.
-       hash_policy=$(sysctl -n net.ipv4.fib_multipath_hash_policy)
-       sysctl -q -w net.ipv4.fib_multipath_hash_policy=1
+       sysctl_set net.ipv4.fib_multipath_hash_policy 1
        ip route replace 198.51.100.0/24 vrf vrf-r1 \
                nexthop via 169.254.2.22 dev $rp12 weight $weight_rp12 \
                nexthop via 169.254.3.23 dev $rp13 weight $weight_rp13

@@ -233,7 +231,7 @@ multipath4_test()
        ip route replace 198.51.100.0/24 vrf vrf-r1 \
                nexthop via 169.254.2.22 dev $rp12 \
                nexthop via 169.254.3.23 dev $rp13
-       sysctl -q -w net.ipv4.fib_multipath_hash_policy=$hash_policy
+       sysctl_restore net.ipv4.fib_multipath_hash_policy
 }
 
 multipath6_l4_test()

@@ -243,13 +241,11 @@ multipath6_l4_test()
        local weight_rp13=$3
        local t0_rp12 t0_rp13 t1_rp12 t1_rp13
        local packets_rp12 packets_rp13
-       local hash_policy
 
        # Transmit multiple flows from h1 to h2 and make sure they are
        # distributed between both multipath links (rp12 and rp13)
        # according to the configured weights.
-       hash_policy=$(sysctl -n net.ipv6.fib_multipath_hash_policy)
-       sysctl -q -w net.ipv6.fib_multipath_hash_policy=1
+       sysctl_set net.ipv6.fib_multipath_hash_policy 1
 
        ip route replace 2001:db8:2::/64 vrf vrf-r1 \
 	       nexthop via fe80:2::22 dev $rp12 weight $weight_rp12 \

@@ -272,7 +268,7 @@ multipath6_l4_test()
 	       nexthop via fe80:2::22 dev $rp12 \
 	       nexthop via fe80:3::23 dev $rp13
 
-       sysctl -q -w net.ipv6.fib_multipath_hash_policy=$hash_policy
+       sysctl_restore net.ipv6.fib_multipath_hash_policy
 }
 
 multipath6_test()

Signature · 6 lines

2.4.11

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Petr Machata

author

2018-05-03 10:37 UTC #3 lore raw

PATCH 3/3 selftests: forwarding: mirror_gre_nh: Unset RP filter

In reply to Petr Machata

The test fails to work if reverse-path filtering is in effect on the mirrored-to host interface, or for all interfaces.

Signed-off-by Petr Machata <petrm at mellanox.com>

Reviewed-by Ido Schimmel <idosch at mellanox.com>

---
 tools/testing/selftests/net/forwarding/mirror_gre_nh.sh | 6 ++++++
 1 file changed, 6 insertions(+)

tools/testing/selftests/net/forwarding/mirror_gre_nh.sh +6 -0

index a0d1ad4..8fa681e 100755
@@ -29,6 +29,9 @@ setup_prepare()
 	swp3=${NETIFS[p5]}
 	h3=${NETIFS[p6]}
 
+	sysctl_set net.ipv4.conf.all.rp_filter 0
+	sysctl_set net.ipv4.conf.$h3.rp_filter 0
+
 	vrf_prepare
 	mirror_gre_topo_create

@@ -60,6 +63,9 @@ cleanup()
 
 	mirror_gre_topo_destroy
 	vrf_cleanup
+
+	sysctl_restore net.ipv4.conf.$h3.rp_filter
+	sysctl_restore net.ipv4.conf.all.rp_filter
 }
 
 test_gretap()

Signature · 6 lines

2.4.11

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

David Miller

maintainer

2018-05-03 17:37 UTC #4 lore raw

In reply to Petr Machata

From: Petr Machata <petrm at mellanox.com>
Date: Thu, 03 May 2018 12:36:52 +0200

Quoted 5 lines

Some selftests need to adjust sysctl settings. In order to be neutral to the system that the test is run on, it is a good practice to change back to the original setting after the test ends. That involves some boilerplate that can be abstracted away.

Quoted 8 lines

In patch #1, introduce two functions, sysctl_set() and sysctl_restore(). The former stores the current value of a given setting, and sets a new value. The latter restores the setting to the previously-stored value.
In patch #2, use these wrappers in a number of tests.
Additionally in patch #3, fix a problem in mirror_gre_nh.sh, which neglected to set a sysctl that's crucial for the test to work.

Luckily that errant 'mlxsw' hides inside of [] so git will get rid of it :-)

Series applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html