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(-)

-- 
2.53.0

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.

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`.

  - 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.
>

侯敏熙 <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.
>>