| Message ID | 20240625172245.233874-1-aconole@redhat.com |
|---|---|
| Headers | show |
| Series | selftests: net: Switch pmtu.sh to use the internal ovs script. | expand |
Simon Horman <horms@kernel.org> writes: > On Tue, Jun 25, 2024 at 01:22:44PM -0400, Aaron Conole wrote: >> The current pmtu test infrastucture requires an installed copy of the >> ovs-vswitchd userspace. This means that any automated or constrained >> environments may not have the requisite tools to run the tests. However, >> the pmtu tests don't require any special classifier processing. Indeed >> they are only using the vswitchd in the most basic mode - as a NORMAL >> switch. >> >> However, the ovs-dpctl kernel utility can now program all the needed basic >> flows to allow traffic to traverse the tunnels and provide support for at >> least testing some basic pmtu scenarios. More complicated flow pipelines >> can be added to the internal ovs test infrastructure, but that is work for >> the future. For now, enable the most common cases - wide mega flows with >> no other prerequisites. >> >> Enhance the pmtu testing to try testing using the internal utility, first. >> As a fallback, if the internal utility isn't running, then try with the >> ovs-vswitchd userspace tools. >> >> Additionally, make sure that when the pyroute2 package is not available >> the ovs-dpctl utility will error out to properly signal an error has >> occurred and skip using the internal utility. > > Hi Aaron, > > I don't feel strongly about this, but it does feel like the > change to ovs-dpctl.py could live in a separate patch. I can do it if others feel like it should be a separate change. I debated it as a separate patch, but I felt that it wasn't really a bug fix, more like a behavior change that would be associated with this pmtu script. I didn't (at the time, and still don't) see a reason to separately backport them, but it could also be considered as a separate orthogonal change, and I'm okay with it being a different patch. Like you, I don't feel strongly either way. If I do separate it, I will also add your Reviewed and Tested tags to that patch. >> Reviewed-by: Stefano Brivio <sbrivio@redhat.com> >> Signed-off-by: Aaron Conole <aconole@redhat.com> > > The above not withstanding, > > > Reviewed-by: Simon Horman <horms@kernel.org> > > I have tested pmtu.sh with this change on Fedora 40 both > with python3-pyroute2 installed, which uses ovs-dpctl, > and without, which uses ovs-vswitchd userspace tools. > > Tested-by: Simon Horman <horms@kernel.org> Thanks! > ...
Hello: This series was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 25 Jun 2024 13:22:38 -0400 you wrote: > Currently, if a user wants to run pmtu.sh and cover all the provided test > cases, they need to install the Open vSwitch userspace utilities. This > dependency is difficult for users as well as CI environments, because the > userspace build and setup may require lots of support and devel packages > to be installed, system setup to be correct, and things like permissions > and selinux policies to be properly configured. > > [...] Here is the summary with links: - [net-next,v3,1/7] selftests: openvswitch: Support explicit tunnel port creation. https://git.kernel.org/netdev/net-next/c/f94ecbc92092 - [net-next,v3,2/7] selftests: openvswitch: Refactor actions parsing. https://git.kernel.org/netdev/net-next/c/37de65a764ed - [net-next,v3,3/7] selftests: openvswitch: Add set() and set_masked() support. https://git.kernel.org/netdev/net-next/c/a4126f90a35f - [net-next,v3,4/7] selftests: openvswitch: Add support for tunnel() key. https://git.kernel.org/netdev/net-next/c/fefe3b7d6bec - [net-next,v3,5/7] selftests: openvswitch: Support implicit ipv6 arguments. https://git.kernel.org/netdev/net-next/c/51458e1084d0 - [net-next,v3,6/7] selftests: net: Use the provided dpctl rather than the vswitchd for tests. https://git.kernel.org/netdev/net-next/c/b7ce46fc614d - [net-next,v3,7/7] selftests: net: add config for openvswitch (no matching commit) You are awesome, thank you!
On Tue, 25 Jun 2024 13:22:38 -0400 Aaron Conole wrote: > Currently, if a user wants to run pmtu.sh and cover all the provided test > cases, they need to install the Open vSwitch userspace utilities. This > dependency is difficult for users as well as CI environments, because the > userspace build and setup may require lots of support and devel packages > to be installed, system setup to be correct, and things like permissions > and selinux policies to be properly configured. Hi Aaron! I merged this yesterday (with slight alphabetical reshuffling of the config options). The pmtu.sh test is solid now, which is great! I also added the OvS tests themselves, and those are not passing, yet: https://netdev.bots.linux.dev/contest.html?test=openvswitch-sh Could you take a look and LMK if these are likely env issues or something bad in the test itself?
Jakub Kicinski <kuba@kernel.org> writes: > On Tue, 25 Jun 2024 13:22:38 -0400 Aaron Conole wrote: >> Currently, if a user wants to run pmtu.sh and cover all the provided test >> cases, they need to install the Open vSwitch userspace utilities. This >> dependency is difficult for users as well as CI environments, because the >> userspace build and setup may require lots of support and devel packages >> to be installed, system setup to be correct, and things like permissions >> and selinux policies to be properly configured. > > Hi Aaron! > > I merged this yesterday (with slight alphabetical reshuffling of > the config options). The pmtu.sh test is solid now, which is great! :) Thanks! That's great to see. > I also added the OvS tests themselves, and those are not passing, yet: > https://netdev.bots.linux.dev/contest.html?test=openvswitch-sh > Could you take a look and LMK if these are likely env issues or > something bad in the test itself? I saw that. I was looking for a place in the nipa repository where I could submit a small fix, because I noticed in the stdout: make -C tools/testing/selftests TARGETS="net/openvswitch" TEST_PROGS=openvvswitch.sh TEST_GEN_PROGS="" run_tests and I think the TEST_PROGS=openvvswitch.sh is misspelled (but it seems to not matter too much for the run_test target).
On Fri, 28 Jun 2024 14:04:09 -0400 Aaron Conole wrote: > > I also added the OvS tests themselves, and those are not passing, yet: > > https://netdev.bots.linux.dev/contest.html?test=openvswitch-sh > > Could you take a look and LMK if these are likely env issues or > > something bad in the test itself? > > I saw that. I was looking for a place in the nipa repository where I > could submit a small fix, because I noticed in the stdout: > > make -C tools/testing/selftests TARGETS="net/openvswitch" > TEST_PROGS=openvvswitch.sh TEST_GEN_PROGS="" run_tests > > and I think the TEST_PROGS=openvvswitch.sh is misspelled (but it seems > to not matter too much for the run_test target). :o that's a weird bug, whatever is echoing back the input from the VMs stdin to stdout is duplicating 74th character?! but as you say looks like the VM gets the right input, it's just the echo. > From what I understand, there are two things causing it to be flaky. > First, the module detection is a bit flaky (and that's why it results is > some 'skip' reports). Additionally, the connection oriented tests > include negative cases and those hit timeouts. The default is to > declare failure after 45s. That can be seen in: > > https://netdev-3.bots.linux.dev/vmksft-net/results/659601/91-openvswitch-sh/stdout > ... > # timeout set to 45 > ... > # TEST: nat_connect_v4 [START] > # Terminated > # Terminated > > This is showing that the timeout is too short. > > I have patches ready for these issues, but I didn't know if you would > like me to submit config config meaning make OvS built in? We have a number of tests using module auto-loading, I don't think there were major issues with it. (well, the rebuilding of modules is a bit questionable with vng, but we do 'make mrproper' between each build to work around that). > and settings files to go under net/openvswitch, > or if you would prefer to see the openvswitch.sh script, and > ovs-dpctl.py utilities move out of their net/openvswitch/ directory. If > the latter, I can submit patches quickly with config and settings (and a > small change to the script itself) that addresses these. If you'd > prefer the former (moving around the files), I'll need to spend some > additional time modifying pmtu and doing a larger test. I don't have a > strong opinion on either approach. Since we talked about it a while back I gave in and implemented support for combining multiple targets in a single runner. So it doesn't matter any more (barring bugs in NIPA), up to you.
Currently, if a user wants to run pmtu.sh and cover all the provided test cases, they need to install the Open vSwitch userspace utilities. This dependency is difficult for users as well as CI environments, because the userspace build and setup may require lots of support and devel packages to be installed, system setup to be correct, and things like permissions and selinux policies to be properly configured. The kernel selftest suite includes an ovs-dpctl.py utility which can interact with the openvswitch module directly. This lets developers and CI environments run without needing too many extra dependencies - just the pyroute2 python package. This series enhances the ovs-dpctl utility to provide support for set() and tunnel() flow specifiers, better ipv6 handling support, and the ability to add tunnel vports, and LWT interfaces. Finally, it modifies the pmtu.sh script to call the ovs-dpctl.py utility rather than the typical OVS userspace utilities. The pmtu.sh can still fall back on the Open vSwitch userspace utilities if the ovs-dpctl.py script can't be used. Aaron Conole (7): selftests: openvswitch: Support explicit tunnel port creation. selftests: openvswitch: Refactor actions parsing. selftests: openvswitch: Add set() and set_masked() support. selftests: openvswitch: Add support for tunnel() key. selftests: openvswitch: Support implicit ipv6 arguments. selftests: net: Use the provided dpctl rather than the vswitchd for tests. selftests: net: add config for openvswitch tools/testing/selftests/net/config | 5 + .../selftests/net/openvswitch/ovs-dpctl.py | 368 +++++++++++++++--- tools/testing/selftests/net/pmtu.sh | 145 +++++-- 3 files changed, 451 insertions(+), 67 deletions(-)