| 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
| ||
|
Message-ID: <20240822184824.3f0c5a28@kernel.org> Date: Thu, 22 Aug 2024 18:48:24 -0700 From: Jakub Kicinski <kuba@...nel.org> To: Paolo Abeni <pabeni@...hat.com> Cc: netdev@...r.kernel.org, Jiri Pirko <jiri@...nulli.us>, Madhu Chittim <madhu.chittim@...el.com>, Sridhar Samudrala <sridhar.samudrala@...el.com>, Simon Horman <horms@...nel.org>, John Fastabend <john.fastabend@...il.com>, Sunil Kovvuri Goutham <sgoutham@...vell.com>, Jamal Hadi Salim <jhs@...atatu.com>, Donald Hunter <donald.hunter@...il.com> Subject: Re: [PATCH v4 net-next 02/12] netlink: spec: add shaper YAML spec On Tue, 20 Aug 2024 17:12:23 +0200 Paolo Abeni wrote: > diff --git a/Documentation/netlink/specs/net_shaper.yaml b/Documentation/netlink/specs/net_shaper.yaml > new file mode 100644 > index 000000000000..a2b7900646ae > --- /dev/null > +++ b/Documentation/netlink/specs/net_shaper.yaml > @@ -0,0 +1,289 @@ > +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) > + > +name: net-shaper > + > +doc: | > + Network device HW rate limiting configuration. > + > + This API allows configuring HR shapers available on the network What's HR? > + device at different levels (queues, network device) and allows > + arbitrary manipulation of the scheduling tree of the involved > + shapers. > + > + Each @shaper is identified within the given device, by an @handle, > + comprising both a @scope and an @id, and can be created via two > + different modifiers: the @set operation, to create and update single s/different modifiers/operations/ > + shaper, and the @group operation, to create and update a scheduling > + group. > + > + Existing shapers can be deleted via the @delete operation. deleted -> deleted / reset > + The user can query the running configuration via the @get operation. The distinction between "scoped" nodes which can be "set" and "detached" "node"s which can only be created via "group" (AFAIU) needs clearer explanation. > +definitions: > + - > + type: enum > + name: scope > + doc: The different scopes where a shaper can be attached. Are they attached or are they the nodes themselves? Maybe just say that scope defines the ID interpretation and that's it. > + render-max: true > + entries: > + - name: unspec > + doc: The scope is not specified. > + - > + name: netdev > + doc: The main shaper for the given network device. > + - > + name: queue > + doc: The shaper is attached to the given device queue. > + - > + name: node > + doc: | > + The shaper allows grouping of queues or others > + node shapers, is not attached to any user-visible Saying it's not attached is confusing. Makes it sound like it exists outside of the scope of a struct net_device. > + network device component, and can be nested to > + either @netdev shapers or other @node shapers. > +attribute-sets: > + - > + name: net-shaper > + attributes: > + - > + name: handle > + type: nest > + nested-attributes: handle > + doc: Unique identifier for the given shaper inside the owning device. > + - > + name: info > + type: nest > + nested-attributes: info > + doc: Fully describes the shaper. > + - > + name: metric > + type: u32 > + enum: metric > + doc: Metric used by the given shaper for bw-min, bw-max and burst. > + - > + name: bw-min > + type: uint > + doc: Minimum guaranteed B/W for the given shaper. s/Minimum g/G/ Please spell out "bandwidth" in user-facing docs. > + - > + name: bw-max > + type: uint > + doc: Shaping B/W for the given shaper or 0 when unlimited. s/Shaping/Maximum/ > + - > + name: burst > + type: uint > + doc: Maximum burst-size for bw-min and bw-max. How about: s/bw-min and bw-max/shaping. Should not be interpreted as a quantum./ ? > + - > + name: priority > + type: u32 > + doc: Scheduling priority for the given shaper. Please clarify that that priority is only valid on children of a scheduling node, and the priority values are only compared between siblings. > + - > + name: weight > + type: u32 > + doc: | > + Weighted round robin weight for given shaper. Relative weight of the input into a round robin node. ? > + The scheduling is applied to all the sibling > + shapers with the same priority. > + - > + name: scope > + type: u32 > + enum: scope > + doc: The given shaper scope. :) > + - > + name: id > + type: u32 > + doc: | > + The given shaper id. "Numeric identifier of a shaper." Do we ever use ID and scope directly in a nest with other attrs? Or are they always wrapped in handle/parent ? If they are always wrapped they can be a standalone attr set / space. > The id semantic depends on the actual > + scope, e.g. for @queue scope it's the queue id, for > + @node scope it's the node identifier. > + - > + name: ifindex > + type: u32 > + doc: Interface index owning the specified shaper. > + - > + name: parent > + type: nest > + nested-attributes: handle > + doc: | > + Identifier for the parent of the affected shaper, > + The parent is usually implied by the shaper handle itself, > + with the only exception of the root shaper in the @group operation. Maybe just say that specifying the parent is only needed for group operations? I think that's what you mean. > + - > + name: leaves > + type: nest > + multi-attr: true > + nested-attributes: info > + doc: | > + Describes a set of leaves shapers for a @group operation. > + - > + name: root > + type: nest > + nested-attributes: root-info > + doc: | > + Describes the root shaper for a @group operation missing full stop > + Differently from @leaves and @shaper allow specifying > + the shaper parent handle, too. Maybe this attr is better called "node", after all. > + - > + name: shaper > + type: nest > + nested-attributes: info > + doc: | > + Describes a single shaper for a @set operation. Hm. How is this different than "info"? $ git grep SHAPER_A_INFO include/uapi/linux/net_shaper.h: NET_SHAPER_A_INFO, $ is "info" supposed to be used? > +operations: > + list: > + - > + name: get > + doc: | > + Get / Dump information about a/all the shaper for a given device. There's no need to "/ dump" and "/all". > + attribute-set: net-shaper > + - > + name: set > + doc: | > + Create or updates the specified shaper. create or update > + On failure the extack is set accordingly. it better be - no need to explain netlink basics > + Can't create @node scope shaper, use > + the @group operation instead. "The set operation can't be used to create a @node scope shaper..." > + attribute-set: net-shaper > + flags: [ admin-perm ] > + > + do: > + pre: net-shaper-nl-pre-doit > + post: net-shaper-nl-post-doit > + request: > + attributes: > + - ifindex > + - shaper > + > + - > + name: delete > + doc: | > + Clear (remove) the specified shaper. When deleting > + a @node shaper, relink all the node's leaves to the relink -> reattach ? > + deleted node parent. delete node's parent > + If, after the removal, the parent shaper has no more > + leaves and the parent shaper scope is @node, even > + the parent node is deleted, recursively. > + On failure the extack is set accordingly. > + attribute-set: net-shaper > + flags: [ admin-perm ] > + > + do: > + pre: net-shaper-nl-pre-doit > + post: net-shaper-nl-post-doit > + request: > + attributes: *ns-binding > + > + - > + name: group > + doc: | > + Creates or updates a scheduling group, adding the specified Please use imperative mood, like in a commit message. adding -> attach(ing) > +++ b/net/shaper/Makefile > @@ -0,0 +1,9 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# Makefile for the Generic HANDSHAKE service > +# > +# Copyright (c) 2024, Red Hat, Inc. Ironic that you added the copyright given the copy/paste fail in the contents... ;)
Powered by blists - more mailing lists