[PATCH net] selftests: net: add missing required classifier

[PATCH net] selftests: net: add missing required classifier

[Paolo Abeni]

the udpgro_fraglist self-test uses the BPF classifiers, but the
current net self-test configuration does not include it, causing
CI failures:

 # selftests: net: udpgro_frglist.sh
 # ipv6
 # tcp - over veth touching data
 # -l 4 -6 -D 2001:db8::1 -t rx -4 -t
 # Error: TC classifier not found.
 # We have an error talking to the kernel
 # Error: TC classifier not found.
 # We have an error talking to the kernel

Add the missing knob.

Fixes: edae34a3ed92 ("selftests net: add UDP GRO fraglist + bpf self-tests")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 tools/testing/selftests/net/config | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index 8da562a9ae87..ca4423ee6dc9 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -42,6 +42,7 @@ CONFIG_MPLS_ROUTING=m
 CONFIG_MPLS_IPTUNNEL=m
 CONFIG_NET_SCH_INGRESS=m
 CONFIG_NET_CLS_FLOWER=m
+CONFIG_NET_CLS_BPF=m
 CONFIG_NET_ACT_TUNNEL_KEY=m
 CONFIG_NET_ACT_MIRRED=m
 CONFIG_BAREUDP=m
-- 
2.43.0

Re: [PATCH net] selftests: net: add missing required classifier

[Maciej Żenczykowski]

On Thu, Jan 25, 2024 at 12:23 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> the udpgro_fraglist self-test uses the BPF classifiers, but the
> current net self-test configuration does not include it, causing
> CI failures:
>
>  # selftests: net: udpgro_frglist.sh
>  # ipv6
>  # tcp - over veth touching data
>  # -l 4 -6 -D 2001:db8::1 -t rx -4 -t
>  # Error: TC classifier not found.
>  # We have an error talking to the kernel
>  # Error: TC classifier not found.
>  # We have an error talking to the kernel
>
> Add the missing knob.
>
> Fixes: edae34a3ed92 ("selftests net: add UDP GRO fraglist + bpf self-tests")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  tools/testing/selftests/net/config | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
> index 8da562a9ae87..ca4423ee6dc9 100644
> --- a/tools/testing/selftests/net/config
> +++ b/tools/testing/selftests/net/config
> @@ -42,6 +42,7 @@ CONFIG_MPLS_ROUTING=m
>  CONFIG_MPLS_IPTUNNEL=m
>  CONFIG_NET_SCH_INGRESS=m
>  CONFIG_NET_CLS_FLOWER=m
> +CONFIG_NET_CLS_BPF=m
>  CONFIG_NET_ACT_TUNNEL_KEY=m
>  CONFIG_NET_ACT_MIRRED=m
>  CONFIG_BAREUDP=m
> --
> 2.43.0
>

Reviewed-by: Maciej Żenczykowski <maze@google.com>

Re: [PATCH net] selftests: net: add missing required classifier

[Eric Dumazet]

On Thu, Jan 25, 2024 at 9:23 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> the udpgro_fraglist self-test uses the BPF classifiers, but the
> current net self-test configuration does not include it, causing
> CI failures:
>
>  # selftests: net: udpgro_frglist.sh
>  # ipv6
>  # tcp - over veth touching data
>  # -l 4 -6 -D 2001:db8::1 -t rx -4 -t
>  # Error: TC classifier not found.
>  # We have an error talking to the kernel
>  # Error: TC classifier not found.
>  # We have an error talking to the kernel
>
> Add the missing knob.
>
> Fixes: edae34a3ed92 ("selftests net: add UDP GRO fraglist + bpf self-tests")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

FYI, while looking at the gro test, I found that using strace was
making it failing as well.

Not sure about this one...

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH net] selftests: net: add missing required classifier

[Paolo Abeni]

On Thu, 2024-01-25 at 09:48 +0100, Eric Dumazet wrote:
> On Thu, Jan 25, 2024 at 9:23 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > 
> > the udpgro_fraglist self-test uses the BPF classifiers, but the
> > current net self-test configuration does not include it, causing
> > CI failures:
> > 
> >  # selftests: net: udpgro_frglist.sh
> >  # ipv6
> >  # tcp - over veth touching data
> >  # -l 4 -6 -D 2001:db8::1 -t rx -4 -t
> >  # Error: TC classifier not found.
> >  # We have an error talking to the kernel
> >  # Error: TC classifier not found.
> >  # We have an error talking to the kernel
> > 
> > Add the missing knob.
> > 
> > Fixes: edae34a3ed92 ("selftests net: add UDP GRO fraglist + bpf self-tests")
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> 
> FYI, while looking at the gro test, I found that using strace was
> making it failing as well.

It looks like the gro.sh (large) tests send the to-be-aggregate
segments individually and relay on the gro flush timeout being large
enough to fit all the relevant write operations. I suspect/hope
something alike:

---
diff --git a/tools/testing/selftests/net/setup_veth.sh b/tools/testing/selftests/net/setup_veth.sh
index a9a1759e035c..1f78a87f6f37 100644
--- a/tools/testing/selftests/net/setup_veth.sh
+++ b/tools/testing/selftests/net/setup_veth.sh
@@ -11,7 +11,7 @@ setup_veth_ns() {
        local -r ns_mac="$4"
 
        [[ -e /var/run/netns/"${ns_name}" ]] || ip netns add "${ns_name}"
-       echo 100000 > "/sys/class/net/${ns_dev}/gro_flush_timeout"
+       echo 1000000 > "/sys/class/net/${ns_dev}/gro_flush_timeout"
        ip link set dev "${ns_dev}" netns "${ns_name}" mtu 65535
        ip -netns "${ns_name}" link set dev "${ns_dev}" up
--- 
should solve the sporadic issues.

> Not sure about this one...

All the udpgro* test should write a single UDP GSO packet and let the
veth segmenting it, hopefully slowing down either ends should not
impact them - but I did not check yet!

Perhaps even the gro.sh tests could be modified alike?

Cheers,

Paolo

Re: [PATCH net] selftests: net: add missing required classifier

[Eric Dumazet]

On Thu, Jan 25, 2024 at 12:38 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On Thu, 2024-01-25 at 09:48 +0100, Eric Dumazet wrote:
> > On Thu, Jan 25, 2024 at 9:23 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > >
> > > the udpgro_fraglist self-test uses the BPF classifiers, but the
> > > current net self-test configuration does not include it, causing
> > > CI failures:
> > >
> > >  # selftests: net: udpgro_frglist.sh
> > >  # ipv6
> > >  # tcp - over veth touching data
> > >  # -l 4 -6 -D 2001:db8::1 -t rx -4 -t
> > >  # Error: TC classifier not found.
> > >  # We have an error talking to the kernel
> > >  # Error: TC classifier not found.
> > >  # We have an error talking to the kernel
> > >
> > > Add the missing knob.
> > >
> > > Fixes: edae34a3ed92 ("selftests net: add UDP GRO fraglist + bpf self-tests")
> > > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> >
> > FYI, while looking at the gro test, I found that using strace was
> > making it failing as well.
>
> It looks like the gro.sh (large) tests send the to-be-aggregate
> segments individually and relay on the gro flush timeout being large
> enough to fit all the relevant write operations. I suspect/hope
> something alike:
>
> ---
> diff --git a/tools/testing/selftests/net/setup_veth.sh b/tools/testing/selftests/net/setup_veth.sh
> index a9a1759e035c..1f78a87f6f37 100644
> --- a/tools/testing/selftests/net/setup_veth.sh
> +++ b/tools/testing/selftests/net/setup_veth.sh
> @@ -11,7 +11,7 @@ setup_veth_ns() {
>         local -r ns_mac="$4"
>
>         [[ -e /var/run/netns/"${ns_name}" ]] || ip netns add "${ns_name}"
> -       echo 100000 > "/sys/class/net/${ns_dev}/gro_flush_timeout"
> +       echo 1000000 > "/sys/class/net/${ns_dev}/gro_flush_timeout"
>         ip link set dev "${ns_dev}" netns "${ns_name}" mtu 65535
>         ip -netns "${ns_name}" link set dev "${ns_dev}" up
> ---
> should solve the sporadic issues.

I think you are right.

I tried multiple values, and found 600,000 was not enough in some cases.

With 1,000,000, I was able to run the test (with the strace overhead)
100 times without a single failure.





>
> > Not sure about this one...
>
> All the udpgro* test should write a single UDP GSO packet and let the
> veth segmenting it, hopefully slowing down either ends should not
> impact them - but I did not check yet!
>
> Perhaps even the gro.sh tests could be modified alike?
>
> Cheers,
>
> Paolo
>

Re: [PATCH net] selftests: net: add missing required classifier

[Paolo Abeni]

On Thu, 2024-01-25 at 15:10 +0100, Eric Dumazet wrote:
> On Thu, Jan 25, 2024 at 12:38 PM Paolo Abeni <pabeni@redhat.com> wrote:
> > 
> > On Thu, 2024-01-25 at 09:48 +0100, Eric Dumazet wrote:
> > > On Thu, Jan 25, 2024 at 9:23 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > > > 
> > > > the udpgro_fraglist self-test uses the BPF classifiers, but the
> > > > current net self-test configuration does not include it, causing
> > > > CI failures:
> > > > 
> > > >  # selftests: net: udpgro_frglist.sh
> > > >  # ipv6
> > > >  # tcp - over veth touching data
> > > >  # -l 4 -6 -D 2001:db8::1 -t rx -4 -t
> > > >  # Error: TC classifier not found.
> > > >  # We have an error talking to the kernel
> > > >  # Error: TC classifier not found.
> > > >  # We have an error talking to the kernel
> > > > 
> > > > Add the missing knob.
> > > > 
> > > > Fixes: edae34a3ed92 ("selftests net: add UDP GRO fraglist + bpf self-tests")
> > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > > 
> > > FYI, while looking at the gro test, I found that using strace was
> > > making it failing as well.
> > 
> > It looks like the gro.sh (large) tests send the to-be-aggregate
> > segments individually and relay on the gro flush timeout being large
> > enough to fit all the relevant write operations. I suspect/hope
> > something alike:
> > 
> > ---
> > diff --git a/tools/testing/selftests/net/setup_veth.sh b/tools/testing/selftests/net/setup_veth.sh
> > index a9a1759e035c..1f78a87f6f37 100644
> > --- a/tools/testing/selftests/net/setup_veth.sh
> > +++ b/tools/testing/selftests/net/setup_veth.sh
> > @@ -11,7 +11,7 @@ setup_veth_ns() {
> >         local -r ns_mac="$4"
> > 
> >         [[ -e /var/run/netns/"${ns_name}" ]] || ip netns add "${ns_name}"
> > -       echo 100000 > "/sys/class/net/${ns_dev}/gro_flush_timeout"
> > +       echo 1000000 > "/sys/class/net/${ns_dev}/gro_flush_timeout"
> >         ip link set dev "${ns_dev}" netns "${ns_name}" mtu 65535
> >         ip -netns "${ns_name}" link set dev "${ns_dev}" up
> > ---
> > should solve the sporadic issues.
> 
> I think you are right.
> 
> I tried multiple values, and found 600,000 was not enough in some cases.
> 
> With 1,000,000, I was able to run the test (with the strace overhead)
> 100 times without a single failure.

Thank you for testing!

Do you prefer I'll send the formal patch or do you prefer otherwise? 

Cheers,

Paolo

Re: [PATCH net] selftests: net: add missing required classifier

[Eric Dumazet]

On Thu, Jan 25, 2024 at 5:40 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
>
> Thank you for testing!
>
> Do you prefer I'll send the formal patch or do you prefer otherwise?

 Please send it, you did the investigations, thanks a lot !

Re: [PATCH net] selftests: net: add missing required classifier

[Jakub Kicinski]

On Thu, 25 Jan 2024 09:22:50 +0100 Paolo Abeni wrote:
> the udpgro_fraglist self-test uses the BPF classifiers, but the
> current net self-test configuration does not include it, causing
> CI failures:

Ah, that's what the other patch was on top of :)
Applied (also slightly reordered), thank you!
-- 
pw-bot: accepted