selftests: openvswitch: Questions about possible enhancements

selftests: openvswitch: Questions about possible enhancements

[Simon Horman]

Hi Aaron, Jakub, all,

I have recently been exercising the Open vSwitch kernel selftests,
using vng, something like this:

	TESTDIR="tools/testing/selftests/net/openvswitch"

        vng -v --run . --user root --cpus 2 \
                --overlay-rwdir "$PWD" -- \
                "modprobe openvswitch && \
		 echo \"timeout=90\" >> \"${TESTDIR}/settings\" && \
                 make -C \"$TESTDIR\" run_tests"

And I have some observations that I'd like to ask about.

1. Building the kernel using the following command does not
   build the openvswitch kernel module.

	vng -v --build \
		--config tools/testing/selftests/net/config

   All that seems to be missing is CONFIG_OPENVSWITCH=m
   and I am wondering what the best way of resolving this is.

   Perhaps I am doing something wrong.
   Or perhaps tools/testing/selftests/net/openvswitch/config
   should be created? If so, should it include (most of?) what is in
   tools/testing/selftests/net/config, or just CONFIG_OPENVSWITCH=m?

2. As per my example above, it seems that a modprobe openvswitch is
   required (if openvswitch is a module).

   Again, perhaps I am doing something wrong. But if not, should this be
   incorporated into tools/testing/selftests/net/openvswitch/openvswitch.sh
   or otherwise automated?

3. I have observed that the last test fails (yesterday, but not today!),
   because the namespace it tries to create already exists. I believe this
   is because it is pending deletion.

   My work-around is as follows:

 ovs_add_netns_and_veths () {
 	info "Adding netns attached: sbx:$1 dp:$2 {$3, $4, $5}"
+	for i in $(seq 10); do
+		ovs_sbx "$1" test -e "/var/run/netns/$3" || break
+		info "Namespace $3 still exists (attempt $i)"
+		ovs_sbx "$1" ip netns del "$3"
+		sleep "$i"
+	done
 	ovs_sbx "$1" ip netns add "$3" || return 1
 	on_exit "ovs_sbx $1 ip netns del $3"
 	ovs_sbx "$1" ip link add "$4" type veth peer name "$5" || return 1

   N.B.: the "netns del" part is probably not needed,
   but I'm not able to exercise it effectively right now.

   I am wondering if a loop like this is appropriate to add, perhaps also
   to namespace deletion. Or if it would be appropriate to port
   openvswitch.sh to use ./tools/testing/selftests/net/lib.sh, which I
   believe handles this.

4. I am observing timeouts whith the default value of 45s.
   Bumping this to 90s seems to help.
   Are there any objections to a patch to bump the timeout?

Re: [ovs-dev] selftests: openvswitch: Questions about possible enhancements

[Simon Horman]

On Wed, Apr 24, 2024 at 05:44:05PM +0100, Simon Horman wrote:
> Hi Aaron, Jakub, all,
> 
> I have recently been exercising the Open vSwitch kernel selftests,
> using vng, something like this:
> 
> 	TESTDIR="tools/testing/selftests/net/openvswitch"
> 
>         vng -v --run . --user root --cpus 2 \
>                 --overlay-rwdir "$PWD" -- \
>                 "modprobe openvswitch && \
> 		 echo \"timeout=90\" >> \"${TESTDIR}/settings\" && \
>                  make -C \"$TESTDIR\" run_tests"
> 
> And I have some observations that I'd like to ask about.
> 
> 1. Building the kernel using the following command does not
>    build the openvswitch kernel module.
> 
> 	vng -v --build \
> 		--config tools/testing/selftests/net/config
> 
>    All that seems to be missing is CONFIG_OPENVSWITCH=m
>    and I am wondering what the best way of resolving this is.
> 
>    Perhaps I am doing something wrong.
>    Or perhaps tools/testing/selftests/net/openvswitch/config
>    should be created? If so, should it include (most of?) what is in
>    tools/testing/selftests/net/config, or just CONFIG_OPENVSWITCH=m?
> 
> 2. As per my example above, it seems that a modprobe openvswitch is
>    required (if openvswitch is a module).
> 
>    Again, perhaps I am doing something wrong. But if not, should this be
>    incorporated into tools/testing/selftests/net/openvswitch/openvswitch.sh
>    or otherwise automated?
> 
> 3. I have observed that the last test fails (yesterday, but not today!),
>    because the namespace it tries to create already exists. I believe this
>    is because it is pending deletion.
> 
>    My work-around is as follows:
> 
>  ovs_add_netns_and_veths () {
>  	info "Adding netns attached: sbx:$1 dp:$2 {$3, $4, $5}"
> +	for i in $(seq 10); do
> +		ovs_sbx "$1" test -e "/var/run/netns/$3" || break
> +		info "Namespace $3 still exists (attempt $i)"
> +		ovs_sbx "$1" ip netns del "$3"
> +		sleep "$i"
> +	done
>  	ovs_sbx "$1" ip netns add "$3" || return 1
>  	on_exit "ovs_sbx $1 ip netns del $3"
>  	ovs_sbx "$1" ip link add "$4" type veth peer name "$5" || return 1
> 
>    N.B.: the "netns del" part is probably not needed,
>    but I'm not able to exercise it effectively right now.
> 
>    I am wondering if a loop like this is appropriate to add, perhaps also
>    to namespace deletion. Or if it would be appropriate to port
>    openvswitch.sh to use ./tools/testing/selftests/net/lib.sh, which I
>    believe handles this.
> 
> 4. I am observing timeouts whith the default value of 45s.
>    Bumping this to 90s seems to help.
>    Are there any objections to a patch to bump the timeout?

  5. openvswitch.sh starts with "#!/bin/sh".
     But substitutions such as "${ns:0:1}0"  fail if /bin/sh is dash.
     Perhaps we should change openvswitch.sh to use bash?

Re: [ovs-dev] selftests: openvswitch: Questions about possible enhancements

[Benjamin Poirier]

On 2024-04-24 18:37 +0100, Simon Horman wrote:
> On Wed, Apr 24, 2024 at 05:44:05PM +0100, Simon Horman wrote:
> > Hi Aaron, Jakub, all,
> > 
> > I have recently been exercising the Open vSwitch kernel selftests,
> > using vng, something like this:
> > 
> > 	TESTDIR="tools/testing/selftests/net/openvswitch"
> > 
> >         vng -v --run . --user root --cpus 2 \
> >                 --overlay-rwdir "$PWD" -- \
> >                 "modprobe openvswitch && \
> > 		 echo \"timeout=90\" >> \"${TESTDIR}/settings\" && \
> >                  make -C \"$TESTDIR\" run_tests"
> > 
> > And I have some observations that I'd like to ask about.
> > 
> > 1. Building the kernel using the following command does not
> >    build the openvswitch kernel module.
> > 
> > 	vng -v --build \
> > 		--config tools/testing/selftests/net/config
> > 
> >    All that seems to be missing is CONFIG_OPENVSWITCH=m
> >    and I am wondering what the best way of resolving this is.
> > 
> >    Perhaps I am doing something wrong.
> >    Or perhaps tools/testing/selftests/net/openvswitch/config
> >    should be created? If so, should it include (most of?) what is in
> >    tools/testing/selftests/net/config, or just CONFIG_OPENVSWITCH=m?

I noticed something similar when testing Jiri's virtio_net selftests
patchset [1].

drivers/net/virtio_net/config includes virtio options but the
test also needs at least CONFIG_NET_VRF=y which is part of net/config.

Whatever the answer to your question, all config files should be
coherent on this matter.

[1] https://lore.kernel.org/netdev/20240424104049.3935572-1-jiri@resnulli.us/

[...]
> 
>   5. openvswitch.sh starts with "#!/bin/sh".
>      But substitutions such as "${ns:0:1}0"  fail if /bin/sh is dash.
>      Perhaps we should change openvswitch.sh to use bash?

I think so. A similar change was done in
c2518da8e6b0 selftests: bonding: Change script interpreter (v6.8-rc1)

Re: selftests: openvswitch: Questions about possible enhancements

[Aaron Conole]

Simon Horman <horms@kernel.org> writes:

> Hi Aaron, Jakub, all,
>
> I have recently been exercising the Open vSwitch kernel selftests,
> using vng, something like this:
>
> 	TESTDIR="tools/testing/selftests/net/openvswitch"
>
>         vng -v --run . --user root --cpus 2 \
>                 --overlay-rwdir "$PWD" -- \
>                 "modprobe openvswitch && \
> 		 echo \"timeout=90\" >> \"${TESTDIR}/settings\" && \
>                  make -C \"$TESTDIR\" run_tests"
>
> And I have some observations that I'd like to ask about.
>
> 1. Building the kernel using the following command does not
>    build the openvswitch kernel module.
>
> 	vng -v --build \
> 		--config tools/testing/selftests/net/config
>
>    All that seems to be missing is CONFIG_OPENVSWITCH=m
>    and I am wondering what the best way of resolving this is.
>
>    Perhaps I am doing something wrong.
>    Or perhaps tools/testing/selftests/net/openvswitch/config
>    should be created? If so, should it include (most of?) what is in
>    tools/testing/selftests/net/config, or just CONFIG_OPENVSWITCH=m?

I have a series that I need to get back to fixing:

https://mail.openvswitch.org/pipermail/ovs-dev/2024-February/411917.html

which does include the config listed, and some of the fixes for things
you've noted.

I think it makes sense to get back to it.

> 2. As per my example above, it seems that a modprobe openvswitch is
>    required (if openvswitch is a module).
>
>    Again, perhaps I am doing something wrong. But if not, should this be
>    incorporated into tools/testing/selftests/net/openvswitch/openvswitch.sh
>    or otherwise automated?
>
> 3. I have observed that the last test fails (yesterday, but not today!),
>    because the namespace it tries to create already exists. I believe this
>    is because it is pending deletion.
>
>    My work-around is as follows:
>
>  ovs_add_netns_and_veths () {
>  	info "Adding netns attached: sbx:$1 dp:$2 {$3, $4, $5}"
> +	for i in $(seq 10); do
> +		ovs_sbx "$1" test -e "/var/run/netns/$3" || break
> +		info "Namespace $3 still exists (attempt $i)"
> +		ovs_sbx "$1" ip netns del "$3"
> +		sleep "$i"
> +	done
>  	ovs_sbx "$1" ip netns add "$3" || return 1
>  	on_exit "ovs_sbx $1 ip netns del $3"
>  	ovs_sbx "$1" ip link add "$4" type veth peer name "$5" || return 1
>
>    N.B.: the "netns del" part is probably not needed,
>    but I'm not able to exercise it effectively right now.
>
>    I am wondering if a loop like this is appropriate to add, perhaps also
>    to namespace deletion. Or if it would be appropriate to port
>    openvswitch.sh to use ./tools/testing/selftests/net/lib.sh, which I
>    believe handles this.
>
> 4. I am observing timeouts whith the default value of 45s.
>    Bumping this to 90s seems to help.
>    Are there any objections to a patch to bump the timeout?

Re: [ovs-dev] selftests: openvswitch: Questions about possible enhancements

[Aaron Conole]

Benjamin Poirier <benjamin.poirier@gmail.com> writes:

> On 2024-04-24 18:37 +0100, Simon Horman wrote:
>> On Wed, Apr 24, 2024 at 05:44:05PM +0100, Simon Horman wrote:
>> > Hi Aaron, Jakub, all,
>> > 
>> > I have recently been exercising the Open vSwitch kernel selftests,
>> > using vng, something like this:
>> > 
>> > 	TESTDIR="tools/testing/selftests/net/openvswitch"
>> > 
>> >         vng -v --run . --user root --cpus 2 \
>> >                 --overlay-rwdir "$PWD" -- \
>> >                 "modprobe openvswitch && \
>> > 		 echo \"timeout=90\" >> \"${TESTDIR}/settings\" && \
>> >                  make -C \"$TESTDIR\" run_tests"
>> > 
>> > And I have some observations that I'd like to ask about.
>> > 
>> > 1. Building the kernel using the following command does not
>> >    build the openvswitch kernel module.
>> > 
>> > 	vng -v --build \
>> > 		--config tools/testing/selftests/net/config
>> > 
>> >    All that seems to be missing is CONFIG_OPENVSWITCH=m
>> >    and I am wondering what the best way of resolving this is.
>> > 
>> >    Perhaps I am doing something wrong.
>> >    Or perhaps tools/testing/selftests/net/openvswitch/config
>> >    should be created? If so, should it include (most of?) what is in
>> >    tools/testing/selftests/net/config, or just CONFIG_OPENVSWITCH=m?
>
> I noticed something similar when testing Jiri's virtio_net selftests
> patchset [1].
>
> drivers/net/virtio_net/config includes virtio options but the
> test also needs at least CONFIG_NET_VRF=y which is part of net/config.
>
> Whatever the answer to your question, all config files should be
> coherent on this matter.
>
> [1] https://lore.kernel.org/netdev/20240424104049.3935572-1-jiri@resnulli.us/
>
> [...]
>> 
>>   5. openvswitch.sh starts with "#!/bin/sh".
>>      But substitutions such as "${ns:0:1}0"  fail if /bin/sh is dash.
>>      Perhaps we should change openvswitch.sh to use bash?
>
> I think so. A similar change was done in
> c2518da8e6b0 selftests: bonding: Change script interpreter (v6.8-rc1)

+1 - I'm okay with it.

Re: selftests: openvswitch: Questions about possible enhancements

[Jakub Kicinski]

On Wed, 24 Apr 2024 17:44:05 +0100 Simon Horman wrote:
> I have recently been exercising the Open vSwitch kernel selftests,
> using vng,

Speaking of ovs tests, we currently don't run them in CI (and suffer
related skips in pmtu.sh) because Amazon Linux doesn't have ovs
packaged and building it looks pretty hard.

Is there an easy way to build just the CLI tooling or get a pre-built
package somewhere?

Or perhaps you'd be willing to run the OvS tests and we can move 
the part of pmtu.sh into OvS test dir?

Re: [ovs-dev] selftests: openvswitch: Questions about possible enhancements

[Simon Horman]

On Wed, Apr 24, 2024 at 01:59:29PM -0400, Benjamin Poirier wrote:
> On 2024-04-24 18:37 +0100, Simon Horman wrote:
> > On Wed, Apr 24, 2024 at 05:44:05PM +0100, Simon Horman wrote:
> > > Hi Aaron, Jakub, all,
> > > 
> > > I have recently been exercising the Open vSwitch kernel selftests,
> > > using vng, something like this:
> > > 
> > > 	TESTDIR="tools/testing/selftests/net/openvswitch"
> > > 
> > >         vng -v --run . --user root --cpus 2 \
> > >                 --overlay-rwdir "$PWD" -- \
> > >                 "modprobe openvswitch && \
> > > 		 echo \"timeout=90\" >> \"${TESTDIR}/settings\" && \
> > >                  make -C \"$TESTDIR\" run_tests"
> > > 
> > > And I have some observations that I'd like to ask about.
> > > 
> > > 1. Building the kernel using the following command does not
> > >    build the openvswitch kernel module.
> > > 
> > > 	vng -v --build \
> > > 		--config tools/testing/selftests/net/config
> > > 
> > >    All that seems to be missing is CONFIG_OPENVSWITCH=m
> > >    and I am wondering what the best way of resolving this is.
> > > 
> > >    Perhaps I am doing something wrong.
> > >    Or perhaps tools/testing/selftests/net/openvswitch/config
> > >    should be created? If so, should it include (most of?) what is in
> > >    tools/testing/selftests/net/config, or just CONFIG_OPENVSWITCH=m?
> 
> I noticed something similar when testing Jiri's virtio_net selftests
> patchset [1].
> 
> drivers/net/virtio_net/config includes virtio options but the
> test also needs at least CONFIG_NET_VRF=y which is part of net/config.
> 
> Whatever the answer to your question, all config files should be
> coherent on this matter.

Yes, agreed. That is the main reason I'm asking about this.

> [1] https://lore.kernel.org/netdev/20240424104049.3935572-1-jiri@resnulli.us/
> 
> [...]
> > 
> >   5. openvswitch.sh starts with "#!/bin/sh".
> >      But substitutions such as "${ns:0:1}0"  fail if /bin/sh is dash.
> >      Perhaps we should change openvswitch.sh to use bash?
> 
> I think so. A similar change was done in
> c2518da8e6b0 selftests: bonding: Change script interpreter (v6.8-rc1)

Thanks, this one seems straightforward.

Re: selftests: openvswitch: Questions about possible enhancements

[Simon Horman]

On Wed, Apr 24, 2024 at 02:14:09PM -0400, Aaron Conole wrote:
> Simon Horman <horms@kernel.org> writes:
> 
> > Hi Aaron, Jakub, all,
> >
> > I have recently been exercising the Open vSwitch kernel selftests,
> > using vng, something like this:
> >
> > 	TESTDIR="tools/testing/selftests/net/openvswitch"
> >
> >         vng -v --run . --user root --cpus 2 \
> >                 --overlay-rwdir "$PWD" -- \
> >                 "modprobe openvswitch && \
> > 		 echo \"timeout=90\" >> \"${TESTDIR}/settings\" && \
> >                  make -C \"$TESTDIR\" run_tests"
> >
> > And I have some observations that I'd like to ask about.
> >
> > 1. Building the kernel using the following command does not
> >    build the openvswitch kernel module.
> >
> > 	vng -v --build \
> > 		--config tools/testing/selftests/net/config
> >
> >    All that seems to be missing is CONFIG_OPENVSWITCH=m
> >    and I am wondering what the best way of resolving this is.
> >
> >    Perhaps I am doing something wrong.
> >    Or perhaps tools/testing/selftests/net/openvswitch/config
> >    should be created? If so, should it include (most of?) what is in
> >    tools/testing/selftests/net/config, or just CONFIG_OPENVSWITCH=m?
> 
> I have a series that I need to get back to fixing:
> 
> https://mail.openvswitch.org/pipermail/ovs-dev/2024-February/411917.html
> 
> which does include the config listed, and some of the fixes for things
> you've noted.
> 
> I think it makes sense to get back to it.

Thanks Aaron,

I was hoping you might say something like that.

WRT to the config itself, as Benjamin mentioned elsewhere in this thread [1]
there is a question about how this should be handled consistently for
all selftests.

[1] https://lore.kernel.org/netdev/ZilIgbIvB04iUal2@f4/

> 
> > 2. As per my example above, it seems that a modprobe openvswitch is
> >    required (if openvswitch is a module).
> >
> >    Again, perhaps I am doing something wrong. But if not, should this be
> >    incorporated into tools/testing/selftests/net/openvswitch/openvswitch.sh
> >    or otherwise automated?
> >
> > 3. I have observed that the last test fails (yesterday, but not today!),
> >    because the namespace it tries to create already exists. I believe this
> >    is because it is pending deletion.
> >
> >    My work-around is as follows:
> >
> >  ovs_add_netns_and_veths () {
> >  	info "Adding netns attached: sbx:$1 dp:$2 {$3, $4, $5}"
> > +	for i in $(seq 10); do
> > +		ovs_sbx "$1" test -e "/var/run/netns/$3" || break
> > +		info "Namespace $3 still exists (attempt $i)"
> > +		ovs_sbx "$1" ip netns del "$3"
> > +		sleep "$i"
> > +	done
> >  	ovs_sbx "$1" ip netns add "$3" || return 1
> >  	on_exit "ovs_sbx $1 ip netns del $3"
> >  	ovs_sbx "$1" ip link add "$4" type veth peer name "$5" || return 1
> >
> >    N.B.: the "netns del" part is probably not needed,
> >    but I'm not able to exercise it effectively right now.
> >
> >    I am wondering if a loop like this is appropriate to add, perhaps also
> >    to namespace deletion. Or if it would be appropriate to port
> >    openvswitch.sh to use ./tools/testing/selftests/net/lib.sh, which I
> >    believe handles this.
> >
> > 4. I am observing timeouts whith the default value of 45s.
> >    Bumping this to 90s seems to help.
> >    Are there any objections to a patch to bump the timeout?

Regarding points 3 and 4.

I did a bit more testing after I sent my email yesterday.
I have two test machines. It turns out, to my surprise, that one is
much slower than the other when running openvswitch.sh with vng.

I am unsure why, but that isn't really on topic. The point
is that I'm currently only seeing problems 3 and 4 manifest
on the slow machine.

I think problem 3 is probably worth solving.
But the timeout question is more subjective.

Re: selftests: openvswitch: Questions about possible enhancements

[Simon Horman]

On Wed, Apr 24, 2024 at 05:30:00PM -0700, Jakub Kicinski wrote:
> On Wed, 24 Apr 2024 17:44:05 +0100 Simon Horman wrote:
> > I have recently been exercising the Open vSwitch kernel selftests,
> > using vng,
> 
> Speaking of ovs tests, we currently don't run them in CI (and suffer
> related skips in pmtu.sh) because Amazon Linux doesn't have ovs
> packaged and building it looks pretty hard.
> 
> Is there an easy way to build just the CLI tooling or get a pre-built
> package somewhere?
> 
> Or perhaps you'd be willing to run the OvS tests and we can move 
> the part of pmtu.sh into OvS test dir?

Thanks Jakub,

The plot thickens.
We'll look into this (Hi Aaron!).

Re: [ovs-dev] selftests: openvswitch: Questions about possible enhancements

[Simon Horman]

+ Aaron

On Thu, Apr 25, 2024 at 09:26:37AM +0100, Simon Horman wrote:
> On Wed, Apr 24, 2024 at 05:30:00PM -0700, Jakub Kicinski wrote:
> > On Wed, 24 Apr 2024 17:44:05 +0100 Simon Horman wrote:
> > > I have recently been exercising the Open vSwitch kernel selftests,
> > > using vng,
> > 
> > Speaking of ovs tests, we currently don't run them in CI (and suffer
> > related skips in pmtu.sh) because Amazon Linux doesn't have ovs
> > packaged and building it looks pretty hard.
> > 
> > Is there an easy way to build just the CLI tooling or get a pre-built
> > package somewhere?
> > 
> > Or perhaps you'd be willing to run the OvS tests and we can move 
> > the part of pmtu.sh into OvS test dir?
> 
> Thanks Jakub,
> 
> The plot thickens.
> We'll look into this (Hi Aaron!).

Hi again,

I took a look into this.

openvswitch.sh does not appear to have any dependencies on Open vSwitch
user-space. My understanding is that, rather, it makes use of
tools/testing/selftests/net/openvswitch/ovs-dpctl.py to talk to the Kernel
using Netlink (which is also what Open vSwitch user-space does).

My brief testing indicates that for this the only dependencies
when running on Amazon Linux 2 are python3 and pyroute2.

I think that it should be possible to port pmtu.sh to use ovs-dpctl.py.
This would require some enhancements to ovs-dpctl.py to handle adding the
tunnel vports (interfaces).

As an aside, to run the Open vSwitch tests in pmtu.sh the openvswitch
kernel module is needed. So I think it would make sense to add
CONFIG_OPENVSWITCH to tools/testing/selftests/net/config.

That would mean that tools/testing/selftests/net/config also has all
the requirements to run openvswitch.sh. If so, we probably wouldn't need to
add tools/testing/selftests/net/openvswitch/config or otherwise do anything
special to configure the kernel for openvswitch.sh.

Re: [ovs-dev] selftests: openvswitch: Questions about possible enhancements

[Jakub Kicinski]

On Thu, 25 Apr 2024 19:57:19 +0100 Simon Horman wrote:
> openvswitch.sh does not appear to have any dependencies on Open vSwitch
> user-space. My understanding is that, rather, it makes use of
> tools/testing/selftests/net/openvswitch/ovs-dpctl.py to talk to the Kernel
> using Netlink (which is also what Open vSwitch user-space does).
> 
> My brief testing indicates that for this the only dependencies
> when running on Amazon Linux 2 are python3 and pyroute2.
> 
> I think that it should be possible to port pmtu.sh to use ovs-dpctl.py.
> This would require some enhancements to ovs-dpctl.py to handle adding the
> tunnel vports (interfaces).
> 
> As an aside, to run the Open vSwitch tests in pmtu.sh the openvswitch
> kernel module is needed. So I think it would make sense to add
> CONFIG_OPENVSWITCH to tools/testing/selftests/net/config.
> 
> That would mean that tools/testing/selftests/net/config also has all
> the requirements to run openvswitch.sh. If so, we probably wouldn't need to
> add tools/testing/selftests/net/openvswitch/config or otherwise do anything
> special to configure the kernel for openvswitch.sh.

That sounds great, for simplicity we could move the ovs files down 
to the .../net/ directory. It'd be cool to not have to do that, and
let us separate tests more clearly into directories. But right now
every directory has its own runner so there's a high price to pay
for a primarily aesthetic gain :(

Re: selftests: openvswitch: Questions about possible enhancements

[Aaron Conole]

Jakub Kicinski <kuba@kernel.org> writes:

> On Wed, 24 Apr 2024 17:44:05 +0100 Simon Horman wrote:
>> I have recently been exercising the Open vSwitch kernel selftests,
>> using vng,
>
> Speaking of ovs tests, we currently don't run them in CI (and suffer
> related skips in pmtu.sh) because Amazon Linux doesn't have ovs
> packaged and building it looks pretty hard.
>
> Is there an easy way to build just the CLI tooling or get a pre-built
> package somewhere?

As Simon notes - we would need some support in the ovs-dpctl.py to set
up the tunnel interfaces, and probably need to set them up for lwt and
classic tunnels.

I have a test branch where I have support for the former, and I can
clean it up and submit it.

> Or perhaps you'd be willing to run the OvS tests and we can move 
> the part of pmtu.sh into OvS test dir?

I guess either would be fine, as long as they can get run.

Re: selftests: openvswitch: Questions about possible enhancements

[Aaron Conole]

Simon Horman <horms@kernel.org> writes:

> On Wed, Apr 24, 2024 at 02:14:09PM -0400, Aaron Conole wrote:
>> Simon Horman <horms@kernel.org> writes:
>> 
>> > Hi Aaron, Jakub, all,
>> >
>> > I have recently been exercising the Open vSwitch kernel selftests,
>> > using vng, something like this:
>> >
>> > 	TESTDIR="tools/testing/selftests/net/openvswitch"
>> >
>> >         vng -v --run . --user root --cpus 2 \
>> >                 --overlay-rwdir "$PWD" -- \
>> >                 "modprobe openvswitch && \
>> > 		 echo \"timeout=90\" >> \"${TESTDIR}/settings\" && \
>> >                  make -C \"$TESTDIR\" run_tests"
>> >
>> > And I have some observations that I'd like to ask about.
>> >
>> > 1. Building the kernel using the following command does not
>> >    build the openvswitch kernel module.
>> >
>> > 	vng -v --build \
>> > 		--config tools/testing/selftests/net/config
>> >
>> >    All that seems to be missing is CONFIG_OPENVSWITCH=m
>> >    and I am wondering what the best way of resolving this is.
>> >
>> >    Perhaps I am doing something wrong.
>> >    Or perhaps tools/testing/selftests/net/openvswitch/config
>> >    should be created? If so, should it include (most of?) what is in
>> >    tools/testing/selftests/net/config, or just CONFIG_OPENVSWITCH=m?
>> 
>> I have a series that I need to get back to fixing:
>> 
>> https://mail.openvswitch.org/pipermail/ovs-dev/2024-February/411917.html
>> 
>> which does include the config listed, and some of the fixes for things
>> you've noted.
>> 
>> I think it makes sense to get back to it.
>
> Thanks Aaron,
>
> I was hoping you might say something like that.
>
> WRT to the config itself, as Benjamin mentioned elsewhere in this thread [1]
> there is a question about how this should be handled consistently for
> all selftests.
>
> [1] https://lore.kernel.org/netdev/ZilIgbIvB04iUal2@f4/

Yeah, I think it makes sense.  There are probably some other bashisms
beyond the substitution noted.  I'll add it to the RFC and rework.

>> 
>> > 2. As per my example above, it seems that a modprobe openvswitch is
>> >    required (if openvswitch is a module).
>> >
>> >    Again, perhaps I am doing something wrong. But if not, should this be
>> >    incorporated into tools/testing/selftests/net/openvswitch/openvswitch.sh
>> >    or otherwise automated?
>> >
>> > 3. I have observed that the last test fails (yesterday, but not today!),
>> >    because the namespace it tries to create already exists. I believe this
>> >    is because it is pending deletion.
>> >
>> >    My work-around is as follows:
>> >
>> >  ovs_add_netns_and_veths () {
>> >  	info "Adding netns attached: sbx:$1 dp:$2 {$3, $4, $5}"
>> > +	for i in $(seq 10); do
>> > +		ovs_sbx "$1" test -e "/var/run/netns/$3" || break
>> > +		info "Namespace $3 still exists (attempt $i)"
>> > +		ovs_sbx "$1" ip netns del "$3"
>> > +		sleep "$i"
>> > +	done
>> >  	ovs_sbx "$1" ip netns add "$3" || return 1
>> >  	on_exit "ovs_sbx $1 ip netns del $3"
>> >  	ovs_sbx "$1" ip link add "$4" type veth peer name "$5" || return 1
>> >
>> >    N.B.: the "netns del" part is probably not needed,
>> >    but I'm not able to exercise it effectively right now.
>> >
>> >    I am wondering if a loop like this is appropriate to add, perhaps also
>> >    to namespace deletion. Or if it would be appropriate to port
>> >    openvswitch.sh to use ./tools/testing/selftests/net/lib.sh, which I
>> >    believe handles this.
>> >
>> > 4. I am observing timeouts whith the default value of 45s.
>> >    Bumping this to 90s seems to help.
>> >    Are there any objections to a patch to bump the timeout?
>
> Regarding points 3 and 4.
>
> I did a bit more testing after I sent my email yesterday.
> I have two test machines. It turns out, to my surprise, that one is
> much slower than the other when running openvswitch.sh with vng.
>
> I am unsure why, but that isn't really on topic. The point
> is that I'm currently only seeing problems 3 and 4 manifest
> on the slow machine.
>
> I think problem 3 is probably worth solving.
> But the timeout question is more subjective.

Re: [ovs-dev] selftests: openvswitch: Questions about possible enhancements

[Aaron Conole]

Jakub Kicinski <kuba@kernel.org> writes:

> On Thu, 25 Apr 2024 19:57:19 +0100 Simon Horman wrote:
>> openvswitch.sh does not appear to have any dependencies on Open vSwitch
>> user-space. My understanding is that, rather, it makes use of
>> tools/testing/selftests/net/openvswitch/ovs-dpctl.py to talk to the Kernel
>> using Netlink (which is also what Open vSwitch user-space does).
>> 
>> My brief testing indicates that for this the only dependencies
>> when running on Amazon Linux 2 are python3 and pyroute2.
>> 
>> I think that it should be possible to port pmtu.sh to use ovs-dpctl.py.
>> This would require some enhancements to ovs-dpctl.py to handle adding the
>> tunnel vports (interfaces).

I have some work from some time back:

https://github.com/apconole/linux-next-work/commit/61d2b9fc68a4e3fc950a24b06232c2fbcbfa0372

https://github.com/apconole/linux-next-work/commit/7262107de7170d44b6dbf6c5ea6f7e6c0bb71d36

https://github.com/apconole/linux-next-work/commit/af5338c9044660fa0eaaa967602aec706bbaeb5b

I was using it to try and build up a test to check recursions in the
datapath, but didn't get a chance to finish.

BUT most of the stuff is there, just needs to be cleaned up.  It would
at least cover the classic case, but the LWT support needs to be added.

>> As an aside, to run the Open vSwitch tests in pmtu.sh the openvswitch
>> kernel module is needed. So I think it would make sense to add
>> CONFIG_OPENVSWITCH to tools/testing/selftests/net/config.
>> 
>> That would mean that tools/testing/selftests/net/config also has all
>> the requirements to run openvswitch.sh. If so, we probably wouldn't need to
>> add tools/testing/selftests/net/openvswitch/config or otherwise do anything
>> special to configure the kernel for openvswitch.sh.
>
> That sounds great, for simplicity we could move the ovs files down 
> to the .../net/ directory. It'd be cool to not have to do that, and
> let us separate tests more clearly into directories. But right now
> every directory has its own runner so there's a high price to pay
> for a primarily aesthetic gain :(

Either one would work for me.  I will repost the RFC addressing some of
the comments.  It should be runnable in a bare VM that has the required
python packages that Simon notes (pyroute2 and python3).

Re: [ovs-dev] selftests: openvswitch: Questions about possible enhancements

[Simon Horman]

On Thu, Apr 25, 2024 at 12:21:51PM -0700, Jakub Kicinski wrote:
> On Thu, 25 Apr 2024 19:57:19 +0100 Simon Horman wrote:
> > openvswitch.sh does not appear to have any dependencies on Open vSwitch
> > user-space. My understanding is that, rather, it makes use of
> > tools/testing/selftests/net/openvswitch/ovs-dpctl.py to talk to the Kernel
> > using Netlink (which is also what Open vSwitch user-space does).
> > 
> > My brief testing indicates that for this the only dependencies
> > when running on Amazon Linux 2 are python3 and pyroute2.
> > 
> > I think that it should be possible to port pmtu.sh to use ovs-dpctl.py.
> > This would require some enhancements to ovs-dpctl.py to handle adding the
> > tunnel vports (interfaces).
> > 
> > As an aside, to run the Open vSwitch tests in pmtu.sh the openvswitch
> > kernel module is needed. So I think it would make sense to add
> > CONFIG_OPENVSWITCH to tools/testing/selftests/net/config.
> > 
> > That would mean that tools/testing/selftests/net/config also has all
> > the requirements to run openvswitch.sh. If so, we probably wouldn't need to
> > add tools/testing/selftests/net/openvswitch/config or otherwise do anything
> > special to configure the kernel for openvswitch.sh.
> 
> That sounds great, for simplicity we could move the ovs files down 
> to the .../net/ directory. It'd be cool to not have to do that, and
> let us separate tests more clearly into directories. But right now
> every directory has its own runner so there's a high price to pay
> for a primarily aesthetic gain :(

Understood. Let's work on getting the Open vSwitch portions of pmtu.sh, and
openvswitch.sh into a little bit better shape. Then we can consider moving
the contents of tools/.../net/openvswitch/. It would certainly be nice to
have the Open vSwitch tests run automatically.