Re: selftests: openvswitch: Questions about possible enhancements

[Aaron Conole <aconole@redhat.com>]

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.