diff mbox series

[PATCHv3] selftests: rtnetlink: load fou module for kci_test_encap_fou() test

Message ID 20200907035010.9154-1-po-hsu.lin@canonical.com (mailing list archive)
State New
Headers show
Series [PATCHv3] selftests: rtnetlink: load fou module for kci_test_encap_fou() test | expand

Commit Message

Po-Hsu Lin Sept. 7, 2020, 3:50 a.m. UTC
The kci_test_encap_fou() test from kci_test_encap() in rtnetlink.sh
needs the fou module to work. Otherwise it will fail with:

  $ ip netns exec "$testns" ip fou add port 7777 ipproto 47
  RTNETLINK answers: No such file or directory
  Error talking to the kernel

Add the CONFIG_NET_FOU into the config file as well. Which needs at
least to be set as a loadable module.

Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 tools/testing/selftests/net/config       | 1 +
 tools/testing/selftests/net/rtnetlink.sh | 6 ++++++
 2 files changed, 7 insertions(+)

Comments

Jakub Kicinski Sept. 7, 2020, 8:12 p.m. UTC | #1
On Mon,  7 Sep 2020 11:50:10 +0800 Po-Hsu Lin wrote:
> The kci_test_encap_fou() test from kci_test_encap() in rtnetlink.sh
> needs the fou module to work. Otherwise it will fail with:
> 
>   $ ip netns exec "$testns" ip fou add port 7777 ipproto 47
>   RTNETLINK answers: No such file or directory
>   Error talking to the kernel
> 
> Add the CONFIG_NET_FOU into the config file as well. Which needs at
> least to be set as a loadable module.
> 
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>

> diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
> index 7c38a90..a711b3e 100755
> --- a/tools/testing/selftests/net/rtnetlink.sh
> +++ b/tools/testing/selftests/net/rtnetlink.sh
> @@ -520,6 +520,11 @@ kci_test_encap_fou()
>  		return $ksft_skip
>  	fi
>  
> +	if ! /sbin/modprobe -q -n fou; then
> +		echo "SKIP: module fou is not found"
> +		return $ksft_skip
> +	fi
> +	/sbin/modprobe -q fou
>  	ip -netns "$testns" fou add port 7777 ipproto 47 2>/dev/null
>  	if [ $? -ne 0 ];then
>  		echo "FAIL: can't add fou port 7777, skipping test"
> @@ -540,6 +545,7 @@ kci_test_encap_fou()
>  		return 1
>  	fi
>  
> +	/sbin/modprobe -q -r fou

I think the common practice is to not remove the module at the end of
the test. It may be used by something else than the test itself.

>  	echo "PASS: fou"
>  }
>
Po-Hsu Lin Sept. 8, 2020, 6:57 a.m. UTC | #2
On Tue, Sep 8, 2020 at 4:12 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon,  7 Sep 2020 11:50:10 +0800 Po-Hsu Lin wrote:
> > The kci_test_encap_fou() test from kci_test_encap() in rtnetlink.sh
> > needs the fou module to work. Otherwise it will fail with:
> >
> >   $ ip netns exec "$testns" ip fou add port 7777 ipproto 47
> >   RTNETLINK answers: No such file or directory
> >   Error talking to the kernel
> >
> > Add the CONFIG_NET_FOU into the config file as well. Which needs at
> > least to be set as a loadable module.
> >
> > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
>
> > diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
> > index 7c38a90..a711b3e 100755
> > --- a/tools/testing/selftests/net/rtnetlink.sh
> > +++ b/tools/testing/selftests/net/rtnetlink.sh
> > @@ -520,6 +520,11 @@ kci_test_encap_fou()
> >               return $ksft_skip
> >       fi
> >
> > +     if ! /sbin/modprobe -q -n fou; then
> > +             echo "SKIP: module fou is not found"
> > +             return $ksft_skip
> > +     fi
> > +     /sbin/modprobe -q fou
> >       ip -netns "$testns" fou add port 7777 ipproto 47 2>/dev/null
> >       if [ $? -ne 0 ];then
> >               echo "FAIL: can't add fou port 7777, skipping test"
> > @@ -540,6 +545,7 @@ kci_test_encap_fou()
> >               return 1
> >       fi
> >
> > +     /sbin/modprobe -q -r fou
>
> I think the common practice is to not remove the module at the end of
> the test. It may be used by something else than the test itself.
>
Hello Jakub,
Thanks for your feedback.

For this case I think it's safe to remove the module here, as it was
never loaded before and thus causing this test to fail.
If other tests in this rtnetlink.sh need this fou module, we should be
able to spot those failures too, however this is the only failure as I
can see.
(pmtu.sh will need fou module to run as well, but it will be loaded there.)

Shouldn't we insert the required module whenever the test needs it? So
that we can run the test itself directly, without depending on other
tests.
Also, I can see modules for tests were being unloaded in other tests as well.

Thanks

> >       echo "PASS: fou"
> >  }
> >
>
Po-Hsu Lin Sept. 18, 2020, 10:39 a.m. UTC | #3
Hello folks,

any thoughts on this patch?
It can make the test pass and reduce the failure numbers in
kselftests, it will be great to have this applied.

Thanks
PHLin


On Tue, Sep 8, 2020 at 2:57 PM Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:
>
> On Tue, Sep 8, 2020 at 4:12 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Mon,  7 Sep 2020 11:50:10 +0800 Po-Hsu Lin wrote:
> > > The kci_test_encap_fou() test from kci_test_encap() in rtnetlink.sh
> > > needs the fou module to work. Otherwise it will fail with:
> > >
> > >   $ ip netns exec "$testns" ip fou add port 7777 ipproto 47
> > >   RTNETLINK answers: No such file or directory
> > >   Error talking to the kernel
> > >
> > > Add the CONFIG_NET_FOU into the config file as well. Which needs at
> > > least to be set as a loadable module.
> > >
> > > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> >
> > > diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
> > > index 7c38a90..a711b3e 100755
> > > --- a/tools/testing/selftests/net/rtnetlink.sh
> > > +++ b/tools/testing/selftests/net/rtnetlink.sh
> > > @@ -520,6 +520,11 @@ kci_test_encap_fou()
> > >               return $ksft_skip
> > >       fi
> > >
> > > +     if ! /sbin/modprobe -q -n fou; then
> > > +             echo "SKIP: module fou is not found"
> > > +             return $ksft_skip
> > > +     fi
> > > +     /sbin/modprobe -q fou
> > >       ip -netns "$testns" fou add port 7777 ipproto 47 2>/dev/null
> > >       if [ $? -ne 0 ];then
> > >               echo "FAIL: can't add fou port 7777, skipping test"
> > > @@ -540,6 +545,7 @@ kci_test_encap_fou()
> > >               return 1
> > >       fi
> > >
> > > +     /sbin/modprobe -q -r fou
> >
> > I think the common practice is to not remove the module at the end of
> > the test. It may be used by something else than the test itself.
> >
> Hello Jakub,
> Thanks for your feedback.
>
> For this case I think it's safe to remove the module here, as it was
> never loaded before and thus causing this test to fail.
> If other tests in this rtnetlink.sh need this fou module, we should be
> able to spot those failures too, however this is the only failure as I
> can see.
> (pmtu.sh will need fou module to run as well, but it will be loaded there.)
>
> Shouldn't we insert the required module whenever the test needs it? So
> that we can run the test itself directly, without depending on other
> tests.
> Also, I can see modules for tests were being unloaded in other tests as well.
>
> Thanks
>
> > >       echo "PASS: fou"
> > >  }
> > >
> >
Po-Hsu Lin Oct. 12, 2020, 5:56 a.m. UTC | #4
Hello,
Is there any update on this patch?
Thanks!
PHLin

On Fri, Sep 18, 2020 at 6:39 PM Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:
>
> Hello folks,
>
> any thoughts on this patch?
> It can make the test pass and reduce the failure numbers in
> kselftests, it will be great to have this applied.
>
> Thanks
> PHLin
>
>
> On Tue, Sep 8, 2020 at 2:57 PM Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:
> >
> > On Tue, Sep 8, 2020 at 4:12 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > >
> > > On Mon,  7 Sep 2020 11:50:10 +0800 Po-Hsu Lin wrote:
> > > > The kci_test_encap_fou() test from kci_test_encap() in rtnetlink.sh
> > > > needs the fou module to work. Otherwise it will fail with:
> > > >
> > > >   $ ip netns exec "$testns" ip fou add port 7777 ipproto 47
> > > >   RTNETLINK answers: No such file or directory
> > > >   Error talking to the kernel
> > > >
> > > > Add the CONFIG_NET_FOU into the config file as well. Which needs at
> > > > least to be set as a loadable module.
> > > >
> > > > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> > >
> > > > diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
> > > > index 7c38a90..a711b3e 100755
> > > > --- a/tools/testing/selftests/net/rtnetlink.sh
> > > > +++ b/tools/testing/selftests/net/rtnetlink.sh
> > > > @@ -520,6 +520,11 @@ kci_test_encap_fou()
> > > >               return $ksft_skip
> > > >       fi
> > > >
> > > > +     if ! /sbin/modprobe -q -n fou; then
> > > > +             echo "SKIP: module fou is not found"
> > > > +             return $ksft_skip
> > > > +     fi
> > > > +     /sbin/modprobe -q fou
> > > >       ip -netns "$testns" fou add port 7777 ipproto 47 2>/dev/null
> > > >       if [ $? -ne 0 ];then
> > > >               echo "FAIL: can't add fou port 7777, skipping test"
> > > > @@ -540,6 +545,7 @@ kci_test_encap_fou()
> > > >               return 1
> > > >       fi
> > > >
> > > > +     /sbin/modprobe -q -r fou
> > >
> > > I think the common practice is to not remove the module at the end of
> > > the test. It may be used by something else than the test itself.
> > >
> > Hello Jakub,
> > Thanks for your feedback.
> >
> > For this case I think it's safe to remove the module here, as it was
> > never loaded before and thus causing this test to fail.
> > If other tests in this rtnetlink.sh need this fou module, we should be
> > able to spot those failures too, however this is the only failure as I
> > can see.
> > (pmtu.sh will need fou module to run as well, but it will be loaded there.)
> >
> > Shouldn't we insert the required module whenever the test needs it? So
> > that we can run the test itself directly, without depending on other
> > tests.
> > Also, I can see modules for tests were being unloaded in other tests as well.
> >
> > Thanks
> >
> > > >       echo "PASS: fou"
> > > >  }
> > > >
> > >
Jakub Kicinski Oct. 12, 2020, 3:28 p.m. UTC | #5
On Mon, 12 Oct 2020 13:56:15 +0800 Po-Hsu Lin wrote:
> Is there any update on this patch?

You received feedback. Don't remove modules after tests, something else
could be using them.
Po-Hsu Lin Oct. 13, 2020, 2:48 a.m. UTC | #6
On Mon, Oct 12, 2020 at 11:28 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 12 Oct 2020 13:56:15 +0800 Po-Hsu Lin wrote:
> > Is there any update on this patch?
>
> You received feedback. Don't remove modules after tests, something else
> could be using them.

Hello Jakub,
I have my feedback regarding your input [1], so I guess it's not
persuasive enough?
Thanks

[1] https://marc.info/?l=linux-kernel&m=159954826414645&w=2
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index 3b42c06b..c5e50ab 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -31,3 +31,4 @@  CONFIG_NET_SCH_ETF=m
 CONFIG_NET_SCH_NETEM=y
 CONFIG_TEST_BLACKHOLE_DEV=m
 CONFIG_KALLSYMS=y
+CONFIG_NET_FOU=m
diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index 7c38a90..a711b3e 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -520,6 +520,11 @@  kci_test_encap_fou()
 		return $ksft_skip
 	fi
 
+	if ! /sbin/modprobe -q -n fou; then
+		echo "SKIP: module fou is not found"
+		return $ksft_skip
+	fi
+	/sbin/modprobe -q fou
 	ip -netns "$testns" fou add port 7777 ipproto 47 2>/dev/null
 	if [ $? -ne 0 ];then
 		echo "FAIL: can't add fou port 7777, skipping test"
@@ -540,6 +545,7 @@  kci_test_encap_fou()
 		return 1
 	fi
 
+	/sbin/modprobe -q -r fou
 	echo "PASS: fou"
 }