[PATCH v2 1/2] usertools/devbind: update coding style
Burakov, Anatoly
anatoly.burakov at intel.com
Wed Dec 4 10:02:03 CET 2024
On 12/3/2024 11:16 PM, Stephen Hemminger wrote:
> On Tue, 3 Dec 2024 11:25:00 +0000
> Anatoly Burakov <anatoly.burakov at intel.com> wrote:
>
>> Devbind is one of the oldest tools in DPDK, and is written in a way that
>> uses a lot of string matching, no type safety, lots of global variables,
>> and has a few inconsistencies in the way it handles data (such as
>> differences between lspci calls and parsing in different circumstances).
>>
>> This patch is a nigh complete rewrite of devbind, with full 100% feature
>> and command-line compatibility with the old version (except for dropping
>> older kernel support), albeit with a few differences in formatting and
>> error messages. All file handling code has also been replaced with
>> context managers.
>>
>> What's different from old code:
>> - Full PEP-484 compliance
>> - Formatted with Ruff
>> - Much better structured code
>> - Clean and consistent control flow
>> - More comments
>> - Better error handling
>> - Fewer lspci calls
>> - Unified lspci parsing
>> - Using /sys/bus/pci/drivers as a source of truth about kernel modules
>> - Check for iproute2 package
>> - Use JSON parsing for iproute2 output
>> - Deprecate --status-dev in favor of optional --status argument
>> - Deprecate kernel <3.15 support and only use driver_override
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>
>> ---
>
> Looks great, like it.
>
> Only suggestion (which you can ignore) would be to make DevbindCtx
> an object with methods bind_devices and print_status, that might simplify.
The intention was that DevbindCtx is for processing command-line
configuration and for keeping reference to Devbind which does actual
work. I feel like the only thing it will simplify is instead of passing
ctx around we'll be passing self. I will look into it though, maybe
there are some opportunities that I'm missing.
>
> Reviewed-by: Stephen HEmminger <stephen at networkplumber.org>
Thanks!
--
Thanks,
Anatoly
More information about the dev
mailing list