diff mbox series

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

Message ID 20241107132231.9271-4-annaemesenyiri@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series SO_PRIORITY cmsg patch summary | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 3 this patch: 3
netdev/build_tools success Errors and warnings before: 2 (+0) this patch: 2 (+0)
netdev/cc_maintainers warning 3 maintainers not CCed: horms@kernel.org shuah@kernel.org linux-kselftest@vger.kernel.org
netdev/build_clang success Errors and warnings before: 4 this patch: 4
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest fail Script cmsg_so_priority.sh not found in tools/testing/selftests/net/Makefile
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch warning CHECK: Alignment should match open parenthesis WARNING: Missing or malformed SPDX-License-Identifier tag in line 2 WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 85 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns WARNING: line length of 98 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest fail net-next-2024-11-07--18-00 (tests: 783)

Commit Message

Anna Emese Nyiri Nov. 7, 2024, 1:22 p.m. UTC 
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.

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

Comments

Willem de Bruijn Nov. 7, 2024, 5:22 p.m. UTC | #1 
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.

> +ip addr add $IP4BR dev br0
> +ip addr add $IP6BR dev br0
> +
> +ip link add vethcab0 type veth peer name red0
> +ip link set vethcab0 master br0
> +ip link set red0 netns red
> +ip netns exec red bash -c "
> +ip link set lo up
> +ip link set red0 up
> +ip addr add $IP4 dev red0
> +ip addr add $IP6 dev red0
> +sysctl -w net.ipv4.ping_group_range='0 2147483647'
> +exit"
> +ip link set vethcab0 up
> +
> +ip link add vethcab1 type veth peer name green0
> +ip link set vethcab1 master br0
> +ip link set green0 netns green
> +ip netns exec green bash -c "
> +ip link set lo up
> +ip link set green0 up
> +ip addr add $TGT4 dev green0
> +ip addr add $TGT6 dev green0
> +exit"
> +ip link set vethcab1 up
> +
> +ip netns exec red bash -c "
> +sudo ethtool -L red0 tx $QUEUE_NUM rx $QUEUE_NUM
> +sudo tc qdisc add dev red0 root mqprio num_tc $QUEUE_NUM queues $queue_config map $map_config hw 0
> +exit"
> +
> +get_queue_bytes() {
> +    ip netns exec red sudo tc -s qdisc show dev red0 | grep 'Sent' | awk '{print $2}'
> +}
> +
> +TOTAL_TESTS=0
> +FAILED_TESTS=0
> +
> +check_result() {
> +    ((TOTAL_TESTS++))
> +    if [ "$1" -ne 0 ]; then
> +        ((FAILED_TESTS++))
> +    fi
> +}
> +
> +
> +for i in 4 6; do
> +    [ $i == 4 ] && TGT=$TGT4_NO_MASK || TGT=$TGT6_NO_MASK
> +
> +    for p in u i r; do
> +        echo "Test IPV$i, prot: $p"
> +        for value in "${priority_values[@]}"; do
> +            ip netns exec red ./cmsg_sender -$i -Q $value -d "${DELAY}" -p $p $TGT $PORT
> +            setsockopt_priority_bytes_num=($(get_queue_bytes))
> +
> +            ip netns exec red ./cmsg_sender -$i -P $value -d "${DELAY}" -p $p $TGT $PORT
> +            cmsg_priority_bytes_num=($(get_queue_bytes))
> +
> +            if [[ "${cmsg_priority_bytes_num[$actual_queue]}" != \
> +            "${setsockopt_priority_bytes_num[$actual_queue]}" ]]; then
> +                check_result 0
> +            else
> +                check_result 1
> +            fi
> +        done
> +    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
>
Willem de Bruijn Nov. 8, 2024, 2:46 p.m. UTC | #2 
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.
Ferenc Fejes Nov. 9, 2024, 6:54 p.m. UTC | #3 
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!
Willem de Bruijn Nov. 9, 2024, 7:56 p.m. UTC | #4 
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.
Ido Schimmel Nov. 10, 2024, 1:27 p.m. UTC | #5 
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@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.

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
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/cmsg_sender.c b/tools/testing/selftests/net/cmsg_sender.c
index 876c2db02a63..6fbe93dd63d2 100644
--- a/tools/testing/selftests/net/cmsg_sender.c
+++ b/tools/testing/selftests/net/cmsg_sender.c
@@ -52,6 +52,7 @@  struct options {
 		unsigned int tclass;
 		unsigned int hlimit;
 		unsigned int priority;
+		unsigned int priority_cmsg;
 	} sockopt;
 	struct {
 		unsigned int family;
@@ -59,6 +60,7 @@  struct options {
 		unsigned int proto;
 	} sock;
 	struct option_cmsg_u32 mark;
+	struct option_cmsg_u32 priority_cmsg;
 	struct {
 		bool ena;
 		unsigned int delay;
@@ -97,6 +99,7 @@  static void __attribute__((noreturn)) cs_usage(const char *bin)
 	       "\n"
 	       "\t\t-m val  Set SO_MARK with given value\n"
 	       "\t\t-M val  Set SO_MARK via setsockopt\n"
+		   "\t\t-Q val  Set SO_PRIORITY via cmsg\n"
 	       "\t\t-d val  Set SO_TXTIME with given delay (usec)\n"
 	       "\t\t-t      Enable time stamp reporting\n"
 	       "\t\t-f val  Set don't fragment via cmsg\n"
@@ -115,7 +118,7 @@  static void cs_parse_args(int argc, char *argv[])
 {
 	int o;
 
-	while ((o = getopt(argc, argv, "46sS:p:P:m:M:n:d:tf:F:c:C:l:L:H:")) != -1) {
+	while ((o = getopt(argc, argv, "46sS:p:P:m:M:n:d:tf:F:c:C:l:L:H:Q:")) != -1) {
 		switch (o) {
 		case 's':
 			opt.silent_send = true;
@@ -148,6 +151,10 @@  static void cs_parse_args(int argc, char *argv[])
 			opt.mark.ena = true;
 			opt.mark.val = atoi(optarg);
 			break;
+		case 'Q':
+			opt.priority_cmsg.ena = true;
+			opt.priority_cmsg.val = atoi(optarg);
+			break;
 		case 'M':
 			opt.sockopt.mark = atoi(optarg);
 			break;
@@ -252,6 +259,8 @@  cs_write_cmsg(int fd, struct msghdr *msg, char *cbuf, size_t cbuf_sz)
 
 	ca_write_cmsg_u32(cbuf, cbuf_sz, &cmsg_len,
 			  SOL_SOCKET, SO_MARK, &opt.mark);
+	ca_write_cmsg_u32(cbuf, cbuf_sz, &cmsg_len,
+			SOL_SOCKET, SO_PRIORITY, &opt.priority_cmsg);
 	ca_write_cmsg_u32(cbuf, cbuf_sz, &cmsg_len,
 			  SOL_IPV6, IPV6_DONTFRAG, &opt.v6.dontfrag);
 	ca_write_cmsg_u32(cbuf, cbuf_sz, &cmsg_len,
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
+
+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
+ip addr add $IP4BR dev br0
+ip addr add $IP6BR dev br0
+
+ip link add vethcab0 type veth peer name red0
+ip link set vethcab0 master br0
+ip link set red0 netns red
+ip netns exec red bash -c "
+ip link set lo up
+ip link set red0 up
+ip addr add $IP4 dev red0
+ip addr add $IP6 dev red0
+sysctl -w net.ipv4.ping_group_range='0 2147483647'
+exit"
+ip link set vethcab0 up
+
+ip link add vethcab1 type veth peer name green0
+ip link set vethcab1 master br0
+ip link set green0 netns green
+ip netns exec green bash -c "
+ip link set lo up
+ip link set green0 up
+ip addr add $TGT4 dev green0
+ip addr add $TGT6 dev green0
+exit"
+ip link set vethcab1 up
+
+ip netns exec red bash -c "
+sudo ethtool -L red0 tx $QUEUE_NUM rx $QUEUE_NUM
+sudo tc qdisc add dev red0 root mqprio num_tc $QUEUE_NUM queues $queue_config map $map_config hw 0
+exit"
+
+get_queue_bytes() {
+    ip netns exec red sudo tc -s qdisc show dev red0 | grep 'Sent' | awk '{print $2}'
+}
+
+TOTAL_TESTS=0
+FAILED_TESTS=0
+
+check_result() {
+    ((TOTAL_TESTS++))
+    if [ "$1" -ne 0 ]; then
+        ((FAILED_TESTS++))
+    fi
+}
+
+
+for i in 4 6; do
+    [ $i == 4 ] && TGT=$TGT4_NO_MASK || TGT=$TGT6_NO_MASK
+
+    for p in u i r; do
+        echo "Test IPV$i, prot: $p"
+        for value in "${priority_values[@]}"; do
+            ip netns exec red ./cmsg_sender -$i -Q $value -d "${DELAY}" -p $p $TGT $PORT
+            setsockopt_priority_bytes_num=($(get_queue_bytes))
+
+            ip netns exec red ./cmsg_sender -$i -P $value -d "${DELAY}" -p $p $TGT $PORT
+            cmsg_priority_bytes_num=($(get_queue_bytes))
+
+            if [[ "${cmsg_priority_bytes_num[$actual_queue]}" != \
+            "${setsockopt_priority_bytes_num[$actual_queue]}" ]]; then
+                check_result 0
+            else
+                check_result 1
+            fi
+        done
+    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