| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aAitarcdcgq9x6uL@shredder>
Date: Wed, 23 Apr 2025 12:05:46 +0300
From: Ido Schimmel <idosch@...sch.org>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
edumazet@...gle.com, pabeni@...hat.com, dsahern@...nel.org,
horms@...nel.org, idosch@...dia.com, kuniyu@...zon.com,
Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net-next 3/3] selftests/net: test tcp connection load
balancing
On Sun, Apr 20, 2025 at 02:04:31PM -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@...gle.com>
>
> Verify that TCP connections use both routes when connecting multiple
> times to a remote service over a two nexthop multipath route.
>
> Use netcat to create the connections. Use tc prio + tc filter to
> count routes taken, counting SYN packets across the two egress
> devices.
>
> To avoid flaky tests when testing inherently randomized behavior,
> set a low bar and pass if even a single SYN is observed on both
> devices.
>
> Signed-off-by: Willem de Bruijn <willemb@...gle.com>
>
> ---
>
> Integrated into fib_nexthops.sh as it covers multipath nexthop
> routing and can reuse all of its setup(), but technically the test
> does not use nexthop *objects* as is, so I can also move into a
> separate file and move common setup code to lib.sh if preferred.
No strong preference, but fib_nexthops.sh explicitly tests nexthop
objects, so including here a test that doesn't use them is a bit weird.
Did you consider putting this in fib_tests.sh instead?
> ---
> tools/testing/selftests/net/fib_nexthops.sh | 83 +++++++++++++++++++++
> 1 file changed, 83 insertions(+)
>
> diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
> index b39f748c2572..93d19e92bd5b 100755
> --- a/tools/testing/selftests/net/fib_nexthops.sh
> +++ b/tools/testing/selftests/net/fib_nexthops.sh
> @@ -31,6 +31,7 @@ IPV4_TESTS="
> ipv4_compat_mode
> ipv4_fdb_grp_fcnal
> ipv4_mpath_select
> + ipv4_mpath_balance
> ipv4_torture
> ipv4_res_torture
> "
> @@ -45,6 +46,7 @@ IPV6_TESTS="
> ipv6_compat_mode
> ipv6_fdb_grp_fcnal
> ipv6_mpath_select
> + ipv6_mpath_balance
> ipv6_torture
> ipv6_res_torture
> "
> @@ -2110,6 +2112,87 @@ ipv4_res_torture()
> log_test 0 0 "IPv4 resilient nexthop group torture test"
> }
>
> +# Install a prio qdisc with separate bands counting IPv4 and IPv6 SYNs
> +tc_add_syn_counter() {
> + local -r dev=$1
> +
> + # qdisc with band 1 for no-match, band 2 for ipv4, band 3 for ipv6
> + ip netns exec $me tc qdisc add dev $dev root handle 1: prio bands 3 \
> + priomap 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
> + ip netns exec $me tc qdisc add dev $dev parent 1:1 handle 2: pfifo
> + ip netns exec $me tc qdisc add dev $dev parent 1:2 handle 4: pfifo
> + ip netns exec $me tc qdisc add dev $dev parent 1:3 handle 6: pfifo
> +
> + # ipv4 filter on SYN flag set: band 2
> + ip netns exec $me tc filter add dev $dev parent 1: protocol ip u32 \
> + match ip protocol 6 0xff \
> + match ip dport 8000 0xffff \
> + match u8 0x02 0xff at 33 \
> + flowid 1:2
> +
> + # ipv6 filter on SYN flag set: band 3
> + ip netns exec $me tc filter add dev $dev parent 1: protocol ipv6 u32 \
> + match ip6 protocol 6 0xff \
> + match ip6 dport 8000 0xffff \
> + match u8 0x02 0xff at 53 \
> + flowid 1:3
> +}
> +
> +tc_get_syn_counter() {
> + ip netns exec $me tc -j -s qdisc show dev $1 handle $2 | jq .[0].packets
> +}
> +
> +ip_mpath_balance() {
> + local -r ipver="-$1"
> + local -r daddr=$2
> + local -r handle="$1:"
> + local -r num_conn=20
> +
> + tc_add_syn_counter veth1
> + tc_add_syn_counter veth3
> +
> + for i in $(seq 1 $num_conn); do
> + ip netns exec $remote nc $ipver -l -p 8000 >/dev/null &
> + echo -n a | ip netns exec $me nc $ipver -q 0 $daddr 8000
I don't have the '-q' option in Fedora:
# ./fib_nexthops.sh -t ipv4_mpath_balance
nc: invalid option -- 'q'
[...]
Tests passed: 0
Tests failed: 1
Tests skipped: 0
We had multiple problems in the past with 'nc' because of different
distributions using different versions. See for example:
ba6fbd383c12dfe6833968e3555ada422720a76f
5e8670610b93158ffacc3241f835454ff26a3469
Maybe use 'socat' instead?
> + done
> +
> + local -r syn0="$(tc_get_syn_counter veth1 $handle)"
> + local -r syn1="$(tc_get_syn_counter veth3 $handle)"
> + local -r syns=$((syn0+syn1))
> +
> + [ "$VERBOSE" = "1" ] && echo "multipath: syns seen: ($syn0,$syn1)"
> +
> + [[ $syns -ge $num_conn ]] && [[ $syn0 -gt 0 ]] && [[ $syn1 -gt 0 ]]
IIUC, this only tests that connections to the same destination address
and destination port are load balanced across all the paths (patch #2),
but it doesn't test that each connection uses the source address of the
egress interface (patch #1). Any reason not to test both? I'm asking
because I expect the current test to pass even without both patches.
I noticed that you are using tc-u32 for the matching, but with tc-flower
you can easily match on both 'src_ip' and 'tcp_flags'.
> +}
> +
> +ipv4_mpath_balance()
> +{
> + $IP route add 172.16.101.1 \
> + nexthop via 172.16.1.2 \
> + nexthop via 172.16.2.2
> +
> + ip netns exec $me \
> + sysctl -q -w net.ipv4.fib_multipath_hash_policy=1
> +
> + ip_mpath_balance 4 172.16.101.1
> +
> + log_test $? 0 "Multipath loadbalance"
> +}
> +
> +ipv6_mpath_balance()
> +{
> + $IP route add 2001:db8:101::1\
> + nexthop via 2001:db8:91::2 \
> + nexthop via 2001:db8:92::2
> +
> + ip netns exec $me \
> + sysctl -q -w net.ipv6.fib_multipath_hash_policy=1
> +
> + ip_mpath_balance 6 2001:db8:101::1
> +
> + log_test $? 0 "Multipath loadbalance"
> +}
> +
> basic()
> {
> echo
> --
> 2.49.0.805.g082f7c87e0-goog
>
>
Powered by blists - more mailing lists