Re: [PATCH v1 1/1] usertools/devbind: update coding style

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon,  2 Dec 2024 15:09:34 +0000
Anatoly Burakov <anatoly.burakov@intel.com> wrote:

> +    # For kernels < 3.15 when binding devices to a generic driver (i.e. one that doesn't have a PCI
> +    # ID table) using new_id, some devices that are not bound to any other driver could be bound
> +    # even if no one has asked them to. hence, we check the list of drivers again, and see if some
> +    # of the previously-unbound devices were erroneously bound.
> +    if not devbind.use_driver_override:

Why is tool still supporting out of date and no longer supported kernel?



> +        choices=[
> +            "baseband",
> +            "compress",
> +            "crypto",
> +            "dma",
> +            "event",
> +            "mempool",
> +            "misc",
> +            "net",
> +            "regex",
> +            "ml",
> +            "all",
> +        ],

Would prefer that all the types are in table/list and the help just
references that list. The next time a type is added, only one place
needs to change.

Also, I would not trust the output format of ip route not to change.
If the utility has to parse output of ip command, use json (-j) instead.

This whole section of code is quite fragile:

>     if devices_type == network_devices:
>         # check what is the interface if any for an ssh connection if
>         # any to this host, so we can mark it later.
>         ssh_if = []
>         route = subprocess.check_output(["ip", "-o", "route"])
>         # filter out all lines for 169.254 routes
>         route = "\n".join(filter(lambda ln: not ln.startswith("169.254"),
>                                  route.decode().splitlines()))
>         rt_info = route.split()
>         for i in range(len(rt_info) - 1):
>             if rt_info[i] == "dev":
>                 ssh_if.append(rt_info[i + 1])