diff mbox series

[net,v3,2/2] selftests: add selftest for chaining of tc ingress handling to egress

Message ID 1664706272-10164-3-git-send-email-paulb@nvidia.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: Fix return value of qdisc ingress handling on success | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 2 maintainers not CCed: shuah@kernel.org linux-kselftest@vger.kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest fail Script test_ingress_egress_chaining.sh not found in tools/testing/selftests/net/Makefile
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 91 exceeds 80 columns WARNING: line length of 93 exceeds 80 columns WARNING: line length of 97 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Paul Blakey Oct. 2, 2022, 10:24 a.m. UTC
This test runs a simple ingress tc setup between two veth pairs,
then adds a egress->ingress rule to test the chaining of tc ingress
pipeline to tc egress piepline.

Signed-off-by: Paul Blakey <paulb@nvidia.com>
---
 .../net/test_ingress_egress_chaining.sh       | 81 +++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100644 tools/testing/selftests/net/test_ingress_egress_chaining.sh

Comments

Paolo Abeni Oct. 4, 2022, 9:36 a.m. UTC | #1
Hello,

On Sun, 2022-10-02 at 13:24 +0300, Paul Blakey wrote:
> This test runs a simple ingress tc setup between two veth pairs,
> then adds a egress->ingress rule to test the chaining of tc ingress
> pipeline to tc egress piepline.
> 
> Signed-off-by: Paul Blakey <paulb@nvidia.com>
> ---
>  .../net/test_ingress_egress_chaining.sh       | 81 +++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 tools/testing/selftests/net/test_ingress_egress_chaining.sh
> 
> diff --git a/tools/testing/selftests/net/test_ingress_egress_chaining.sh b/tools/testing/selftests/net/test_ingress_egress_chaining.sh
> new file mode 100644
> index 000000000000..4775f5657e68
> --- /dev/null
> +++ b/tools/testing/selftests/net/test_ingress_egress_chaining.sh
> @@ -0,0 +1,81 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# This test runs a simple ingress tc setup between two veth pairs,
> +# and chains a single egress rule to test ingress chaining to egress.
> +#
> +# Kselftest framework requirement - SKIP code is 4.
> +ksft_skip=4
> +
> +if [ "$(id -u)" -ne 0 ];then
> +	echo "SKIP: Need root privileges"
> +	exit $ksft_skip
> +fi
> +
> +if [ ! -x "$(command -v iperf)" ]; then
> +	echo "SKIP: Could not run test without iperf tool"

You just need to establish a TCP connection towards a given IP, right?

Than you can use the existing self-tests program:

# listener:
./udpgso_bench_rx -t & 

# client:
./udpgso_bench_tx -t -l <transfer time> -4  -D <listener IP>

and avoid dependencies on external tools.

> +	exit $ksft_skip
> +fi
> +
> +needed_mods="act_mirred cls_flower sch_ingress"
> +for mod in $needed_mods; do
> +	modinfo $mod &>/dev/null || { echo "SKIP: Need act_mirred module"; exit $ksft_skip; }
> +done
> +
> +ns="ns$((RANDOM%899+100))"
> +veth1="veth1$((RANDOM%899+100))"
> +veth2="veth2$((RANDOM%899+100))"
> +peer1="peer1$((RANDOM%899+100))"
> +peer2="peer2$((RANDOM%899+100))"
> +
> +function fail() {
> +	echo "FAIL: $@" >> /dev/stderr
> +	exit 1
> +}
> +
> +function cleanup() {
> +	killall -q -9 iperf
> +	ip link del $veth1 &> /dev/null
> +	ip link del $veth2 &> /dev/null
> +	ip netns del $ns &> /dev/null
> +}
> +trap cleanup EXIT
> +
> +function config() {
> +	echo "Setup veth pairs [$veth1, $peer1], and veth pair [$veth2, $peer2]"
> +	ip link add $veth1 type veth peer name $peer1
> +	ip link add $veth2 type veth peer name $peer2
> +	ifconfig $peer1 5.5.5.6/24 up

Please use the modern 'ip addr' syntax. More importantly, it's better
if you move both peers in separate netns, to avoid 'random' self-test
failure due to the specific local routing configuration.

Additionally you could pick addresses from tests blocks (192.0.2.0/24,
198.51.100.0/24, 203.0.113.0/24) or at least from private ranges.

> +	ip netns add $ns
> +	ip link set dev $peer2 netns $ns
> +	ip netns exec $ns ifconfig $peer2 5.5.5.5/24 up
> +	ifconfig $veth2 0 up
> +	ifconfig $veth1 0 up

Please use 'ip link' ...

> +
> +	echo "Add tc filter ingress->egress forwarding $veth1 <-> $veth2"
> +	tc qdisc add dev $veth2 ingress
> +	tc qdisc add dev $veth1 ingress
> +	tc filter add dev $veth2 ingress prio 1 proto all flower \
> +		action mirred egress redirect dev $veth1
> +	tc filter add dev $veth1 ingress prio 1 proto all flower \
> +		action mirred egress redirect dev $veth2
> +
> +	echo "Add tc filter egress->ingress forwarding $peer1 -> $veth1, bypassing the veth pipe"
> +	tc qdisc add dev $peer1 clsact
> +	tc filter add dev $peer1 egress prio 20 proto ip flower \
> +		action mirred ingress redirect dev $veth1
> +}
> +
> +function test_run() {
> +	echo "Run iperf"
> +	iperf -s -D

Depending on the timing, the server can create the listener socket
after that the client tried to connect, causing random failures. 

You should introduce some explicit, small, delay to give the server the
time to start-up, e.g.:

# start server
sleep 0.2
# start client


Thanks!

Paolo
Paolo Abeni Oct. 4, 2022, 9:40 a.m. UTC | #2
On Sun, 2022-10-02 at 13:24 +0300, Paul Blakey wrote:
> This test runs a simple ingress tc setup between two veth pairs,
> then adds a egress->ingress rule to test the chaining of tc ingress
> pipeline to tc egress piepline.
> 
> Signed-off-by: Paul Blakey <paulb@nvidia.com>
> ---

whoops... I forgot an important item: you should add the new test to
the net self-tests Makefile:

TEST_PROGS += test_ingress_egress_chaining.sh

Thanks!

Paolo
Paul Blakey Oct. 12, 2022, 8:12 a.m. UTC | #3
On 04/10/2022 12:36, Paolo Abeni wrote:
> Hello,
> 
> On Sun, 2022-10-02 at 13:24 +0300, Paul Blakey wrote:
>> This test runs a simple ingress tc setup between two veth pairs,
>> then adds a egress->ingress rule to test the chaining of tc ingress
>> pipeline to tc egress piepline.
>>
>> Signed-off-by: Paul Blakey <paulb@nvidia.com>
>> ---
>>   .../net/test_ingress_egress_chaining.sh       | 81 +++++++++++++++++++
>>   1 file changed, 81 insertions(+)
>>   create mode 100644 tools/testing/selftests/net/test_ingress_egress_chaining.sh
>>
>> diff --git a/tools/testing/selftests/net/test_ingress_egress_chaining.sh b/tools/testing/selftests/net/test_ingress_egress_chaining.sh
>> new file mode 100644
>> index 000000000000..4775f5657e68
>> --- /dev/null
>> +++ b/tools/testing/selftests/net/test_ingress_egress_chaining.sh
>> @@ -0,0 +1,81 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +# This test runs a simple ingress tc setup between two veth pairs,
>> +# and chains a single egress rule to test ingress chaining to egress.
>> +#
>> +# Kselftest framework requirement - SKIP code is 4.
>> +ksft_skip=4
>> +
>> +if [ "$(id -u)" -ne 0 ];then
>> +	echo "SKIP: Need root privileges"
>> +	exit $ksft_skip
>> +fi
>> +
>> +if [ ! -x "$(command -v iperf)" ]; then
>> +	echo "SKIP: Could not run test without iperf tool"
> 
> You just need to establish a TCP connection towards a given IP, right?
> 
> Than you can use the existing self-tests program:
> 
> # listener:
> ./udpgso_bench_rx -t &
> 
> # client:
> ./udpgso_bench_tx -t -l <transfer time> -4  -D <listener IP>
> 
> and avoid dependencies on external tools.
> 
>> +	exit $ksft_skip
>> +fi
>> +
>> +needed_mods="act_mirred cls_flower sch_ingress"
>> +for mod in $needed_mods; do
>> +	modinfo $mod &>/dev/null || { echo "SKIP: Need act_mirred module"; exit $ksft_skip; }
>> +done
>> +
>> +ns="ns$((RANDOM%899+100))"
>> +veth1="veth1$((RANDOM%899+100))"
>> +veth2="veth2$((RANDOM%899+100))"
>> +peer1="peer1$((RANDOM%899+100))"
>> +peer2="peer2$((RANDOM%899+100))"
>> +
>> +function fail() {
>> +	echo "FAIL: $@" >> /dev/stderr
>> +	exit 1
>> +}
>> +
>> +function cleanup() {
>> +	killall -q -9 iperf
>> +	ip link del $veth1 &> /dev/null
>> +	ip link del $veth2 &> /dev/null
>> +	ip netns del $ns &> /dev/null
>> +}
>> +trap cleanup EXIT
>> +
>> +function config() {
>> +	echo "Setup veth pairs [$veth1, $peer1], and veth pair [$veth2, $peer2]"
>> +	ip link add $veth1 type veth peer name $peer1
>> +	ip link add $veth2 type veth peer name $peer2
>> +	ifconfig $peer1 5.5.5.6/24 up
> 
> Please use the modern 'ip addr' syntax. More importantly, it's better
> if you move both peers in separate netns, to avoid 'random' self-test
> failure due to the specific local routing configuration.

I have one of the peers outside a namespace because I needed the egress 
tc rule to see both devices.

Besides that I will change as requested, Thanks.

> 
> Additionally you could pick addresses from tests blocks (192.0.2.0/24,
> 198.51.100.0/24, 203.0.113.0/24) or at least from private ranges.
> 
>> +	ip netns add $ns
>> +	ip link set dev $peer2 netns $ns
>> +	ip netns exec $ns ifconfig $peer2 5.5.5.5/24 up
>> +	ifconfig $veth2 0 up
>> +	ifconfig $veth1 0 up
> 
> Please use 'ip link' ...
> 
>> +
>> +	echo "Add tc filter ingress->egress forwarding $veth1 <-> $veth2"
>> +	tc qdisc add dev $veth2 ingress
>> +	tc qdisc add dev $veth1 ingress
>> +	tc filter add dev $veth2 ingress prio 1 proto all flower \
>> +		action mirred egress redirect dev $veth1
>> +	tc filter add dev $veth1 ingress prio 1 proto all flower \
>> +		action mirred egress redirect dev $veth2
>> +
>> +	echo "Add tc filter egress->ingress forwarding $peer1 -> $veth1, bypassing the veth pipe"
>> +	tc qdisc add dev $peer1 clsact
>> +	tc filter add dev $peer1 egress prio 20 proto ip flower \
>> +		action mirred ingress redirect dev $veth1
>> +}
>> +
>> +function test_run() {
>> +	echo "Run iperf"
>> +	iperf -s -D
> 
> Depending on the timing, the server can create the listener socket
> after that the client tried to connect, causing random failures.
> 
> You should introduce some explicit, small, delay to give the server the
> time to start-up, e.g.:
> 
> # start server
> sleep 0.2
> # start client
> 
> 
> Thanks!
> 
> Paolo
>
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/test_ingress_egress_chaining.sh b/tools/testing/selftests/net/test_ingress_egress_chaining.sh
new file mode 100644
index 000000000000..4775f5657e68
--- /dev/null
+++ b/tools/testing/selftests/net/test_ingress_egress_chaining.sh
@@ -0,0 +1,81 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# This test runs a simple ingress tc setup between two veth pairs,
+# and chains a single egress rule to test ingress chaining to egress.
+#
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
+if [ "$(id -u)" -ne 0 ];then
+	echo "SKIP: Need root privileges"
+	exit $ksft_skip
+fi
+
+if [ ! -x "$(command -v iperf)" ]; then
+	echo "SKIP: Could not run test without iperf tool"
+	exit $ksft_skip
+fi
+
+needed_mods="act_mirred cls_flower sch_ingress"
+for mod in $needed_mods; do
+	modinfo $mod &>/dev/null || { echo "SKIP: Need act_mirred module"; exit $ksft_skip; }
+done
+
+ns="ns$((RANDOM%899+100))"
+veth1="veth1$((RANDOM%899+100))"
+veth2="veth2$((RANDOM%899+100))"
+peer1="peer1$((RANDOM%899+100))"
+peer2="peer2$((RANDOM%899+100))"
+
+function fail() {
+	echo "FAIL: $@" >> /dev/stderr
+	exit 1
+}
+
+function cleanup() {
+	killall -q -9 iperf
+	ip link del $veth1 &> /dev/null
+	ip link del $veth2 &> /dev/null
+	ip netns del $ns &> /dev/null
+}
+trap cleanup EXIT
+
+function config() {
+	echo "Setup veth pairs [$veth1, $peer1], and veth pair [$veth2, $peer2]"
+	ip link add $veth1 type veth peer name $peer1
+	ip link add $veth2 type veth peer name $peer2
+	ifconfig $peer1 5.5.5.6/24 up
+	ip netns add $ns
+	ip link set dev $peer2 netns $ns
+	ip netns exec $ns ifconfig $peer2 5.5.5.5/24 up
+	ifconfig $veth2 0 up
+	ifconfig $veth1 0 up
+
+	echo "Add tc filter ingress->egress forwarding $veth1 <-> $veth2"
+	tc qdisc add dev $veth2 ingress
+	tc qdisc add dev $veth1 ingress
+	tc filter add dev $veth2 ingress prio 1 proto all flower \
+		action mirred egress redirect dev $veth1
+	tc filter add dev $veth1 ingress prio 1 proto all flower \
+		action mirred egress redirect dev $veth2
+
+	echo "Add tc filter egress->ingress forwarding $peer1 -> $veth1, bypassing the veth pipe"
+	tc qdisc add dev $peer1 clsact
+	tc filter add dev $peer1 egress prio 20 proto ip flower \
+		action mirred ingress redirect dev $veth1
+}
+
+function test_run() {
+	echo "Run iperf"
+	iperf -s -D
+	timeout -k 2 10 ip netns exec $ns iperf -c 5.5.5.6 -i 1 -t 2 || fail "iperf failed"
+	echo "Test passed"
+}
+
+config
+test_run
+trap - EXIT
+cleanup
+
+