| 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: <6808f6a11ba60_20419294d4@willemb.c.googlers.com.notmuch>
Date: Wed, 23 Apr 2025 10:18:09 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Ido Schimmel <idosch@...sch.org>,
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
Ido Schimmel wrote:
> 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?
Ok, that is a more logical location.
The main reason for fib_nexthops.sh was that it can reuse all of its
setup().
But I can probably use route_setup and manually add ns remote and
veth5 and 6. Will take a look.
> > ---
> > 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?
Ack, will change.
> > + 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'.
Will do. Thanks!
> > +}
> > +
> > +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