netdev - Re: [PATCH iproute2-next v2 00/11] Add a tool for configuration of DCB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0bdc221e-e9dc-347c-a2fe-40efe358da00@gmail.com>
Date:   Sat, 31 Oct 2020 09:51:08 -0600
From:   David Ahern <dsahern@...il.com>
To:     Petr Machata <me@...chata.org>, netdev@...r.kernel.org,
        stephen@...workplumber.org
Cc:     john.fastabend@...il.com, jiri@...dia.com, idosch@...dia.com,
        Jakub Kicinski <kuba@...nel.org>,
        Roman Mashak <mrv@...atatu.com>
Subject: Re: [PATCH iproute2-next v2 00/11] Add a tool for configuration of
 DCB

On 10/30/20 6:29 AM, Petr Machata wrote:
> The Linux DCB interface allows configuration of a broad range of
> hardware-specific attributes, such as TC scheduling, flow control, per-port
> buffer configuration, TC rate, etc.
> 

...

> The patchset proceeds as follows:
> 
> - Many tools in iproute2 have an option to work in batch mode, where the
>   commands to run are given in a file. The code to handle batching is
>   largely the same independent of the tool in question. In patch #1, add a
>   helper to handle the batching, and migrate individual tools to use it.
> 
> - A number of configuration options come in a form of an on-off switch.
>   This in turn can be considered a special case of parsing one of a given
>   set of strings. In patch #2, extract helpers to parse one of a number of
>   strings, on top of which build an on-off parser.
> 
>   Currently each tool open-codes the logic to parse the on-off toggle. A
>   future patch set will migrate instances of this code over to the new
>   helpers.
> 
> - The on/off toggles from previous list item sometimes need to be dumped.
>   While in the FP output, one typically wishes to maintain consistency with
>   the command line and show actual strings, "on" and "off", in JSON output
>   one would rather use booleans. This logic is somewhat annoying to have to
>   open-code time and again. Therefore in patch #3, add a helper to do just
>   that.
> 
> - The DCB tool is built on top of libmnl. Several routines will be
>   basically the same in DCB as they are currently in devlink. In patches
>   #4-#6, extract them to a new module, mnl_utils, for easy reuse.
> 
> - Much of DCB is built around arrays. A syntax similar to the iplink_vlan's
>   ingress-qos-map / egress-qos-map is very handy for describing changes
>   done to such arrays. Therefore in patch #7, extract a helper,
>   parse_mapping(), which manages parsing of key-value arrays. In patch #8,
>   fix a buglet in the helper, and in patch #9, extend it to allow setting
>   of all array elements in one go.
> 
> - In patch #10, add a skeleton of "dcb", which contains common helpers and
>   dispatches to subtools for handling of individual objects. The skeleton
>   is empty as of this patch.
> 
>   In patch #11, add "dcb_ets", a module for handling of specifically DCB
>   ETS objects.
> 
>   The intention is to gradually add handlers for at least PFC, APP, peer
>   configuration, buffers and rates.
> 
> [1] https://github.com/Mellanox/mlnx-tools/tree/master/ofed_scripts
> 

overall this looks really good to me. Thanks for taking the time to do
the refactoring.