[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <673ba52a4374f_1d652429476@willemb.c.googlers.com.notmuch>
Date: Mon, 18 Nov 2024 15:35:54 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Anna Emese Nyiri <annaemesenyiri@...il.com>,
netdev@...r.kernel.org
Cc: fejes@....elte.hu,
annaemesenyiri@...il.com,
edumazet@...gle.com,
kuba@...nel.org,
pabeni@...hat.com,
willemb@...gle.com,
idosch@...sch.org
Subject: Re: [PATCH net-next v4 3/3] selftests: net: test SO_PRIORITY
ancillary data with cmsg_sender
Anna Emese Nyiri wrote:
> Extend cmsg_sender.c with a new option '-Q' to send SO_PRIORITY
> ancillary data.
>
> cmsg_so_priority.sh script added to validate SO_PRIORITY behavior
> by creating VLAN device with egress QoS mapping and testing packet
> priorities using flower filters. Verify that packets with different
> priorities are correctly matched and counted by filters for multiple
> protocols and IP versions.
>
> Suggested-by: Ido Schimmel <idosch@...sch.org>
> Signed-off-by: Anna Emese Nyiri <annaemesenyiri@...il.com>
> ---
> tools/testing/selftests/net/cmsg_sender.c | 11 +-
> .../testing/selftests/net/cmsg_so_priority.sh | 147 ++++++++++++++++++
> 2 files changed, 157 insertions(+), 1 deletion(-)
> create mode 100755 tools/testing/selftests/net/cmsg_so_priority.sh
>
> +++ b/tools/testing/selftests/net/cmsg_so_priority.sh
> @@ -0,0 +1,147 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +IP4=192.0.2.1/24
> +TGT4=192.0.2.2/24
> +TGT4_NO_MASK=192.0.2.2
nit, avoid duplication:
TGT4_NO_MASK=192.0.2.2
TGT4="${TGT4_NO_MASK}/24"
etc.
Or even drop the versions with the suffix and add that
explicitly where used.
> +TGT4_RAW=192.0.2.3/24
> +TGT4_RAW_NO_MASK=192.0.2.3
> +IP6=2001:db8::1/64
> +TGT6=2001:db8::2/64
> +TGT6_NO_MASK=2001:db8::2
> +TGT6_RAW=2001:db8::3/64
> +TGT6_RAW_NO_MASK=2001:db8::3
> +PORT=1234
> +DELAY=4000
> +
> +
> +create_filter() {
> +
> + local ns=$1
> + local dev=$2
> + local handle=$3
> + local vlan_prio=$4
> + local ip_type=$5
> + local proto=$6
> + local dst_ip=$7
> +
> + local cmd="tc -n $ns filter add dev $dev egress pref 1 handle $handle \
> + proto 802.1q flower vlan_prio $vlan_prio vlan_ethtype $ip_type"
nit: indentation on line break. Break inside string is ideally avoided too.
perhaps just avoid the string and below call
tc -n "$ns" filter add dev "$dev" \
egress pref 1 handle "$handle" proto 802.1q \
dst_ip "$dst_ip" "$ip_proto_opt"
flower vlan_prio "$vlan_prio" vlan_ethtype "$ip_type" \
action pass
> +
> + if [[ "$proto" == "u" ]]; then
> + ip_proto="udp"
> + elif [[ "$ip_type" == "ipv4" && "$proto" == "i" ]]; then
> + ip_proto="icmp"
> + elif [[ "$ip_type" == "ipv6" && "$proto" == "i" ]]; then
> + ip_proto="icmpv6"
> + fi
> +
> + if [[ "$proto" != "r" ]]; then
> + cmd="$cmd ip_proto $ip_proto"
> + fi
> +
> + cmd="$cmd dst_ip $dst_ip action pass"
> +
> + eval $cmd
> +}
> +
> +TOTAL_TESTS=0
> +FAILED_TESTS=0
> +
> +check_result() {
> + ((TOTAL_TESTS++))
> + if [ "$1" -ne 0 ]; then
> + ((FAILED_TESTS++))
> + fi
> +}
> +
> +cleanup() {
> + ip link del dummy1 2>/dev/null
Both devices are in ns1.
No need to clean up the devices explicitly. Just deleting the netns
will remove them.
> + ip -n ns1 link del dummy1.10 2>/dev/null
> + ip netns del ns1 2>/dev/null
> +}
> +
> +trap cleanup EXIT
> +
> +
> +
> +ip netns add ns1
> +
> +ip -n ns1 link set dev lo up
> +ip -n ns1 link add name dummy1 up type dummy
> +
> +ip -n ns1 link add link dummy1 name dummy1.10 up type vlan id 10 \
> + egress-qos-map 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7
> +
> +ip -n ns1 address add $IP4 dev dummy1.10
> +ip -n ns1 address add $IP6 dev dummy1.10
> +
> +ip netns exec ns1 bash -c "
> +sysctl -w net.ipv4.ping_group_range='0 2147483647'
> +exit"
Same point on indentation on line continuation.
No need for explicit exit.
> +
> +
> +ip -n ns1 neigh add $TGT4_NO_MASK lladdr 00:11:22:33:44:55 nud permanent dev \
> + dummy1.10
> +ip -n ns1 neigh add $TGT6_NO_MASK lladdr 00:11:22:33:44:55 nud permanent dev dummy1.10
> +ip -n ns1 neigh add $TGT4_RAW_NO_MASK lladdr 00:11:22:33:44:66 nud permanent dev dummy1.10
> +ip -n ns1 neigh add $TGT6_RAW_NO_MASK lladdr 00:11:22:33:44:66 nud permanent dev dummy1.10
> +
> +tc -n ns1 qdisc add dev dummy1 clsact
> +
> +FILTER_COUNTER=10
> +
> +for i in 4 6; do
> + for proto in u i r; do
> + echo "Test IPV$i, prot: $proto"
> + for priority in {0..7}; do
> + if [[ $i == 4 && $proto == "r" ]]; then
> + TGT=$TGT4_RAW_NO_MASK
> + elif [[ $i == 6 && $proto == "r" ]]; then
> + TGT=$TGT6_RAW_NO_MASK
> + elif [ $i == 4 ]; then
> + TGT=$TGT4_NO_MASK
> + else
> + TGT=$TGT6_NO_MASK
> + fi
> +
> + handle="${FILTER_COUNTER}${priority}"
> +
> + create_filter ns1 dummy1 $handle $priority ipv$i $proto $TGT
> +
> + pkts=$(tc -n ns1 -j -s filter show dev dummy1 egress \
> + | jq ".[] | select(.options.handle == ${handle}) | \
Can jq be assumed installed on all machines?
> + .options.actions[0].stats.packets")
> +
> + if [[ $pkts == 0 ]]; then
> + check_result 0
> + else
> + echo "prio $priority: expected 0, got $pkts"
> + check_result 1
> + fi
> +
> + ip netns exec ns1 ./cmsg_sender -$i -Q $priority -d "${DELAY}" -p $proto $TGT $PORT
> + ip netns exec ns1 ./cmsg_sender -$i -P $priority -d "${DELAY}" -p $proto $TGT $PORT
> +
> +
> + pkts=$(tc -n ns1 -j -s filter show dev dummy1 egress \
> + | jq ".[] | select(.options.handle == ${handle}) | \
> + .options.actions[0].stats.packets")
> + if [[ $pkts == 2 ]]; then
> + check_result 0
> + else
> + echo "prio $priority: expected 2, got $pkts"
> + check_result 1
I'd test -Q and -p separately. A bit of extra code to repeat the pkts
read. But worth it.
> + fi
> + done
> + FILTER_COUNTER=$((FILTER_COUNTER + 10))
> + done
> +done
> +
> +if [ $FAILED_TESTS -ne 0 ]; then
> + echo "FAIL - $FAILED_TESTS/$TOTAL_TESTS tests failed"
> + exit 1
> +else
> + echo "OK - All $TOTAL_TESTS tests passed"
> + exit 0
> +fi
> --
> 2.43.0
>
Powered by blists - more mailing lists