Re: [PATCH iproute2-next v2 00/11] Add a tool for configuration of DCB
From: David Ahern <hidden>
Date: 2020-10-31 15:51:13
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.