| 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: <CAKm6_RtbJoysoteLH0Q6QQ4XH=wshSHgBeQ6DJRMos7-7G-xwg@mail.gmail.com>
Date: Tue, 19 Nov 2024 18:20:05 +0100
From: Anna Nyiri <annaemesenyiri@...il.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: netdev@...r.kernel.org, fejes@....elte.hu, 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
Thank you for the feedback, I will update the test.
Since the features are working well and the issues in the test are
cosmetic, could it be merged in its current state?
After the merge, I would submit the correction as a fix.
Willem de Bruijn <willemdebruijn.kernel@...il.com> ezt írta (időpont:
2024. nov. 18., H, 21:35):
>
> 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