2023-08-01 22:08:24

by Aaron Conole

[permalink] [raw]
Subject: [PATCH v3 net-next 0/5] selftests: openvswitch: add flow programming cases

The openvswitch selftests currently contain a few cases for managing the
datapath, which includes creating datapath instances, adding interfaces,
and doing some basic feature / upcall tests. This is useful to validate
the control path.

Add the ability to program some of the more common flows with actions. This
can be improved overtime to include regression testing, etc.

v2->v3:
1. Dropped support for ipv6 in nat() case
2. Fixed a spelling mistake in 2/5 commit message.

v1->v2:
1. Fix issue when parsing ipv6 in the NAT action
2. Fix issue calculating length during ctact parsing
3. Fix error message when invalid bridge is passed
4. Fold in Adrian's patch to support key masks


Aaron Conole (4):
selftests: openvswitch: add an initial flow programming case
selftests: openvswitch: add a test for ipv4 forwarding
selftests: openvswitch: add basic ct test case parsing
selftests: openvswitch: add ct-nat test case with ipv4

Adrian Moreno (1):
selftests: openvswitch: support key masks

.../selftests/net/openvswitch/openvswitch.sh | 223 +++++++
.../selftests/net/openvswitch/ovs-dpctl.py | 588 +++++++++++++++++-
2 files changed, 787 insertions(+), 24 deletions(-)

--
2.40.1



2023-08-01 22:22:56

by Aaron Conole

[permalink] [raw]
Subject: [PATCH v3 net-next 3/5] selftests: openvswitch: add a test for ipv4 forwarding

This is a simple ipv4 bidirectional connectivity test.

Signed-off-by: Aaron Conole <[email protected]>
Reviewed-by: Adrian Moreno <[email protected]>
---
NOTE: 4 lines flag the line-length checkpatch warning, but there didn't
seem to be a good way of breaking the lines smaller.

.../selftests/net/openvswitch/openvswitch.sh | 40 +++++++++++++++++++
1 file changed, 40 insertions(+)

diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index 5cdacb3c8c925..5d60a9466dab3 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -12,6 +12,7 @@ TRACING=0

tests="
arp_ping eth-arp: Basic arp ping between two NS
+ connect_v4 ip4-xon: Basic ipv4 ping between two NS
netlink_checks ovsnl: validate netlink attrs and settings
upcall_interfaces ovs: test the upcall interfaces"

@@ -192,6 +193,45 @@ test_arp_ping () {
return 0
}

+# connect_v4 test
+# - client has 1500 byte MTU
+# - server has 1500 byte MTU
+# - use ICMP to ping in each direction
+test_connect_v4 () {
+
+ sbx_add "test_connect_v4" || return $?
+
+ ovs_add_dp "test_connect_v4" cv4 || return 1
+
+ info "create namespaces"
+ for ns in client server; do
+ ovs_add_netns_and_veths "test_connect_v4" "cv4" "$ns" \
+ "${ns:0:1}0" "${ns:0:1}1" || return 1
+ done
+
+
+ ip netns exec client ip addr add 172.31.110.10/24 dev c1
+ ip netns exec client ip link set c1 up
+ ip netns exec server ip addr add 172.31.110.20/24 dev s1
+ ip netns exec server ip link set s1 up
+
+ # Add forwarding for ARP and ip packets - completely wildcarded
+ ovs_add_flow "test_connect_v4" cv4 \
+ 'in_port(1),eth(),eth_type(0x0806),arp()' '2' || return 1
+ ovs_add_flow "test_connect_v4" cv4 \
+ 'in_port(2),eth(),eth_type(0x0806),arp()' '1' || return 1
+ ovs_add_flow "test_connect_v4" cv4 \
+ 'in_port(1),eth(),eth_type(0x0800),ipv4(src=172.31.110.10)' '2' || return 1
+ ovs_add_flow "test_connect_v4" cv4 \
+ 'in_port(2),eth(),eth_type(0x0800),ipv4(src=172.31.110.20)' '1' || return 1
+
+ # do a ping
+ ovs_sbx "test_connect_v4" ip netns exec client ping 172.31.110.20 -c 3 || return 1
+
+ info "done..."
+ return 0
+}
+
# netlink_validation
# - Create a dp
# - check no warning with "old version" simulation
--
2.40.1


2023-08-01 22:34:25

by Aaron Conole

[permalink] [raw]
Subject: [PATCH v3 net-next 1/5] selftests: openvswitch: add an initial flow programming case

The openvswitch self-tests can test much of the control side of
the module (ie: what a vswitchd implementation would process),
but the actual packet forwarding cases aren't supported, making
the testing of limited value.

Add some flow parsing and an initial ARP based test case using
arping utility. This lets us display flows, add some basic
output flows with simple matches, and test against a known good
forwarding case.

Signed-off-by: Aaron Conole <[email protected]>
Reviewed-by: Adrian Moreno <[email protected]>
---
NOTE: 3 lines flag the line-length checkpatch warning, but there didn't
seem to be a good way of breaking the lines smaller for 2 of them.
The third would still flag, even if broken at what looks like a
good point to break it.

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

diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index 3117a4be0cd04..5cdacb3c8c925 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -11,6 +11,7 @@ VERBOSE=0
TRACING=0

tests="
+ arp_ping eth-arp: Basic arp ping between two NS
netlink_checks ovsnl: validate netlink attrs and settings
upcall_interfaces ovs: test the upcall interfaces"

@@ -127,6 +128,16 @@ ovs_add_netns_and_veths () {
return 0
}

+ovs_add_flow () {
+ info "Adding flow to DP: sbx:$1 br:$2 flow:$3 act:$4"
+ ovs_sbx "$1" python3 $ovs_base/ovs-dpctl.py add-flow "$2" "$3" "$4"
+ if [ $? -ne 0 ]; then
+ echo "Flow [ $3 : $4 ] failed" >> ${ovs_dir}/debug.log
+ return 1
+ fi
+ return 0
+}
+
usage() {
echo
echo "$0 [OPTIONS] [TEST]..."
@@ -141,6 +152,46 @@ usage() {
exit 1
}

+# arp_ping test
+# - client has 1500 byte MTU
+# - server has 1500 byte MTU
+# - send ARP ping between two ns
+test_arp_ping () {
+
+ which arping >/dev/null 2>&1 || return $ksft_skip
+
+ sbx_add "test_arp_ping" || return $?
+
+ ovs_add_dp "test_arp_ping" arpping || return 1
+
+ info "create namespaces"
+ for ns in client server; do
+ ovs_add_netns_and_veths "test_arp_ping" "arpping" "$ns" \
+ "${ns:0:1}0" "${ns:0:1}1" || return 1
+ done
+
+ # Setup client namespace
+ ip netns exec client ip addr add 172.31.110.10/24 dev c1
+ ip netns exec client ip link set c1 up
+ HW_CLIENT=`ip netns exec client ip link show dev c1 | grep -E 'link/ether [0-9a-f:]+' | awk '{print $2;}'`
+ info "Client hwaddr: $HW_CLIENT"
+
+ # Setup server namespace
+ ip netns exec server ip addr add 172.31.110.20/24 dev s1
+ ip netns exec server ip link set s1 up
+ HW_SERVER=`ip netns exec server ip link show dev s1 | grep -E 'link/ether [0-9a-f:]+' | awk '{print $2;}'`
+ info "Server hwaddr: $HW_SERVER"
+
+ ovs_add_flow "test_arp_ping" arpping \
+ "in_port(1),eth(),eth_type(0x0806),arp(sip=172.31.110.10,tip=172.31.110.20,sha=$HW_CLIENT,tha=ff:ff:ff:ff:ff:ff)" '2' || return 1
+ ovs_add_flow "test_arp_ping" arpping \
+ "in_port(2),eth(),eth_type(0x0806),arp()" '1' || return 1
+
+ ovs_sbx "test_arp_ping" ip netns exec client arping -I c1 172.31.110.20 -c 1 || return 1
+
+ return 0
+}
+
# netlink_validation
# - Create a dp
# - check no warning with "old version" simulation
diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
index 1c8b36bc15d48..27e15c636456e 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -9,9 +9,12 @@ import errno
import ipaddress
import logging
import multiprocessing
+import re
import struct
import sys
import time
+import types
+import uuid

try:
from pyroute2 import NDB
@@ -59,6 +62,104 @@ def macstr(mac):
return outstr


+def strspn(str1, str2):
+ tot = 0
+ for char in str1:
+ if str2.find(char) == -1:
+ return tot
+ tot += 1
+ return tot
+
+
+def intparse(statestr, defmask="0xffffffff"):
+ totalparse = strspn(statestr, "0123456789abcdefABCDEFx/")
+ # scan until "/"
+ count = strspn(statestr, "x0123456789abcdefABCDEF")
+
+ firstnum = statestr[:count]
+ if firstnum[-1] == "/":
+ firstnum = firstnum[:-1]
+ k = int(firstnum, 0)
+
+ m = None
+ if defmask is not None:
+ secondnum = defmask
+ if statestr[count] == "/":
+ secondnum = statestr[count + 1 :] # this is wrong...
+ m = int(secondnum, 0)
+
+ return statestr[totalparse + 1 :], k, m
+
+
+def parse_flags(flag_str, flag_vals):
+ bitResult = 0
+ maskResult = 0
+
+ if len(flag_str) == 0:
+ return flag_str, bitResult, maskResult
+
+ if flag_str[0].isdigit():
+ idx = 0
+ while flag_str[idx].isdigit() or flag_str[idx] == "x":
+ idx += 1
+ digits = flag_str[:idx]
+ flag_str = flag_str[idx:]
+
+ bitResult = int(digits, 0)
+ maskResult = int(digits, 0)
+
+ while len(flag_str) > 0 and (flag_str[0] == "+" or flag_str[0] == "-"):
+ if flag_str[0] == "+":
+ setFlag = True
+ elif flag_str[0] == "-":
+ setFlag = False
+
+ flag_str = flag_str[1:]
+
+ flag_len = 0
+ while (
+ flag_str[flag_len] != "+"
+ and flag_str[flag_len] != "-"
+ and flag_str[flag_len] != ","
+ and flag_str[flag_len] != ")"
+ ):
+ flag_len += 1
+
+ flag = flag_str[0:flag_len]
+
+ if flag in flag_vals:
+ if maskResult & flag_vals[flag]:
+ raise KeyError(
+ "Flag %s set once, cannot be set in multiples" % flag
+ )
+
+ if setFlag:
+ bitResult |= flag_vals[flag]
+
+ maskResult |= flag_vals[flag]
+ else:
+ raise KeyError("Missing flag value: %s" % flag)
+
+ flag_str = flag_str[flag_len:]
+
+ return flag_str, bitResult, maskResult
+
+
+def parse_ct_state(statestr):
+ ct_flags = {
+ "new": 1 << 0,
+ "est": 1 << 1,
+ "rel": 1 << 2,
+ "rpl": 1 << 3,
+ "inv": 1 << 4,
+ "trk": 1 << 5,
+ "snat": 1 << 6,
+ "dnat": 1 << 7,
+ }
+
+ return parse_flags(statestr, ct_flags)
+
+
def convert_mac(mac_str, mask=False):
if mac_str is None or mac_str == "":
mac_str = "00:00:00:00:00:00"
@@ -79,6 +180,62 @@ def convert_ipv4(ip, mask=False):
return int(ipaddress.IPv4Address(ip))


+def parse_starts_block(block_str, scanstr, returnskipped, scanregex=False):
+ if scanregex:
+ m = re.search(scanstr, block_str)
+ if m is None:
+ if returnskipped:
+ return block_str
+ return False
+ if returnskipped:
+ block_str = block_str[len(m.group(0)) :]
+ return block_str
+ return True
+
+ if block_str.startswith(scanstr):
+ if returnskipped:
+ block_str = block_str[len(scanstr) :]
+ else:
+ return True
+
+ if returnskipped:
+ return block_str
+
+ return False
+
+
+def parse_extract_field(
+ block_str, fieldstr, scanfmt, convert, masked=False, defval=None
+):
+ if fieldstr and not block_str.startswith(fieldstr):
+ return block_str, defval
+
+ if fieldstr:
+ str_skiplen = len(fieldstr)
+ str_skipped = block_str[str_skiplen:]
+ if str_skiplen == 0:
+ return str_skipped, defval
+ else:
+ str_skiplen = 0
+ str_skipped = block_str
+
+ m = re.search(scanfmt, str_skipped)
+ if m is None:
+ raise ValueError("Bad fmt string")
+
+ data = m.group(0)
+ if convert:
+ data = convert(m.group(0))
+
+ str_skipped = str_skipped[len(m.group(0)) :]
+ if masked:
+ if str_skipped[0] == "/":
+ raise ValueError("Masking support TBD...")
+
+ str_skipped = str_skipped[strspn(str_skipped, ", ") :]
+ return str_skipped, data
+
+
class ovs_dp_msg(genlmsg):
# include the OVS version
# We need a custom header rather than just being able to rely on
@@ -278,6 +435,50 @@ class ovsactions(nla):

return print_str

+ def parse(self, actstr):
+ while len(actstr) != 0:
+ parsed = False
+ if actstr.startswith("drop"):
+ # for now, drops have no explicit action, so we
+ # don't need to set any attributes. The final
+ # act of the processing chain will just drop the packet
+ return
+
+ elif parse_starts_block(actstr, "^(\d+)", False, True):
+ actstr, output = parse_extract_field(
+ actstr, None, "(\d+)", lambda x: int(x), False, "0"
+ )
+ self["attrs"].append(["OVS_ACTION_ATTR_OUTPUT", output])
+ parsed = True
+ elif parse_starts_block(actstr, "recirc(", False):
+ actstr, recircid = parse_extract_field(
+ actstr,
+ "recirc(",
+ "([0-9a-fA-Fx]+)",
+ lambda x: int(x, 0),
+ False,
+ 0,
+ )
+ self["attrs"].append(["OVS_ACTION_ATTR_RECIRC", recircid])
+ parsed = True
+
+ parse_flat_map = (
+ ("ct_clear", "OVS_ACTION_ATTR_CT_CLEAR"),
+ ("pop_vlan", "OVS_ACTION_ATTR_POP_VLAN"),
+ ("pop_eth", "OVS_ACTION_ATTR_POP_ETH"),
+ ("pop_nsh", "OVS_ACTION_ATTR_POP_NSH"),
+ )
+
+ for flat_act in parse_flat_map:
+ if parse_starts_block(actstr, flat_act[0], False):
+ actstr += len(flat_act[0])
+ self["attrs"].append([flat_act[1]])
+ actstr = actstr[strspn(actstr, ", ") :]
+ parsed = True
+
+ if not parsed:
+ raise ValueError("Action str: '%s' not supported" % actstr)
+

class ovskey(nla):
nla_flags = NLA_F_NESTED
@@ -347,6 +548,53 @@ class ovskey(nla):
init=init,
)

+ def parse(self, flowstr, typeInst):
+ if not flowstr.startswith(self.proto_str):
+ return None, None
+
+ k = typeInst()
+ m = typeInst()
+
+ flowstr = flowstr[len(self.proto_str) :]
+ if flowstr.startswith("("):
+ flowstr = flowstr[1:]
+
+ keybits = b""
+ maskbits = b""
+ for f in self.fields_map:
+ if flowstr.startswith(f[1]):
+ # the following assumes that the field looks
+ # something like 'field.' where '.' is a
+ # character that we don't exactly care about.
+ flowstr = flowstr[len(f[1]) + 1 :]
+ splitchar = 0
+ for c in flowstr:
+ if c == "," or c == ")":
+ break
+ splitchar += 1
+ data = flowstr[:splitchar]
+ flowstr = flowstr[splitchar:]
+ else:
+ data = None
+
+ if len(f) > 4:
+ func = f[4]
+ else:
+ func = f[3]
+ k[f[0]] = func(data)
+ if len(f) > 4:
+ m[f[0]] = func(data, True)
+ else:
+ m[f[0]] = func(data)
+
+ flowstr = flowstr[strspn(flowstr, ", ") :]
+ if len(flowstr) == 0:
+ return flowstr, k, m
+
+ flowstr = flowstr[strspn(flowstr, "), ") :]
+
+ return flowstr, k, m
+
def dpstr(self, masked=None, more=False):
outstr = self.proto_str + "("
first = False
@@ -810,6 +1058,71 @@ class ovskey(nla):
class ovs_key_mpls(nla):
fields = (("lse", ">I"),)

+ def parse(self, flowstr, mask=None):
+ for field in (
+ ("OVS_KEY_ATTR_PRIORITY", "skb_priority", intparse),
+ ("OVS_KEY_ATTR_SKB_MARK", "skb_mark", intparse),
+ ("OVS_KEY_ATTR_RECIRC_ID", "recirc_id", intparse),
+ ("OVS_KEY_ATTR_DP_HASH", "dp_hash", intparse),
+ ("OVS_KEY_ATTR_CT_STATE", "ct_state", parse_ct_state),
+ ("OVS_KEY_ATTR_CT_ZONE", "ct_zone", intparse),
+ ("OVS_KEY_ATTR_CT_MARK", "ct_mark", intparse),
+ ("OVS_KEY_ATTR_IN_PORT", "in_port", intparse),
+ (
+ "OVS_KEY_ATTR_ETHERNET",
+ "eth",
+ ovskey.ethaddr,
+ ),
+ (
+ "OVS_KEY_ATTR_ETHERTYPE",
+ "eth_type",
+ lambda x: intparse(x, "0xffff"),
+ ),
+ (
+ "OVS_KEY_ATTR_IPV4",
+ "ipv4",
+ ovskey.ovs_key_ipv4,
+ ),
+ (
+ "OVS_KEY_ATTR_IPV6",
+ "ipv6",
+ ovskey.ovs_key_ipv6,
+ ),
+ (
+ "OVS_KEY_ATTR_ARP",
+ "arp",
+ ovskey.ovs_key_arp,
+ ),
+ (
+ "OVS_KEY_ATTR_TCP",
+ "tcp",
+ ovskey.ovs_key_tcp,
+ ),
+ (
+ "OVS_KEY_ATTR_TCP_FLAGS",
+ "tcp_flags",
+ lambda x: parse_flags(x, None),
+ ),
+ ):
+ fld = field[1] + "("
+ if not flowstr.startswith(fld):
+ continue
+
+ if not isinstance(field[2], types.FunctionType):
+ nk = field[2]()
+ flowstr, k, m = nk.parse(flowstr, field[2])
+ else:
+ flowstr = flowstr[len(fld) :]
+ flowstr, k, m = field[2](flowstr)
+
+ if m and mask is not None:
+ mask["attrs"].append([field[0], m])
+ self["attrs"].append([field[0], k])
+
+ flowstr = flowstr[strspn(flowstr, "),") :]
+
+ return flowstr
+
def dpstr(self, mask=None, more=False):
print_str = ""

@@ -1358,11 +1671,92 @@ class OvsFlow(GenericNetlinkSocket):

return print_str

+ def parse(self, flowstr, actstr, dpidx=0):
+ OVS_UFID_F_OMIT_KEY = 1 << 0
+ OVS_UFID_F_OMIT_MASK = 1 << 1
+ OVS_UFID_F_OMIT_ACTIONS = 1 << 2
+
+ self["cmd"] = 0
+ self["version"] = 0
+ self["reserved"] = 0
+ self["dpifindex"] = 0
+
+ if flowstr.startswith("ufid:"):
+ count = 5
+ while flowstr[count] != ",":
+ count += 1
+ ufidstr = flowstr[5:count]
+ flowstr = flowstr[count + 1 :]
+ else:
+ ufidstr = str(uuid.uuid4())
+ uuidRawObj = uuid.UUID(ufidstr).fields
+
+ self["attrs"].append(
+ [
+ "OVS_FLOW_ATTR_UFID",
+ [
+ uuidRawObj[0],
+ uuidRawObj[1] << 16 | uuidRawObj[2],
+ uuidRawObj[3] << 24
+ | uuidRawObj[4] << 16
+ | uuidRawObj[5] & (0xFF << 32) >> 32,
+ uuidRawObj[5] & (0xFFFFFFFF),
+ ],
+ ]
+ )
+ self["attrs"].append(
+ [
+ "OVS_FLOW_ATTR_UFID_FLAGS",
+ int(
+ OVS_UFID_F_OMIT_KEY
+ | OVS_UFID_F_OMIT_MASK
+ | OVS_UFID_F_OMIT_ACTIONS
+ ),
+ ]
+ )
+
+ k = ovskey()
+ m = ovskey()
+ k.parse(flowstr, m)
+ self["attrs"].append(["OVS_FLOW_ATTR_KEY", k])
+ self["attrs"].append(["OVS_FLOW_ATTR_MASK", m])
+
+ a = ovsactions()
+ a.parse(actstr)
+ self["attrs"].append(["OVS_FLOW_ATTR_ACTIONS", a])
+
def __init__(self):
GenericNetlinkSocket.__init__(self)

self.bind(OVS_FLOW_FAMILY, OvsFlow.ovs_flow_msg)

+ def add_flow(self, dpifindex, flowmsg):
+ """
+ Send a new flow message to the kernel.
+
+ dpifindex should be a valid datapath obtained by calling
+ into the OvsDatapath lookup
+
+ flowmsg is a flow object obtained by calling a dpparse
+ """
+
+ flowmsg["cmd"] = OVS_FLOW_CMD_NEW
+ 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.
@@ -1514,6 +1908,11 @@ def main(argv):
dumpflcmd = subparsers.add_parser("dump-flows")
dumpflcmd.add_argument("dumpdp", help="Datapath Name")

+ addflcmd = subparsers.add_parser("add-flow")
+ addflcmd.add_argument("flbr", help="Datapath name")
+ addflcmd.add_argument("flow", help="Flow specification")
+ addflcmd.add_argument("acts", help="Flow actions")
+
args = parser.parse_args()

if args.verbose > 0:
@@ -1589,6 +1988,14 @@ def main(argv):
rep = ovsflow.dump(rep["dpifindex"])
for flow in rep:
print(flow.dpstr(True if args.verbose > 0 else False))
+ elif hasattr(args, "flbr"):
+ rep = ovsdp.info(args.flbr, 0)
+ if rep is None:
+ print("DP '%s' not found." % args.flbr)
+ return 1
+ flow = OvsFlow.ovs_flow_msg()
+ flow.parse(args.flow, args.acts, rep["dpifindex"])
+ ovsflow.add_flow(rep["dpifindex"], flow)

return 0

--
2.40.1


2023-08-01 23:34:51

by Aaron Conole

[permalink] [raw]
Subject: [PATCH v3 net-next 5/5] selftests: openvswitch: add ct-nat test case with ipv4

Building on the previous work, add a very simplistic NAT case
using ipv4. This just tests dnat transformation

Signed-off-by: Aaron Conole <[email protected]>
---
NOTE: A number of lines flag the line-length checkpatch
warning, but there didnt seem to be a really good
way of breaking the lines smaller.

.../selftests/net/openvswitch/openvswitch.sh | 64 +++++++++++++++++++
.../selftests/net/openvswitch/ovs-dpctl.py | 62 ++++++++++++++++++
2 files changed, 126 insertions(+)

diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index 40a66c72af0f0..dced4f612a78c 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -14,6 +14,7 @@ tests="
arp_ping eth-arp: Basic arp ping between two NS
ct_connect_v4 ip4-ct-xon: Basic ipv4 tcp connection using ct
connect_v4 ip4-xon: Basic ipv4 ping between two NS
+ nat_connect_v4 ip4-nat-xon: Basic ipv4 tcp connection via NAT
netlink_checks ovsnl: validate netlink attrs and settings
upcall_interfaces ovs: test the upcall interfaces"

@@ -300,6 +301,69 @@ test_connect_v4 () {
return 0
}

+# nat_connect_v4 test
+# - client has 1500 byte MTU
+# - server has 1500 byte MTU
+# - use ICMP to ping in each direction
+# - only allow CT state stuff to pass through new in c -> s
+test_nat_connect_v4 () {
+ which nc >/dev/null 2>/dev/null || return $ksft_skip
+
+ sbx_add "test_nat_connect_v4" || return $?
+
+ ovs_add_dp "test_nat_connect_v4" nat4 || return 1
+ info "create namespaces"
+ for ns in client server; do
+ ovs_add_netns_and_veths "test_nat_connect_v4" "nat4" "$ns" \
+ "${ns:0:1}0" "${ns:0:1}1" || return 1
+ done
+
+ ip netns exec client ip addr add 172.31.110.10/24 dev c1
+ ip netns exec client ip link set c1 up
+ ip netns exec server ip addr add 172.31.110.20/24 dev s1
+ ip netns exec server ip link set s1 up
+
+ ip netns exec client ip route add default via 172.31.110.20
+
+ ovs_add_flow "test_nat_connect_v4" nat4 \
+ 'in_port(1),eth(),eth_type(0x0806),arp()' '2' || return 1
+ ovs_add_flow "test_nat_connect_v4" nat4 \
+ 'in_port(2),eth(),eth_type(0x0806),arp()' '1' || return 1
+ ovs_add_flow "test_nat_connect_v4" nat4 \
+ "ct_state(-trk),in_port(1),eth(),eth_type(0x0800),ipv4(dst=192.168.0.20)" \
+ "ct(commit,nat(dst=172.31.110.20)),recirc(0x1)"
+ ovs_add_flow "test_nat_connect_v4" nat4 \
+ "ct_state(-trk),in_port(2),eth(),eth_type(0x0800),ipv4()" \
+ "ct(commit,nat),recirc(0x2)"
+
+ ovs_add_flow "test_nat_connect_v4" nat4 \
+ "recirc_id(0x1),ct_state(+trk-inv),in_port(1),eth(),eth_type(0x0800),ipv4()" "2"
+ ovs_add_flow "test_nat_connect_v4" nat4 \
+ "recirc_id(0x2),ct_state(+trk-inv),in_port(2),eth(),eth_type(0x0800),ipv4()" "1"
+
+ # do a ping
+ ovs_sbx "test_nat_connect_v4" ip netns exec client ping 192.168.0.20 -c 3 || return 1
+
+ # create an echo server in 'server'
+ echo "server" | \
+ ovs_netns_spawn_daemon "test_nat_connect_v4" "server" \
+ nc -lvnp 4443
+ ovs_sbx "test_nat_connect_v4" ip netns exec client nc -i 1 -zv 192.168.0.20 4443 || return 1
+
+ # Now test in the other direction (should fail)
+ echo "client" | \
+ ovs_netns_spawn_daemon "test_nat_connect_v4" "client" \
+ nc -lvnp 4443
+ ovs_sbx "test_nat_connect_v4" ip netns exec client nc -i 1 -zv 172.31.110.10 4443
+ if [ $? == 0 ]; then
+ info "connect to client was successful"
+ return 1
+ fi
+
+ info "done..."
+ return 0
+}
+
# netlink_validation
# - Create a dp
# - check no warning with "old version" simulation
diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
index 1a3dc3762361b..fbdac15e31344 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -530,6 +530,68 @@ class ovsactions(nla):
else:
ctact["attrs"].append([scan[1], None])
actstr = actstr[strspn(actstr, ", ") :]
+ # it seems strange to put this here, but nat() is a complex
+ # sub-action and this lets it sit anywhere in the ct() action
+ if actstr.startswith("nat"):
+ actstr = actstr[3:]
+ natact = ovsactions.ctact.natattr()
+
+ if actstr.startswith("("):
+ t = None
+ actstr = actstr[1:]
+ if actstr.startswith("src"):
+ t = "OVS_NAT_ATTR_SRC"
+ actstr = actstr[3:]
+ elif actstr.startswith("dst"):
+ t = "OVS_NAT_ATTR_DST"
+ actstr = actstr[3:]
+
+ actstr, ip_block_min = parse_extract_field(
+ actstr, "=", "([0-9a-fA-F\.]+)", str, False
+ )
+ actstr, ip_block_max = parse_extract_field(
+ actstr, "-", "([0-9a-fA-F\.]+)", str, False
+ )
+
+ actstr, proto_min = parse_extract_field(
+ actstr, ":", "(\d+)", int, False
+ )
+ actstr, proto_max = parse_extract_field(
+ actstr, "-", "(\d+)", int, False
+ )
+
+ if t is not None:
+ natact["attrs"].append([t, None])
+
+ if ip_block_min is not None:
+ natact["attrs"].append(
+ ["OVS_NAT_ATTR_IP_MIN", ip_block_min]
+ )
+ if ip_block_max is not None:
+ natact["attrs"].append(
+ ["OVS_NAT_ATTR_IP_MAX", ip_block_max]
+ )
+ if proto_min is not None:
+ natact["attrs"].append(
+ ["OVS_NAT_ATTR_PROTO_MIN", proto_min]
+ )
+ if proto_max is not None:
+ natact["attrs"].append(
+ ["OVS_NAT_ATTR_PROTO_MAX", proto_max]
+ )
+
+ for natscan in (
+ ("persistent", "OVS_NAT_ATTR_PERSISTENT"),
+ ("hash", "OVS_NAT_ATTR_PROTO_HASH"),
+ ("random", "OVS_NAT_ATTR_PROTO_RANDOM"),
+ ):
+ if actstr.startswith(natscan[0]):
+ actstr = actstr[len(natscan[0]) :]
+ natact["attrs"].append([natscan[1], None])
+ actstr = actstr[strspn(actstr, ", ") :]
+
+ ctact["attrs"].append(["OVS_CT_ATTR_NAT", natact])
+ actstr = actstr[strspn(actstr, ",) ") :]

self["attrs"].append(["OVS_ACTION_ATTR_CT", ctact])
parsed = True
--
2.40.1


2023-08-03 09:44:37

by Simon Horman

[permalink] [raw]
Subject: Re: [ovs-dev] [PATCH v3 net-next 0/5] selftests: openvswitch: add flow programming cases

On Tue, Aug 01, 2023 at 05:22:21PM -0400, Aaron Conole wrote:
> The openvswitch selftests currently contain a few cases for managing the
> datapath, which includes creating datapath instances, adding interfaces,
> and doing some basic feature / upcall tests. This is useful to validate
> the control path.
>
> Add the ability to program some of the more common flows with actions. This
> can be improved overtime to include regression testing, etc.

Thanks Aaron.

For series,

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


2023-08-03 14:02:43

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH v3 net-next 0/5] selftests: openvswitch: add flow programming cases

Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <[email protected]>:

On Tue, 1 Aug 2023 17:22:21 -0400 you wrote:
> The openvswitch selftests currently contain a few cases for managing the
> datapath, which includes creating datapath instances, adding interfaces,
> and doing some basic feature / upcall tests. This is useful to validate
> the control path.
>
> Add the ability to program some of the more common flows with actions. This
> can be improved overtime to include regression testing, etc.
>
> [...]

Here is the summary with links:
- [v3,net-next,1/5] selftests: openvswitch: add an initial flow programming case
https://git.kernel.org/netdev/net-next/c/918423fda910
- [v3,net-next,2/5] selftests: openvswitch: support key masks
https://git.kernel.org/netdev/net-next/c/9f1179fbbd84
- [v3,net-next,3/5] selftests: openvswitch: add a test for ipv4 forwarding
https://git.kernel.org/netdev/net-next/c/05398aa40953
- [v3,net-next,4/5] selftests: openvswitch: add basic ct test case parsing
https://git.kernel.org/netdev/net-next/c/2893ba9c1d1a
- [v3,net-next,5/5] selftests: openvswitch: add ct-nat test case with ipv4
https://git.kernel.org/netdev/net-next/c/60f10077eec6

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



2023-08-03 16:18:01

by Paolo Abeni

[permalink] [raw]
Subject: Re: [PATCH v3 net-next 0/5] selftests: openvswitch: add flow programming cases

On Tue, 2023-08-01 at 17:22 -0400, Aaron Conole wrote:
> The openvswitch selftests currently contain a few cases for managing the
> datapath, which includes creating datapath instances, adding interfaces,
> and doing some basic feature / upcall tests. This is useful to validate
> the control path.
>
> Add the ability to program some of the more common flows with actions. This
> can be improved overtime to include regression testing, etc.
>
> v2->v3:
> 1. Dropped support for ipv6 in nat() case
> 2. Fixed a spelling mistake in 2/5 commit message.
>
> v1->v2:
> 1. Fix issue when parsing ipv6 in the NAT action
> 2. Fix issue calculating length during ctact parsing
> 3. Fix error message when invalid bridge is passed
> 4. Fold in Adrian's patch to support key masks

FTR, this apparently requires an [un?]fairly recent version of
pyroute2. Perhaps you could explicitly check for a minimum working
version and otherwise bail out (skip) the add-flow tests.

Cheers,

Paolo



2023-08-04 13:28:22

by Aaron Conole

[permalink] [raw]
Subject: Re: [PATCH v3 net-next 0/5] selftests: openvswitch: add flow programming cases

Paolo Abeni <[email protected]> writes:

> On Tue, 2023-08-01 at 17:22 -0400, Aaron Conole wrote:
>> The openvswitch selftests currently contain a few cases for managing the
>> datapath, which includes creating datapath instances, adding interfaces,
>> and doing some basic feature / upcall tests. This is useful to validate
>> the control path.
>>
>> Add the ability to program some of the more common flows with actions. This
>> can be improved overtime to include regression testing, etc.
>>
>> v2->v3:
>> 1. Dropped support for ipv6 in nat() case
>> 2. Fixed a spelling mistake in 2/5 commit message.
>>
>> v1->v2:
>> 1. Fix issue when parsing ipv6 in the NAT action
>> 2. Fix issue calculating length during ctact parsing
>> 3. Fix error message when invalid bridge is passed
>> 4. Fold in Adrian's patch to support key masks
>
> FTR, this apparently requires an [un?]fairly recent version of
> pyroute2. Perhaps you could explicitly check for a minimum working
> version and otherwise bail out (skip) the add-flow tests.

I'll make sure to get a follow up out ASAP that includes some check for
supported version and will skip if not. I will try to also include some
additional robustness checks.

Thanks Paolo!

> Cheers,
>
> Paolo