| Message ID | 20240603183121.2305013-1-amorenoz@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [net-next,1/2] selftests: openvswitch: fix action formatting | expand |
Adrian Moreno <amorenoz@redhat.com> writes: > Netlink flags, although they don't have payload at the netlink level, > are represented as having a "True" value in pyroute2. > > Without it, trying to add a flow with a flag-type action (e.g: pop_vlan) > fails with the following traceback: > > Traceback (most recent call last): > File "[...]/ovs-dpctl.py", line 2498, in <module> > sys.exit(main(sys.argv)) > ^^^^^^^^^^^^^^ > File "[...]/ovs-dpctl.py", line 2487, in main > ovsflow.add_flow(rep["dpifindex"], flow) > File "[...]/ovs-dpctl.py", line 2136, in add_flow > reply = self.nlm_request( > ^^^^^^^^^^^^^^^^^ > File "[...]/pyroute2/netlink/nlsocket.py", line 822, in nlm_request > return tuple(self._genlm_request(*argv, **kwarg)) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File "[...]/pyroute2/netlink/generic/__init__.py", line 126, in > nlm_request > return tuple(super().nlm_request(*argv, **kwarg)) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File "[...]/pyroute2/netlink/nlsocket.py", line 1124, in nlm_request > self.put(msg, msg_type, msg_flags, msg_seq=msg_seq) > File "[...]/pyroute2/netlink/nlsocket.py", line 389, in put > self.sendto_gate(msg, addr) > File "[...]/pyroute2/netlink/nlsocket.py", line 1056, in sendto_gate > msg.encode() > File "[...]/pyroute2/netlink/__init__.py", line 1245, in encode > offset = self.encode_nlas(offset) > ^^^^^^^^^^^^^^^^^^^^^^^^ > File "[...]/pyroute2/netlink/__init__.py", line 1560, in encode_nlas > nla_instance.setvalue(cell[1]) > File "[...]/pyroute2/netlink/__init__.py", line 1265, in setvalue > nlv.setvalue(nla_tuple[1]) > ~~~~~~~~~^^^ > IndexError: list index out of range > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- Acked-by: Aaron Conole <aconole@redhat.com> I don't know which pyroute2 version I had used when I tested this previously, but even on my current system I get this error now. Thanks for the fix. > tools/testing/selftests/net/openvswitch/ovs-dpctl.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py > index b76907ac0092..a2395c3f37a1 100644 > --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py > +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py > @@ -537,7 +537,7 @@ class ovsactions(nla): > for flat_act in parse_flat_map: > if parse_starts_block(actstr, flat_act[0], False): > actstr = actstr[len(flat_act[0]):] > - self["attrs"].append([flat_act[1]]) > + self["attrs"].append([flat_act[1], True]) > actstr = actstr[strspn(actstr, ", ") :] > parsed = True
On Mon, 03 Jun 2024 15:00:03 -0400 Aaron Conole wrote: > I agree, this is an issue. BUT I think it might be better to just > filter by field type up front. See: > > https://github.com/apconole/linux-next-work/commit/7262107de7170d44b6dbf6c5ea6f7e6c0bb71d36#diff-3e72e7405c6bb4e9842bed5f63883ca930387086bb40d4034e92ed83a5decb4bR441 > > That version I think ends up being much easier to follow. If you want > to take it for your series, feel free. If you disagree, maybe there's > something I'm not considering about it. > > NOTE that version is just a bunch of independent changes that are > squashed together. I have a cleaner version. > > I can also bundle up the series I have so far and submit, but I didn't > want to do that until I got all the pmtu.sh support working. Maybe it > makes sense to send it now though. Simon, Jakub - wdyt? I'd say - hold onto the changes until pmtu.sh works, unless there's *any* reason for a particular patch to go in early, eg: - patch fixes existing bug - someone else needs a patch - ... - a patch which falls under some of the criteria above depends on the patch
On Mon, Jun 03, 2024 at 03:00:03PM GMT, Aaron Conole wrote: > Adrian Moreno <amorenoz@redhat.com> writes: > > > In the action formatting function ("dpstr"), the iteration is made over > > the nla_map, so if there are more than one attribute from the same type > > we only print the first one. > > > > Fix this by iterating over the actual attributes. > > > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > > --- > > .../selftests/net/openvswitch/ovs-dpctl.py | 48 +++++++++++-------- > > 1 file changed, 27 insertions(+), 21 deletions(-) > > > > diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py > > index 1dd057afd3fb..b76907ac0092 100644 > > --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py > > +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py > > @@ -437,40 +437,46 @@ class ovsactions(nla): > > def dpstr(self, more=False): > > print_str = "" > > > > - for field in self.nla_map: > > - if field[1] == "none" or self.get_attr(field[0]) is None: > > + for attr_name, value in self["attrs"]: > > + attr_desc = next(filter(lambda x: x[0] == attr_name, self.nla_map), > > + None) > > + if not attr_desc: > > + raise ValueError("Unknown attribute: %s" % attr) > > + > > + attr_type = attr_desc[1] > > + > > + if attr_type == "none": > > I agree, this is an issue. BUT I think it might be better to just > filter by field type up front. See: > > https://github.com/apconole/linux-next-work/commit/7262107de7170d44b6dbf6c5ea6f7e6c0bb71d36#diff-3e72e7405c6bb4e9842bed5f63883ca930387086bb40d4034e92ed83a5decb4bR441 > > That version I think ends up being much easier to follow. If you want > to take it for your series, feel free. If you disagree, maybe there's > something I'm not considering about it. > I agree. It's better to check field attribute names first. I found this during manual testing of the "emit_sample" series but I ended up not needing it for the automated one, so I'm OK waiting for your cleanup series. In fact, I also have some patches that try some rework of this part. In particular, I tried to unify all attributes under a common base class that would handle printing and parsing. That way, most cases would fall into "print_str += datum.dpstr(more)" and the "if/elif" block would shrink significantly. > NOTE that version is just a bunch of independent changes that are > squashed together. I have a cleaner version. > > I can also bundle up the series I have so far and submit, but I didn't > want to do that until I got all the pmtu.sh support working. Maybe it > makes sense to send it now though. Simon, Jakub - wdyt? > > > continue > > if print_str != "": > > print_str += "," > > > > - if field[1] == "uint32": > > - if field[0] == "OVS_ACTION_ATTR_OUTPUT": > > - print_str += "%d" % int(self.get_attr(field[0])) > > - elif field[0] == "OVS_ACTION_ATTR_RECIRC": > > - print_str += "recirc(0x%x)" % int(self.get_attr(field[0])) > > - elif field[0] == "OVS_ACTION_ATTR_TRUNC": > > - print_str += "trunc(%d)" % int(self.get_attr(field[0])) > > - elif field[0] == "OVS_ACTION_ATTR_DROP": > > - print_str += "drop(%d)" % int(self.get_attr(field[0])) > > - elif field[1] == "flag": > > - if field[0] == "OVS_ACTION_ATTR_CT_CLEAR": > > + if attr_type == "uint32": > > + if attr_name == "OVS_ACTION_ATTR_OUTPUT": > > + print_str += "%d" % int(value) > > + elif attr_name == "OVS_ACTION_ATTR_RECIRC": > > + print_str += "recirc(0x%x)" % int(value) > > + elif attr_name == "OVS_ACTION_ATTR_TRUNC": > > + print_str += "trunc(%d)" % int(value) > > + elif attr_name == "OVS_ACTION_ATTR_DROP": > > + print_str += "drop(%d)" % int(value) > > + elif attr_type == "flag": > > + if attr_name == "OVS_ACTION_ATTR_CT_CLEAR": > > print_str += "ct_clear" > > - elif field[0] == "OVS_ACTION_ATTR_POP_VLAN": > > + elif attr_name == "OVS_ACTION_ATTR_POP_VLAN": > > print_str += "pop_vlan" > > - elif field[0] == "OVS_ACTION_ATTR_POP_ETH": > > + elif attr_name == "OVS_ACTION_ATTR_POP_ETH": > > print_str += "pop_eth" > > - elif field[0] == "OVS_ACTION_ATTR_POP_NSH": > > + elif attr_name == "OVS_ACTION_ATTR_POP_NSH": > > print_str += "pop_nsh" > > - elif field[0] == "OVS_ACTION_ATTR_POP_MPLS": > > + elif attr_name == "OVS_ACTION_ATTR_POP_MPLS": > > print_str += "pop_mpls" > > else: > > - datum = self.get_attr(field[0]) > > - if field[0] == "OVS_ACTION_ATTR_CLONE": > > + if attr_name == "OVS_ACTION_ATTR_CLONE": > > print_str += "clone(" > > - print_str += datum.dpstr(more) > > + print_str += value.dpstr(more) > > print_str += ")" > > else: > > - print_str += datum.dpstr(more) > > + print_str += value.dpstr(more) > > > > return print_str >
Adrián Moreno <amorenoz@redhat.com> writes: > On Mon, Jun 03, 2024 at 03:00:03PM GMT, Aaron Conole wrote: >> Adrian Moreno <amorenoz@redhat.com> writes: >> >> > In the action formatting function ("dpstr"), the iteration is made over >> > the nla_map, so if there are more than one attribute from the same type >> > we only print the first one. >> > >> > Fix this by iterating over the actual attributes. >> > >> > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >> > --- >> > .../selftests/net/openvswitch/ovs-dpctl.py | 48 +++++++++++-------- >> > 1 file changed, 27 insertions(+), 21 deletions(-) >> > >> > diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py >> > index 1dd057afd3fb..b76907ac0092 100644 >> > --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py >> > +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py >> > @@ -437,40 +437,46 @@ class ovsactions(nla): >> > def dpstr(self, more=False): >> > print_str = "" >> > >> > - for field in self.nla_map: >> > - if field[1] == "none" or self.get_attr(field[0]) is None: >> > + for attr_name, value in self["attrs"]: >> > + attr_desc = next(filter(lambda x: x[0] == attr_name, self.nla_map), >> > + None) >> > + if not attr_desc: >> > + raise ValueError("Unknown attribute: %s" % attr) >> > + >> > + attr_type = attr_desc[1] >> > + >> > + if attr_type == "none": >> >> I agree, this is an issue. BUT I think it might be better to just >> filter by field type up front. See: >> >> https://github.com/apconole/linux-next-work/commit/7262107de7170d44b6dbf6c5ea6f7e6c0bb71d36#diff-3e72e7405c6bb4e9842bed5f63883ca930387086bb40d4034e92ed83a5decb4bR441 >> >> That version I think ends up being much easier to follow. If you want >> to take it for your series, feel free. If you disagree, maybe there's >> something I'm not considering about it. >> > > I agree. It's better to check field attribute names first. I found this > during manual testing of the "emit_sample" series but I ended up not > needing it for the automated one, so I'm OK waiting for your cleanup > series. I'll get stuff out this week for it. > In fact, I also have some patches that try some rework of this part. In > particular, I tried to unify all attributes under a common base class > that would handle printing and parsing. That way, most cases would fall > into "print_str += datum.dpstr(more)" and the "if/elif" block would > shrink significantly. That sounds very good. >> NOTE that version is just a bunch of independent changes that are >> squashed together. I have a cleaner version. >> >> I can also bundle up the series I have so far and submit, but I didn't >> want to do that until I got all the pmtu.sh support working. Maybe it >> makes sense to send it now though. Simon, Jakub - wdyt? >> >> > continue >> > if print_str != "": >> > print_str += "," >> > >> > - if field[1] == "uint32": >> > - if field[0] == "OVS_ACTION_ATTR_OUTPUT": >> > - print_str += "%d" % int(self.get_attr(field[0])) >> > - elif field[0] == "OVS_ACTION_ATTR_RECIRC": >> > - print_str += "recirc(0x%x)" % int(self.get_attr(field[0])) >> > - elif field[0] == "OVS_ACTION_ATTR_TRUNC": >> > - print_str += "trunc(%d)" % int(self.get_attr(field[0])) >> > - elif field[0] == "OVS_ACTION_ATTR_DROP": >> > - print_str += "drop(%d)" % int(self.get_attr(field[0])) >> > - elif field[1] == "flag": >> > - if field[0] == "OVS_ACTION_ATTR_CT_CLEAR": >> > + if attr_type == "uint32": >> > + if attr_name == "OVS_ACTION_ATTR_OUTPUT": >> > + print_str += "%d" % int(value) >> > + elif attr_name == "OVS_ACTION_ATTR_RECIRC": >> > + print_str += "recirc(0x%x)" % int(value) >> > + elif attr_name == "OVS_ACTION_ATTR_TRUNC": >> > + print_str += "trunc(%d)" % int(value) >> > + elif attr_name == "OVS_ACTION_ATTR_DROP": >> > + print_str += "drop(%d)" % int(value) >> > + elif attr_type == "flag": >> > + if attr_name == "OVS_ACTION_ATTR_CT_CLEAR": >> > print_str += "ct_clear" >> > - elif field[0] == "OVS_ACTION_ATTR_POP_VLAN": >> > + elif attr_name == "OVS_ACTION_ATTR_POP_VLAN": >> > print_str += "pop_vlan" >> > - elif field[0] == "OVS_ACTION_ATTR_POP_ETH": >> > + elif attr_name == "OVS_ACTION_ATTR_POP_ETH": >> > print_str += "pop_eth" >> > - elif field[0] == "OVS_ACTION_ATTR_POP_NSH": >> > + elif attr_name == "OVS_ACTION_ATTR_POP_NSH": >> > print_str += "pop_nsh" >> > - elif field[0] == "OVS_ACTION_ATTR_POP_MPLS": >> > + elif attr_name == "OVS_ACTION_ATTR_POP_MPLS": >> > print_str += "pop_mpls" >> > else: >> > - datum = self.get_attr(field[0]) >> > - if field[0] == "OVS_ACTION_ATTR_CLONE": >> > + if attr_name == "OVS_ACTION_ATTR_CLONE": >> > print_str += "clone(" >> > - print_str += datum.dpstr(more) >> > + print_str += value.dpstr(more) >> > print_str += ")" >> > else: >> > - print_str += datum.dpstr(more) >> > + print_str += value.dpstr(more) >> > >> > return print_str >>
On Mon, Jun 03, 2024 at 03:02:46PM GMT, Aaron Conole wrote: > Adrian Moreno <amorenoz@redhat.com> writes: > > > Netlink flags, although they don't have payload at the netlink level, > > are represented as having a "True" value in pyroute2. > > > > Without it, trying to add a flow with a flag-type action (e.g: pop_vlan) > > fails with the following traceback: > > > > Traceback (most recent call last): > > File "[...]/ovs-dpctl.py", line 2498, in <module> > > sys.exit(main(sys.argv)) > > ^^^^^^^^^^^^^^ > > File "[...]/ovs-dpctl.py", line 2487, in main > > ovsflow.add_flow(rep["dpifindex"], flow) > > File "[...]/ovs-dpctl.py", line 2136, in add_flow > > reply = self.nlm_request( > > ^^^^^^^^^^^^^^^^^ > > File "[...]/pyroute2/netlink/nlsocket.py", line 822, in nlm_request > > return tuple(self._genlm_request(*argv, **kwarg)) > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > File "[...]/pyroute2/netlink/generic/__init__.py", line 126, in > > nlm_request > > return tuple(super().nlm_request(*argv, **kwarg)) > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > File "[...]/pyroute2/netlink/nlsocket.py", line 1124, in nlm_request > > self.put(msg, msg_type, msg_flags, msg_seq=msg_seq) > > File "[...]/pyroute2/netlink/nlsocket.py", line 389, in put > > self.sendto_gate(msg, addr) > > File "[...]/pyroute2/netlink/nlsocket.py", line 1056, in sendto_gate > > msg.encode() > > File "[...]/pyroute2/netlink/__init__.py", line 1245, in encode > > offset = self.encode_nlas(offset) > > ^^^^^^^^^^^^^^^^^^^^^^^^ > > File "[...]/pyroute2/netlink/__init__.py", line 1560, in encode_nlas > > nla_instance.setvalue(cell[1]) > > File "[...]/pyroute2/netlink/__init__.py", line 1265, in setvalue > > nlv.setvalue(nla_tuple[1]) > > ~~~~~~~~~^^^ > > IndexError: list index out of range > > > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > > --- > > Acked-by: Aaron Conole <aconole@redhat.com> > > I don't know which pyroute2 version I had used when I tested this > previously, but even on my current system I get this error now. Thanks > for the fix. > Thanks Aaron. I'll resend as v2 with your ack as a stand-alone patch since the other patch of this series will be fixed by your soon-to-come series. > > tools/testing/selftests/net/openvswitch/ovs-dpctl.py | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py > > index b76907ac0092..a2395c3f37a1 100644 > > --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py > > +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py > > @@ -537,7 +537,7 @@ class ovsactions(nla): > > for flat_act in parse_flat_map: > > if parse_starts_block(actstr, flat_act[0], False): > > actstr = actstr[len(flat_act[0]):] > > - self["attrs"].append([flat_act[1]]) > > + self["attrs"].append([flat_act[1], True]) > > actstr = actstr[strspn(actstr, ", ") :] > > parsed = True >
Adrián Moreno <amorenoz@redhat.com> writes: > On Mon, Jun 03, 2024 at 03:02:46PM GMT, Aaron Conole wrote: >> Adrian Moreno <amorenoz@redhat.com> writes: >> >> > Netlink flags, although they don't have payload at the netlink level, >> > are represented as having a "True" value in pyroute2. >> > >> > Without it, trying to add a flow with a flag-type action (e.g: pop_vlan) >> > fails with the following traceback: >> > >> > Traceback (most recent call last): >> > File "[...]/ovs-dpctl.py", line 2498, in <module> >> > sys.exit(main(sys.argv)) >> > ^^^^^^^^^^^^^^ >> > File "[...]/ovs-dpctl.py", line 2487, in main >> > ovsflow.add_flow(rep["dpifindex"], flow) >> > File "[...]/ovs-dpctl.py", line 2136, in add_flow >> > reply = self.nlm_request( >> > ^^^^^^^^^^^^^^^^^ >> > File "[...]/pyroute2/netlink/nlsocket.py", line 822, in nlm_request >> > return tuple(self._genlm_request(*argv, **kwarg)) >> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> > File "[...]/pyroute2/netlink/generic/__init__.py", line 126, in >> > nlm_request >> > return tuple(super().nlm_request(*argv, **kwarg)) >> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> > File "[...]/pyroute2/netlink/nlsocket.py", line 1124, in nlm_request >> > self.put(msg, msg_type, msg_flags, msg_seq=msg_seq) >> > File "[...]/pyroute2/netlink/nlsocket.py", line 389, in put >> > self.sendto_gate(msg, addr) >> > File "[...]/pyroute2/netlink/nlsocket.py", line 1056, in sendto_gate >> > msg.encode() >> > File "[...]/pyroute2/netlink/__init__.py", line 1245, in encode >> > offset = self.encode_nlas(offset) >> > ^^^^^^^^^^^^^^^^^^^^^^^^ >> > File "[...]/pyroute2/netlink/__init__.py", line 1560, in encode_nlas >> > nla_instance.setvalue(cell[1]) >> > File "[...]/pyroute2/netlink/__init__.py", line 1265, in setvalue >> > nlv.setvalue(nla_tuple[1]) >> > ~~~~~~~~~^^^ >> > IndexError: list index out of range >> > >> > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >> > --- >> >> Acked-by: Aaron Conole <aconole@redhat.com> >> >> I don't know which pyroute2 version I had used when I tested this >> previously, but even on my current system I get this error now. Thanks >> for the fix. >> > > Thanks Aaron. I'll resend as v2 with your ack as a stand-alone patch > since the other patch of this series will be fixed by your soon-to-come > series. Thanks! >> > tools/testing/selftests/net/openvswitch/ovs-dpctl.py | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py >> > index b76907ac0092..a2395c3f37a1 100644 >> > --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py >> > +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py >> > @@ -537,7 +537,7 @@ class ovsactions(nla): >> > for flat_act in parse_flat_map: >> > if parse_starts_block(actstr, flat_act[0], False): >> > actstr = actstr[len(flat_act[0]):] >> > - self["attrs"].append([flat_act[1]]) >> > + self["attrs"].append([flat_act[1], True]) >> > actstr = actstr[strspn(actstr, ", ") :] >> > parsed = True >>
diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py index 1dd057afd3fb..b76907ac0092 100644 --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py @@ -437,40 +437,46 @@ class ovsactions(nla): def dpstr(self, more=False): print_str = "" - for field in self.nla_map: - if field[1] == "none" or self.get_attr(field[0]) is None: + for attr_name, value in self["attrs"]: + attr_desc = next(filter(lambda x: x[0] == attr_name, self.nla_map), + None) + if not attr_desc: + raise ValueError("Unknown attribute: %s" % attr) + + attr_type = attr_desc[1] + + if attr_type == "none": continue if print_str != "": print_str += "," - if field[1] == "uint32": - if field[0] == "OVS_ACTION_ATTR_OUTPUT": - print_str += "%d" % int(self.get_attr(field[0])) - elif field[0] == "OVS_ACTION_ATTR_RECIRC": - print_str += "recirc(0x%x)" % int(self.get_attr(field[0])) - elif field[0] == "OVS_ACTION_ATTR_TRUNC": - print_str += "trunc(%d)" % int(self.get_attr(field[0])) - elif field[0] == "OVS_ACTION_ATTR_DROP": - print_str += "drop(%d)" % int(self.get_attr(field[0])) - elif field[1] == "flag": - if field[0] == "OVS_ACTION_ATTR_CT_CLEAR": + if attr_type == "uint32": + if attr_name == "OVS_ACTION_ATTR_OUTPUT": + print_str += "%d" % int(value) + elif attr_name == "OVS_ACTION_ATTR_RECIRC": + print_str += "recirc(0x%x)" % int(value) + elif attr_name == "OVS_ACTION_ATTR_TRUNC": + print_str += "trunc(%d)" % int(value) + elif attr_name == "OVS_ACTION_ATTR_DROP": + print_str += "drop(%d)" % int(value) + elif attr_type == "flag": + if attr_name == "OVS_ACTION_ATTR_CT_CLEAR": print_str += "ct_clear" - elif field[0] == "OVS_ACTION_ATTR_POP_VLAN": + elif attr_name == "OVS_ACTION_ATTR_POP_VLAN": print_str += "pop_vlan" - elif field[0] == "OVS_ACTION_ATTR_POP_ETH": + elif attr_name == "OVS_ACTION_ATTR_POP_ETH": print_str += "pop_eth" - elif field[0] == "OVS_ACTION_ATTR_POP_NSH": + elif attr_name == "OVS_ACTION_ATTR_POP_NSH": print_str += "pop_nsh" - elif field[0] == "OVS_ACTION_ATTR_POP_MPLS": + elif attr_name == "OVS_ACTION_ATTR_POP_MPLS": print_str += "pop_mpls" else: - datum = self.get_attr(field[0]) - if field[0] == "OVS_ACTION_ATTR_CLONE": + if attr_name == "OVS_ACTION_ATTR_CLONE": print_str += "clone(" - print_str += datum.dpstr(more) + print_str += value.dpstr(more) print_str += ")" else: - print_str += datum.dpstr(more) + print_str += value.dpstr(more) return print_str
In the action formatting function ("dpstr"), the iteration is made over the nla_map, so if there are more than one attribute from the same type we only print the first one. Fix this by iterating over the actual attributes. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- .../selftests/net/openvswitch/ovs-dpctl.py | 48 +++++++++++-------- 1 file changed, 27 insertions(+), 21 deletions(-)