Message-ID: <ZzC0uV1RqEnpAkwf@shredder>
Date: Sun, 10 Nov 2024 15:27:21 +0200
From: Ido Schimmel <idosch@...sch.org>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: Ferenc Fejes <fejes@....elte.hu>,
	Anna Emese Nyiri <annaemesenyiri@...il.com>, netdev@...r.kernel.org,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com
Subject: Re: [PATCH net-next v3 3/3] selftest: net: test SO_PRIORITY
 ancillary data with cmsg_sender

On Sat, Nov 09, 2024 at 02:56:29PM -0500, Willem de Bruijn wrote:
> Ferenc Fejes wrote:
> > On Fri, 2024-11-08 at 09:46 -0500, Willem de Bruijn wrote:
> > > Willem de Bruijn wrote:
> > > > Anna Emese Nyiri wrote:
> > > > > Extend cmsg_sender.c with a new option '-Q' to send SO_PRIORITY
> > > > > ancillary data.
> > > > > 
> > > > > Add the `cmsg_so_priority.sh` script, which sets up two network  
> > > > > namespaces (red and green) and uses the `cmsg_sender.c` program
> > > > > to  
> > > > > send messages between them. The script sets packet priorities
> > > > > both  
> > > > > via `setsockopt` and control messages (cmsg) and verifies
> > > > > whether  
> > > > > packets are routed to the same queue based on priority settings.
> > > > 
> > > > qdisc. queue is a more generic term, generally referring to hw
> > > > queues.
> > > >  
> > > > > 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 | 115
> > > > > ++++++++++++++++++
> > > > >  2 files changed, 125 insertions(+), 1 deletion(-)
> > > > >  create mode 100755
> > > > > tools/testing/selftests/net/cmsg_so_priority.sh
> > > >  
> > > > > diff --git a/tools/testing/selftests/net/cmsg_so_priority.sh
> > > > > b/tools/testing/selftests/net/cmsg_so_priority.sh
> > > > > new file mode 100755
> > > > > index 000000000000..706d29b251e9
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/selftests/net/cmsg_so_priority.sh
> > > > > @@ -0,0 +1,115 @@
> > > > > +#!/bin/bash
> > > > 
> > > > SPDX header
> > > > 
> > > > > +
> > > > > +source lib.sh
> > > > > +
> > > > > +IP4=192.168.0.2/16
> > > > > +TGT4=192.168.0.3/16
> > > > > +TGT4_NO_MASK=192.168.0.3
> > > > > +IP6=2001:db8::2/64
> > > > > +TGT6=2001:db8::3/64
> > > > > +TGT6_NO_MASK=2001:db8::3
> > > > > +IP4BR=192.168.0.1/16
> > > > > +IP6BR=2001:db8::1/64
> > > > > +PORT=8080
> > > > > +DELAY=400000
> > > > > +QUEUE_NUM=4
> > > > > +
> > > > > +
> > > > > +cleanup() {
> > > > > +    ip netns del red 2>/dev/null
> > > > > +    ip netns del green 2>/dev/null
> > > > > +    ip link del br0 2>/dev/null
> > > > > +    ip link del vethcab0 2>/dev/null
> > > > > +    ip link del vethcab1 2>/dev/null
> > > > > +}
> > > > > +
> > > > > +trap cleanup EXIT
> > > > > +
> > > > > +priority_values=($(seq 0 $((QUEUE_NUM - 1))))
> > > > > +
> > > > > +queue_config=""
> > > > > +for ((i=0; i<$QUEUE_NUM; i++)); do
> > > > > +    queue_config+=" 1@$i"
> > > > > +done
> > > > > +
> > > > > +map_config=$(seq 0 $((QUEUE_NUM - 1)) | tr '\n' ' ')
> > > > > +
> > > > > +ip netns add red
> > > > > +ip netns add green
> > > > > +ip link add br0 type bridge
> > > > > +ip link set br0 up
> > > > 
> > > > Is this bridge needed? Can this just use a veth pair as is.
> > > > 
> > > > More importantly, it would be great if we can deduplicate this kind
> > > > of
> > > > setup boilerplate across similar tests as much as possible.
> > > 
> > > As a matter of fact, similar to cmsg_so_mark, this test can probably
> > > use a dummy netdevice, no need for a second namespace and dev.
> > > 
> > > cmsg_so_mark.sh is probably small enough that it is fine to copy that
> > > and create a duplicate. As trying to extend it to cover both tests
> > > will probably double it in size and will just be harder to follow.
> > 
> > I'm afraid we don't have "ip rule" match argument for skb->priority
> > like we have for skb->mark (ip rule fwmark). AFAIU cmsg_so_mark.sh uses
> > rule matches to confirm if skb->mark is correct or not.
> > 
> > Using nftables meta priority would work with a dummy device:
> > add table so_prio
> > add chain so_prio so_prio_chain { type filter hook output priority 0; }
> > add rule so_prio so_prio_chain meta priority $SOPRIOVALUE counter
> > 
> > Is there anything simpler? I am afraid we cannot use nftables in
> > selftests, or can we? Thanks!
> 
> I'd use traffic shaper (tc). There are a variety of qdiscs/schedulers
> and classifiers that act on skb->priority.

When I initially suggested the test I was thinking about creating a VLAN
device with egress QoS map and then matching on VLAN priority with
flower. Something like [1]. It is just a quick hack. Proper test should
test with all combinations of IPv4 / IPv6 / UDP / ICMP / RAW / cmsg /
sockopt (like cmsg_so_mark.sh).

[1]
#!/bin/bash

ip netns del ns1 &> /dev/null
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 192.0.2.1/24 dev dummy1.10
ip -n ns1 neigh add 192.0.2.2 lladdr 00:11:22:33:44:55 nud permanent dev \
	dummy1.10

tc -n ns1 qdisc add dev dummy1 clsact
for i in {0..7}; do
	tc -n ns1 filter add dev dummy1 egress pref 1 handle 10${i} \
		proto 802.1q flower vlan_prio $i vlan_ethtype ipv4 \
		ip_proto udp dst_ip 192.0.2.2 action drop
done

for i in {0..7}; do
	pkts=$(tc -n ns1 -j -s filter show dev dummy1 egress \
		| jq ".[] | select(.options.handle == 10$i) | \
		.options.actions[0].stats.packets")
	[[ $pkts == 0 ]] || echo "prio $i: expected 0, got $pkts"

	ip netns exec ns1 ./cmsg_sender -4 -p udp -P $i 192.0.2.2 1234

	pkts=$(tc -n ns1 -j -s filter show dev dummy1 egress \
		| jq ".[] | select(.options.handle == 10$i) | \
		.options.actions[0].stats.packets")
	[[ $pkts == 1 ]] || echo "prio $i: expected 1, got $pkts"
done

Powered by blists - more mailing lists