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

[David Ahern <dsahern@gmail.com>]

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.