Messages in this thread | | | Date | Fri, 28 Jul 2023 17:13:06 +0200 | Subject | Re: [ovs-dev] [PATCH v2 net-next 1/5] selftests: openvswitch: add an initial flow programming case | From | Adrian Moreno <> |
| |
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
| |