2023-10-11 19:50:55

by Aaron Conole

[permalink] [raw]
Subject: [PATCH net v2 0/4] selftests: openvswitch: Minor fixes for some systems

A number of corner cases were caught when trying to run the selftests on
older systems. Missed skip conditions, some error cases, and outdated
python setups would all report failures but the issue would actually be
related to some other condition rather than the selftest suite.

Address these individual cases.

Aaron Conole (4):
selftests: openvswitch: Add version check for pyroute2
selftests: openvswitch: Catch cases where the tests are killed
selftests: openvswitch: Skip drop testing on older kernels
selftests: openvswitch: Fix the ct_tuple for v4

.../selftests/net/openvswitch/openvswitch.sh | 21 +++++++-
.../selftests/net/openvswitch/ovs-dpctl.py | 48 ++++++++++++++++++-
2 files changed, 66 insertions(+), 3 deletions(-)

--
2.41.0


2023-10-11 19:51:40

by Aaron Conole

[permalink] [raw]
Subject: [PATCH net v2 1/4] selftests: openvswitch: Add version check for pyroute2

Paolo Abeni reports that on some systems the pyroute2 version isn't
new enough to run the test suite. Ensure that we support a minimum
version of 0.6 for all cases (which does include the existing ones).
The 0.6.1 version was released in May of 2021, so should be
propagated to most installations at this point.

The alternative that Paolo proposed was to only skip when the
add-flow is being run. This would be okay for most cases, except
if a future test case is added that needs to do flow dump without
an associated add (just guessing). In that case, it could also be
broken and we would need additional skip logic anyway. Just draw
a line in the sand now.

Fixes: 25f16c873fb1 ("selftests: add openvswitch selftest suite")
Reported-by: Paolo Abeni <[email protected]>
Closes: https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Aaron Conole <[email protected]>
---
v2: Include version information

tools/testing/selftests/net/openvswitch/openvswitch.sh | 2 +-
tools/testing/selftests/net/openvswitch/ovs-dpctl.py | 10 +++++++++-
2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index 9c2012d70b08e..220c3356901ef 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -525,7 +525,7 @@ run_test() {
fi

if python3 ovs-dpctl.py -h 2>&1 | \
- grep "Need to install the python" >/dev/null 2>&1; then
+ grep -E "Need to (install|upgrade) the python" >/dev/null 2>&1; then
stdbuf -o0 printf "TEST: %-60s [PYLIB]\n" "${tdesc}"
return $ksft_skip
fi
diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
index 912dc8c490858..e4c24d5edf20c 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -28,8 +28,10 @@ try:
from pyroute2.netlink import nlmsg_atoms
from pyroute2.netlink.exceptions import NetlinkError
from pyroute2.netlink.generic import GenericNetlinkSocket
+ import pyroute2
+
except ModuleNotFoundError:
- print("Need to install the python pyroute2 package.")
+ print("Need to install the python pyroute2 package >= 0.6.")
sys.exit(0)


@@ -1998,6 +2000,12 @@ def main(argv):
nlmsg_atoms.ovskey = ovskey
nlmsg_atoms.ovsactions = ovsactions

+ # version check for pyroute2
+ prverscheck = pyroute2.__version__.split(".")
+ if int(prverscheck[0]) == 0 and int(prverscheck[1]) < 6:
+ print("Need to upgrade the python pyroute2 package to >= 0.6.")
+ sys.exit(0)
+
parser = argparse.ArgumentParser()
parser.add_argument(
"-v",
--
2.41.0

2023-10-11 19:51:47

by Aaron Conole

[permalink] [raw]
Subject: [PATCH net v2 3/4] selftests: openvswitch: Skip drop testing on older kernels

Kernels that don't have support for openvswitch drop reasons also
won't have the drop counter reasons, so we should skip the test
completely. It previously wasn't possible to build a test case
for this without polluting the datapath, so we introduce a mechanism
to clear all the flows from a datapath allowing us to test for
explicit drop actions, and then clear the flows to build the
original test case.

Fixes: 4242029164d6 ("selftests: openvswitch: add explicit drop testcase")
Signed-off-by: Aaron Conole <[email protected]>
---
v2: Address intermingled tabs vs. spaces

.../selftests/net/openvswitch/openvswitch.sh | 17 ++++++++++
.../selftests/net/openvswitch/ovs-dpctl.py | 34 +++++++++++++++++++
2 files changed, 51 insertions(+)

diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index 2a0112be7ead5..f8499d4c87f3f 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -144,6 +144,12 @@ ovs_add_flow () {
return 0
}

+ovs_del_flows () {
+ info "Deleting all flows from DP: sbx:$1 br:$2"
+ ovs_sbx "$1" python3 $ovs_base/ovs-dpctl.py del-flows "$2"
+ return 0
+}
+
ovs_drop_record_and_run () {
local sbx=$1
shift
@@ -200,6 +206,17 @@ test_drop_reason() {
ip netns exec server ip addr add 172.31.110.20/24 dev s1
ip netns exec server ip link set s1 up

+ # Check if drop reasons can be sent
+ ovs_add_flow "test_drop_reason" dropreason \
+ 'in_port(1),eth(),eth_type(0x0806),arp()' 'drop(10)' 2>/dev/null
+ if [ $? == 1 ]; then
+ info "no support for drop reasons - skipping"
+ ovs_exit_sig
+ return $ksft_skip
+ fi
+
+ ovs_del_flows "test_drop_reason" dropreason
+
# Allow ARP
ovs_add_flow "test_drop_reason" dropreason \
'in_port(1),eth(),eth_type(0x0806),arp()' '2' || return 1
diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
index e4c24d5edf20c..10b8f31548f84 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -1906,6 +1906,32 @@ class OvsFlow(GenericNetlinkSocket):
raise ne
return reply

+ def del_flows(self, dpifindex):
+ """
+ Send a del message to the kernel that will drop all flows.
+
+ dpifindex should be a valid datapath obtained by calling
+ into the OvsDatapath lookup
+ """
+
+ flowmsg = OvsFlow.ovs_flow_msg()
+ flowmsg["cmd"] = OVS_FLOW_CMD_DEL
+ flowmsg["version"] = OVS_DATAPATH_VERSION
+ flowmsg["reserved"] = 0
+ flowmsg["dpifindex"] = dpifindex
+
+ try:
+ reply = self.nlm_request(
+ flowmsg,
+ msg_type=self.prid,
+ msg_flags=NLM_F_REQUEST | NLM_F_ACK,
+ )
+ reply = reply[0]
+ except NetlinkError as ne:
+ print(flowmsg)
+ raise ne
+ return reply
+
def dump(self, dpifindex, flowspec=None):
"""
Returns a list of messages containing flows.
@@ -2068,6 +2094,9 @@ def main(argv):
addflcmd.add_argument("flow", help="Flow specification")
addflcmd.add_argument("acts", help="Flow actions")

+ delfscmd = subparsers.add_parser("del-flows")
+ delfscmd.add_argument("flsbr", help="Datapath name")
+
args = parser.parse_args()

if args.verbose > 0:
@@ -2151,6 +2180,11 @@ def main(argv):
flow = OvsFlow.ovs_flow_msg()
flow.parse(args.flow, args.acts, rep["dpifindex"])
ovsflow.add_flow(rep["dpifindex"], flow)
+ elif hasattr(args, "flsbr"):
+ rep = ovsdp.info(args.flsbr, 0)
+ if rep is None:
+ print("DP '%s' not found." % args.flsbr)
+ ovsflow.del_flows(rep["dpifindex"])

return 0

--
2.41.0

2023-10-11 19:51:50

by Aaron Conole

[permalink] [raw]
Subject: [PATCH net v2 4/4] selftests: openvswitch: Fix the ct_tuple for v4

The ct_tuple v4 data structure decode / encode routines were using
the v6 IP address decode and relying on default encode. This could
cause exceptions during encode / decode depending on how a ct4
tuple would appear in a netlink message.

Caught during code review.

Fixes: e52b07aa1a54 ("selftests: openvswitch: add flow dump support")
Signed-off-by: Aaron Conole <[email protected]>
---
v2: More detailed explanation for fix.

tools/testing/selftests/net/openvswitch/ovs-dpctl.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
index 10b8f31548f84..b97e621face95 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -1119,12 +1119,14 @@ class ovskey(nla):
"src",
lambda x: str(ipaddress.IPv4Address(x)),
int,
+ convert_ipv4,
),
(
"dst",
"dst",
- lambda x: str(ipaddress.IPv6Address(x)),
+ lambda x: str(ipaddress.IPv4Address(x)),
int,
+ convert_ipv4,
),
("tp_src", "tp_src", "%d", int),
("tp_dst", "tp_dst", "%d", int),
--
2.41.0

2023-10-13 20:07:19

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH net v2 0/4] selftests: openvswitch: Minor fixes for some systems

On Wed, Oct 11, 2023 at 03:49:35PM -0400, Aaron Conole wrote:
> A number of corner cases were caught when trying to run the selftests on
> older systems. Missed skip conditions, some error cases, and outdated
> python setups would all report failures but the issue would actually be
> related to some other condition rather than the selftest suite.
>
> Address these individual cases.
>
> Aaron Conole (4):
> selftests: openvswitch: Add version check for pyroute2
> selftests: openvswitch: Catch cases where the tests are killed
> selftests: openvswitch: Skip drop testing on older kernels
> selftests: openvswitch: Fix the ct_tuple for v4
>
> .../selftests/net/openvswitch/openvswitch.sh | 21 +++++++-
> .../selftests/net/openvswitch/ovs-dpctl.py | 48 ++++++++++++++++++-
> 2 files changed, 66 insertions(+), 3 deletions(-)

Thanks Aaron,

this looks like a good incremental improvement to me.

For series,

Reviewed-by: Simon Horman <[email protected]>

2023-10-15 19:10:39

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net v2 0/4] selftests: openvswitch: Minor fixes for some systems

Hello:

This series was applied to netdev/net.git (main)
by David S. Miller <[email protected]>:

On Wed, 11 Oct 2023 15:49:35 -0400 you wrote:
> A number of corner cases were caught when trying to run the selftests on
> older systems. Missed skip conditions, some error cases, and outdated
> python setups would all report failures but the issue would actually be
> related to some other condition rather than the selftest suite.
>
> Address these individual cases.
>
> [...]

Here is the summary with links:
- [net,v2,1/4] selftests: openvswitch: Add version check for pyroute2
https://git.kernel.org/netdev/net/c/92e37f20f20a
- [net,v2,2/4] selftests: openvswitch: Catch cases where the tests are killed
https://git.kernel.org/netdev/net/c/af846afad5ca
- [net,v2,3/4] selftests: openvswitch: Skip drop testing on older kernels
https://git.kernel.org/netdev/net/c/76035fd12cb9
- [net,v2,4/4] selftests: openvswitch: Fix the ct_tuple for v4
https://git.kernel.org/netdev/net/c/8eff0e062201

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html