Re: [PATCH net-next 0/5] selftests: openvswitch: pylint cleanup for ovs-dpctl.py

[Aaron Conole <aconole@redhat.com>]

侯敏熙 <houminxi@gmail.com> writes:

> Hello Aaron
>
> Thanks for the suggestion, Aaron.  I dug into this a bit and I think
> it's worth doing.
>
> The good news is that 3 of 4 OVS families already have ynl specs
> in-tree (ovs_datapath, ovs_flow, ovs_vport).  The flow spec is pretty
> thorough -- covers all the key/action/tunnel nested attrs.  pyynl
> handles genetlink, dump, and nested encode/decode, so the library side
> looks solid.  There's even an OVS C sample under tools/net/ynl/samples/.
>
> Gaps I found:
>
>   - No ovs_packet.yaml spec.  We'd need to write one for upcall
>     handling (MISS/ACTION/EXECUTE).  Shouldn't be too bad, maybe
>     100 lines based on the other specs.
>
>   - The existing specs are missing a few operation defs (flow DEL/SET,
>     vport SET, datapath SET).  Small additions.
>
>   - ovs-dpctl.py also uses pyroute2.iproute for tunnel interface
>     creation and NDB for interface lookup.  Those aren't OVS netlink,
>     so we'd either keep a minimal pyroute2 dep for just that or
>     shell out to `ip`.

We could also add support for using just a direct netlink socket to
query for interfaces.  This means we would not need to shell out to
iproute2 binary.  But I think if iproute2 isn't available then the
selftests would also not pass, so subprocess is probably okay.

>   - The ODP string parser (~1500 lines for flow key/action encoding)
>     is orthogonal to the netlink wire format, so that stays regardless.
>
> I think the natural order would be: start with datapath ops (simplest,
> spec already complete), then vport, then flow (bulk of the work), then
> packet/upcall after writing the spec.  The f-string fixes from my
> current series still apply.
>
> Minxi
>
> Aaron Conole <aconole@redhat.com> 于2026年5月15日周五 14:53写道：
>>
>> Minxi Hou <houminxi@gmail.com> writes:
>>
>> > This series cleans up all pylint warnings in ovs-dpctl.py,
>> > bringing the score from 7.62/10 to 10.00/10.
>> >
>> > This series applies on top of:
>> >   [PATCH net-next v10 1/2] selftests: openvswitch: add vlan() and
>> >   encap() flow string parsing
>> >   https://lore.kernel.org/netdev/20260512070841.1183581-2-houminxi@gmail.com/
>> >
>> > Patch 1 converts 86 %-format strings to f-strings.
>> > Patch 2 fixes misc warnings (unused import, bare except, unused
>> > variables, redundant expressions).
>> > Patch 3 renames classes to PascalCase and variables to snake_case.
>> > Patch 4 adds one-line docstrings to all definitions.
>> > Patch 5 suppresses complexity warnings from pyroute2 constraints.
>> >
>> > Tested with vng on x86_64, all OVS selftests pass.
>> >
>> > Minxi Hou (5):
>> >   selftests: openvswitch: convert %-formatting to f-strings
>> >   selftests: openvswitch: fix misc pylint warnings in ovs-dpctl.py
>> >   selftests: openvswitch: rename classes and variables in ovs-dpctl.py
>> >   selftests: openvswitch: add missing docstrings in ovs-dpctl.py
>> >   selftests: openvswitch: suppress pylint complexity warnings
>> >
>> >  .../selftests/net/openvswitch/ovs-dpctl.py    | 794 ++++++++++--------
>> >  1 file changed, 428 insertions(+), 366 deletions(-)
>>
>> Thinking about this series, it might be better to go a bit further and
>> just drop the pyroute2 in favor of using ynl to generate the netlink
>> encode/decode.  The value that ovs-dpctl.py brings is the ability to
>> interoperate with the ovs-vswitchd odp encoding.  That we are using
>> pyroute2 to do the actual 'wire' format of netlink is incidental.
>>
>> Some of the work would still be needed (I think some of the f-string
>> adjustments) but that would allow trimming a good chunk of the code and
>> let us just rely on the in-tree ynl rather than pyroute2 dependency.
>>
>> WDYT?  Ilya and I can work with you offline if you are interested in
>> pursuing this approach.  I think it should make the overall code much
>> better to work with, too.
>>