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 <[email protected]> >>> --- >>> 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
