[PATCH net] selftests: net: add missing config for big tcp tests

[PATCH net] selftests: net: add missing config for big tcp tests

[Paolo Abeni]

The big_tcp test-case requires a few kernel knobs currently
not specified in the net selftests config, causing the
following failure:

  # selftests: net: big_tcp.sh
  # Error: Failed to load TC action module.
  # We have an error talking to the kernel
...
  # Testing for BIG TCP:
  # CLI GSO | GW GRO | GW GSO | SER GRO
  # ./big_tcp.sh: line 107: test: !=: unary operator expected
...
  # on        on       on       on      : [FAIL_on_link1]

Add the missing configs

Fixes: 6bb382bcf742 ("selftests: add a selftest for big tcp")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 tools/testing/selftests/net/config | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index b511f43cd197..c0e8482f82d3 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -29,6 +29,7 @@ CONFIG_NF_NAT=m
 CONFIG_IP6_NF_IPTABLES=m
 CONFIG_IP_NF_IPTABLES=m
 CONFIG_IP6_NF_NAT=m
+CONFIG_IP6_NF_RAW=m
 CONFIG_IP_NF_NAT=m
 CONFIG_IPV6_GRE=m
 CONFIG_IPV6_SEG6_LWTUNNEL=y
@@ -41,9 +42,11 @@ CONFIG_MACVLAN=y
 CONFIG_MACVTAP=y
 CONFIG_MPLS=y
 CONFIG_MPTCP=y
+CONFIG_IP_NF_RAW=m
 CONFIG_NF_TABLES=m
 CONFIG_NF_TABLES_IPV6=y
 CONFIG_NF_TABLES_IPV4=y
+CONFIG_NF_FLOW_TABLE=m
 CONFIG_NFT_NAT=m
 CONFIG_NET_ACT_GACT=m
 CONFIG_NET_CLS_BASIC=m
@@ -71,6 +74,7 @@ CONFIG_NET_CLS_FLOWER=m
 CONFIG_NET_CLS_BPF=m
 CONFIG_NET_ACT_TUNNEL_KEY=m
 CONFIG_NET_ACT_MIRRED=m
+CONFIG_NET_ACT_CT=m
 CONFIG_BAREUDP=m
 CONFIG_IPV6_IOAM6_LWTUNNEL=y
 CONFIG_CRYPTO_SM4_GENERIC=y
@@ -79,5 +83,6 @@ CONFIG_TUN=y
 CONFIG_VXLAN=m
 CONFIG_IP_SCTP=m
 CONFIG_NETFILTER_XT_MATCH_POLICY=m
+CONFIG_NETFILTER_XT_MATCH_LENGTH=m
 CONFIG_CRYPTO_ARIA=y
 CONFIG_XFRM_INTERFACE=m
-- 
2.43.0

Re: [PATCH net] selftests: net: add missing config for big tcp tests

[Aaron Conole]

Paolo Abeni <pabeni@redhat.com> writes:

> The big_tcp test-case requires a few kernel knobs currently
> not specified in the net selftests config, causing the
> following failure:
>
>   # selftests: net: big_tcp.sh
>   # Error: Failed to load TC action module.
>   # We have an error talking to the kernel
> ...
>   # Testing for BIG TCP:
>   # CLI GSO | GW GRO | GW GSO | SER GRO
>   # ./big_tcp.sh: line 107: test: !=: unary operator expected
> ...
>   # on        on       on       on      : [FAIL_on_link1]
>
> Add the missing configs
>
> Fixes: 6bb382bcf742 ("selftests: add a selftest for big tcp")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---

Thanks for the fix.

Maybe we should also add the config for NET_ACT_CT since we will
invoke it on setup.  I guess there's some dependency that must be
pulling it in for us already so we don't explicitly call for it, but we
do require it in setup() if I understand correctly.  I don't think it
should hold up this patch though.

Acked-by: Aaron Conole <aconole@redhat.com>

Re: [PATCH net] selftests: net: add missing config for big tcp tests

[Paolo Abeni]

On Fri, 2024-01-26 at 11:18 -0500, Aaron Conole wrote:
> Paolo Abeni <pabeni@redhat.com> writes:
> 
> > The big_tcp test-case requires a few kernel knobs currently
> > not specified in the net selftests config, causing the
> > following failure:
> > 
> >   # selftests: net: big_tcp.sh
> >   # Error: Failed to load TC action module.
> >   # We have an error talking to the kernel
> > ...
> >   # Testing for BIG TCP:
> >   # CLI GSO | GW GRO | GW GSO | SER GRO
> >   # ./big_tcp.sh: line 107: test: !=: unary operator expected
> > ...
> >   # on        on       on       on      : [FAIL_on_link1]
> > 
> > Add the missing configs
> > 
> > Fixes: 6bb382bcf742 ("selftests: add a selftest for big tcp")
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> 
> Thanks for the fix.
> 
> Maybe we should also add the config for NET_ACT_CT since we will
> invoke it on setup.  I guess there's some dependency that must be
> pulling it in for us already so we don't explicitly call for it, but we
> do require it in setup() if I understand correctly.  

This patch already adds NET_ACT_CT=m:

> @@ -71,6 +74,7 @@ CONFIG_NET_CLS_FLOWER=m
>  CONFIG_NET_CLS_BPF=m
>  CONFIG_NET_ACT_TUNNEL_KEY=m
>  CONFIG_NET_ACT_MIRRED=m
> +CONFIG_NET_ACT_CT=m
>  CONFIG_BAREUDP=m
>  CONFIG_IPV6_IOAM6_LWTUNNEL=y
>  CONFIG_CRYPTO_SM4_GENERIC=y

Cheers,

Paolo

Re: [PATCH net] selftests: net: add missing config for big tcp tests

[Xin Long]

On Fri, Jan 26, 2024 at 10:32 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> The big_tcp test-case requires a few kernel knobs currently
> not specified in the net selftests config, causing the
> following failure:
>
>   # selftests: net: big_tcp.sh
>   # Error: Failed to load TC action module.
>   # We have an error talking to the kernel
> ...
>   # Testing for BIG TCP:
>   # CLI GSO | GW GRO | GW GSO | SER GRO
>   # ./big_tcp.sh: line 107: test: !=: unary operator expected
> ...
>   # on        on       on       on      : [FAIL_on_link1]
>
> Add the missing configs
>
> Fixes: 6bb382bcf742 ("selftests: add a selftest for big tcp")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Acked-by: Xin Long <lucien.xin@gmail.com>

Re: [PATCH net] selftests: net: add missing config for big tcp tests

[Jakub Kicinski]

On Fri, 26 Jan 2024 16:32:36 +0100 Paolo Abeni wrote:
> The big_tcp test-case requires a few kernel knobs currently
> not specified in the net selftests config, causing the
> following failure:
> 
>   # selftests: net: big_tcp.sh
>   # Error: Failed to load TC action module.
>   # We have an error talking to the kernel
> ...
>   # Testing for BIG TCP:
>   # CLI GSO | GW GRO | GW GSO | SER GRO
>   # ./big_tcp.sh: line 107: test: !=: unary operator expected
> ...
>   # on        on       on       on      : [FAIL_on_link1]
> 
> Add the missing configs
> 
> Fixes: 6bb382bcf742 ("selftests: add a selftest for big tcp")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Ah, great, I was missing RF_RAW in the local hack.
I applied manually because looks like this change is on top of
something:

patching file tools/testing/selftests/net/config
Hunk #3 succeeded at 73 with fuzz 1 (offset -1 lines).
Hunk #4 succeeded at 82 (offset -1 lines).

While at it I reordered the values a little bit to be closer to what 
I think would get us closer to alphasort. Hope you don't mind.

Re: [PATCH net] selftests: net: add missing config for big tcp tests

[Paolo Abeni]

On Fri, 2024-01-26 at 11:55 -0800, Jakub Kicinski wrote:
> On Fri, 26 Jan 2024 16:32:36 +0100 Paolo Abeni wrote:
> > The big_tcp test-case requires a few kernel knobs currently
> > not specified in the net selftests config, causing the
> > following failure:
> > 
> >   # selftests: net: big_tcp.sh
> >   # Error: Failed to load TC action module.
> >   # We have an error talking to the kernel
> > ...
> >   # Testing for BIG TCP:
> >   # CLI GSO | GW GRO | GW GSO | SER GRO
> >   # ./big_tcp.sh: line 107: test: !=: unary operator expected
> > ...
> >   # on        on       on       on      : [FAIL_on_link1]
> > 
> > Add the missing configs
> > 
> > Fixes: 6bb382bcf742 ("selftests: add a selftest for big tcp")
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> 
> Ah, great, I was missing RF_RAW in the local hack.
> I applied manually because looks like this change is on top of
> something:
> 
> patching file tools/testing/selftests/net/config
> Hunk #3 succeeded at 73 with fuzz 1 (offset -1 lines).
> Hunk #4 succeeded at 82 (offset -1 lines).

Ooops... yes, indeed it's on top of the few patches I sent in the past
days.

> While at it I reordered the values a little bit to be closer to what 
> I think would get us closer to alphasort. Hope you don't mind.

Sure thing I don't mind! I'm sorry for the extra work on you.

Cheers,

Paolo

Re: [PATCH net] selftests: net: add missing config for big tcp tests

[Paolo Abeni]

On Mon, 2024-01-29 at 10:11 +0100, Paolo Abeni wrote:
> On Fri, 2024-01-26 at 11:55 -0800, Jakub Kicinski wrote:
> > On Fri, 26 Jan 2024 16:32:36 +0100 Paolo Abeni wrote:
> > > The big_tcp test-case requires a few kernel knobs currently
> > > not specified in the net selftests config, causing the
> > > following failure:
> > > 
> > >   # selftests: net: big_tcp.sh
> > >   # Error: Failed to load TC action module.
> > >   # We have an error talking to the kernel
> > > ...
> > >   # Testing for BIG TCP:
> > >   # CLI GSO | GW GRO | GW GSO | SER GRO
> > >   # ./big_tcp.sh: line 107: test: !=: unary operator expected
> > > ...
> > >   # on        on       on       on      : [FAIL_on_link1]
> > > 
> > > Add the missing configs
> > > 
> > > Fixes: 6bb382bcf742 ("selftests: add a selftest for big tcp")
> > > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > 
> > Ah, great, I was missing RF_RAW in the local hack.
> > I applied manually because looks like this change is on top of
> > something:
> > 
> > patching file tools/testing/selftests/net/config
> > Hunk #3 succeeded at 73 with fuzz 1 (offset -1 lines).
> > Hunk #4 succeeded at 82 (offset -1 lines).
> 
> Ooops... yes, indeed it's on top of the few patches I sent in the past
> days.
> 
> > While at it I reordered the values a little bit to be closer to what 
> > I think would get us closer to alphasort. Hope you don't mind.
> 
> Sure thing I don't mind! I'm sorry for the extra work on you.

Uhm... while the self-test doesn't emit anymore the message related to
the missing modules, it still fails in the CI env and I can't reproduce
the failures in my local env (the same for the gro.sh script).

If I understand correctly, the tests run under double virtualization (a
VM on top AWS?), is that correct? I guess the extra slowdown/overhead
will need more care.

Thanks,

Paolo

Re: [PATCH net] selftests: net: add missing config for big tcp tests

[Jakub Kicinski]

On Mon, 29 Jan 2024 17:31:33 +0100 Paolo Abeni wrote:
> Uhm... while the self-test doesn't emit anymore the message related to
> the missing modules, it still fails in the CI env and I can't reproduce
> the failures in my local env (the same for the gro.sh script).
> 
> If I understand correctly, the tests run under double virtualization (a
> VM on top AWS?), is that correct? I guess the extra slowdown/overhead
> will need more care.

Yes, it's VM inside a VM without nested virtualization support.
A weird setup, granted, but when we move to bare metal I'd like
to enable KASAN, which will probably cause a similar slowdown..

You could possibly get a similar slowdown by disabling HW virt /
KVM?

FWIW far the 4 types of issues we've seen were:
 - config missing
 - OS doesn't ifup by default
 - OS tools are old / buggy
 - VM-in-VM is just too slow.

There's a bunch of failures in forwarding which look like perf issues.
I wonder if we should introduce something in the settings file to let
tests know that they are running in very slow env?

Re: [PATCH net] selftests: net: add missing config for big tcp tests

[Paolo Abeni]

On Mon, 2024-01-29 at 08:39 -0800, Jakub Kicinski wrote:
> On Mon, 29 Jan 2024 17:31:33 +0100 Paolo Abeni wrote:
> > Uhm... while the self-test doesn't emit anymore the message related to
> > the missing modules, it still fails in the CI env and I can't reproduce
> > the failures in my local env (the same for the gro.sh script).
> > 
> > If I understand correctly, the tests run under double virtualization (a
> > VM on top AWS?), is that correct? I guess the extra slowdown/overhead
> > will need more care.
> 
> Yes, it's VM inside a VM without nested virtualization support.
> A weird setup, granted, but when we move to bare metal I'd like
> to enable KASAN, which will probably cause a similar slowdown..
> 
> You could possibly get a similar slowdown by disabling HW virt /
> KVM?

Thanks, the above helped - that is, I can reproduce the failure running
the self-tests in a VM with KVM disabled in the host. Funnily enough I
can't use plain virtme for that - the virtme VM crashes on boot,
possibly due to the wrong 'machine' argument passed to qemu.

In any case I can't see a sane way to cope with such slow environments
except skipping the sensitive cases.

> FWIW far the 4 types of issues we've seen were:
>  - config missing
>  - OS doesn't ifup by default
>  - OS tools are old / buggy
>  - VM-in-VM is just too slow.
> 
> There's a bunch of failures in forwarding which look like perf issues.
> I wonder if we should introduce something in the settings file to let
> tests know that they are running in very slow env?

I was wondering about passing such info to the test e.g. via an env
variable:

vng --run . --user root -- HOST_IS_DAMN_SLOW=true
./tools/testing/selftests/kselftest_install/run_kselftest.sh -t
<whatever>

In any case some tests should be updated to skip the relevant cases
accordingly, right?

Cheers,

Paolo

Re: [PATCH net] selftests: net: add missing config for big tcp tests

[Jakub Kicinski]

On Tue, 30 Jan 2024 19:41:10 +0100 Paolo Abeni wrote:
> > Yes, it's VM inside a VM without nested virtualization support.
> > A weird setup, granted, but when we move to bare metal I'd like
> > to enable KASAN, which will probably cause a similar slowdown..
> > 
> > You could possibly get a similar slowdown by disabling HW virt /
> > KVM?  
> 
> Thanks, the above helped - that is, I can reproduce the failure running
> the self-tests in a VM with KVM disabled in the host. Funnily enough I
> can't use plain virtme for that - the virtme VM crashes on boot,
> possibly due to the wrong 'machine' argument passed to qemu.

FWIW I think you can fix this by passing -o " -cpu Haswell" to vng. 
Yet another piece of knowledge I wish I didn't have and which I should
probably put somewhere public :(

> In any case I can't see a sane way to cope with such slow environments
> except skipping the sensitive cases.
> 
> > FWIW far the 4 types of issues we've seen were:
> >  - config missing
> >  - OS doesn't ifup by default
> >  - OS tools are old / buggy
> >  - VM-in-VM is just too slow.
> > 
> > There's a bunch of failures in forwarding which look like perf issues.
> > I wonder if we should introduce something in the settings file to let
> > tests know that they are running in very slow env?  
> 
> I was wondering about passing such info to the test e.g. via an env
> variable:
> 
> vng --run . --user root -- HOST_IS_DAMN_SLOW=true
> ./tools/testing/selftests/kselftest_install/run_kselftest.sh -t
> <whatever>
> 
> In any case some tests should be updated to skip the relevant cases
> accordingly, right?

Which reminds me I need to send the meeting notes from the netdev call
:) We went with KSFT_MACHINE_SLOW=yes for now, the NIPA machines should
have it set now.