Re: [PATCH net-next v3 3/3] selftest: net: test SO_PRIORITY ancillary data with cmsg_sender

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

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@idosch.org>
> > > > Signed-off-by: Anna Emese Nyiri <annaemesenyiri@gmail.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.