* [PATCH v2 net-next 1/5] selftests: openvswitch: add an initial flow programming case
2023-07-28 11:59 [PATCH v2 net-next 0/5] selftests: openvswitch: add flow programming cases Aaron Conole
@ 2023-07-28 11:59 ` Aaron Conole
2023-07-28 15:13 ` [ovs-dev] " Adrian Moreno
2023-07-28 11:59 ` [PATCH v2 net-next 2/5] selftests: openvswitch: support key masks Aaron Conole
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Aaron Conole @ 2023-07-28 11:59 UTC (permalink / raw)
To: netdev
Cc: dev, linux-kselftest, linux-kernel, Shuah Khan, Paolo Abeni,
Jakub Kicinski, Eric Dumazet, David S. Miller, Pravin B Shelar,
Ilya Maximets
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 <aconole@redhat.com>
---
NOTE: 3 lines flag the line-length checkpatch warning, but there didn't
seem to bea 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 3117a4be0cd0..5cdacb3c8c92 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 1c8b36bc15d4..a11ba9f7ea6e 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [ovs-dev] [PATCH v2 net-next 1/5] selftests: openvswitch: add an initial flow programming case
2023-07-28 11:59 ` [PATCH v2 net-next 1/5] selftests: openvswitch: add an initial flow programming case Aaron Conole
@ 2023-07-28 15:13 ` Adrian Moreno
0 siblings, 0 replies; 12+ messages in thread
From: Adrian Moreno @ 2023-07-28 15:13 UTC (permalink / raw)
To: Aaron Conole, netdev
Cc: dev, linux-kernel, Ilya Maximets, Eric Dumazet, linux-kselftest,
Jakub Kicinski, Paolo Abeni, Shuah Khan, David S. Miller
On 7/28/23 13:59, Aaron Conole wrote:
> 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 <aconole@redhat.com>
Reviewed-by: Adrian Moreno <amorenoz@redhat.com>
> ---
> NOTE: 3 lines flag the line-length checkpatch warning, but there didn't
> seem to bea 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 3117a4be0cd0..5cdacb3c8c92 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 1c8b36bc15d4..a11ba9f7ea6e 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
>
--
Adrián Moreno
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 net-next 2/5] selftests: openvswitch: support key masks
2023-07-28 11:59 [PATCH v2 net-next 0/5] selftests: openvswitch: add flow programming cases Aaron Conole
2023-07-28 11:59 ` [PATCH v2 net-next 1/5] selftests: openvswitch: add an initial flow programming case Aaron Conole
@ 2023-07-28 11:59 ` Aaron Conole
2023-07-30 15:42 ` [ovs-dev] " Simon Horman
2023-07-28 11:59 ` [PATCH v2 net-next 3/5] selftests: openvswitch: add a test for ipv4 forwarding Aaron Conole
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Aaron Conole @ 2023-07-28 11:59 UTC (permalink / raw)
To: netdev
Cc: dev, linux-kselftest, linux-kernel, Shuah Khan, Paolo Abeni,
Jakub Kicinski, Eric Dumazet, David S. Miller, Pravin B Shelar,
Ilya Maximets
From: Adrian Moreno <amorenoz@redhat.com>
From: Adrian Moreno <amorenoz@redhat.com>
The default value for the mask actually depends on the value (e.g: if
the value is non-null, the default is full-mask), so change the convert
functions to accept the full, possibly masked string and let them figure
out how to parse the differnt values.
Also, implement size-aware int parsing.
With this patch we can now express flows such as the following:
"eth(src=0a:ca:fe:ca:fe:0a/ff:ff:00:00:ff:00)"
"eth(src=0a:ca:fe:ca:fe:0a)" -> mask = ff:ff:ff:ff:ff:ff
"ipv4(src=192.168.1.1)" -> mask = 255.255.255.255
"ipv4(src=192.168.1.1/24)"
"ipv4(src=192.168.1.1/255.255.255.0)"
"tcp(src=8080)" -> mask = 0xffff
"tcp(src=8080/0xf0f0)"
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Acked-by: Aaron Conole <aconole@redhat.com>
---
.../selftests/net/openvswitch/ovs-dpctl.py | 96 ++++++++++++-------
1 file changed, 64 insertions(+), 32 deletions(-)
diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
index a11ba9f7ea6e..2b869e89c51d 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -160,25 +160,45 @@ def parse_ct_state(statestr):
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"
- if mask is True and mac_str != "00:00:00:00:00:00":
- mac_str = "FF:FF:FF:FF:FF:FF"
- mac_split = mac_str.split(":")
- ret = bytearray([int(i, 16) for i in mac_split])
- return bytes(ret)
+def convert_mac(data):
+ def to_bytes(mac):
+ mac_split = mac.split(":")
+ ret = bytearray([int(i, 16) for i in mac_split])
+ return bytes(ret)
+ mac_str, _, mask_str = data.partition('/')
-def convert_ipv4(ip, mask=False):
- if ip is None:
- ip = 0
- if mask is True:
- if ip != 0:
- ip = int(ipaddress.IPv4Address(ip)) & 0xFFFFFFFF
+ if not mac_str:
+ mac_str = mask_str = "00:00:00:00:00:00"
+ elif not mask_str:
+ mask_str = "FF:FF:FF:FF:FF:FF"
- return int(ipaddress.IPv4Address(ip))
+ return to_bytes(mac_str), to_bytes(mask_str)
+def convert_ipv4(data):
+ ip, _, mask = data.partition('/')
+
+ if not ip:
+ ip = mask = 0
+ elif not mask:
+ mask = 0xFFFFFFFF
+ elif mask.isdigit():
+ mask = (0xFFFFFFFF << (32 - int(mask))) & 0xFFFFFFFF
+
+ return int(ipaddress.IPv4Address(ip)), int(ipaddress.IPv4Address(mask))
+
+def convert_int(size):
+ def convert_int_sized(data):
+ value, _, mask = data.partition('/')
+
+ if not value:
+ return 0, 0
+ elif not mask:
+ return int(value, 0), pow(2, size) - 1
+ else:
+ return int(value, 0), int(mask, 0)
+
+ return convert_int_sized
def parse_starts_block(block_str, scanstr, returnskipped, scanregex=False):
if scanregex:
@@ -525,8 +545,10 @@ class ovskey(nla):
)
fields_map = (
- ("src", "src", "%d", lambda x: int(x) if x is not None else 0),
- ("dst", "dst", "%d", lambda x: int(x) if x is not None else 0),
+ ("src", "src", "%d", lambda x: int(x) if x else 0,
+ convert_int(16)),
+ ("dst", "dst", "%d", lambda x: int(x) if x else 0,
+ convert_int(16)),
)
def __init__(
@@ -575,17 +597,13 @@ class ovskey(nla):
data = flowstr[:splitchar]
flowstr = flowstr[splitchar:]
else:
- data = None
+ data = ""
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)
+ k[f[0]], m[f[0]] = f[4](data)
else:
- m[f[0]] = func(data)
+ k[f[0]] = f[3](data)
+ m[f[0]] = f[3](data)
flowstr = flowstr[strspn(flowstr, ", ") :]
if len(flowstr) == 0:
@@ -689,10 +707,14 @@ class ovskey(nla):
int,
convert_ipv4,
),
- ("proto", "proto", "%d", lambda x: int(x) if x is not None else 0),
- ("tos", "tos", "%d", lambda x: int(x) if x is not None else 0),
- ("ttl", "ttl", "%d", lambda x: int(x) if x is not None else 0),
- ("frag", "frag", "%d", lambda x: int(x) if x is not None else 0),
+ ("proto", "proto", "%d", lambda x: int(x) if x else 0,
+ convert_int(8)),
+ ("tos", "tos", "%d", lambda x: int(x) if x else 0,
+ convert_int(8)),
+ ("ttl", "ttl", "%d", lambda x: int(x) if x else 0,
+ convert_int(8)),
+ ("frag", "frag", "%d", lambda x: int(x) if x else 0,
+ convert_int(8)),
)
def __init__(
@@ -828,8 +850,8 @@ class ovskey(nla):
)
fields_map = (
- ("type", "type", "%d", int),
- ("code", "code", "%d", int),
+ ("type", "type", "%d", lambda x: int(x) if x else 0),
+ ("code", "code", "%d", lambda x: int(x) if x else 0),
)
def __init__(
@@ -894,7 +916,7 @@ class ovskey(nla):
int,
convert_ipv4,
),
- ("op", "op", "%d", lambda x: int(x) if x is not None else 0),
+ ("op", "op", "%d", lambda x: int(x) if x else 0),
(
"sha",
"sha",
@@ -1098,6 +1120,16 @@ class ovskey(nla):
"tcp",
ovskey.ovs_key_tcp,
),
+ (
+ "OVS_KEY_ATTR_UDP",
+ "udp",
+ ovskey.ovs_key_udp,
+ ),
+ (
+ "OVS_KEY_ATTR_ICMP",
+ "icmp",
+ ovskey.ovs_key_icmp,
+ ),
(
"OVS_KEY_ATTR_TCP_FLAGS",
"tcp_flags",
--
2.40.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [ovs-dev] [PATCH v2 net-next 2/5] selftests: openvswitch: support key masks
2023-07-28 11:59 ` [PATCH v2 net-next 2/5] selftests: openvswitch: support key masks Aaron Conole
@ 2023-07-30 15:42 ` Simon Horman
0 siblings, 0 replies; 12+ messages in thread
From: Simon Horman @ 2023-07-30 15:42 UTC (permalink / raw)
To: Aaron Conole
Cc: netdev, dev, linux-kernel, Ilya Maximets, Eric Dumazet,
linux-kselftest, Jakub Kicinski, Paolo Abeni, Shuah Khan,
David S. Miller
On Fri, Jul 28, 2023 at 07:59:37AM -0400, Aaron Conole wrote:
> From: Adrian Moreno <amorenoz@redhat.com>
>
> From: Adrian Moreno <amorenoz@redhat.com>
>
> The default value for the mask actually depends on the value (e.g: if
> the value is non-null, the default is full-mask), so change the convert
> functions to accept the full, possibly masked string and let them figure
> out how to parse the differnt values.
nit: differnt -> different
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 net-next 3/5] selftests: openvswitch: add a test for ipv4 forwarding
2023-07-28 11:59 [PATCH v2 net-next 0/5] selftests: openvswitch: add flow programming cases Aaron Conole
2023-07-28 11:59 ` [PATCH v2 net-next 1/5] selftests: openvswitch: add an initial flow programming case Aaron Conole
2023-07-28 11:59 ` [PATCH v2 net-next 2/5] selftests: openvswitch: support key masks Aaron Conole
@ 2023-07-28 11:59 ` Aaron Conole
2023-07-28 15:08 ` [ovs-dev] " Adrian Moreno
2023-07-28 11:59 ` [PATCH v2 net-next 4/5] selftests: openvswitch: add basic ct test case parsing Aaron Conole
2023-07-28 11:59 ` [PATCH v2 net-next 5/5] selftests: openvswitch: add ct-nat test case with ipv4 Aaron Conole
4 siblings, 1 reply; 12+ messages in thread
From: Aaron Conole @ 2023-07-28 11:59 UTC (permalink / raw)
To: netdev
Cc: dev, linux-kselftest, linux-kernel, Shuah Khan, Paolo Abeni,
Jakub Kicinski, Eric Dumazet, David S. Miller, Pravin B Shelar,
Ilya Maximets
This is a simple ipv4 bidirectional connectivity test.
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
.../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 5cdacb3c8c92..5d60a9466dab 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [ovs-dev] [PATCH v2 net-next 3/5] selftests: openvswitch: add a test for ipv4 forwarding
2023-07-28 11:59 ` [PATCH v2 net-next 3/5] selftests: openvswitch: add a test for ipv4 forwarding Aaron Conole
@ 2023-07-28 15:08 ` Adrian Moreno
0 siblings, 0 replies; 12+ messages in thread
From: Adrian Moreno @ 2023-07-28 15:08 UTC (permalink / raw)
To: Aaron Conole, netdev
Cc: dev, linux-kernel, Ilya Maximets, Eric Dumazet, linux-kselftest,
Jakub Kicinski, Paolo Abeni, Shuah Khan, David S. Miller
On 7/28/23 13:59, Aaron Conole wrote:
> This is a simple ipv4 bidirectional connectivity test.
>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
Reviewed-by: Adrian Moreno <amorenoz@redhat.com>
> ---
> .../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 5cdacb3c8c92..5d60a9466dab 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
--
Adrián Moreno
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 net-next 4/5] selftests: openvswitch: add basic ct test case parsing
2023-07-28 11:59 [PATCH v2 net-next 0/5] selftests: openvswitch: add flow programming cases Aaron Conole
` (2 preceding siblings ...)
2023-07-28 11:59 ` [PATCH v2 net-next 3/5] selftests: openvswitch: add a test for ipv4 forwarding Aaron Conole
@ 2023-07-28 11:59 ` Aaron Conole
2023-07-28 15:08 ` [ovs-dev] " Adrian Moreno
2023-07-28 11:59 ` [PATCH v2 net-next 5/5] selftests: openvswitch: add ct-nat test case with ipv4 Aaron Conole
4 siblings, 1 reply; 12+ messages in thread
From: Aaron Conole @ 2023-07-28 11:59 UTC (permalink / raw)
To: netdev
Cc: dev, linux-kselftest, linux-kernel, Shuah Khan, Paolo Abeni,
Jakub Kicinski, Eric Dumazet, David S. Miller, Pravin B Shelar,
Ilya Maximets
Forwarding via ct() action is an important use case for openvswitch, but
generally would require using a full ovs-vswitchd to get working. Add a
ct action parser for basic ct test case.
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
NOTE: 3 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 | 68 +++++++++++++++++++
.../selftests/net/openvswitch/ovs-dpctl.py | 39 +++++++++++
2 files changed, 107 insertions(+)
diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index 5d60a9466dab..40a66c72af0f 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
+ ct_connect_v4 ip4-ct-xon: Basic ipv4 tcp connection using ct
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"
@@ -193,6 +194,73 @@ test_arp_ping () {
return 0
}
+# ct_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_ct_connect_v4 () {
+
+ which nc >/dev/null 2>/dev/null || return $ksft_skip
+
+ sbx_add "test_ct_connect_v4" || return $?
+
+ ovs_add_dp "test_ct_connect_v4" ct4 || return 1
+ info "create namespaces"
+ for ns in client server; do
+ ovs_add_netns_and_veths "test_ct_connect_v4" "ct4" "$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_ct_connect_v4" ct4 \
+ 'in_port(1),eth(),eth_type(0x0806),arp()' '2' || return 1
+ ovs_add_flow "test_ct_connect_v4" ct4 \
+ 'in_port(2),eth(),eth_type(0x0806),arp()' '1' || return 1
+ ovs_add_flow "test_ct_connect_v4" ct4 \
+ 'ct_state(-trk),eth(),eth_type(0x0800),ipv4()' \
+ 'ct(commit),recirc(0x1)' || return 1
+ ovs_add_flow "test_ct_connect_v4" ct4 \
+ 'recirc_id(0x1),ct_state(+trk+new),in_port(1),eth(),eth_type(0x0800),ipv4(src=172.31.110.10)' \
+ '2' || return 1
+ ovs_add_flow "test_ct_connect_v4" ct4 \
+ 'recirc_id(0x1),ct_state(+trk+est),in_port(1),eth(),eth_type(0x0800),ipv4(src=172.31.110.10)' \
+ '2' || return 1
+ ovs_add_flow "test_ct_connect_v4" ct4 \
+ 'recirc_id(0x1),ct_state(+trk+est),in_port(2),eth(),eth_type(0x0800),ipv4(dst=172.31.110.10)' \
+ '1' || return 1
+ ovs_add_flow "test_ct_connect_v4" ct4 \
+ 'recirc_id(0x1),ct_state(+trk+inv),eth(),eth_type(0x0800),ipv4()' 'drop' || \
+ return 1
+
+ # do a ping
+ ovs_sbx "test_ct_connect_v4" ip netns exec client ping 172.31.110.20 -c 3 || return 1
+
+ # create an echo server in 'server'
+ echo "server" | \
+ ovs_netns_spawn_daemon "test_ct_connect_v4" "server" \
+ nc -lvnp 4443
+ ovs_sbx "test_ct_connect_v4" ip netns exec client nc -i 1 -zv 172.31.110.20 4443 || return 1
+
+ # Now test in the other direction (should fail)
+ echo "client" | \
+ ovs_netns_spawn_daemon "test_ct_connect_v4" "client" \
+ nc -lvnp 4443
+ ovs_sbx "test_ct_connect_v4" ip netns exec client nc -i 1 -zv 172.31.110.10 4443
+ if [ $? == 0 ]; then
+ info "ct connect to client was successful"
+ return 1
+ fi
+
+ info "done..."
+ return 0
+}
+
# connect_v4 test
# - client has 1500 byte MTU
# - server has 1500 byte MTU
diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
index 2b869e89c51d..6e258ab9e635 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -62,6 +62,15 @@ def macstr(mac):
return outstr
+def strcspn(str1, str2):
+ tot = 0
+ for char in str1:
+ if str2.find(char) != -1:
+ return tot
+ tot += 1
+ return tot
+
+
def strspn(str1, str2):
tot = 0
for char in str1:
@@ -496,6 +505,36 @@ class ovsactions(nla):
actstr = actstr[strspn(actstr, ", ") :]
parsed = True
+ if parse_starts_block(actstr, "ct(", False):
+ actstr = actstr[len("ct(") :]
+ ctact = ovsactions.ctact()
+
+ for scan in (
+ ("commit", "OVS_CT_ATTR_COMMIT", None),
+ ("force_commit", "OVS_CT_ATTR_FORCE_COMMIT", None),
+ ("zone", "OVS_CT_ATTR_ZONE", int),
+ ("mark", "OVS_CT_ATTR_MARK", int),
+ ("helper", "OVS_CT_ATTR_HELPER", lambda x, y: str(x)),
+ ("timeout", "OVS_CT_ATTR_TIMEOUT", lambda x, y: str(x)),
+ ):
+ if actstr.startswith(scan[0]):
+ actstr = actstr[len(scan[0]) :]
+ if scan[2] is not None:
+ if actstr[0] != "=":
+ raise ValueError("Invalid ct attr")
+ actstr = actstr[1:]
+ pos = strcspn(actstr, ",)")
+ datum = scan[2](actstr[:pos], 0)
+ ctact["attrs"].append([scan[1], datum])
+ actstr = actstr[pos:]
+ else:
+ ctact["attrs"].append([scan[1], None])
+ actstr = actstr[strspn(actstr, ", ") :]
+
+ self["attrs"].append(["OVS_ACTION_ATTR_CT", ctact])
+ parsed = True
+
+ actstr = actstr[strspn(actstr, "), ") :]
if not parsed:
raise ValueError("Action str: '%s' not supported" % actstr)
--
2.40.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [ovs-dev] [PATCH v2 net-next 4/5] selftests: openvswitch: add basic ct test case parsing
2023-07-28 11:59 ` [PATCH v2 net-next 4/5] selftests: openvswitch: add basic ct test case parsing Aaron Conole
@ 2023-07-28 15:08 ` Adrian Moreno
0 siblings, 0 replies; 12+ messages in thread
From: Adrian Moreno @ 2023-07-28 15:08 UTC (permalink / raw)
To: Aaron Conole, netdev
Cc: dev, linux-kernel, Ilya Maximets, Eric Dumazet, linux-kselftest,
Jakub Kicinski, Paolo Abeni, Shuah Khan, David S. Miller
On 7/28/23 13:59, Aaron Conole wrote:
> Forwarding via ct() action is an important use case for openvswitch, but
> generally would require using a full ovs-vswitchd to get working. Add a
> ct action parser for basic ct test case.
>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
Reviewed-by: Adrian Moreno <amorenoz@redhat.com>
> ---
> NOTE: 3 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 | 68 +++++++++++++++++++
> .../selftests/net/openvswitch/ovs-dpctl.py | 39 +++++++++++
> 2 files changed, 107 insertions(+)
>
> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> index 5d60a9466dab..40a66c72af0f 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
> + ct_connect_v4 ip4-ct-xon: Basic ipv4 tcp connection using ct
> 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"
> @@ -193,6 +194,73 @@ test_arp_ping () {
> return 0
> }
>
> +# ct_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_ct_connect_v4 () {
> +
> + which nc >/dev/null 2>/dev/null || return $ksft_skip
> +
> + sbx_add "test_ct_connect_v4" || return $?
> +
> + ovs_add_dp "test_ct_connect_v4" ct4 || return 1
> + info "create namespaces"
> + for ns in client server; do
> + ovs_add_netns_and_veths "test_ct_connect_v4" "ct4" "$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_ct_connect_v4" ct4 \
> + 'in_port(1),eth(),eth_type(0x0806),arp()' '2' || return 1
> + ovs_add_flow "test_ct_connect_v4" ct4 \
> + 'in_port(2),eth(),eth_type(0x0806),arp()' '1' || return 1
> + ovs_add_flow "test_ct_connect_v4" ct4 \
> + 'ct_state(-trk),eth(),eth_type(0x0800),ipv4()' \
> + 'ct(commit),recirc(0x1)' || return 1
> + ovs_add_flow "test_ct_connect_v4" ct4 \
> + 'recirc_id(0x1),ct_state(+trk+new),in_port(1),eth(),eth_type(0x0800),ipv4(src=172.31.110.10)' \
> + '2' || return 1
> + ovs_add_flow "test_ct_connect_v4" ct4 \
> + 'recirc_id(0x1),ct_state(+trk+est),in_port(1),eth(),eth_type(0x0800),ipv4(src=172.31.110.10)' \
> + '2' || return 1
> + ovs_add_flow "test_ct_connect_v4" ct4 \
> + 'recirc_id(0x1),ct_state(+trk+est),in_port(2),eth(),eth_type(0x0800),ipv4(dst=172.31.110.10)' \
> + '1' || return 1
> + ovs_add_flow "test_ct_connect_v4" ct4 \
> + 'recirc_id(0x1),ct_state(+trk+inv),eth(),eth_type(0x0800),ipv4()' 'drop' || \
> + return 1
> +
> + # do a ping
> + ovs_sbx "test_ct_connect_v4" ip netns exec client ping 172.31.110.20 -c 3 || return 1
> +
> + # create an echo server in 'server'
> + echo "server" | \
> + ovs_netns_spawn_daemon "test_ct_connect_v4" "server" \
> + nc -lvnp 4443
> + ovs_sbx "test_ct_connect_v4" ip netns exec client nc -i 1 -zv 172.31.110.20 4443 || return 1
> +
> + # Now test in the other direction (should fail)
> + echo "client" | \
> + ovs_netns_spawn_daemon "test_ct_connect_v4" "client" \
> + nc -lvnp 4443
> + ovs_sbx "test_ct_connect_v4" ip netns exec client nc -i 1 -zv 172.31.110.10 4443
> + if [ $? == 0 ]; then
> + info "ct connect to client was successful"
> + return 1
> + fi
> +
> + info "done..."
> + return 0
> +}
> +
> # connect_v4 test
> # - client has 1500 byte MTU
> # - server has 1500 byte MTU
> diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> index 2b869e89c51d..6e258ab9e635 100644
> --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> @@ -62,6 +62,15 @@ def macstr(mac):
> return outstr
>
>
> +def strcspn(str1, str2):
> + tot = 0
> + for char in str1:
> + if str2.find(char) != -1:
> + return tot
> + tot += 1
> + return tot
> +
> +
> def strspn(str1, str2):
> tot = 0
> for char in str1:
> @@ -496,6 +505,36 @@ class ovsactions(nla):
> actstr = actstr[strspn(actstr, ", ") :]
> parsed = True
>
> + if parse_starts_block(actstr, "ct(", False):
> + actstr = actstr[len("ct(") :]
> + ctact = ovsactions.ctact()
> +
> + for scan in (
> + ("commit", "OVS_CT_ATTR_COMMIT", None),
> + ("force_commit", "OVS_CT_ATTR_FORCE_COMMIT", None),
> + ("zone", "OVS_CT_ATTR_ZONE", int),
> + ("mark", "OVS_CT_ATTR_MARK", int),
> + ("helper", "OVS_CT_ATTR_HELPER", lambda x, y: str(x)),
> + ("timeout", "OVS_CT_ATTR_TIMEOUT", lambda x, y: str(x)),
> + ):
> + if actstr.startswith(scan[0]):
> + actstr = actstr[len(scan[0]) :]
> + if scan[2] is not None:
> + if actstr[0] != "=":
> + raise ValueError("Invalid ct attr")
> + actstr = actstr[1:]
> + pos = strcspn(actstr, ",)")
> + datum = scan[2](actstr[:pos], 0)
> + ctact["attrs"].append([scan[1], datum])
> + actstr = actstr[pos:]
> + else:
> + ctact["attrs"].append([scan[1], None])
> + actstr = actstr[strspn(actstr, ", ") :]
> +
> + self["attrs"].append(["OVS_ACTION_ATTR_CT", ctact])
> + parsed = True
> +
> + actstr = actstr[strspn(actstr, "), ") :]
> if not parsed:
> raise ValueError("Action str: '%s' not supported" % actstr)
>
--
Adrián Moreno
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 net-next 5/5] selftests: openvswitch: add ct-nat test case with ipv4
2023-07-28 11:59 [PATCH v2 net-next 0/5] selftests: openvswitch: add flow programming cases Aaron Conole
` (3 preceding siblings ...)
2023-07-28 11:59 ` [PATCH v2 net-next 4/5] selftests: openvswitch: add basic ct test case parsing Aaron Conole
@ 2023-07-28 11:59 ` Aaron Conole
2023-07-28 14:31 ` Adrian Moreno
4 siblings, 1 reply; 12+ messages in thread
From: Aaron Conole @ 2023-07-28 11:59 UTC (permalink / raw)
To: netdev
Cc: dev, linux-kselftest, linux-kernel, Shuah Khan, Paolo Abeni,
Jakub Kicinski, Eric Dumazet, David S. Miller, Pravin B Shelar,
Ilya Maximets
Building on the previous work, add a very simplistic NAT case
using ipv4. This just tests dnat transformation
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
.../selftests/net/openvswitch/openvswitch.sh | 64 ++++++++++++++++
.../selftests/net/openvswitch/ovs-dpctl.py | 75 +++++++++++++++++++
2 files changed, 139 insertions(+)
diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index 40a66c72af0f..dced4f612a78 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 6e258ab9e635..258c9ef263d9 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -530,6 +530,81 @@ 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
+ )
+
+ # [XXXX:YYY::Z]:123
+ # following RFC 3986
+ # More complete parsing, ala RFC5952 isn't
+ # supported.
+ if actstr.startswith("]"):
+ actstr = actstr[1:]
+ if ip_block_min is not None and \
+ ip_block_min.startswith("["):
+ ip_block_min = ip_block_min[1:]
+ if ip_block_max is not None and \
+ ip_block_max.startswith("["):
+ ip_block_max = ip_block_max[1:]
+
+ 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 (
+ ("persist", "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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 net-next 5/5] selftests: openvswitch: add ct-nat test case with ipv4
2023-07-28 11:59 ` [PATCH v2 net-next 5/5] selftests: openvswitch: add ct-nat test case with ipv4 Aaron Conole
@ 2023-07-28 14:31 ` Adrian Moreno
2023-07-31 18:53 ` Aaron Conole
0 siblings, 1 reply; 12+ messages in thread
From: Adrian Moreno @ 2023-07-28 14:31 UTC (permalink / raw)
To: Aaron Conole, netdev
Cc: dev, linux-kselftest, linux-kernel, Shuah Khan, Paolo Abeni,
Jakub Kicinski, Eric Dumazet, David S. Miller, Pravin B Shelar,
Ilya Maximets
On 7/28/23 13:59, Aaron Conole wrote:
> Building on the previous work, add a very simplistic NAT case
> using ipv4. This just tests dnat transformation
>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
> .../selftests/net/openvswitch/openvswitch.sh | 64 ++++++++++++++++
> .../selftests/net/openvswitch/ovs-dpctl.py | 75 +++++++++++++++++++
> 2 files changed, 139 insertions(+)
>
> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> index 40a66c72af0f..dced4f612a78 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 6e258ab9e635..258c9ef263d9 100644
> --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> @@ -530,6 +530,81 @@ 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
> + )
> +
> + # [XXXX:YYY::Z]:123
> + # following RFC 3986
> + # More complete parsing, ala RFC5952 isn't
> + # supported.
> + if actstr.startswith("]"):
> + actstr = actstr[1:]
> + if ip_block_min is not None and \
> + ip_block_min.startswith("["):
> + ip_block_min = ip_block_min[1:]
> + if ip_block_max is not None and \
> + ip_block_max.startswith("["):
> + ip_block_max = ip_block_max[1:]
> +
> + actstr, proto_min = parse_extract_field(
> + actstr, ":", "(\d+)", int, False
> + )
> + actstr, proto_max = parse_extract_field(
> + actstr, "-", "(\d+)", int, False
> + )
I'm still struggling to make this part work:
On the one hand, ipv6 seems not fully supported by ovs-dpctl.py. If I try adding
an ipv6 flow I end up needing to add a function such as as the following and use
it to parse "ipv6()" field:
def convert_ipv6(data):
ip, _, mask = data.partition('/')
max_ip = pow(2, 128) - 1
if not ip:
ip = mask = 0
elif not mask:
mask = max
elif mask.isdigit():
mask = (max_ip << (128 - int(mask))) & max_ip
return ipaddress.IPv6Address(ip).packed, ipaddress.IPv6Address(mask).packed
OTOH, trying to support ipv6 makes ct ip/port range parsing more complex, for
instance, this action: "ct(nat(src=10.0.0.240-10.0.0.254:32768-65535))"
fails, because it's parsed as:
ip_block_min = 10.0.0.240
ip_block_max = 10.0.0.254:32768
proto_min = None
proto_max = 65535
I would say we could drop ipv6 support for nat() action, making it simpler to
parse or first detect if we're parsing ipv4 or ipv6 and use appropriate regexp
on each case. E.g:
https://github.com/openvswitch/ovs/blob/d460c473ebf9e9ab16da44cbfbb13a4911352195/python/ovs/flow/decoders.py#L430-L486
Another approach would be to stop trying to be human friendly and use an easier
to parse syntax, something closer to key-value, e.g:
"ct(ip_block_min=10.0.0.240, ip_block_max=10.0.0.254, proto_min=32768,
proto_max=65535)". It'd be more verbose and not compatible with ovs tooling but
this is a testing tool afterall. WDYT?
> +
> + 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 (
> + ("persist", "OVS_NAT_ATTR_PERSISTENT"),
odp-util.c defines this flag as "persistent", not sure if you intend to keep it
compatible at all.
> + ("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
--
Adrián Moreno
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 net-next 5/5] selftests: openvswitch: add ct-nat test case with ipv4
2023-07-28 14:31 ` Adrian Moreno
@ 2023-07-31 18:53 ` Aaron Conole
0 siblings, 0 replies; 12+ messages in thread
From: Aaron Conole @ 2023-07-31 18:53 UTC (permalink / raw)
To: Adrian Moreno
Cc: netdev, dev, linux-kselftest, linux-kernel, Shuah Khan,
Paolo Abeni, Jakub Kicinski, Eric Dumazet, David S. Miller,
Pravin B Shelar, Ilya Maximets
Adrian Moreno <amorenoz@redhat.com> writes:
> On 7/28/23 13:59, Aaron Conole wrote:
>> Building on the previous work, add a very simplistic NAT case
>> using ipv4. This just tests dnat transformation
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>> .../selftests/net/openvswitch/openvswitch.sh | 64 ++++++++++++++++
>> .../selftests/net/openvswitch/ovs-dpctl.py | 75 +++++++++++++++++++
>> 2 files changed, 139 insertions(+)
>> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh
>> b/tools/testing/selftests/net/openvswitch/openvswitch.sh
>> index 40a66c72af0f..dced4f612a78 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 6e258ab9e635..258c9ef263d9 100644
>> --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
>> +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
>> @@ -530,6 +530,81 @@ 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
>> + )
>> +
>> + # [XXXX:YYY::Z]:123
>> + # following RFC 3986
>> + # More complete parsing, ala RFC5952 isn't
>> + # supported.
>> + if actstr.startswith("]"):
>> + actstr = actstr[1:]
>> + if ip_block_min is not None and \
>> + ip_block_min.startswith("["):
>> + ip_block_min = ip_block_min[1:]
>> + if ip_block_max is not None and \
>> + ip_block_max.startswith("["):
>> + ip_block_max = ip_block_max[1:]
>> +
>> + actstr, proto_min = parse_extract_field(
>> + actstr, ":", "(\d+)", int, False
>> + )
>> + actstr, proto_max = parse_extract_field(
>> + actstr, "-", "(\d+)", int, False
>> + )
>
> I'm still struggling to make this part work:
> On the one hand, ipv6 seems not fully supported by ovs-dpctl.py. If I
> try adding an ipv6 flow I end up needing to add a function such as as
> the following and use it to parse "ipv6()" field:
Let's just drop the ipv6 stuff from the NAT action support, and we can
add it later when I address the flow key side of it.
> def convert_ipv6(data):
> ip, _, mask = data.partition('/')
> max_ip = pow(2, 128) - 1
>
> if not ip:
> ip = mask = 0
> elif not mask:
> mask = max
> elif mask.isdigit():
> mask = (max_ip << (128 - int(mask))) & max_ip
>
> return ipaddress.IPv6Address(ip).packed, ipaddress.IPv6Address(mask).packed
>
> OTOH, trying to support ipv6 makes ct ip/port range parsing more
> complex, for instance, this action:
> "ct(nat(src=10.0.0.240-10.0.0.254:32768-65535))"
>
> fails, because it's parsed as:
> ip_block_min = 10.0.0.240
> ip_block_max = 10.0.0.254:32768
> proto_min = None
> proto_max = 65535
>
> I would say we could drop ipv6 support for nat() action, making it
> simpler to parse or first detect if we're parsing ipv4 or ipv6 and use
> appropriate regexp on each case. E.g:
> https://github.com/openvswitch/ovs/blob/d460c473ebf9e9ab16da44cbfbb13a4911352195/python/ovs/flow/decoders.py#L430-L486
ACK
> Another approach would be to stop trying to be human friendly and use
> an easier to parse syntax, something closer to key-value, e.g:
> "ct(ip_block_min=10.0.0.240, ip_block_max=10.0.0.254, proto_min=32768,
> proto_max=65535)". It'd be more verbose and not compatible with ovs
> tooling but this is a testing tool afterall. WDYT?
I do like that we can use the flow output from the running ovs-vswitchd
to generate test cases. I'd like, as much as possible, to try and keep
it compatible.
>> +
>> + 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 (
>> + ("persist", "OVS_NAT_ATTR_PERSISTENT"),
>
> odp-util.c defines this flag as "persistent", not sure if you intend
> to keep it compatible at all.
I will adjust when I resubmit.
>> + ("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
^ permalink raw reply [flat|nested] 12+ messages in thread