Adrián Moreno <amorenoz@xxxxxxxxxx> writes: > 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@xxxxxxxxxx> >> --- >> .../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? Yes, you're right. This was some debug code that I was messing with and it made it into this submission. I've dropped it :) Thanks for the review! > >> + for act in actsmsg: >> + print_str += act.dpstr(more) >> + else: >> + print_str += actsmsg.dpstr(more) >> >> return print_str >> >> -- >> 2.45.1 >>