Adrian Moreno <amorenoz@xxxxxxxxxx> writes:
> On 6/28/23 18:27, 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@xxxxxxxxxx>
>> ---
>> 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 | 408 ++++++++++++++++++
>> 2 files changed, 459 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..799bfb3064b90 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,61 @@ 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...")
>> +
>> + 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 +434,52 @@ class ovsactions(nla):
>> return print_str
>> + def parse(self, actstr):
>> + parsed = False
>> + while len(actstr) != 0:
>> + 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"
>> + )
>> + actstr = actstr[strspn(actstr, ", ") :]
>> + 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,
>> + )
>> + actstr = actstr[strspn(actstr, "), ") :]
>> + 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 +549,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 +1059,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 +1672,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 +1909,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 +1989,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"):
>
> These checks on the attributes means every command must have
> attributes with different names. So if we then add del-br it must not
> have an attribute called "flbr". We could rename it to "fladdbr"
> (following the other commands) or match on the subcommand name, which
> would be cleaner imho and less error, e.g: all the datapath attributes
> can be called the same (see below).
I agree we can adjust this to use the subparser dest= and just check for
the specific command passed. I will do that in a different series, just
to keep it as a separate thing.
>> + rep = ovsdp.info(args.flbr, 0)
>> + if rep is None:
>> + print("DP '%s' not found." % args.dumpdp)
>
> "dumpdp" is not an attribute of this subcommand.
I'll fix this.
>> + return 1
>> + flow = OvsFlow.ovs_flow_msg()
>> + flow.parse(args.flow, args.acts, rep["dpifindex"])
>> + ovsflow.add_flow(rep["dpifindex"], flow)
>> return 0
>>
