On Thu, Jun 13, 2024 at 02:13:30PM -0400, Aaron Conole wrote:
This will be used when setting details about the tunnel to use as transport. There is a difference between the ODP format between tunnel(): the 'key' flag is not actually a flag field, so we don't support it in the same way that the vswitchd userspace supports displaying it.
Signed-off-by: Aaron Conole aconole@redhat.com
...
@@ -1265,6 +1265,165 @@ class ovskey(nla): init=init, )
- class ovs_key_tunnel(nla):
nla_flags = NLA_F_NESTEDnla_map = (("OVS_TUNNEL_KEY_ATTR_ID", "be64"),("OVS_TUNNEL_KEY_ATTR_IPV4_SRC", "ipaddr"),("OVS_TUNNEL_KEY_ATTR_IPV4_DST", "ipaddr"),("OVS_TUNNEL_KEY_ATTR_TOS", "uint8"),("OVS_TUNNEL_KEY_ATTR_TTL", "uint8"),("OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT", "flag"),("OVS_TUNNEL_KEY_ATTR_CSUM", "flag"),("OVS_TUNNEL_KEY_ATTR_OAM", "flag"),("OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS", "array(uint32)"),("OVS_TUNNEL_KEY_ATTR_TP_SRC", "be16"),("OVS_TUNNEL_KEY_ATTR_TP_DST", "be16"),("OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS", "none"),("OVS_TUNNEL_KEY_ATTR_IPV6_SRC", "ipaddr"),("OVS_TUNNEL_KEY_ATTR_IPV6_DST", "ipaddr"),("OVS_TUNNEL_KEY_ATTR_PAD", "none"),("OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS", "none"),("OVS_TUNNEL_KEY_ATTR_IPV4_INFO_BRIDGE", "flag"),)def parse(self, flowstr, mask=None):if not flowstr.startswith("tunnel("):return None, Nonek = ovskey.ovs_key_tunnel()if mask is not None:mask = ovskey.ovs_key_tunnel()flowstr = flowstr[len("tunnel("):]v6_address = Nonefields = [("tun_id=", r"(\d+)", int, "OVS_TUNNEL_KEY_ATTR_ID",0xffffffffffffffff, None, None),("src=", r"([0-9a-fA-F\.]+)", str,"OVS_TUNNEL_KEY_ATTR_IPV4_SRC", "255.255.255.255", "0.0.0.0",False),("dst=", r"([0-9a-fA-F\.]+)", str,"OVS_TUNNEL_KEY_ATTR_IPV4_DST", "255.255.255.255", "0.0.0.0",False),("ipv6_src=", r"([0-9a-fA-F:]+)", str,"OVS_TUNNEL_KEY_ATTR_IPV6_SRC","ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", "::", True),("ipv6_dst=", r"([0-9a-fA-F:]+)", str,"OVS_TUNNEL_KEY_ATTR_IPV6_DST","ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", "::", True),("tos=", r"(\d+)", int, "OVS_TUNNEL_KEY_ATTR_TOS", 255, 0,None),("ttl=", r"(\d+)", int, "OVS_TUNNEL_KEY_ATTR_TTL", 255, 0,None),("tp_src=", r"(\d+)", int, "OVS_TUNNEL_KEY_ATTR_TP_SRC",65535, 0, None),("tp_dst=", r"(\d+)", int, "OVS_TUNNEL_KEY_ATTR_TP_DST",65535, 0, None),]forced_include = ["OVS_TUNNEL_KEY_ATTR_TTL"]for prefix, regex, typ, attr_name, mask_val, default_val, v46_flag in fields:flowstr, value = parse_extract_field(flowstr, prefix, regex, typ, False)if not attr_name:raise Exception("Bad list value in tunnel fields")if value is None and attr_name in forced_include:value = default_valmask_val = default_valif value is not None:if v6_address is None and v46_flag is not None:v6_address = v46_flag
By my reading, at this point v6_address will only be None if v46_flag is not None. IF so, the condition below seems excessive.
if v6_address is not None and v46_flag is not None \and v46_flag != v6_address:raise ValueError("Cannot mix v6 and v4 addresses")
I wonder if we can instead express this as (completely untested!):
if v46_flag is not None: if v6_address is None: v6_address = v46_flag if v46_flag != v6_address: raise ValueError("Cannot mix v6 and v4 addresses")
k["attrs"].append([attr_name, value])if mask is not None:mask["attrs"].append([attr_name, mask_val])else:if v6_address is not None and v46_flag is not None \and v46_flag != v6_address:continueif v6_address is None and v46_flag is not None:continue
And I wonder if this is a bit easier on the eyes (also completely untested):
if v46_flag is not None: if v6_address is None or v46_flag != v6_address: continue
if mask is not None:mask["attrs"].append([attr_name, default_val])if k["attrs"][0][0] != "OVS_TUNNEL_KEY_ATTR_ID":raise ValueError("Needs a tunid set")
...
@@ -1745,7 +1905,7 @@ class OvsVport(GenericNetlinkSocket): ) TUNNEL_DEFAULTS = [("geneve", 6081),
("vxlan", 4798)]
("vxlan", 4789)]for tnl in TUNNEL_DEFAULTS: if ptype == tnl[0]:
As noted in my response to PATCH 1/7, I think that the change in the hunk above belongs there rather than here.