Re: [RFC net-next 3/7] selftests: openvswitch: Add set() and set_masked() support.

Adrián Moreno
2024-06-17T12:18:48Z

On Thu, Jun 13, 2024 at 02:13:29PM GMT, Aaron Conole wrote:
> These will be used in upcoming commits to set specific attributes for
> interacting with tunnels. Since set() will use the key parsing routine, we
> also make sure to prepend it with an open paren, for the action parsing to
> properly understand it.
>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
> .../selftests/net/openvswitch/ovs-dpctl.py | 39 +++++++++++++++++--
> 1 file changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> index 73768f3af6e5..fee64c31d4d4 100644
> --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> @@ -284,7 +284,7 @@ class ovsactions(nla):
> ("OVS_ACTION_ATTR_UNSPEC", "none"),
> ("OVS_ACTION_ATTR_OUTPUT", "uint32"),
> ("OVS_ACTION_ATTR_USERSPACE", "userspace"),
> - ("OVS_ACTION_ATTR_SET", "none"),
> + ("OVS_ACTION_ATTR_SET", "ovskey"),
> ("OVS_ACTION_ATTR_PUSH_VLAN", "none"),
> ("OVS_ACTION_ATTR_POP_VLAN", "flag"),
> ("OVS_ACTION_ATTR_SAMPLE", "none"),
> @@ -292,7 +292,7 @@ class ovsactions(nla):
> ("OVS_ACTION_ATTR_HASH", "none"),
> ("OVS_ACTION_ATTR_PUSH_MPLS", "none"),
> ("OVS_ACTION_ATTR_POP_MPLS", "flag"),
> - ("OVS_ACTION_ATTR_SET_MASKED", "none"),
> + ("OVS_ACTION_ATTR_SET_MASKED", "ovskey"),
> ("OVS_ACTION_ATTR_CT", "ctact"),
> ("OVS_ACTION_ATTR_TRUNC", "uint32"),
> ("OVS_ACTION_ATTR_PUSH_ETH", "none"),
> @@ -469,6 +469,14 @@ class ovsactions(nla):
> print_str += "clone("
> print_str += datum.dpstr(more)
> print_str += ")"
> + elif field[0] == "OVS_ACTION_ATTR_SET" or \
> + field[0] == "OVS_ACTION_ATTR_SET_MASKED":
> + print_str += "set"
> + if field[0] == "OVS_ACTION_ATTR_SET_MASKED":
> + print_str += "_masked"
> + print_str += "("
> + print_str += datum.dpstr(more)
> + print_str += ")"
> else:
> try:
> print_str += datum.dpstr(more)
> @@ -547,6 +555,25 @@ class ovsactions(nla):
> self["attrs"].append(("OVS_ACTION_ATTR_CLONE", subacts))
> actstr = actstr[parsedLen:]
> parsed = True
> + elif parse_starts_block(actstr, "set(", False):
> + parencount += 1
> + k = ovskey()
> + actstr = actstr[len("set("):]
> + actstr = k.parse(actstr, None)
> + self["attrs"].append(("OVS_ACTION_ATTR_SET", k))
> + if not actstr.startswith(")"):
> + actstr = ")" + actstr
> + parsed = True
> + elif parse_starts_block(actstr, "set_masked(", False):
> + parencount += 1
> + k = ovskey()
> + m = ovskey()
> + actstr = actstr[len("set_masked("):]
> + actstr = k.parse(actstr, m)
> + self["attrs"].append(("OVS_ACTION_ATTR_SET_MASKED", [k, m]))
> + if not actstr.startswith(")"):
> + actstr = ")" + actstr
> + parsed = True
> elif parse_starts_block(actstr, "ct(", False):
> parencount += 1
> actstr = actstr[len("ct(") :]
> @@ -1312,7 +1339,7 @@ class ovskey(nla):
> mask["attrs"].append([field[0], m])
> self["attrs"].append([field[0], k])
>
> - flowstr = flowstr[strspn(flowstr, "),") :]
> + flowstr = flowstr[strspn(flowstr, "), ") :]
>
> return flowstr
>
> @@ -1898,7 +1925,11 @@ class OvsFlow(GenericNetlinkSocket):
> ):
> print_str += "drop"
> else:
> - print_str += actsmsg.dpstr(more)
> + if type(actsmsg) == "list":

nit: I belive the recommended way of comparing types is using
"isinstance":

https://www.flake8rules.com/rules/E721.html

Also, I don't see what can make actmsg be a list. It should always be an
instance of "ovsactions", right?

> + for act in actsmsg:
> + print_str += act.dpstr(more)
> + else:
> + print_str += actsmsg.dpstr(more)
>
> return print_str
>
> --
> 2.45.1
>