| Message ID | 20211122112256.2011194-8-amorenoz@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | python: add flow parsing library | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
On 22 Nov 2021, at 12:22, Adrian Moreno wrote: > Introduce OFPFlow class and all its decoders. > > Most of the decoders are generic (from decoders.py). Some have special > syntax and need a specific implementation. > > Decoders for nat are moved to the common decoders.py because it's syntax > is shared with other types of flows (e.g: dpif flows). > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- > python/automake.mk | 4 +- > python/ovs/flows/decoders.py | 93 ++++++++ > python/ovs/flows/ofp.py | 400 +++++++++++++++++++++++++++++++++++ > python/ovs/flows/ofp_act.py | 233 ++++++++++++++++++++ > 4 files changed, 729 insertions(+), 1 deletion(-) > create mode 100644 python/ovs/flows/ofp.py > create mode 100644 python/ovs/flows/ofp_act.py > > diff --git a/python/automake.mk b/python/automake.mk > index 136da26bd..d1464d7f6 100644 > --- a/python/automake.mk > +++ b/python/automake.mk > @@ -46,7 +46,9 @@ ovs_pyfiles = \ > python/ovs/flows/decoders.py \ > python/ovs/flows/kv.py \ > python/ovs/flows/list.py \ > - python/ovs/flows/flow.py > + python/ovs/flows/flow.py \ > + python/ovs/flows/ofp.py \ > + python/ovs/flows/ofp_act.py > > # These python files are used at build time but not runtime, > # so they are not installed. > diff --git a/python/ovs/flows/decoders.py b/python/ovs/flows/decoders.py > index bf7a94ae8..3def9f279 100644 > --- a/python/ovs/flows/decoders.py > +++ b/python/ovs/flows/decoders.py > @@ -6,6 +6,7 @@ object. > """ > > import netaddr > +import re > > > class Decoder: > @@ -358,3 +359,95 @@ class IPMask(Decoder): > > def to_json(self): > return str(self) > + > + > +def decode_free_output(value): > + """Decodes the output value when found free > + (without the 'output' keyword)""" This is the last time I’ll add it for the remaining patches to be reviewed. But please check that all sentences start with a Captical and end with a dot. > + try: > + return "output", {"port": int(value)} Any reason not to try to remove the “ here? return "output", {"port": int(value.strip('"'))} > + except ValueError: > + return "output", {"port": value.strip('"')} > + > + > +ipv4 = r"[\d\.]+" Should this not try to match at least 1 and at most 3 numbers? Maybe something like [\d{1,3}\.]+. > +ipv4_capture = r"({ipv4})".format(ipv4=ipv4) > +ipv6 = r"[\w:]+" > +ipv6_capture = r"(?:\[*)?({ipv6})(?:\]*)?".format(ipv6=ipv6) > +port_range = r":(\d+)(?:-(\d+))?" Same as for IPv4, guess {0, 4} > +ip_range_regexp = r"{ip_cap}(?:-{ip_cap})?(?:{port_range})?" > +ipv4_port_regex = re.compile( > + ip_range_regexp.format(ip_cap=ipv4_capture, port_range=port_range) > +) > +ipv6_port_regex = re.compile( > + ip_range_regexp.format(ip_cap=ipv6_capture, port_range=port_range) > +) > + > + > +def decode_ip_port_range(value): > + """ > + Decodes an IP and port range: > + {ip_start}-{ip-end}:{port_start}-{port_end} > + > + IPv6 addresses are surrounded by "[" and "]" if port ranges are also > + present > + > + Returns the following dictionary: > + { > + "addrs": { > + "start": {ip_start} > + "end": {ip_end} > + } > + "ports": { > + "start": {port_start}, > + "end": {port_end} > + } > + (the "ports" key might be omitted) > + """ > + if value.count(":") > 1: > + match = ipv6_port_regex.match(value) > + else: > + match = ipv4_port_regex.match(value) Do we need to check if there is a match? It might be good to error out if the string supplied does not match the expected input? Or if garbage is following the part we could decode? > + > + ip_start = match.group(1) > + ip_end = match.group(2) > + port_start = match.group(3) > + port_end = match.group(4) > + > + result = { > + "addrs": { > + "start": netaddr.IPAddress(ip_start), > + "end": netaddr.IPAddress(ip_end or ip_start), > + } > + } > + if port_start: > + result["ports"] = { > + "start": int(port_start), > + "end": int(port_end or port_start), > + } > + > + return result > + > + > +def decode_nat(value): > + """Decodes the 'nat' keyword of the ct action""" Can we add an example of the nat format? > + if not value: > + return True Why returning True is no data is present? I would expect None. > + > + result = dict() > + type_parts = value.split("=") > + result["type"] = type_parts[0] > + > + if len(type_parts) > 1: > + value_parts = type_parts[1].split(",") > + if len(type_parts) != 2: > + raise ValueError("Malformed nat action: %s" % value) > + > + ip_port_range = decode_ip_port_range(value_parts[0]) > + > + result = {"type": type_parts[0], **ip_port_range} > + > + for flag in value_parts[1:]: > + result[flag] = True > + > + return result > diff --git a/python/ovs/flows/ofp.py b/python/ovs/flows/ofp.py > new file mode 100644 > index 000000000..e56b08967 > --- /dev/null > +++ b/python/ovs/flows/ofp.py > @@ -0,0 +1,400 @@ > +""" Defines the parsers needed to parse ofproto flows > +""" > + > +import functools > + > +from ovs.flows.kv import KVParser, KVDecoders, nested_kv_decoder > +from ovs.flows.ofp_fields import field_decoders > +from ovs.flows.flow import Flow, Section > +from ovs.flows.list import ListDecoders, nested_list_decoder > +from ovs.flows.decoders import ( > + decode_default, > + decode_flag, > + decode_int, > + decode_time, > + decode_mask, > + IPMask, > + EthMask, > + decode_free_output, > + decode_nat, > +) > +from ovs.flows.ofp_act import ( > + decode_output, > + decode_field, > + decode_controller, > + decode_bundle, > + decode_bundle_load, > + decode_encap_ethernet, > + decode_load_field, > + decode_set_field, > + decode_move_field, > + decode_dec_ttl, > + decode_chk_pkt_larger, > + decode_zone, > + decode_exec, > + decode_learn, > +) > + > + > +class OFPFlow(Flow): > + """OFPFLow represents an OpenFlow Flow""" Can we describe args; sections, orig, and id? Guess just copy them from Flow() > + > + def __init__(self, sections, orig="", id=None): > + """Constructor""" > + super(OFPFlow, self).__init__(sections, orig, id) > + > + def __str__(self): > + if self._orig: > + return self._orig > + else: > + return self.to_string() > + > + def to_string(self): > + """Print a text representation of the flow""" > + string = "Info: {}\n" + self.info > + string += "Match : {}\n" + self.match > + string += "Actions: {}\n " + self.actions It’s getting late, so maybe I should stop reviewing. But I’m wondering where self.info/self.match/self.actions are set? Also, _orig is only None, if explicitly set during initialization, else it’s “”. > + return string > + > + > +class OFPFlowFactory: Do we need OFPFlowFactory(object):, some times flake reports this as H238. But might no longer be valid with the latest python3. > + """OpenFlow Flow Factory is a class capable of creating OFPFLow objects""" > + > + def __init__(self): > + self.info_decoders = self._info_decoders() > + self.match_decoders = KVDecoders( The name suggests that we return KVDecoders() as the __info_decoders() does, I think all three should return the same. > + {**self._field_decoders(), **self._flow_match_decoders()} > + ) > + self.act_decoders = self._act_decoders() > + > + def from_string(self, ofp_string, id=None): > + """Parse a ofproto flow string > + > + The string is expected to have the follwoing format: > + [flow data] [match] actions=[actions] Should we maybe add an example, as it’s not that straight forward, for example: cookie=0x0, duration=1.001s, table=0, n_packets=1, n_bytes=64, ip,in_port=enp5s0f0 actions=output:vnet0 [flow data], [match] actions=[actions] > + > + :param ofp_string: a ofproto string as dumped by ovs-ofctl tool An ofproto string, as dumped by the ovs-ofctl tool. > + :type ofp_string: str > + > + :return: an OFPFlow with the content of the flow string > + :rtype: OFPFlow > + """ > + if " reply " in ofp_string: > + return None > + > + sections = list() > + parts = ofp_string.split("actions=") > + if len(parts) != 2: > + raise ValueError("malformed ofproto flow: %s" % ofp_string) > + > + actions = parts[1] > + > + field_parts = parts[0].rstrip(" ").rpartition(" ") > + if len(field_parts) != 3: > + raise ValueError("malformed ofproto flow: %s" % ofp_string) > + > + info = field_parts[0] > + match = field_parts[2] > + > + iparser = KVParser(self.info_decoders) > + iparser.parse(info) > + isection = Section( > + name="info", > + pos=ofp_string.find(info), > + string=info, > + data=iparser.kv(), > + ) > + sections.append(isection) > + > + mparser = KVParser(self.match_decoders) > + mparser.parse(match) > + msection = Section( > + name="match", > + pos=ofp_string.find(match), > + string=match, > + data=mparser.kv(), > + ) > + sections.append(msection) > + > + aparser = KVParser(self.act_decoders) > + aparser.parse(actions) > + asection = Section( > + name="actions", > + pos=ofp_string.find(actions), > + string=actions, > + data=aparser.kv(), > + is_list=True, > + ) > + sections.append(asection) > + > + return OFPFlow(sections, ofp_string, id) > + > + @classmethod > + def _info_decoders(cls): > + """Generate the match decoders""" > + info = { > + "table": decode_int, > + "duration": decode_time, > + "n_packet": decode_int, > + "n_bytes": decode_int, > + "cookie": decode_int, > + "idle_timeout": decode_time, > + "hard_timeout": decode_time, > + "hard_age": decode_time, > + } > + return KVDecoders(info) > + > + @classmethod > + def _flow_match_decoders(cls): > + """Returns the decoders for key-values that are part of the flow match > + but not a flow field""" > + return { > + "priority": decode_int, > + } Guess this should be return KVDecoders({ "priority": decode_int, })) > + > + @classmethod > + def _field_decoders(cls): > + shorthands = [ > + "eth", > + "ip", > + "ipv6", > + "icmp", > + "icmp6", > + "tcp", > + "tcp6", > + "udp", > + "udp6", > + "sctp", > + "arp", > + "rarp", > + "mpls", > + "mplsm", > + ] > + > + fields = {**field_decoders, **{key: decode_flag for key in shorthands}} > + > + # vlan_vid field is special. Although it is technically 12 bit wide, > + # bit 12 is allowed to be set to 1 to indicate that the vlan header is > + # present (see section VLAN FIELDS in > + # http://www.openvswitch.org/support/dist-docs/ovs-fields.7.txt) > + # Therefore, override the generated vlan_vid field size > + fields["vlan_vid"] = decode_mask(13) > + return fields This should return KVDecoders(fields). Guess this is true for all class methods below! Or, you should choose to always return the list. But not mix the two. > + > + @classmethod > + def _output_actions_decoders(cls): > + """Returns the decoders for the output actions""" If you have them, I think it might be useful, to add some example strings to the individual decoders. This way, it's easy to see what they intend to decode. > + return { > + "output": decode_output, > + "drop": decode_flag, > + "controller": decode_controller, > + "enqueue": nested_list_decoder( > + ListDecoders([("port", decode_default), ("queue", int)]), > + delims=[",", ":"], > + ), > + "bundle": decode_bundle, > + "bundle_load": decode_bundle_load, > + "group": decode_default, > + } > + > + @classmethod > + def _encap_actions_decoders(cls): > + """Returns the decoders for the encap actions""" > + > + return { > + "pop_vlan": decode_flag, > + "strip_vlan": decode_flag, > + "push_vlan": decode_default, > + "decap": decode_flag, > + "encap": nested_kv_decoder( > + KVDecoders( > + { > + "nsh": nested_kv_decoder( > + KVDecoders( > + { > + "md_type": decode_default, > + "tlv": nested_list_decoder( > + ListDecoders( > + [ > + ("class", decode_int), > + ("type", decode_int), > + ("value", decode_int), > + ] > + ) > + ), > + } > + ) > + ), > + }, > + default=None, > + default_free=decode_encap_ethernet, > + ) > + ), > + } > + > + @classmethod > + def _field_action_decoders(cls): > + """Returns the decoders for the field modification actions""" > + # Field modification actions > + field_default_decoders = [ > + "set_mpls_label", > + "set_mpls_tc", > + "set_mpls_ttl", > + "mod_nw_tos", > + "mod_nw_ecn", > + "mod_tcp_src", > + "mod_tcp_dst", > + ] > + return { > + "load": decode_load_field, > + "set_field": functools.partial( > + decode_set_field, KVDecoders(cls._field_decoders()) > + ), > + "move": decode_move_field, > + "mod_dl_dst": EthMask, > + "mod_dl_src": EthMask, > + "mod_nw_dst": IPMask, > + "mod_nw_src": IPMask, > + "dec_ttl": decode_dec_ttl, > + "dec_mpls_ttl": decode_flag, > + "dec_nsh_ttl": decode_flag, > + "check_pkt_larger": decode_chk_pkt_larger, > + **{field: decode_default for field in field_default_decoders}, > + } > + > + @classmethod > + def _meta_action_decoders(cls): > + """Returns the decoders for the metadata actions""" > + meta_default_decoders = ["set_tunnel", "set_tunnel64", "set_queue"] > + return { > + "pop_queue": decode_flag, > + **{field: decode_default for field in meta_default_decoders}, > + } > + > + @classmethod > + def _fw_action_decoders(cls): > + """Returns the decoders for the Firewalling actions""" > + return { > + "ct": nested_kv_decoder( > + KVDecoders( > + { > + "commit": decode_flag, > + "zone": decode_zone, > + "table": decode_int, > + "nat": decode_nat, > + "force": decode_flag, > + "exec": functools.partial( > + decode_exec, > + KVDecoders( > + { > + **cls._encap_actions_decoders(), > + **cls._field_action_decoders(), > + **cls._meta_action_decoders(), > + } > + ), > + ), > + "alg": decode_default, > + } > + ) > + ), > + "ct_clear": decode_flag, > + } > + > + @classmethod > + def _control_action_decoders(cls): > + return { > + "resubmit": nested_list_decoder( > + ListDecoders( > + [ > + ("port", decode_default), > + ("table", decode_int), > + ("ct", decode_flag), > + ] > + ) > + ), > + "push": decode_field, > + "pop": decode_field, > + "exit": decode_flag, > + "multipath": nested_list_decoder( > + ListDecoders( > + [ > + ("fields", decode_default), > + ("basis", decode_int), > + ("algorithm", decode_default), > + ("n_links", decode_int), > + ("arg", decode_int), > + ("dst", decode_field), > + ] > + ) > + ), > + } > + > + @classmethod > + def _clone_actions_decoders(cls, action_decoders): > + """Generate the decoders for clone actions > + > + Args: > + action_decoders (dict): The decoders of the supported nested > + actions > + """ > + return { > + "learn": decode_learn( > + { > + **action_decoders, > + "fin_timeout": nested_kv_decoder( > + KVDecoders( > + { > + "idle_timeout": decode_time, > + "hard_timeout": decode_time, > + } > + ) > + ), > + } > + ), > + "clone": functools.partial( > + decode_exec, KVDecoders(action_decoders) > + ), > + } > + > + @classmethod > + def _other_action_decoders(cls): > + """Recoders for other actions (see man(7) ovs-actions)""" > + return { > + "conjunction": nested_list_decoder( > + ListDecoders( > + [("id", decode_int), ("k", decode_int), ("n", decode_int)] > + ), > + delims=[",", "/"], > + ), > + "note": decode_default, > + "sample": nested_kv_decoder( > + KVDecoders( > + { > + "probability": decode_int, > + "collector_set_id": decode_int, > + "obs_domain_id": decode_int, > + "obs_point_id": decode_int, > + "sampling_port": decode_default, > + "ingress": decode_flag, > + "egress": decode_flag, > + } > + ) > + ), > + } > + > + @classmethod > + def _act_decoders(cls): > + """Generate the actions decoders""" > + > + actions = { > + **cls._output_actions_decoders(), > + **cls._encap_actions_decoders(), > + **cls._field_action_decoders(), > + **cls._meta_action_decoders(), > + **cls._fw_action_decoders(), > + **cls._control_action_decoders(), > + **cls._other_action_decoders(), > + } > + clone_actions = cls._clone_actions_decoders(actions) > + actions.update(clone_actions) > + return KVDecoders(actions, default_free=decode_free_output) > diff --git a/python/ovs/flows/ofp_act.py b/python/ovs/flows/ofp_act.py > new file mode 100644 > index 000000000..bc6574999 > --- /dev/null > +++ b/python/ovs/flows/ofp_act.py > @@ -0,0 +1,233 @@ > +""" Defines decoders for openflow actions > +""" > + > +import functools > + > +from ovs.flows.kv import nested_kv_decoder, KVDecoders, KeyValue, KVParser > +from ovs.flows.decoders import ( > + decode_default, > + decode_time, > + decode_flag, > + decode_int, > +) > +from ovs.flows.ofp_fields import field_decoders > + > + > +def decode_output(value): > + """Decodes the output value > + > + Does not support field specification > + """ > + if len(value.split(",")) > 1: > + return nested_kv_decoder()(value) > + try: > + return {"port": int(value)} > + except ValueError: > + return {"port": value.strip('"')} > + > + > +def decode_controller(value): > + """Decodes the controller action""" > + if not value: > + return KeyValue("output", "controller") > + else: > + # Try controller:max_len > + try: > + max_len = int(value) > + return { > + "max_len": max_len, > + } > + except ValueError: > + pass > + # controller(key[=val], ...) > + return nested_kv_decoder()(value) > + > + > +def decode_bundle_load(value): > + return decode_bundle(value, True) > + > + > +def decode_bundle(value, load=False): > + """Decode bundle action""" > + result = {} > + keys = ["fields", "basis", "algorithm", "ofport"] > + if load: > + keys.append("dst") > + > + for key in keys: > + parts = value.partition(",") > + nvalue = parts[0] > + value = parts[2] > + if key == "ofport": > + continue > + result[key] = decode_default(nvalue) > + > + # Handle members: > + mvalues = value.split("members:") > + result["members"] = [int(port) for port in mvalues[1].split(",")] > + return result > + > + > +def decode_encap_ethernet(value): > + """Decodes encap ethernet value""" Was confused with ethertype here, but I think ethernet does not have a value does it? Just encap(ethernet)? > + return "ethernet", int(value, 0) > + > + > +def decode_field(value): > + """Decodes a field as defined in the 'Field Specification' of the actions > + man page: http://www.openvswitch.org/support/dist-docs/ovs-actions.7.txt > + """ > + parts = value.strip("]\n\r").split("[") > + result = { > + "field": parts[0], > + } > + > + if len(parts) > 1 and parts[1]: > + field_range = parts[1].split("..") > + start = field_range[0] > + end = field_range[1] if len(field_range) > 1 else start > + if start: > + result["start"] = int(start) > + if end: > + result["end"] = int(end) > + > + return result > + > + > +def decode_load_field(value): > + """Decodes 'load:value->dst' actions""" > + parts = value.split("->") > + if len(parts) != 2: > + raise ValueError("Malformed load action : %s" % value) > + > + # If the load action is performed within a learn() action, > + # The value can be specified as another field. > + try: > + return {"value": int(parts[0], 0), "dst": decode_field(parts[1])} > + except ValueError: > + return {"src": decode_field(parts[0]), "dst": decode_field(parts[1])} > + > + > +def decode_set_field(field_decoders, value): > + """Decodes 'set_field:value/mask->dst' actions > + > + The value is decoded by field_decoders which is a KVDecoders instance > + Args: > + field_decoders > + """ > + parts = value.split("->") > + if len(parts) != 2: > + raise ValueError("Malformed set_field action : %s" % value) > + > + val = parts[0] > + dst = parts[1] > + > + val_result = field_decoders.decode(dst, val) > + > + return { > + "value": {val_result[0]: val_result[1]}, > + "dst": decode_field(dst), > + } > + > + > +def decode_move_field(value): > + """Decodes 'move:src->dst' actions""" > + parts = value.split("->") > + if len(parts) != 2: > + raise ValueError("Malformed move action : %s" % value) > + > + return { > + "src": decode_field(parts[0]), > + "dst": decode_field(parts[1]), > + } > + > + > +def decode_dec_ttl(value): > + """Decodes dec_ttl and dec_ttl(id, id[2], ...) actions""" > + if not value: > + return True > + return [int(idx) for idx in value.split(",")] > + > + > +def decode_chk_pkt_larger(value): > + """Decodes 'check_pkt_larger(pkt_len)->dst' actions""" > + parts = value.split("->") > + if len(parts) != 2: > + raise ValueError("Malformed check_pkt_larger action : %s" % value) > + > + pkt_len = int(parts[0].strip("()")) > + dst = decode_field(parts[1]) > + return {"pkt_len": pkt_len, "dst": dst} > + > + > +# CT decoders > +def decode_zone(value): > + """Decodes the 'zone' keyword of the ct action""" > + try: > + return int(value, 0) > + except ValueError: > + pass > + return decode_field(value) > + > + > +def decode_exec(action_decoders, value): > + """Decodes the 'exec' keyword of the ct action > + > + Args: > + decode_actions (KVDecoders): the decoders to be used to decode the > + nested exec > + value (string): the string to be decoded > + """ > + exec_parser = KVParser(action_decoders) > + exec_parser.parse(value) > + return [{kv.key: kv.value} for kv in exec_parser.kv()] > + > + > +def decode_learn(action_decoders): It’s getting late, and I have a hard time (focussing ;) understanding where the value for this one comes from? I'll pick it up from here when I continue the review. > + """Create the decoder to be used to decode the 'learn' action. > + > + The learn action can include any nested action, therefore we need decoders > + for all possible actions. > + > + Args: > + action_decoders (dict): dictionary of decoders to be used in nested > + action decoding > + > + """ > + > + def decode_learn_field(decoder, value): > + """Generates a decoder to be used for the 'field' argument of the > + 'learn' action. > + > + The field can hold a value that should be decoded, either as a field, > + or as a the value (see man(7) ovs-actions) > + > + Args: > + decoder (callable): The decoder > + > + """ > + if value in field_decoders.keys(): > + # It's a field > + return value > + else: > + return decoder(value) > + > + learn_field_decoders = { > + field: functools.partial(decode_learn_field, decoder) > + for field, decoder in field_decoders.items() > + } > + learn_decoders = { > + **action_decoders, > + **learn_field_decoders, > + "idle_timeout": decode_time, > + "hard_timeout": decode_time, > + "priority": decode_int, > + "cooke": decode_int, > + "send_flow_rem": decode_flag, > + "table": decode_int, > + "delete_learned": decode_flag, > + "limit": decode_int, > + "result_dst": decode_field, > + } > + > + return functools.partial(decode_exec, KVDecoders(learn_decoders)) > -- > 2.31.1
On 12/17/21 18:19, Eelco Chaudron wrote: > > > On 22 Nov 2021, at 12:22, Adrian Moreno wrote: > >> Introduce OFPFlow class and all its decoders. >> >> Most of the decoders are generic (from decoders.py). Some have special >> syntax and need a specific implementation. >> >> Decoders for nat are moved to the common decoders.py because it's syntax >> is shared with other types of flows (e.g: dpif flows). >> >> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >> --- >> python/automake.mk | 4 +- >> python/ovs/flows/decoders.py | 93 ++++++++ >> python/ovs/flows/ofp.py | 400 +++++++++++++++++++++++++++++++++++ >> python/ovs/flows/ofp_act.py | 233 ++++++++++++++++++++ >> 4 files changed, 729 insertions(+), 1 deletion(-) >> create mode 100644 python/ovs/flows/ofp.py >> create mode 100644 python/ovs/flows/ofp_act.py >> >> diff --git a/python/automake.mk b/python/automake.mk >> index 136da26bd..d1464d7f6 100644 >> --- a/python/automake.mk >> +++ b/python/automake.mk >> @@ -46,7 +46,9 @@ ovs_pyfiles = \ >> python/ovs/flows/decoders.py \ >> python/ovs/flows/kv.py \ >> python/ovs/flows/list.py \ >> - python/ovs/flows/flow.py >> + python/ovs/flows/flow.py \ >> + python/ovs/flows/ofp.py \ >> + python/ovs/flows/ofp_act.py >> >> # These python files are used at build time but not runtime, >> # so they are not installed. >> diff --git a/python/ovs/flows/decoders.py b/python/ovs/flows/decoders.py >> index bf7a94ae8..3def9f279 100644 >> --- a/python/ovs/flows/decoders.py >> +++ b/python/ovs/flows/decoders.py >> @@ -6,6 +6,7 @@ object. >> """ >> >> import netaddr >> +import re >> >> >> class Decoder: >> @@ -358,3 +359,95 @@ class IPMask(Decoder): >> >> def to_json(self): >> return str(self) >> + >> + >> +def decode_free_output(value): >> + """Decodes the output value when found free >> + (without the 'output' keyword)""" > > This is the last time I’ll add it for the remaining patches to be reviewed. > But please check that all sentences start with a Captical and end with a dot. > Sure. Sorry about that. >> + try: >> + return "output", {"port": int(value)} > > Any reason not to try to remove the “ here? > return "output", {"port": int(value.strip('"'))} > >> + except ValueError: >> + return "output", {"port": value.strip('"')} >> + >> + >> +ipv4 = r"[\d\.]+" > > Should this not try to match at least 1 and at most 3 numbers? > Maybe something like [\d{1,3}\.]+. > >> +ipv4_capture = r"({ipv4})".format(ipv4=ipv4) >> +ipv6 = r"[\w:]+" >> +ipv6_capture = r"(?:\[*)?({ipv6})(?:\]*)?".format(ipv6=ipv6) >> +port_range = r":(\d+)(?:-(\d+))?" > > Same as for IPv4, guess {0, 4} > I agree, this regexp are too simplistic. Will improve them a bit (though a fully accurate ipv6 regexp might bee a bit over-complicated for this usecase). >> +ip_range_regexp = r"{ip_cap}(?:-{ip_cap})?(?:{port_range})?" >> +ipv4_port_regex = re.compile( >> + ip_range_regexp.format(ip_cap=ipv4_capture, port_range=port_range) >> +) >> +ipv6_port_regex = re.compile( >> + ip_range_regexp.format(ip_cap=ipv6_capture, port_range=port_range) >> +) >> + >> + >> +def decode_ip_port_range(value): >> + """ >> + Decodes an IP and port range: >> + {ip_start}-{ip-end}:{port_start}-{port_end} >> + >> + IPv6 addresses are surrounded by "[" and "]" if port ranges are also >> + present >> + >> + Returns the following dictionary: >> + { >> + "addrs": { >> + "start": {ip_start} >> + "end": {ip_end} >> + } >> + "ports": { >> + "start": {port_start}, >> + "end": {port_end} >> + } >> + (the "ports" key might be omitted) >> + """ >> + if value.count(":") > 1: >> + match = ipv6_port_regex.match(value) >> + else: >> + match = ipv4_port_regex.match(value) > > Do we need to check if there is a match? > It might be good to error out if the string supplied does not match the expected input? Or if garbage is following the part we could decode? > You're right. The error will happen in the next line if there is no match but a more expressive error could be thrown. >> + >> + ip_start = match.group(1) >> + ip_end = match.group(2) >> + port_start = match.group(3) >> + port_end = match.group(4) >> + >> + result = { >> + "addrs": { >> + "start": netaddr.IPAddress(ip_start), >> + "end": netaddr.IPAddress(ip_end or ip_start), >> + } >> + } >> + if port_start: >> + result["ports"] = { >> + "start": int(port_start), >> + "end": int(port_end or port_start), >> + } >> + >> + return result >> + >> + >> +def decode_nat(value): >> + """Decodes the 'nat' keyword of the ct action""" > > Can we add an example of the nat format? Yes, will do. > >> + if not value: >> + return True > > Why returning True is no data is present? I would expect None. In general keys without values are decoded as key: True. Other places in the code calls them "flags", e.g: "drop" action is decoded as {"drop": True}. In this case "ct" without a value is a flag. But I can see how this can be confusing. I'll add a comment to clarify. > >> + >> + result = dict() >> + type_parts = value.split("=") >> + result["type"] = type_parts[0] >> + >> + if len(type_parts) > 1: >> + value_parts = type_parts[1].split(",") >> + if len(type_parts) != 2: >> + raise ValueError("Malformed nat action: %s" % value) >> + >> + ip_port_range = decode_ip_port_range(value_parts[0]) >> + >> + result = {"type": type_parts[0], **ip_port_range} >> + >> + for flag in value_parts[1:]: >> + result[flag] = True >> + >> + return result >> diff --git a/python/ovs/flows/ofp.py b/python/ovs/flows/ofp.py >> new file mode 100644 >> index 000000000..e56b08967 >> --- /dev/null >> +++ b/python/ovs/flows/ofp.py >> @@ -0,0 +1,400 @@ >> +""" Defines the parsers needed to parse ofproto flows >> +""" >> + >> +import functools >> + >> +from ovs.flows.kv import KVParser, KVDecoders, nested_kv_decoder >> +from ovs.flows.ofp_fields import field_decoders >> +from ovs.flows.flow import Flow, Section >> +from ovs.flows.list import ListDecoders, nested_list_decoder >> +from ovs.flows.decoders import ( >> + decode_default, >> + decode_flag, >> + decode_int, >> + decode_time, >> + decode_mask, >> + IPMask, >> + EthMask, >> + decode_free_output, >> + decode_nat, >> +) >> +from ovs.flows.ofp_act import ( >> + decode_output, >> + decode_field, >> + decode_controller, >> + decode_bundle, >> + decode_bundle_load, >> + decode_encap_ethernet, >> + decode_load_field, >> + decode_set_field, >> + decode_move_field, >> + decode_dec_ttl, >> + decode_chk_pkt_larger, >> + decode_zone, >> + decode_exec, >> + decode_learn, >> +) >> + >> + >> +class OFPFlow(Flow): >> + """OFPFLow represents an OpenFlow Flow""" > > Can we describe args; sections, orig, and id? Guess just copy them from Flow() Sure, will do. >> + >> + def __init__(self, sections, orig="", id=None): >> + """Constructor""" >> + super(OFPFlow, self).__init__(sections, orig, id) >> + >> + def __str__(self): >> + if self._orig: >> + return self._orig >> + else: >> + return self.to_string() >> + >> + def to_string(self): >> + """Print a text representation of the flow""" >> + string = "Info: {}\n" + self.info >> + string += "Match : {}\n" + self.match >> + string += "Actions: {}\n " + self.actions > > It’s getting late, so maybe I should stop reviewing. But I’m wondering where self.info/self.match/self.actions are set? self.[info,match,actions] are created as attributes by the base class (Flow), though that mechanism could be argued to be a bit over-engineered. > Also, _orig is only None, if explicitly set during initialization, else it’s “”. Right, either case "if self._orig" will be False. > >> + return string >> + >> + >> +class OFPFlowFactory: > > Do we need OFPFlowFactory(object):, some times flake reports this as H238. But might no longer be valid with the latest python3. Not 100% sure either, I'll check. Thanks. > >> + """OpenFlow Flow Factory is a class capable of creating OFPFLow objects""" >> + >> + def __init__(self): >> + self.info_decoders = self._info_decoders() >> + self.match_decoders = KVDecoders( > > The name suggests that we return KVDecoders() as the __info_decoders() does, I think all three should return the same. > What name exactly? Do you mean the _flow_match_decoders and _field_decoders? Or do you mean match_decoders should be a function rather than the object directly? >> + {**self._field_decoders(), **self._flow_match_decoders()} >> + ) >> + self.act_decoders = self._act_decoders() >> + >> + def from_string(self, ofp_string, id=None): >> + """Parse a ofproto flow string >> + >> + The string is expected to have the follwoing format: >> + [flow data] [match] actions=[actions] > > Should we maybe add an example, as it’s not that straight forward, for example: > > cookie=0x0, duration=1.001s, table=0, n_packets=1, n_bytes=64, ip,in_port=enp5s0f0 actions=output:vnet0 > > > [flow data], [match] actions=[actions] > Sure, will do. Thanks. >> + >> + :param ofp_string: a ofproto string as dumped by ovs-ofctl tool > > An ofproto string, as dumped by the ovs-ofctl tool. > >> + :type ofp_string: str >> + >> + :return: an OFPFlow with the content of the flow string >> + :rtype: OFPFlow >> + """ >> + if " reply " in ofp_string: >> + return None >> + >> + sections = list() >> + parts = ofp_string.split("actions=") >> + if len(parts) != 2: >> + raise ValueError("malformed ofproto flow: %s" % ofp_string) >> + >> + actions = parts[1] >> + >> + field_parts = parts[0].rstrip(" ").rpartition(" ") >> + if len(field_parts) != 3: >> + raise ValueError("malformed ofproto flow: %s" % ofp_string) >> + >> + info = field_parts[0] >> + match = field_parts[2] >> + >> + iparser = KVParser(self.info_decoders) >> + iparser.parse(info) >> + isection = Section( >> + name="info", >> + pos=ofp_string.find(info), >> + string=info, >> + data=iparser.kv(), >> + ) >> + sections.append(isection) >> + >> + mparser = KVParser(self.match_decoders) >> + mparser.parse(match) >> + msection = Section( >> + name="match", >> + pos=ofp_string.find(match), >> + string=match, >> + data=mparser.kv(), >> + ) >> + sections.append(msection) >> + >> + aparser = KVParser(self.act_decoders) >> + aparser.parse(actions) >> + asection = Section( >> + name="actions", >> + pos=ofp_string.find(actions), >> + string=actions, >> + data=aparser.kv(), >> + is_list=True, >> + ) >> + sections.append(asection) >> + >> + return OFPFlow(sections, ofp_string, id) >> + >> + @classmethod >> + def _info_decoders(cls): >> + """Generate the match decoders""" >> + info = { >> + "table": decode_int, >> + "duration": decode_time, >> + "n_packet": decode_int, >> + "n_bytes": decode_int, >> + "cookie": decode_int, >> + "idle_timeout": decode_time, >> + "hard_timeout": decode_time, >> + "hard_age": decode_time, >> + } >> + return KVDecoders(info) >> + >> + @classmethod >> + def _flow_match_decoders(cls): >> + """Returns the decoders for key-values that are part of the flow match >> + but not a flow field""" >> + return { >> + "priority": decode_int, >> + } > > Guess this should be return KVDecoders({ > "priority": decode_int, > })) > >> + >> + @classmethod >> + def _field_decoders(cls): >> + shorthands = [ >> + "eth", >> + "ip", >> + "ipv6", >> + "icmp", >> + "icmp6", >> + "tcp", >> + "tcp6", >> + "udp", >> + "udp6", >> + "sctp", >> + "arp", >> + "rarp", >> + "mpls", >> + "mplsm", >> + ] >> + >> + fields = {**field_decoders, **{key: decode_flag for key in shorthands}} >> + >> + # vlan_vid field is special. Although it is technically 12 bit wide, >> + # bit 12 is allowed to be set to 1 to indicate that the vlan header is >> + # present (see section VLAN FIELDS in >> + # http://www.openvswitch.org/support/dist-docs/ovs-fields.7.txt) >> + # Therefore, override the generated vlan_vid field size >> + fields["vlan_vid"] = decode_mask(13) >> + return fields > > This should return KVDecoders(fields). > Guess this is true for all class methods below! Or, you should choose to always return the list. But not mix the two. The problem with the fields is that they make up different KVDecoders, that's why we return the dictionary. They are included in the KVDecoders() used for the "match" part of the flow as well as for the "set_field" action. I agree that using the same function nomenclature is confusing. I'll change to reflect what it really does > >> + >> + @classmethod >> + def _output_actions_decoders(cls): >> + """Returns the decoders for the output actions""" > > If you have them, I think it might be useful, to add some example strings to the individual decoders. This way, it's easy to see what they intend to decode. > >> + return { >> + "output": decode_output, >> + "drop": decode_flag, >> + "controller": decode_controller, >> + "enqueue": nested_list_decoder( >> + ListDecoders([("port", decode_default), ("queue", int)]), >> + delims=[",", ":"], >> + ), >> + "bundle": decode_bundle, >> + "bundle_load": decode_bundle_load, >> + "group": decode_default, >> + } >> + >> + @classmethod >> + def _encap_actions_decoders(cls): >> + """Returns the decoders for the encap actions""" >> + >> + return { >> + "pop_vlan": decode_flag, >> + "strip_vlan": decode_flag, >> + "push_vlan": decode_default, >> + "decap": decode_flag, >> + "encap": nested_kv_decoder( >> + KVDecoders( >> + { >> + "nsh": nested_kv_decoder( >> + KVDecoders( >> + { >> + "md_type": decode_default, >> + "tlv": nested_list_decoder( >> + ListDecoders( >> + [ >> + ("class", decode_int), >> + ("type", decode_int), >> + ("value", decode_int), >> + ] >> + ) >> + ), >> + } >> + ) >> + ), >> + }, >> + default=None, >> + default_free=decode_encap_ethernet, >> + ) >> + ), >> + } >> + >> + @classmethod >> + def _field_action_decoders(cls): >> + """Returns the decoders for the field modification actions""" >> + # Field modification actions >> + field_default_decoders = [ >> + "set_mpls_label", >> + "set_mpls_tc", >> + "set_mpls_ttl", >> + "mod_nw_tos", >> + "mod_nw_ecn", >> + "mod_tcp_src", >> + "mod_tcp_dst", >> + ] >> + return { >> + "load": decode_load_field, >> + "set_field": functools.partial( >> + decode_set_field, KVDecoders(cls._field_decoders()) >> + ), >> + "move": decode_move_field, >> + "mod_dl_dst": EthMask, >> + "mod_dl_src": EthMask, >> + "mod_nw_dst": IPMask, >> + "mod_nw_src": IPMask, >> + "dec_ttl": decode_dec_ttl, >> + "dec_mpls_ttl": decode_flag, >> + "dec_nsh_ttl": decode_flag, >> + "check_pkt_larger": decode_chk_pkt_larger, >> + **{field: decode_default for field in field_default_decoders}, >> + } >> + >> + @classmethod >> + def _meta_action_decoders(cls): >> + """Returns the decoders for the metadata actions""" >> + meta_default_decoders = ["set_tunnel", "set_tunnel64", "set_queue"] >> + return { >> + "pop_queue": decode_flag, >> + **{field: decode_default for field in meta_default_decoders}, >> + } >> + >> + @classmethod >> + def _fw_action_decoders(cls): >> + """Returns the decoders for the Firewalling actions""" >> + return { >> + "ct": nested_kv_decoder( >> + KVDecoders( >> + { >> + "commit": decode_flag, >> + "zone": decode_zone, >> + "table": decode_int, >> + "nat": decode_nat, >> + "force": decode_flag, >> + "exec": functools.partial( >> + decode_exec, >> + KVDecoders( >> + { >> + **cls._encap_actions_decoders(), >> + **cls._field_action_decoders(), >> + **cls._meta_action_decoders(), >> + } >> + ), >> + ), >> + "alg": decode_default, >> + } >> + ) >> + ), >> + "ct_clear": decode_flag, >> + } >> + >> + @classmethod >> + def _control_action_decoders(cls): >> + return { >> + "resubmit": nested_list_decoder( >> + ListDecoders( >> + [ >> + ("port", decode_default), >> + ("table", decode_int), >> + ("ct", decode_flag), >> + ] >> + ) >> + ), >> + "push": decode_field, >> + "pop": decode_field, >> + "exit": decode_flag, >> + "multipath": nested_list_decoder( >> + ListDecoders( >> + [ >> + ("fields", decode_default), >> + ("basis", decode_int), >> + ("algorithm", decode_default), >> + ("n_links", decode_int), >> + ("arg", decode_int), >> + ("dst", decode_field), >> + ] >> + ) >> + ), >> + } >> + >> + @classmethod >> + def _clone_actions_decoders(cls, action_decoders): >> + """Generate the decoders for clone actions >> + >> + Args: >> + action_decoders (dict): The decoders of the supported nested >> + actions >> + """ >> + return { >> + "learn": decode_learn( >> + { >> + **action_decoders, >> + "fin_timeout": nested_kv_decoder( >> + KVDecoders( >> + { >> + "idle_timeout": decode_time, >> + "hard_timeout": decode_time, >> + } >> + ) >> + ), >> + } >> + ), >> + "clone": functools.partial( >> + decode_exec, KVDecoders(action_decoders) >> + ), >> + } >> + >> + @classmethod >> + def _other_action_decoders(cls): >> + """Recoders for other actions (see man(7) ovs-actions)""" >> + return { >> + "conjunction": nested_list_decoder( >> + ListDecoders( >> + [("id", decode_int), ("k", decode_int), ("n", decode_int)] >> + ), >> + delims=[",", "/"], >> + ), >> + "note": decode_default, >> + "sample": nested_kv_decoder( >> + KVDecoders( >> + { >> + "probability": decode_int, >> + "collector_set_id": decode_int, >> + "obs_domain_id": decode_int, >> + "obs_point_id": decode_int, >> + "sampling_port": decode_default, >> + "ingress": decode_flag, >> + "egress": decode_flag, >> + } >> + ) >> + ), >> + } >> + >> + @classmethod >> + def _act_decoders(cls): >> + """Generate the actions decoders""" >> + >> + actions = { >> + **cls._output_actions_decoders(), >> + **cls._encap_actions_decoders(), >> + **cls._field_action_decoders(), >> + **cls._meta_action_decoders(), >> + **cls._fw_action_decoders(), >> + **cls._control_action_decoders(), >> + **cls._other_action_decoders(), >> + } >> + clone_actions = cls._clone_actions_decoders(actions) >> + actions.update(clone_actions) >> + return KVDecoders(actions, default_free=decode_free_output) >> diff --git a/python/ovs/flows/ofp_act.py b/python/ovs/flows/ofp_act.py >> new file mode 100644 >> index 000000000..bc6574999 >> --- /dev/null >> +++ b/python/ovs/flows/ofp_act.py >> @@ -0,0 +1,233 @@ >> +""" Defines decoders for openflow actions >> +""" >> + >> +import functools >> + >> +from ovs.flows.kv import nested_kv_decoder, KVDecoders, KeyValue, KVParser >> +from ovs.flows.decoders import ( >> + decode_default, >> + decode_time, >> + decode_flag, >> + decode_int, >> +) >> +from ovs.flows.ofp_fields import field_decoders >> + >> + >> +def decode_output(value): >> + """Decodes the output value >> + >> + Does not support field specification >> + """ >> + if len(value.split(",")) > 1: >> + return nested_kv_decoder()(value) >> + try: >> + return {"port": int(value)} >> + except ValueError: >> + return {"port": value.strip('"')} >> + >> + >> +def decode_controller(value): >> + """Decodes the controller action""" >> + if not value: >> + return KeyValue("output", "controller") >> + else: >> + # Try controller:max_len >> + try: >> + max_len = int(value) >> + return { >> + "max_len": max_len, >> + } >> + except ValueError: >> + pass >> + # controller(key[=val], ...) >> + return nested_kv_decoder()(value) >> + >> + >> +def decode_bundle_load(value): >> + return decode_bundle(value, True) >> + >> + >> +def decode_bundle(value, load=False): >> + """Decode bundle action""" >> + result = {} >> + keys = ["fields", "basis", "algorithm", "ofport"] >> + if load: >> + keys.append("dst") >> + >> + for key in keys: >> + parts = value.partition(",") >> + nvalue = parts[0] >> + value = parts[2] >> + if key == "ofport": >> + continue >> + result[key] = decode_default(nvalue) >> + >> + # Handle members: >> + mvalues = value.split("members:") >> + result["members"] = [int(port) for port in mvalues[1].split(",")] >> + return result >> + >> + >> +def decode_encap_ethernet(value): >> + """Decodes encap ethernet value""" > > Was confused with ethertype here, but I think ethernet does not have a value does it? > Just encap(ethernet)? > Right. Re-reading the man page it definitely seems so. I got confused the first time. I'll change it. Thanks! >> + return "ethernet", int(value, 0) >> + >> + >> +def decode_field(value): >> + """Decodes a field as defined in the 'Field Specification' of the actions >> + man page: http://www.openvswitch.org/support/dist-docs/ovs-actions.7.txt >> + """ >> + parts = value.strip("]\n\r").split("[") >> + result = { >> + "field": parts[0], >> + } >> + >> + if len(parts) > 1 and parts[1]: >> + field_range = parts[1].split("..") >> + start = field_range[0] >> + end = field_range[1] if len(field_range) > 1 else start >> + if start: >> + result["start"] = int(start) >> + if end: >> + result["end"] = int(end) >> + >> + return result >> + >> + >> +def decode_load_field(value): >> + """Decodes 'load:value->dst' actions""" >> + parts = value.split("->") >> + if len(parts) != 2: >> + raise ValueError("Malformed load action : %s" % value) >> + >> + # If the load action is performed within a learn() action, >> + # The value can be specified as another field. >> + try: >> + return {"value": int(parts[0], 0), "dst": decode_field(parts[1])} >> + except ValueError: >> + return {"src": decode_field(parts[0]), "dst": decode_field(parts[1])} >> + >> + >> +def decode_set_field(field_decoders, value): >> + """Decodes 'set_field:value/mask->dst' actions >> + >> + The value is decoded by field_decoders which is a KVDecoders instance >> + Args: >> + field_decoders >> + """ >> + parts = value.split("->") >> + if len(parts) != 2: >> + raise ValueError("Malformed set_field action : %s" % value) >> + >> + val = parts[0] >> + dst = parts[1] >> + >> + val_result = field_decoders.decode(dst, val) >> + >> + return { >> + "value": {val_result[0]: val_result[1]}, >> + "dst": decode_field(dst), >> + } >> + >> + >> +def decode_move_field(value): >> + """Decodes 'move:src->dst' actions""" >> + parts = value.split("->") >> + if len(parts) != 2: >> + raise ValueError("Malformed move action : %s" % value) >> + >> + return { >> + "src": decode_field(parts[0]), >> + "dst": decode_field(parts[1]), >> + } >> + >> + >> +def decode_dec_ttl(value): >> + """Decodes dec_ttl and dec_ttl(id, id[2], ...) actions""" >> + if not value: >> + return True >> + return [int(idx) for idx in value.split(",")] >> + >> + >> +def decode_chk_pkt_larger(value): >> + """Decodes 'check_pkt_larger(pkt_len)->dst' actions""" >> + parts = value.split("->") >> + if len(parts) != 2: >> + raise ValueError("Malformed check_pkt_larger action : %s" % value) >> + >> + pkt_len = int(parts[0].strip("()")) >> + dst = decode_field(parts[1]) >> + return {"pkt_len": pkt_len, "dst": dst} >> + >> + >> +# CT decoders >> +def decode_zone(value): >> + """Decodes the 'zone' keyword of the ct action""" >> + try: >> + return int(value, 0) >> + except ValueError: >> + pass >> + return decode_field(value) >> + >> + >> +def decode_exec(action_decoders, value): >> + """Decodes the 'exec' keyword of the ct action >> + >> + Args: >> + decode_actions (KVDecoders): the decoders to be used to decode the >> + nested exec >> + value (string): the string to be decoded >> + """ >> + exec_parser = KVParser(action_decoders) >> + exec_parser.parse(value) >> + return [{kv.key: kv.value} for kv in exec_parser.kv()] >> + >> + >> +def decode_learn(action_decoders): > > It’s getting late, and I have a hard time (focussing ;) understanding where the value for this one comes from? I'll pick it up from here when I continue the review. > I hope I can clarify. Learn action has two added complexities: - It accepts any other action key-value. For this we need to create a wrapper around the pre-calculated action_decoders. - The way fields can be specified is augmented. Not only we have 'field=value', but we also have 'field=_src_' (where _src_ is another field name) and just 'field'. For this we need to create a wrapper of field_decoders that, for each "field=X" key-value we check if X is a field_name or if it's acually a value that we need to send to the appropriate field_decoder to process. >> + """Create the decoder to be used to decode the 'learn' action. >> + >> + The learn action can include any nested action, therefore we need decoders >> + for all possible actions. >> + >> + Args: >> + action_decoders (dict): dictionary of decoders to be used in nested >> + action decoding >> + >> + """ >> + >> + def decode_learn_field(decoder, value): >> + """Generates a decoder to be used for the 'field' argument of the >> + 'learn' action. >> + >> + The field can hold a value that should be decoded, either as a field, >> + or as a the value (see man(7) ovs-actions) >> + >> + Args: >> + decoder (callable): The decoder >> + >> + """ >> + if value in field_decoders.keys(): >> + # It's a field >> + return value >> + else: >> + return decoder(value) >> + >> + learn_field_decoders = { >> + field: functools.partial(decode_learn_field, decoder) >> + for field, decoder in field_decoders.items() >> + } >> + learn_decoders = { >> + **action_decoders, >> + **learn_field_decoders, >> + "idle_timeout": decode_time, >> + "hard_timeout": decode_time, >> + "priority": decode_int, >> + "cooke": decode_int, >> + "send_flow_rem": decode_flag, >> + "table": decode_int, >> + "delete_learned": decode_flag, >> + "limit": decode_int, >> + "result_dst": decode_field, >> + } >> + >> + return functools.partial(decode_exec, KVDecoders(learn_decoders)) >> -- >> 2.31.1 >
On 20 Dec 2021, at 12:01, Adrian Moreno wrote: > On 12/17/21 18:19, Eelco Chaudron wrote: >> >> >> On 22 Nov 2021, at 12:22, Adrian Moreno wrote: >> >>> Introduce OFPFlow class and all its decoders. >>> >>> Most of the decoders are generic (from decoders.py). Some have special >>> syntax and need a specific implementation. >>> >>> Decoders for nat are moved to the common decoders.py because it's syntax >>> is shared with other types of flows (e.g: dpif flows). >>> >>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >>> --- >>> python/automake.mk | 4 +- >>> python/ovs/flows/decoders.py | 93 ++++++++ >>> python/ovs/flows/ofp.py | 400 +++++++++++++++++++++++++++++++++++ >>> python/ovs/flows/ofp_act.py | 233 ++++++++++++++++++++ >>> 4 files changed, 729 insertions(+), 1 deletion(-) >>> create mode 100644 python/ovs/flows/ofp.py >>> create mode 100644 python/ovs/flows/ofp_act.py >>> >>> diff --git a/python/automake.mk b/python/automake.mk >>> index 136da26bd..d1464d7f6 100644 >>> --- a/python/automake.mk >>> +++ b/python/automake.mk >>> @@ -46,7 +46,9 @@ ovs_pyfiles = \ >>> python/ovs/flows/decoders.py \ >>> python/ovs/flows/kv.py \ >>> python/ovs/flows/list.py \ >>> - python/ovs/flows/flow.py >>> + python/ovs/flows/flow.py \ >>> + python/ovs/flows/ofp.py \ >>> + python/ovs/flows/ofp_act.py >>> >>> # These python files are used at build time but not runtime, >>> # so they are not installed. >>> diff --git a/python/ovs/flows/decoders.py b/python/ovs/flows/decoders.py >>> index bf7a94ae8..3def9f279 100644 >>> --- a/python/ovs/flows/decoders.py >>> +++ b/python/ovs/flows/decoders.py >>> @@ -6,6 +6,7 @@ object. >>> """ >>> >>> import netaddr >>> +import re >>> >>> >>> class Decoder: >>> @@ -358,3 +359,95 @@ class IPMask(Decoder): >>> >>> def to_json(self): >>> return str(self) >>> + >>> + >>> +def decode_free_output(value): >>> + """Decodes the output value when found free >>> + (without the 'output' keyword)""" >> >> This is the last time I’ll add it for the remaining patches to be reviewed. >> But please check that all sentences start with a Captical and end with a dot. >> > > Sure. Sorry about that. > > >>> + try: >>> + return "output", {"port": int(value)} >> >> Any reason not to try to remove the “ here? >> return "output", {"port": int(value.strip('"'))} >> >>> + except ValueError: >>> + return "output", {"port": value.strip('"')} >>> + >>> + >>> +ipv4 = r"[\d\.]+" >> >> Should this not try to match at least 1 and at most 3 numbers? >> Maybe something like [\d{1,3}\.]+. >> >>> +ipv4_capture = r"({ipv4})".format(ipv4=ipv4) >>> +ipv6 = r"[\w:]+" >>> +ipv6_capture = r"(?:\[*)?({ipv6})(?:\]*)?".format(ipv6=ipv6) >>> +port_range = r":(\d+)(?:-(\d+))?" >> >> Same as for IPv4, guess {0, 4} >> > > I agree, this regexp are too simplistic. Will improve them a bit (though a fully accurate ipv6 regexp might bee a bit over-complicated for this usecase). > Thanks, I did a quick google and found these: https://www.oreilly.com/library/view/regular-expressions-cookbook/9780596802837/ch07s16.html https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch08s17.html But I guess it does not take care of the IPV6 dotted notation if OVS supports it. This seems to have it: https://community.helpsystems.com/forums/intermapper/miscellaneous-topics/5acc4fcf-fa83-e511-80cf-0050568460e4?_ga=2.113564423.1432958022.1523882681-2146416484.1523557976 >>> +ip_range_regexp = r"{ip_cap}(?:-{ip_cap})?(?:{port_range})?" >>> +ipv4_port_regex = re.compile( >>> + ip_range_regexp.format(ip_cap=ipv4_capture, port_range=port_range) >>> +) >>> +ipv6_port_regex = re.compile( >>> + ip_range_regexp.format(ip_cap=ipv6_capture, port_range=port_range) >>> +) >>> + >>> + >>> +def decode_ip_port_range(value): >>> + """ >>> + Decodes an IP and port range: >>> + {ip_start}-{ip-end}:{port_start}-{port_end} >>> + >>> + IPv6 addresses are surrounded by "[" and "]" if port ranges are also >>> + present >>> + >>> + Returns the following dictionary: >>> + { >>> + "addrs": { >>> + "start": {ip_start} >>> + "end": {ip_end} >>> + } >>> + "ports": { >>> + "start": {port_start}, >>> + "end": {port_end} >>> + } >>> + (the "ports" key might be omitted) >>> + """ >>> + if value.count(":") > 1: >>> + match = ipv6_port_regex.match(value) >>> + else: >>> + match = ipv4_port_regex.match(value) >> >> Do we need to check if there is a match? >> It might be good to error out if the string supplied does not match the expected input? Or if garbage is following the part we could decode? >> > > You're right. The error will happen in the next line if there is no match but a more expressive error could be thrown. > >>> + >>> + ip_start = match.group(1) >>> + ip_end = match.group(2) >>> + port_start = match.group(3) >>> + port_end = match.group(4) >>> + >>> + result = { >>> + "addrs": { >>> + "start": netaddr.IPAddress(ip_start), >>> + "end": netaddr.IPAddress(ip_end or ip_start), >>> + } >>> + } >>> + if port_start: >>> + result["ports"] = { >>> + "start": int(port_start), >>> + "end": int(port_end or port_start), >>> + } >>> + >>> + return result >>> + >>> + >>> +def decode_nat(value): >>> + """Decodes the 'nat' keyword of the ct action""" >> >> Can we add an example of the nat format? > > Yes, will do. > >> >>> + if not value: >>> + return True >> >> Why returning True is no data is present? I would expect None. > > In general keys without values are decoded as key: True. Other places in the code calls them "flags", e.g: "drop" action is decoded as {"drop": True}. In this case "ct" without a value is a flag. > > But I can see how this can be confusing. I'll add a comment to clarify. Thanks! >> >>> + >>> + result = dict() >>> + type_parts = value.split("=") >>> + result["type"] = type_parts[0] >>> + >>> + if len(type_parts) > 1: >>> + value_parts = type_parts[1].split(",") >>> + if len(type_parts) != 2: >>> + raise ValueError("Malformed nat action: %s" % value) >>> + >>> + ip_port_range = decode_ip_port_range(value_parts[0]) >>> + >>> + result = {"type": type_parts[0], **ip_port_range} >>> + >>> + for flag in value_parts[1:]: >>> + result[flag] = True >>> + >>> + return result >>> diff --git a/python/ovs/flows/ofp.py b/python/ovs/flows/ofp.py >>> new file mode 100644 >>> index 000000000..e56b08967 >>> --- /dev/null >>> +++ b/python/ovs/flows/ofp.py >>> @@ -0,0 +1,400 @@ >>> +""" Defines the parsers needed to parse ofproto flows >>> +""" >>> + >>> +import functools >>> + >>> +from ovs.flows.kv import KVParser, KVDecoders, nested_kv_decoder >>> +from ovs.flows.ofp_fields import field_decoders >>> +from ovs.flows.flow import Flow, Section >>> +from ovs.flows.list import ListDecoders, nested_list_decoder >>> +from ovs.flows.decoders import ( >>> + decode_default, >>> + decode_flag, >>> + decode_int, >>> + decode_time, >>> + decode_mask, >>> + IPMask, >>> + EthMask, >>> + decode_free_output, >>> + decode_nat, >>> +) >>> +from ovs.flows.ofp_act import ( >>> + decode_output, >>> + decode_field, >>> + decode_controller, >>> + decode_bundle, >>> + decode_bundle_load, >>> + decode_encap_ethernet, >>> + decode_load_field, >>> + decode_set_field, >>> + decode_move_field, >>> + decode_dec_ttl, >>> + decode_chk_pkt_larger, >>> + decode_zone, >>> + decode_exec, >>> + decode_learn, >>> +) >>> + >>> + >>> +class OFPFlow(Flow): >>> + """OFPFLow represents an OpenFlow Flow""" >> >> Can we describe args; sections, orig, and id? Guess just copy them from Flow() > > Sure, will do. > >>> + >>> + def __init__(self, sections, orig="", id=None): >>> + """Constructor""" >>> + super(OFPFlow, self).__init__(sections, orig, id) >>> + >>> + def __str__(self): >>> + if self._orig: >>> + return self._orig >>> + else: >>> + return self.to_string() >>> + >>> + def to_string(self): >>> + """Print a text representation of the flow""" >>> + string = "Info: {}\n" + self.info >>> + string += "Match : {}\n" + self.match >>> + string += "Actions: {}\n " + self.actions >> >> It’s getting late, so maybe I should stop reviewing. But I’m wondering where self.info/self.match/self.actions are set? > > self.[info,match,actions] are created as attributes by the base class (Flow), though that mechanism could be argued to be a bit over-engineered. > >> Also, _orig is only None, if explicitly set during initialization, else it’s “”. > > Right, either case "if self._orig" will be False. Cool, not sure what happened but I totally missed this had the parent class of Flow. So ignore these comments :( >> >>> + return string >>> + >>> + >>> +class OFPFlowFactory: See my comments on patch 8, where I do think we should get rid of this class, and update the OFPFlow class to take this string at init. Being more OOO it would look like this: def __init__(self, ofp_string, id=None): """Constructor""" sections = self._sections_from_string(ofp_string) super(OFPFlow, self).__init__(sections, ofp_string, id) Where sections_from_string(ofp_string) is just the same code as in this class but returns the sections. @staticmethod def _sections_from_string(ofp_string): ... >> >> Do we need OFPFlowFactory(object):, some times flake reports this as H238. But might no longer be valid with the latest python3. > > Not 100% sure either, I'll check. Thanks. > >> >>> + """OpenFlow Flow Factory is a class capable of creating OFPFLow objects""" >>> + >>> + def __init__(self): >>> + self.info_decoders = self._info_decoders() >>> + self.match_decoders = KVDecoders( >> >> The name suggests that we return KVDecoders() as the __info_decoders() does, I think all three should return the same. >> > > What name exactly? Do you mean the _flow_match_decoders and _field_decoders? > Or do you mean match_decoders should be a function rather than the object directly? Trying to remember what my thoughts were ;) Guess I wondered why _flow_match_decoders() was not doing the ‘return KVDecoders({**self._field_decoders(), **self._flow_match_decoders()})’. >>> + {**self._field_decoders(), **self._flow_match_decoders()} >>> + ) >>> + self.act_decoders = self._act_decoders() >>> + >>> + def from_string(self, ofp_string, id=None): >>> + """Parse a ofproto flow string >>> + >>> + The string is expected to have the follwoing format: >>> + [flow data] [match] actions=[actions] >> >> Should we maybe add an example, as it’s not that straight forward, for example: >> >> cookie=0x0, duration=1.001s, table=0, n_packets=1, n_bytes=64, ip,in_port=enp5s0f0 actions=output:vnet0 >> >> >> [flow data], [match] actions=[actions] >> > > Sure, will do. Thanks. > >>> + >>> + :param ofp_string: a ofproto string as dumped by ovs-ofctl tool >> >> An ofproto string, as dumped by the ovs-ofctl tool. >> >>> + :type ofp_string: str >>> + >>> + :return: an OFPFlow with the content of the flow string >>> + :rtype: OFPFlow >>> + """ >>> + if " reply " in ofp_string: >>> + return None >>> + >>> + sections = list() >>> + parts = ofp_string.split("actions=") >>> + if len(parts) != 2: >>> + raise ValueError("malformed ofproto flow: %s" % ofp_string) >>> + >>> + actions = parts[1] >>> + >>> + field_parts = parts[0].rstrip(" ").rpartition(" ") >>> + if len(field_parts) != 3: >>> + raise ValueError("malformed ofproto flow: %s" % ofp_string) >>> + >>> + info = field_parts[0] >>> + match = field_parts[2] >>> + >>> + iparser = KVParser(self.info_decoders) >>> + iparser.parse(info) >>> + isection = Section( >>> + name="info", >>> + pos=ofp_string.find(info), >>> + string=info, >>> + data=iparser.kv(), >>> + ) >>> + sections.append(isection) >>> + >>> + mparser = KVParser(self.match_decoders) >>> + mparser.parse(match) >>> + msection = Section( >>> + name="match", >>> + pos=ofp_string.find(match), >>> + string=match, >>> + data=mparser.kv(), >>> + ) >>> + sections.append(msection) >>> + >>> + aparser = KVParser(self.act_decoders) >>> + aparser.parse(actions) >>> + asection = Section( >>> + name="actions", >>> + pos=ofp_string.find(actions), >>> + string=actions, >>> + data=aparser.kv(), >>> + is_list=True, >>> + ) >>> + sections.append(asection) >>> + >>> + return OFPFlow(sections, ofp_string, id) >>> + >>> + @classmethod >>> + def _info_decoders(cls): >>> + """Generate the match decoders""" >>> + info = { >>> + "table": decode_int, >>> + "duration": decode_time, >>> + "n_packet": decode_int, >>> + "n_bytes": decode_int, >>> + "cookie": decode_int, >>> + "idle_timeout": decode_time, >>> + "hard_timeout": decode_time, >>> + "hard_age": decode_time, >>> + } >>> + return KVDecoders(info) >>> + >>> + @classmethod >>> + def _flow_match_decoders(cls): >>> + """Returns the decoders for key-values that are part of the flow match >>> + but not a flow field""" >>> + return { >>> + "priority": decode_int, >>> + } >> >> Guess this should be return KVDecoders({ >> "priority": decode_int, >> })) >> >>> + >>> + @classmethod >>> + def _field_decoders(cls): >>> + shorthands = [ >>> + "eth", >>> + "ip", >>> + "ipv6", >>> + "icmp", >>> + "icmp6", >>> + "tcp", >>> + "tcp6", >>> + "udp", >>> + "udp6", >>> + "sctp", >>> + "arp", >>> + "rarp", >>> + "mpls", >>> + "mplsm", >>> + ] >>> + >>> + fields = {**field_decoders, **{key: decode_flag for key in shorthands}} >>> + >>> + # vlan_vid field is special. Although it is technically 12 bit wide, >>> + # bit 12 is allowed to be set to 1 to indicate that the vlan header is >>> + # present (see section VLAN FIELDS in >>> + # http://www.openvswitch.org/support/dist-docs/ovs-fields.7.txt) >>> + # Therefore, override the generated vlan_vid field size >>> + fields["vlan_vid"] = decode_mask(13) >>> + return fields >> >> This should return KVDecoders(fields). >> Guess this is true for all class methods below! Or, you should choose to always return the list. But not mix the two. > > The problem with the fields is that they make up different KVDecoders, that's why we return the dictionary. They are included in the KVDecoders() used for the "match" part of the flow as well as for the "set_field" action. I agree that using the same function nomenclature is confusing. I'll change to reflect what it really does Thanks >>> + >>> + @classmethod >>> + def _output_actions_decoders(cls): >>> + """Returns the decoders for the output actions""" >> >> If you have them, I think it might be useful, to add some example strings to the individual decoders. This way, it's easy to see what they intend to decode. >> >>> + return { >>> + "output": decode_output, >>> + "drop": decode_flag, >>> + "controller": decode_controller, >>> + "enqueue": nested_list_decoder( >>> + ListDecoders([("port", decode_default), ("queue", int)]), >>> + delims=[",", ":"], >>> + ), >>> + "bundle": decode_bundle, >>> + "bundle_load": decode_bundle_load, >>> + "group": decode_default, >>> + } >>> + >>> + @classmethod >>> + def _encap_actions_decoders(cls): >>> + """Returns the decoders for the encap actions""" >>> + >>> + return { >>> + "pop_vlan": decode_flag, >>> + "strip_vlan": decode_flag, >>> + "push_vlan": decode_default, >>> + "decap": decode_flag, >>> + "encap": nested_kv_decoder( >>> + KVDecoders( >>> + { >>> + "nsh": nested_kv_decoder( >>> + KVDecoders( >>> + { >>> + "md_type": decode_default, >>> + "tlv": nested_list_decoder( >>> + ListDecoders( >>> + [ >>> + ("class", decode_int), >>> + ("type", decode_int), >>> + ("value", decode_int), >>> + ] >>> + ) >>> + ), >>> + } >>> + ) >>> + ), >>> + }, >>> + default=None, >>> + default_free=decode_encap_ethernet, >>> + ) >>> + ), >>> + } >>> + >>> + @classmethod >>> + def _field_action_decoders(cls): >>> + """Returns the decoders for the field modification actions""" >>> + # Field modification actions >>> + field_default_decoders = [ >>> + "set_mpls_label", >>> + "set_mpls_tc", >>> + "set_mpls_ttl", >>> + "mod_nw_tos", >>> + "mod_nw_ecn", >>> + "mod_tcp_src", >>> + "mod_tcp_dst", >>> + ] >>> + return { >>> + "load": decode_load_field, >>> + "set_field": functools.partial( >>> + decode_set_field, KVDecoders(cls._field_decoders()) >>> + ), >>> + "move": decode_move_field, >>> + "mod_dl_dst": EthMask, >>> + "mod_dl_src": EthMask, >>> + "mod_nw_dst": IPMask, >>> + "mod_nw_src": IPMask, >>> + "dec_ttl": decode_dec_ttl, >>> + "dec_mpls_ttl": decode_flag, >>> + "dec_nsh_ttl": decode_flag, >>> + "check_pkt_larger": decode_chk_pkt_larger, >>> + **{field: decode_default for field in field_default_decoders}, >>> + } >>> + >>> + @classmethod >>> + def _meta_action_decoders(cls): >>> + """Returns the decoders for the metadata actions""" >>> + meta_default_decoders = ["set_tunnel", "set_tunnel64", "set_queue"] >>> + return { >>> + "pop_queue": decode_flag, >>> + **{field: decode_default for field in meta_default_decoders}, >>> + } >>> + >>> + @classmethod >>> + def _fw_action_decoders(cls): >>> + """Returns the decoders for the Firewalling actions""" >>> + return { >>> + "ct": nested_kv_decoder( >>> + KVDecoders( >>> + { >>> + "commit": decode_flag, >>> + "zone": decode_zone, >>> + "table": decode_int, >>> + "nat": decode_nat, >>> + "force": decode_flag, >>> + "exec": functools.partial( >>> + decode_exec, >>> + KVDecoders( >>> + { >>> + **cls._encap_actions_decoders(), >>> + **cls._field_action_decoders(), >>> + **cls._meta_action_decoders(), >>> + } >>> + ), >>> + ), >>> + "alg": decode_default, >>> + } >>> + ) >>> + ), >>> + "ct_clear": decode_flag, >>> + } >>> + >>> + @classmethod >>> + def _control_action_decoders(cls): >>> + return { >>> + "resubmit": nested_list_decoder( >>> + ListDecoders( >>> + [ >>> + ("port", decode_default), >>> + ("table", decode_int), >>> + ("ct", decode_flag), >>> + ] >>> + ) >>> + ), >>> + "push": decode_field, >>> + "pop": decode_field, >>> + "exit": decode_flag, >>> + "multipath": nested_list_decoder( >>> + ListDecoders( >>> + [ >>> + ("fields", decode_default), >>> + ("basis", decode_int), >>> + ("algorithm", decode_default), >>> + ("n_links", decode_int), >>> + ("arg", decode_int), >>> + ("dst", decode_field), >>> + ] >>> + ) >>> + ), >>> + } >>> + >>> + @classmethod >>> + def _clone_actions_decoders(cls, action_decoders): >>> + """Generate the decoders for clone actions >>> + >>> + Args: >>> + action_decoders (dict): The decoders of the supported nested >>> + actions >>> + """ >>> + return { >>> + "learn": decode_learn( >>> + { >>> + **action_decoders, >>> + "fin_timeout": nested_kv_decoder( >>> + KVDecoders( >>> + { >>> + "idle_timeout": decode_time, >>> + "hard_timeout": decode_time, >>> + } >>> + ) >>> + ), >>> + } >>> + ), >>> + "clone": functools.partial( >>> + decode_exec, KVDecoders(action_decoders) >>> + ), >>> + } >>> + >>> + @classmethod >>> + def _other_action_decoders(cls): >>> + """Recoders for other actions (see man(7) ovs-actions)""" >>> + return { >>> + "conjunction": nested_list_decoder( >>> + ListDecoders( >>> + [("id", decode_int), ("k", decode_int), ("n", decode_int)] >>> + ), >>> + delims=[",", "/"], >>> + ), >>> + "note": decode_default, >>> + "sample": nested_kv_decoder( >>> + KVDecoders( >>> + { >>> + "probability": decode_int, >>> + "collector_set_id": decode_int, >>> + "obs_domain_id": decode_int, >>> + "obs_point_id": decode_int, >>> + "sampling_port": decode_default, >>> + "ingress": decode_flag, >>> + "egress": decode_flag, >>> + } >>> + ) >>> + ), >>> + } >>> + >>> + @classmethod >>> + def _act_decoders(cls): >>> + """Generate the actions decoders""" >>> + >>> + actions = { >>> + **cls._output_actions_decoders(), >>> + **cls._encap_actions_decoders(), >>> + **cls._field_action_decoders(), >>> + **cls._meta_action_decoders(), >>> + **cls._fw_action_decoders(), >>> + **cls._control_action_decoders(), >>> + **cls._other_action_decoders(), >>> + } >>> + clone_actions = cls._clone_actions_decoders(actions) >>> + actions.update(clone_actions) >>> + return KVDecoders(actions, default_free=decode_free_output) >>> diff --git a/python/ovs/flows/ofp_act.py b/python/ovs/flows/ofp_act.py >>> new file mode 100644 >>> index 000000000..bc6574999 >>> --- /dev/null >>> +++ b/python/ovs/flows/ofp_act.py >>> @@ -0,0 +1,233 @@ >>> +""" Defines decoders for openflow actions >>> +""" >>> + >>> +import functools >>> + >>> +from ovs.flows.kv import nested_kv_decoder, KVDecoders, KeyValue, KVParser >>> +from ovs.flows.decoders import ( >>> + decode_default, >>> + decode_time, >>> + decode_flag, >>> + decode_int, >>> +) >>> +from ovs.flows.ofp_fields import field_decoders >>> + >>> + >>> +def decode_output(value): >>> + """Decodes the output value >>> + >>> + Does not support field specification >>> + """ >>> + if len(value.split(",")) > 1: >>> + return nested_kv_decoder()(value) >>> + try: >>> + return {"port": int(value)} >>> + except ValueError: >>> + return {"port": value.strip('"')} >>> + >>> + >>> +def decode_controller(value): >>> + """Decodes the controller action""" >>> + if not value: >>> + return KeyValue("output", "controller") >>> + else: >>> + # Try controller:max_len >>> + try: >>> + max_len = int(value) >>> + return { >>> + "max_len": max_len, >>> + } >>> + except ValueError: >>> + pass >>> + # controller(key[=val], ...) >>> + return nested_kv_decoder()(value) >>> + >>> + >>> +def decode_bundle_load(value): >>> + return decode_bundle(value, True) >>> + >>> + >>> +def decode_bundle(value, load=False): >>> + """Decode bundle action""" >>> + result = {} >>> + keys = ["fields", "basis", "algorithm", "ofport"] >>> + if load: >>> + keys.append("dst") >>> + >>> + for key in keys: >>> + parts = value.partition(",") >>> + nvalue = parts[0] >>> + value = parts[2] >>> + if key == "ofport": >>> + continue >>> + result[key] = decode_default(nvalue) >>> + >>> + # Handle members: >>> + mvalues = value.split("members:") >>> + result["members"] = [int(port) for port in mvalues[1].split(",")] >>> + return result >>> + >>> + >>> +def decode_encap_ethernet(value): >>> + """Decodes encap ethernet value""" >> >> Was confused with ethertype here, but I think ethernet does not have a value does it? >> Just encap(ethernet)? >> > > Right. Re-reading the man page it definitely seems so. I got confused the first time. I'll change it. > Thanks! > >>> + return "ethernet", int(value, 0) >>> + >>> + >>> +def decode_field(value): >>> + """Decodes a field as defined in the 'Field Specification' of the actions >>> + man page: http://www.openvswitch.org/support/dist-docs/ovs-actions.7.txt >>> + """ >>> + parts = value.strip("]\n\r").split("[") >>> + result = { >>> + "field": parts[0], >>> + } >>> + >>> + if len(parts) > 1 and parts[1]: >>> + field_range = parts[1].split("..") >>> + start = field_range[0] >>> + end = field_range[1] if len(field_range) > 1 else start >>> + if start: >>> + result["start"] = int(start) >>> + if end: >>> + result["end"] = int(end) >>> + >>> + return result >>> + >>> + >>> +def decode_load_field(value): >>> + """Decodes 'load:value->dst' actions""" >>> + parts = value.split("->") >>> + if len(parts) != 2: >>> + raise ValueError("Malformed load action : %s" % value) >>> + >>> + # If the load action is performed within a learn() action, >>> + # The value can be specified as another field. >>> + try: >>> + return {"value": int(parts[0], 0), "dst": decode_field(parts[1])} >>> + except ValueError: >>> + return {"src": decode_field(parts[0]), "dst": decode_field(parts[1])} >>> + >>> + >>> +def decode_set_field(field_decoders, value): >>> + """Decodes 'set_field:value/mask->dst' actions >>> + >>> + The value is decoded by field_decoders which is a KVDecoders instance >>> + Args: >>> + field_decoders >>> + """ >>> + parts = value.split("->") >>> + if len(parts) != 2: >>> + raise ValueError("Malformed set_field action : %s" % value) >>> + >>> + val = parts[0] >>> + dst = parts[1] >>> + >>> + val_result = field_decoders.decode(dst, val) >>> + >>> + return { >>> + "value": {val_result[0]: val_result[1]}, >>> + "dst": decode_field(dst), >>> + } >>> + >>> + >>> +def decode_move_field(value): >>> + """Decodes 'move:src->dst' actions""" >>> + parts = value.split("->") >>> + if len(parts) != 2: >>> + raise ValueError("Malformed move action : %s" % value) >>> + >>> + return { >>> + "src": decode_field(parts[0]), >>> + "dst": decode_field(parts[1]), >>> + } >>> + >>> + >>> +def decode_dec_ttl(value): >>> + """Decodes dec_ttl and dec_ttl(id, id[2], ...) actions""" >>> + if not value: >>> + return True >>> + return [int(idx) for idx in value.split(",")] >>> + >>> + >>> +def decode_chk_pkt_larger(value): >>> + """Decodes 'check_pkt_larger(pkt_len)->dst' actions""" >>> + parts = value.split("->") >>> + if len(parts) != 2: >>> + raise ValueError("Malformed check_pkt_larger action : %s" % value) >>> + >>> + pkt_len = int(parts[0].strip("()")) >>> + dst = decode_field(parts[1]) >>> + return {"pkt_len": pkt_len, "dst": dst} >>> + >>> + >>> +# CT decoders >>> +def decode_zone(value): >>> + """Decodes the 'zone' keyword of the ct action""" >>> + try: >>> + return int(value, 0) >>> + except ValueError: >>> + pass >>> + return decode_field(value) >>> + >>> + >>> +def decode_exec(action_decoders, value): >>> + """Decodes the 'exec' keyword of the ct action >>> + >>> + Args: >>> + decode_actions (KVDecoders): the decoders to be used to decode the >>> + nested exec >>> + value (string): the string to be decoded >>> + """ >>> + exec_parser = KVParser(action_decoders) >>> + exec_parser.parse(value) >>> + return [{kv.key: kv.value} for kv in exec_parser.kv()] >>> + >>> + >>> +def decode_learn(action_decoders): >> >> It’s getting late, and I have a hard time (focussing ;) understanding where the value for this one comes from? I'll pick it up from here when I continue the review. >> > > I hope I can clarify. Learn action has two added complexities: > > - It accepts any other action key-value. For this we need to create a wrapper around the pre-calculated action_decoders. > > - The way fields can be specified is augmented. Not only we have 'field=value', but we also have 'field=_src_' (where _src_ is another field name) and just 'field'. For this we need to create a wrapper of field_decoders that, for each "field=X" key-value we check if X is a field_name or if it's acually a value that we need to send to the appropriate field_decoder to process. Ack thanks, it makes sense now. >>> + """Create the decoder to be used to decode the 'learn' action. >>> + >>> + The learn action can include any nested action, therefore we need decoders >>> + for all possible actions. >>> + >>> + Args: >>> + action_decoders (dict): dictionary of decoders to be used in nested >>> + action decoding >>> + >>> + """ >>> + >>> + def decode_learn_field(decoder, value): >>> + """Generates a decoder to be used for the 'field' argument of the >>> + 'learn' action. >>> + >>> + The field can hold a value that should be decoded, either as a field, >>> + or as a the value (see man(7) ovs-actions) >>> + >>> + Args: >>> + decoder (callable): The decoder >>> + >>> + """ >>> + if value in field_decoders.keys(): >>> + # It's a field >>> + return value >>> + else: >>> + return decoder(value) >>> + >>> + learn_field_decoders = { >>> + field: functools.partial(decode_learn_field, decoder) >>> + for field, decoder in field_decoders.items() >>> + } >>> + learn_decoders = { >>> + **action_decoders, >>> + **learn_field_decoders, >>> + "idle_timeout": decode_time, >>> + "hard_timeout": decode_time, >>> + "priority": decode_int, >>> + "cooke": decode_int, >>> + "send_flow_rem": decode_flag, >>> + "table": decode_int, >>> + "delete_learned": decode_flag, >>> + "limit": decode_int, >>> + "result_dst": decode_field, >>> + } >>> + >>> + return functools.partial(decode_exec, KVDecoders(learn_decoders)) >>> -- >>> 2.31.1 >> > > -- > Adrián Moreno
On 12/24/21 15:07, Eelco Chaudron wrote: >>>> +class OFPFlowFactory: > > See my comments on patch 8, where I do think we should get rid of this class, and update the OFPFlow class to take this string at init. > Being more OOO it would look like this: > > def __init__(self, ofp_string, id=None): > """Constructor""" > sections = self._sections_from_string(ofp_string) > super(OFPFlow, self).__init__(sections, ofp_string, id) > > Where sections_from_string(ofp_string) is just the same code as in this class but returns the sections. > > @staticmethod > def _sections_from_string(ofp_string): > ... > That was more or less how I originally implemented it. I then refactored it into a factory class because when parsing thousands of flows I saw a considerable amount of time being spent on parser "preparation", this is: creating all the KVParser objects. So I created the factory just to cache this data. Another alternative would be to make them global (for the class) and initialize them the first time we parse a flow so we could keep the nicer syntax. I'm trying to avoid running functions on module load. WDYT? >>> >>> Do we need OFPFlowFactory(object):, some times flake reports this as H238. But might no longer be valid with the latest python3. >> >> Not 100% sure either, I'll check. Thanks. >> >>> >>>> + """OpenFlow Flow Factory is a class capable of creating OFPFLow objects""" >>>> + >>>> + def __init__(self): >>>> + self.info_decoders = self._info_decoders() >>>> + self.match_decoders = KVDecoders( >>> >>> The name suggests that we return KVDecoders() as the __info_decoders() does, I think all three should return the same. >>> >> >> What name exactly? Do you mean the _flow_match_decoders and _field_decoders? >> Or do you mean match_decoders should be a function rather than the object directly? > > Trying to remember what my thoughts were ;) Guess I wondered why _flow_match_decoders() was not doing the ‘return KVDecoders({**self._field_decoders(), **self._flow_match_decoders()})’. > I'll unify the names to make them more consistent. [...] >>>> + >>>> + @classmethod >>>> + def _output_actions_decoders(cls): >>>> + """Returns the decoders for the output actions""" >>> >>> If you have them, I think it might be useful, to add some example strings to the individual decoders. This way, it's easy to see what they intend to decode. >>> Ack. [...] >> >>>> +def decode_learn(action_decoders): >>> >>> It’s getting late, and I have a hard time (focussing ;) understanding where the value for this one comes from? I'll pick it up from here when I continue the review. >>> >> >> I hope I can clarify. Learn action has two added complexities: >> >> - It accepts any other action key-value. For this we need to create a wrapper around the pre-calculated action_decoders. >> >> - The way fields can be specified is augmented. Not only we have 'field=value', but we also have 'field=_src_' (where _src_ is another field name) and just 'field'. For this we need to create a wrapper of field_decoders that, for each "field=X" key-value we check if X is a field_name or if it's acually a value that we need to send to the appropriate field_decoder to process. > > Ack thanks, it makes sense now. I'll add a comment to make it clearer.
On 12 Jan 2022, at 16:46, Adrian Moreno wrote: > On 12/24/21 15:07, Eelco Chaudron wrote: >>>>> +class OFPFlowFactory: >> >> See my comments on patch 8, where I do think we should get rid of this class, and update the OFPFlow class to take this string at init. >> Being more OOO it would look like this: >> >> def __init__(self, ofp_string, id=None): >> """Constructor""" >> sections = self._sections_from_string(ofp_string) >> super(OFPFlow, self).__init__(sections, ofp_string, id) >> >> Where sections_from_string(ofp_string) is just the same code as in this class but returns the sections. >> >> @staticmethod >> def _sections_from_string(ofp_string): >> ... >> > > That was more or less how I originally implemented it. I then refactored it into a factory class because when parsing thousands of flows I saw a considerable amount of time being spent on parser "preparation", this is: creating all the KVParser objects. So I created the factory just to cache this data. > > Another alternative would be to make them global (for the class) and initialize them the first time we parse a flow so we could keep the nicer syntax. I'm trying to avoid running functions on module load. WDYT? Yes making use of static class variables as you suggest in patch 8 would do the job. Thanks for following up on all my comments :) >>>> >>>> Do we need OFPFlowFactory(object):, some times flake reports this as H238. But might no longer be valid with the latest python3. >>> >>> Not 100% sure either, I'll check. Thanks. >>> >>>> >>>>> + """OpenFlow Flow Factory is a class capable of creating OFPFLow objects""" >>>>> + >>>>> + def __init__(self): >>>>> + self.info_decoders = self._info_decoders() >>>>> + self.match_decoders = KVDecoders( >>>> >>>> The name suggests that we return KVDecoders() as the __info_decoders() does, I think all three should return the same. >>>> >>> >>> What name exactly? Do you mean the _flow_match_decoders and _field_decoders? >>> Or do you mean match_decoders should be a function rather than the object directly? >> >> Trying to remember what my thoughts were ;) Guess I wondered why _flow_match_decoders() was not doing the ‘return KVDecoders({**self._field_decoders(), **self._flow_match_decoders()})’. >> > > I'll unify the names to make them more consistent. > > [...] > >>>>> + >>>>> + @classmethod >>>>> + def _output_actions_decoders(cls): >>>>> + """Returns the decoders for the output actions""" >>>> >>>> If you have them, I think it might be useful, to add some example strings to the individual decoders. This way, it's easy to see what they intend to decode. >>>> > > Ack. > > [...] >>> >>>>> +def decode_learn(action_decoders): >>>> >>>> It’s getting late, and I have a hard time (focussing ;) understanding where the value for this one comes from? I'll pick it up from here when I continue the review. >>>> >>> >>> I hope I can clarify. Learn action has two added complexities: >>> >>> - It accepts any other action key-value. For this we need to create a wrapper around the pre-calculated action_decoders. >>> >>> - The way fields can be specified is augmented. Not only we have 'field=value', but we also have 'field=_src_' (where _src_ is another field name) and just 'field'. For this we need to create a wrapper of field_decoders that, for each "field=X" key-value we check if X is a field_name or if it's acually a value that we need to send to the appropriate field_decoder to process. >> >> Ack thanks, it makes sense now. > > > I'll add a comment to make it clearer. > > > > -- > Adrián Moreno
diff --git a/python/automake.mk b/python/automake.mk index 136da26bd..d1464d7f6 100644 --- a/python/automake.mk +++ b/python/automake.mk @@ -46,7 +46,9 @@ ovs_pyfiles = \ python/ovs/flows/decoders.py \ python/ovs/flows/kv.py \ python/ovs/flows/list.py \ - python/ovs/flows/flow.py + python/ovs/flows/flow.py \ + python/ovs/flows/ofp.py \ + python/ovs/flows/ofp_act.py # These python files are used at build time but not runtime, # so they are not installed. diff --git a/python/ovs/flows/decoders.py b/python/ovs/flows/decoders.py index bf7a94ae8..3def9f279 100644 --- a/python/ovs/flows/decoders.py +++ b/python/ovs/flows/decoders.py @@ -6,6 +6,7 @@ object. """ import netaddr +import re class Decoder: @@ -358,3 +359,95 @@ class IPMask(Decoder): def to_json(self): return str(self) + + +def decode_free_output(value): + """Decodes the output value when found free + (without the 'output' keyword)""" + try: + return "output", {"port": int(value)} + except ValueError: + return "output", {"port": value.strip('"')} + + +ipv4 = r"[\d\.]+" +ipv4_capture = r"({ipv4})".format(ipv4=ipv4) +ipv6 = r"[\w:]+" +ipv6_capture = r"(?:\[*)?({ipv6})(?:\]*)?".format(ipv6=ipv6) +port_range = r":(\d+)(?:-(\d+))?" +ip_range_regexp = r"{ip_cap}(?:-{ip_cap})?(?:{port_range})?" +ipv4_port_regex = re.compile( + ip_range_regexp.format(ip_cap=ipv4_capture, port_range=port_range) +) +ipv6_port_regex = re.compile( + ip_range_regexp.format(ip_cap=ipv6_capture, port_range=port_range) +) + + +def decode_ip_port_range(value): + """ + Decodes an IP and port range: + {ip_start}-{ip-end}:{port_start}-{port_end} + + IPv6 addresses are surrounded by "[" and "]" if port ranges are also + present + + Returns the following dictionary: + { + "addrs": { + "start": {ip_start} + "end": {ip_end} + } + "ports": { + "start": {port_start}, + "end": {port_end} + } + (the "ports" key might be omitted) + """ + if value.count(":") > 1: + match = ipv6_port_regex.match(value) + else: + match = ipv4_port_regex.match(value) + + ip_start = match.group(1) + ip_end = match.group(2) + port_start = match.group(3) + port_end = match.group(4) + + result = { + "addrs": { + "start": netaddr.IPAddress(ip_start), + "end": netaddr.IPAddress(ip_end or ip_start), + } + } + if port_start: + result["ports"] = { + "start": int(port_start), + "end": int(port_end or port_start), + } + + return result + + +def decode_nat(value): + """Decodes the 'nat' keyword of the ct action""" + if not value: + return True + + result = dict() + type_parts = value.split("=") + result["type"] = type_parts[0] + + if len(type_parts) > 1: + value_parts = type_parts[1].split(",") + if len(type_parts) != 2: + raise ValueError("Malformed nat action: %s" % value) + + ip_port_range = decode_ip_port_range(value_parts[0]) + + result = {"type": type_parts[0], **ip_port_range} + + for flag in value_parts[1:]: + result[flag] = True + + return result diff --git a/python/ovs/flows/ofp.py b/python/ovs/flows/ofp.py new file mode 100644 index 000000000..e56b08967 --- /dev/null +++ b/python/ovs/flows/ofp.py @@ -0,0 +1,400 @@ +""" Defines the parsers needed to parse ofproto flows +""" + +import functools + +from ovs.flows.kv import KVParser, KVDecoders, nested_kv_decoder +from ovs.flows.ofp_fields import field_decoders +from ovs.flows.flow import Flow, Section +from ovs.flows.list import ListDecoders, nested_list_decoder +from ovs.flows.decoders import ( + decode_default, + decode_flag, + decode_int, + decode_time, + decode_mask, + IPMask, + EthMask, + decode_free_output, + decode_nat, +) +from ovs.flows.ofp_act import ( + decode_output, + decode_field, + decode_controller, + decode_bundle, + decode_bundle_load, + decode_encap_ethernet, + decode_load_field, + decode_set_field, + decode_move_field, + decode_dec_ttl, + decode_chk_pkt_larger, + decode_zone, + decode_exec, + decode_learn, +) + + +class OFPFlow(Flow): + """OFPFLow represents an OpenFlow Flow""" + + def __init__(self, sections, orig="", id=None): + """Constructor""" + super(OFPFlow, self).__init__(sections, orig, id) + + def __str__(self): + if self._orig: + return self._orig + else: + return self.to_string() + + def to_string(self): + """Print a text representation of the flow""" + string = "Info: {}\n" + self.info + string += "Match : {}\n" + self.match + string += "Actions: {}\n " + self.actions + return string + + +class OFPFlowFactory: + """OpenFlow Flow Factory is a class capable of creating OFPFLow objects""" + + def __init__(self): + self.info_decoders = self._info_decoders() + self.match_decoders = KVDecoders( + {**self._field_decoders(), **self._flow_match_decoders()} + ) + self.act_decoders = self._act_decoders() + + def from_string(self, ofp_string, id=None): + """Parse a ofproto flow string + + The string is expected to have the follwoing format: + [flow data] [match] actions=[actions] + + :param ofp_string: a ofproto string as dumped by ovs-ofctl tool + :type ofp_string: str + + :return: an OFPFlow with the content of the flow string + :rtype: OFPFlow + """ + if " reply " in ofp_string: + return None + + sections = list() + parts = ofp_string.split("actions=") + if len(parts) != 2: + raise ValueError("malformed ofproto flow: %s" % ofp_string) + + actions = parts[1] + + field_parts = parts[0].rstrip(" ").rpartition(" ") + if len(field_parts) != 3: + raise ValueError("malformed ofproto flow: %s" % ofp_string) + + info = field_parts[0] + match = field_parts[2] + + iparser = KVParser(self.info_decoders) + iparser.parse(info) + isection = Section( + name="info", + pos=ofp_string.find(info), + string=info, + data=iparser.kv(), + ) + sections.append(isection) + + mparser = KVParser(self.match_decoders) + mparser.parse(match) + msection = Section( + name="match", + pos=ofp_string.find(match), + string=match, + data=mparser.kv(), + ) + sections.append(msection) + + aparser = KVParser(self.act_decoders) + aparser.parse(actions) + asection = Section( + name="actions", + pos=ofp_string.find(actions), + string=actions, + data=aparser.kv(), + is_list=True, + ) + sections.append(asection) + + return OFPFlow(sections, ofp_string, id) + + @classmethod + def _info_decoders(cls): + """Generate the match decoders""" + info = { + "table": decode_int, + "duration": decode_time, + "n_packet": decode_int, + "n_bytes": decode_int, + "cookie": decode_int, + "idle_timeout": decode_time, + "hard_timeout": decode_time, + "hard_age": decode_time, + } + return KVDecoders(info) + + @classmethod + def _flow_match_decoders(cls): + """Returns the decoders for key-values that are part of the flow match + but not a flow field""" + return { + "priority": decode_int, + } + + @classmethod + def _field_decoders(cls): + shorthands = [ + "eth", + "ip", + "ipv6", + "icmp", + "icmp6", + "tcp", + "tcp6", + "udp", + "udp6", + "sctp", + "arp", + "rarp", + "mpls", + "mplsm", + ] + + fields = {**field_decoders, **{key: decode_flag for key in shorthands}} + + # vlan_vid field is special. Although it is technically 12 bit wide, + # bit 12 is allowed to be set to 1 to indicate that the vlan header is + # present (see section VLAN FIELDS in + # http://www.openvswitch.org/support/dist-docs/ovs-fields.7.txt) + # Therefore, override the generated vlan_vid field size + fields["vlan_vid"] = decode_mask(13) + return fields + + @classmethod + def _output_actions_decoders(cls): + """Returns the decoders for the output actions""" + return { + "output": decode_output, + "drop": decode_flag, + "controller": decode_controller, + "enqueue": nested_list_decoder( + ListDecoders([("port", decode_default), ("queue", int)]), + delims=[",", ":"], + ), + "bundle": decode_bundle, + "bundle_load": decode_bundle_load, + "group": decode_default, + } + + @classmethod + def _encap_actions_decoders(cls): + """Returns the decoders for the encap actions""" + + return { + "pop_vlan": decode_flag, + "strip_vlan": decode_flag, + "push_vlan": decode_default, + "decap": decode_flag, + "encap": nested_kv_decoder( + KVDecoders( + { + "nsh": nested_kv_decoder( + KVDecoders( + { + "md_type": decode_default, + "tlv": nested_list_decoder( + ListDecoders( + [ + ("class", decode_int), + ("type", decode_int), + ("value", decode_int), + ] + ) + ), + } + ) + ), + }, + default=None, + default_free=decode_encap_ethernet, + ) + ), + } + + @classmethod + def _field_action_decoders(cls): + """Returns the decoders for the field modification actions""" + # Field modification actions + field_default_decoders = [ + "set_mpls_label", + "set_mpls_tc", + "set_mpls_ttl", + "mod_nw_tos", + "mod_nw_ecn", + "mod_tcp_src", + "mod_tcp_dst", + ] + return { + "load": decode_load_field, + "set_field": functools.partial( + decode_set_field, KVDecoders(cls._field_decoders()) + ), + "move": decode_move_field, + "mod_dl_dst": EthMask, + "mod_dl_src": EthMask, + "mod_nw_dst": IPMask, + "mod_nw_src": IPMask, + "dec_ttl": decode_dec_ttl, + "dec_mpls_ttl": decode_flag, + "dec_nsh_ttl": decode_flag, + "check_pkt_larger": decode_chk_pkt_larger, + **{field: decode_default for field in field_default_decoders}, + } + + @classmethod + def _meta_action_decoders(cls): + """Returns the decoders for the metadata actions""" + meta_default_decoders = ["set_tunnel", "set_tunnel64", "set_queue"] + return { + "pop_queue": decode_flag, + **{field: decode_default for field in meta_default_decoders}, + } + + @classmethod + def _fw_action_decoders(cls): + """Returns the decoders for the Firewalling actions""" + return { + "ct": nested_kv_decoder( + KVDecoders( + { + "commit": decode_flag, + "zone": decode_zone, + "table": decode_int, + "nat": decode_nat, + "force": decode_flag, + "exec": functools.partial( + decode_exec, + KVDecoders( + { + **cls._encap_actions_decoders(), + **cls._field_action_decoders(), + **cls._meta_action_decoders(), + } + ), + ), + "alg": decode_default, + } + ) + ), + "ct_clear": decode_flag, + } + + @classmethod + def _control_action_decoders(cls): + return { + "resubmit": nested_list_decoder( + ListDecoders( + [ + ("port", decode_default), + ("table", decode_int), + ("ct", decode_flag), + ] + ) + ), + "push": decode_field, + "pop": decode_field, + "exit": decode_flag, + "multipath": nested_list_decoder( + ListDecoders( + [ + ("fields", decode_default), + ("basis", decode_int), + ("algorithm", decode_default), + ("n_links", decode_int), + ("arg", decode_int), + ("dst", decode_field), + ] + ) + ), + } + + @classmethod + def _clone_actions_decoders(cls, action_decoders): + """Generate the decoders for clone actions + + Args: + action_decoders (dict): The decoders of the supported nested + actions + """ + return { + "learn": decode_learn( + { + **action_decoders, + "fin_timeout": nested_kv_decoder( + KVDecoders( + { + "idle_timeout": decode_time, + "hard_timeout": decode_time, + } + ) + ), + } + ), + "clone": functools.partial( + decode_exec, KVDecoders(action_decoders) + ), + } + + @classmethod + def _other_action_decoders(cls): + """Recoders for other actions (see man(7) ovs-actions)""" + return { + "conjunction": nested_list_decoder( + ListDecoders( + [("id", decode_int), ("k", decode_int), ("n", decode_int)] + ), + delims=[",", "/"], + ), + "note": decode_default, + "sample": nested_kv_decoder( + KVDecoders( + { + "probability": decode_int, + "collector_set_id": decode_int, + "obs_domain_id": decode_int, + "obs_point_id": decode_int, + "sampling_port": decode_default, + "ingress": decode_flag, + "egress": decode_flag, + } + ) + ), + } + + @classmethod + def _act_decoders(cls): + """Generate the actions decoders""" + + actions = { + **cls._output_actions_decoders(), + **cls._encap_actions_decoders(), + **cls._field_action_decoders(), + **cls._meta_action_decoders(), + **cls._fw_action_decoders(), + **cls._control_action_decoders(), + **cls._other_action_decoders(), + } + clone_actions = cls._clone_actions_decoders(actions) + actions.update(clone_actions) + return KVDecoders(actions, default_free=decode_free_output) diff --git a/python/ovs/flows/ofp_act.py b/python/ovs/flows/ofp_act.py new file mode 100644 index 000000000..bc6574999 --- /dev/null +++ b/python/ovs/flows/ofp_act.py @@ -0,0 +1,233 @@ +""" Defines decoders for openflow actions +""" + +import functools + +from ovs.flows.kv import nested_kv_decoder, KVDecoders, KeyValue, KVParser +from ovs.flows.decoders import ( + decode_default, + decode_time, + decode_flag, + decode_int, +) +from ovs.flows.ofp_fields import field_decoders + + +def decode_output(value): + """Decodes the output value + + Does not support field specification + """ + if len(value.split(",")) > 1: + return nested_kv_decoder()(value) + try: + return {"port": int(value)} + except ValueError: + return {"port": value.strip('"')} + + +def decode_controller(value): + """Decodes the controller action""" + if not value: + return KeyValue("output", "controller") + else: + # Try controller:max_len + try: + max_len = int(value) + return { + "max_len": max_len, + } + except ValueError: + pass + # controller(key[=val], ...) + return nested_kv_decoder()(value) + + +def decode_bundle_load(value): + return decode_bundle(value, True) + + +def decode_bundle(value, load=False): + """Decode bundle action""" + result = {} + keys = ["fields", "basis", "algorithm", "ofport"] + if load: + keys.append("dst") + + for key in keys: + parts = value.partition(",") + nvalue = parts[0] + value = parts[2] + if key == "ofport": + continue + result[key] = decode_default(nvalue) + + # Handle members: + mvalues = value.split("members:") + result["members"] = [int(port) for port in mvalues[1].split(",")] + return result + + +def decode_encap_ethernet(value): + """Decodes encap ethernet value""" + return "ethernet", int(value, 0) + + +def decode_field(value): + """Decodes a field as defined in the 'Field Specification' of the actions + man page: http://www.openvswitch.org/support/dist-docs/ovs-actions.7.txt + """ + parts = value.strip("]\n\r").split("[") + result = { + "field": parts[0], + } + + if len(parts) > 1 and parts[1]: + field_range = parts[1].split("..") + start = field_range[0] + end = field_range[1] if len(field_range) > 1 else start + if start: + result["start"] = int(start) + if end: + result["end"] = int(end) + + return result + + +def decode_load_field(value): + """Decodes 'load:value->dst' actions""" + parts = value.split("->") + if len(parts) != 2: + raise ValueError("Malformed load action : %s" % value) + + # If the load action is performed within a learn() action, + # The value can be specified as another field. + try: + return {"value": int(parts[0], 0), "dst": decode_field(parts[1])} + except ValueError: + return {"src": decode_field(parts[0]), "dst": decode_field(parts[1])} + + +def decode_set_field(field_decoders, value): + """Decodes 'set_field:value/mask->dst' actions + + The value is decoded by field_decoders which is a KVDecoders instance + Args: + field_decoders + """ + parts = value.split("->") + if len(parts) != 2: + raise ValueError("Malformed set_field action : %s" % value) + + val = parts[0] + dst = parts[1] + + val_result = field_decoders.decode(dst, val) + + return { + "value": {val_result[0]: val_result[1]}, + "dst": decode_field(dst), + } + + +def decode_move_field(value): + """Decodes 'move:src->dst' actions""" + parts = value.split("->") + if len(parts) != 2: + raise ValueError("Malformed move action : %s" % value) + + return { + "src": decode_field(parts[0]), + "dst": decode_field(parts[1]), + } + + +def decode_dec_ttl(value): + """Decodes dec_ttl and dec_ttl(id, id[2], ...) actions""" + if not value: + return True + return [int(idx) for idx in value.split(",")] + + +def decode_chk_pkt_larger(value): + """Decodes 'check_pkt_larger(pkt_len)->dst' actions""" + parts = value.split("->") + if len(parts) != 2: + raise ValueError("Malformed check_pkt_larger action : %s" % value) + + pkt_len = int(parts[0].strip("()")) + dst = decode_field(parts[1]) + return {"pkt_len": pkt_len, "dst": dst} + + +# CT decoders +def decode_zone(value): + """Decodes the 'zone' keyword of the ct action""" + try: + return int(value, 0) + except ValueError: + pass + return decode_field(value) + + +def decode_exec(action_decoders, value): + """Decodes the 'exec' keyword of the ct action + + Args: + decode_actions (KVDecoders): the decoders to be used to decode the + nested exec + value (string): the string to be decoded + """ + exec_parser = KVParser(action_decoders) + exec_parser.parse(value) + return [{kv.key: kv.value} for kv in exec_parser.kv()] + + +def decode_learn(action_decoders): + """Create the decoder to be used to decode the 'learn' action. + + The learn action can include any nested action, therefore we need decoders + for all possible actions. + + Args: + action_decoders (dict): dictionary of decoders to be used in nested + action decoding + + """ + + def decode_learn_field(decoder, value): + """Generates a decoder to be used for the 'field' argument of the + 'learn' action. + + The field can hold a value that should be decoded, either as a field, + or as a the value (see man(7) ovs-actions) + + Args: + decoder (callable): The decoder + + """ + if value in field_decoders.keys(): + # It's a field + return value + else: + return decoder(value) + + learn_field_decoders = { + field: functools.partial(decode_learn_field, decoder) + for field, decoder in field_decoders.items() + } + learn_decoders = { + **action_decoders, + **learn_field_decoders, + "idle_timeout": decode_time, + "hard_timeout": decode_time, + "priority": decode_int, + "cooke": decode_int, + "send_flow_rem": decode_flag, + "table": decode_int, + "delete_learned": decode_flag, + "limit": decode_int, + "result_dst": decode_field, + } + + return functools.partial(decode_exec, KVDecoders(learn_decoders))
Introduce OFPFlow class and all its decoders. Most of the decoders are generic (from decoders.py). Some have special syntax and need a specific implementation. Decoders for nat are moved to the common decoders.py because it's syntax is shared with other types of flows (e.g: dpif flows). Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- python/automake.mk | 4 +- python/ovs/flows/decoders.py | 93 ++++++++ python/ovs/flows/ofp.py | 400 +++++++++++++++++++++++++++++++++++ python/ovs/flows/ofp_act.py | 233 ++++++++++++++++++++ 4 files changed, 729 insertions(+), 1 deletion(-) create mode 100644 python/ovs/flows/ofp.py create mode 100644 python/ovs/flows/ofp_act.py