| Message ID | 13747e9505c47d88c22a12a372ea94755c6ba3b2.1722357745.git.pabeni@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: introduce TX H/W shaping API | expand |
Paolo Abeni <pabeni@redhat.com> writes: > diff --git a/Documentation/netlink/specs/shaper.yaml b/Documentation/netlink/specs/shaper.yaml > new file mode 100644 > index 000000000000..7327f5596fdb > --- /dev/null > +++ b/Documentation/netlink/specs/shaper.yaml It's probably more user-friendly to use the same filename as the spec name, so net-shaper.yaml > @@ -0,0 +1,262 @@ > +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) > + > +name: net-shaper > + > +doc: Network device HW Rate Limiting offload > + > +definitions: > + - > + type: enum > + name: scope > + doc: the different scopes where a shaper can be attached Nit: upper case 'The' to be consistent with rest of docs. > + render-max: true > + entries: > + - name: unspec > + doc: The scope is not specified What are the semantics of 'unspec' ? When can it be used? > + - > + name: port > + doc: The root for the whole H/W > + - > + name: netdev > + doc: The main shaper for the given network device. What are the semantic differences between netdev and port? > + - > + name: queue > + doc: The shaper is attached to the given device queue. > + - > + name: detached > + doc: | > + The shaper is not attached to any user-visible network > + device component and allows nesting and grouping of > + queues or others detached shapers. I assume that shapers are always owned by the netdev regardless of attach status? > + - > + type: enum > + name: metric > + doc: different metric each shaper can support Nit: upper case here as well. > + entries: > + - > + name: bps > + doc: Shaper operates on a bits per second basis > + - > + name: pps > + doc: Shaper operates on a packets per second basis > + > +attribute-sets: > + - > + name: net-shaper > + attributes: > + - > + name: ifindex > + type: u32 > + doc: Interface index owing the specified shaper[s] Typo: this should be 'owning' ? > + - > + name: handle > + type: nest > + nested-attributes: handle > + doc: Unique identifier for the given 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 > + - > + name: bw-max > + type: uint > + doc: Shaping B/W for the given shaper or 0 when unlimited > + - > + name: burst > + type: uint > + doc: Maximum burst-size for bw-min and bw-max > + - > + name: priority > + type: u32 > + doc: Scheduling priority for the given shaper > + - > + name: weight > + type: u32 > + doc: | > + Weighted round robin weight for given shaper. > + The scheduling is applied to all the sibling > + shapers with the same priority > + - > + name: scope > + type: u32 > + enum: scope > + doc: The given handle scope > + - > + name: id > + type: u32 > + doc: | > + The given handle id. The id semantic depends on the actual > + scope, e.g. for 'queue' scope it's the queue id, for > + 'detached' scope it's the shaper group identifier. If scope and id are only ever used as attributes of a handle then they would be better specified as a separate attribute-set, instead of mixing them in here and using a subset. You use 'quoted' references here and @refs elsewhere. It would be good to be consistent. See note below about @ in htmldocs. > + - > + name: parent > + type: nest > + nested-attributes: handle > + doc: | > + Identifier for the parent of the affected shaper, > + The parent handle value is implied by the shaper handle itself, > + except for the output shaper in the 'group' operation. Nit: quoted ref again here > + - > + name: inputs > + type: nest > + multi-attr: true > + nested-attributes: ns-info > + doc: | > + Describes a set of inputs shapers for a @group operation The @group renders exactly as-is in the generated htmldocs. There may be a more .rst friendly markup you can use that will render better. > + - > + name: output > + type: nest > + nested-attributes: ns-output-info > + doc: | > + Describes the output shaper for a @group operation > + Differently from @inputs and @shaper allow specifying > + the shaper parent handle, too. > + Nit: remove the extra blank line here > + - > + name: shaper > + type: nest > + nested-attributes: ns-info > + doc: | > + Describes a single shaper for a @set operation > + - > + name: handle > + subset-of: net-shaper > + attributes: > + - > + name: scope > + - > + name: id > + - > + name: ns-info > + subset-of: net-shaper > + attributes: > + - > + name: handle > + - > + name: metric > + - > + name: bw-min > + - > + name: bw-max > + - > + name: burst > + - > + name: priority > + - > + name: weight > + - > + name: ns-output-info > + subset-of: net-shaper > + attributes: > + - > + name: parent > + - > + name: handle > + - > + name: metric > + - > + name: bw-min > + - > + name: bw-max > + - > + name: burst > + - > + name: priority > + - > + name: weight > + > +operations: > + list: > + - > + name: get > + doc: | > + Get / Dump information about a/all the shaper for a given device > + attribute-set: net-shaper > + > + do: > + request: > + attributes: > + - ifindex > + - handle > + reply: > + attributes: &ns-attrs > + - parent > + - handle > + - metric > + - bw-min > + - bw-max > + - burst > + - priority > + - weight > + > + dump: > + request: > + attributes: > + - ifindex > + reply: > + attributes: *ns-attrs > + - > + name: set > + doc: | > + Create or configures the specified shaper. > + On failures the extack is set accordingly. > + Can't create @detached scope shaper, use > + the @group operation instead. > + attribute-set: net-shaper > + flags: [ admin-perm ] > + > + do: > + request: > + attributes: > + - ifindex > + - shaper > + > + - > + name: delete > + doc: | > + Clear (remove) the specified shaper. If after the removal > + the parent shaper has no more children and the parent > + shaper scope is @detached, even the parent is deleted, > + recursively. > + On failures the extack is set accordingly. > + attribute-set: net-shaper > + flags: [ admin-perm ] > + > + do: > + request: > + attributes: > + - ifindex > + - handle > + > + - > + name: group > + doc: | > + Group the specified input shapers under the specified > + output shaper, eventually creating the latter, if needed. > + Input shapers scope must be either @queue or @detached. It says above that you cannot create a detached shaper, so how do you create one to use as an input shaper here? Is this group op more like a multi-create op? > + Output shaper scope must be either @detached or @netdev. > + When using an output @detached scope shaper, if the > + @handle @id is not specified, a new shaper of such scope > + is created and, otherwise the specified output shaper > + must be already existing. > + The operation is atomic, on failures the extack is set > + accordingly and no change is applied to the device > + shaping configuration, otherwise the output shaper > + handle is provided as reply. > + attribute-set: net-shaper > + flags: [ admin-perm ] Does there need to be a reciprocal 'ungroup' operation? Without it, create / group / delete seems like they will have ambiguous semantics. > + do: > + request: > + attributes: > + - ifindex > + - inputs > + - output > + reply: > + attributes: > + - handle
Tue, Jul 30, 2024 at 10:39:45PM CEST, pabeni@redhat.com wrote: >Define the user-space visible interface to query, configure and delete >network shapers via yaml definition. > >Add dummy implementations for the relevant NL callbacks. > >set() and delete() operations touch a single shaper creating/updating or >deleting it. >The group() operation creates a shaper's group, nesting multiple input >shapers under the specified output shaper. > >Signed-off-by: Paolo Abeni <pabeni@redhat.com> >--- >RFC v1 -> RFC v2: > - u64 -> uint > - net_shapers -> net-shapers > - documented all the attributes > - dropped [ admin-perm ] for get() op > - group op > - set/delete touch a single shaper >--- > Documentation/netlink/specs/shaper.yaml | 262 ++++++++++++++++++++++++ > MAINTAINERS | 1 + > include/uapi/linux/net_shaper.h | 74 +++++++ > net/Kconfig | 3 + > net/Makefile | 1 + > net/shaper/Makefile | 9 + > net/shaper/shaper.c | 34 +++ > net/shaper/shaper_nl_gen.c | 117 +++++++++++ > net/shaper/shaper_nl_gen.h | 27 +++ > 9 files changed, 528 insertions(+) > create mode 100644 Documentation/netlink/specs/shaper.yaml > create mode 100644 include/uapi/linux/net_shaper.h > create mode 100644 net/shaper/Makefile > create mode 100644 net/shaper/shaper.c > create mode 100644 net/shaper/shaper_nl_gen.c > create mode 100644 net/shaper/shaper_nl_gen.h > >diff --git a/Documentation/netlink/specs/shaper.yaml b/Documentation/netlink/specs/shaper.yaml >new file mode 100644 >index 000000000000..7327f5596fdb >--- /dev/null >+++ b/Documentation/netlink/specs/shaper.yaml >@@ -0,0 +1,262 @@ >+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) >+ >+name: net-shaper >+ >+doc: Network device HW Rate Limiting offload >+ >+definitions: >+ - >+ type: enum >+ name: scope >+ doc: the different scopes where a shaper can be attached >+ render-max: true >+ entries: >+ - name: unspec >+ doc: The scope is not specified >+ - >+ name: port >+ doc: The root for the whole H/W What is this "port"? >+ - >+ name: netdev >+ doc: The main shaper for the given network device. >+ - >+ name: queue >+ doc: The shaper is attached to the given device queue. >+ - >+ name: detached >+ doc: | >+ The shaper is not attached to any user-visible network >+ device component and allows nesting and grouping of >+ queues or others detached shapers. What is the purpose of the "detached" thing? >+ - >+ type: enum >+ name: metric >+ doc: different metric each shaper can support >+ entries: >+ - >+ name: bps >+ doc: Shaper operates on a bits per second basis >+ - >+ name: pps >+ doc: Shaper operates on a packets per second basis >+ [..] >+ >+operations: >+ list: >+ - >+ name: get >+ doc: | >+ Get / Dump information about a/all the shaper for a given device >+ attribute-set: net-shaper >+ >+ do: >+ request: >+ attributes: >+ - ifindex >+ - handle >+ reply: >+ attributes: &ns-attrs >+ - parent >+ - handle >+ - metric >+ - bw-min >+ - bw-max >+ - burst >+ - priority >+ - weight >+ >+ dump: >+ request: >+ attributes: >+ - ifindex >+ reply: >+ attributes: *ns-attrs >+ - >+ name: set >+ doc: | >+ Create or configures the specified shaper. >+ On failures the extack is set accordingly. >+ Can't create @detached scope shaper, use >+ the @group operation instead. >+ attribute-set: net-shaper >+ flags: [ admin-perm ] >+ >+ do: >+ request: >+ attributes: >+ - ifindex >+ - shaper >+ >+ - >+ name: delete >+ doc: | >+ Clear (remove) the specified shaper. If after the removal >+ the parent shaper has no more children and the parent >+ shaper scope is @detached, even the parent is deleted, >+ recursively. >+ On failures the extack is set accordingly. >+ attribute-set: net-shaper >+ flags: [ admin-perm ] >+ >+ do: >+ request: >+ attributes: >+ - ifindex >+ - handle >+ >+ - >+ name: group >+ doc: | >+ Group the specified input shapers under the specified >+ output shaper, eventually creating the latter, if needed. >+ Input shapers scope must be either @queue or @detached. >+ Output shaper scope must be either @detached or @netdev. >+ When using an output @detached scope shaper, if the >+ @handle @id is not specified, a new shaper of such scope >+ is created and, otherwise the specified output shaper >+ must be already existing. I'm lost. Could this designt be described in details in the doc I asked in the cover letter? :/ Please. >+ The operation is atomic, on failures the extack is set >+ accordingly and no change is applied to the device >+ shaping configuration, otherwise the output shaper >+ handle is provided as reply. >+ attribute-set: net-shaper [..]
On 7/31/24 23:13, Donald Hunter wrote: > Paolo Abeni <pabeni@redhat.com> writes: > >> diff --git a/Documentation/netlink/specs/shaper.yaml b/Documentation/netlink/specs/shaper.yaml >> new file mode 100644 >> index 000000000000..7327f5596fdb >> --- /dev/null >> +++ b/Documentation/netlink/specs/shaper.yaml > > It's probably more user-friendly to use the same filename as the spec > name, so net-shaper.yaml No big objection on my side, but if we enforce 'Name:' to be $(basename $file .yaml), the 'Name' field becomes redundant. [...] >> + render-max: true >> + entries: >> + - name: unspec >> + doc: The scope is not specified > > What are the semantics of 'unspec' ? When can it be used? I guess at this point it can be dropped. It was introduced in a previous incarnation to represent the port parent - the port does not have a parent, being the root of the hierarchy. >> + - >> + name: port >> + doc: The root for the whole H/W >> + - >> + name: netdev >> + doc: The main shaper for the given network device. > > What are the semantic differences between netdev and port? netdev == Linux network device port == wire plug >> + - >> + name: queue >> + doc: The shaper is attached to the given device queue. >> + - >> + name: detached >> + doc: | >> + The shaper is not attached to any user-visible network >> + device component and allows nesting and grouping of >> + queues or others detached shapers. > > I assume that shapers are always owned by the netdev regardless of > attach status? If you mean that it's up to the netdev clean them up on (netdev) removal, yes. >> +>> + - >> + name: inputs >> + type: nest >> + multi-attr: true >> + nested-attributes: ns-info >> + doc: | >> + Describes a set of inputs shapers for a @group operation > > The @group renders exactly as-is in the generated htmldocs. There may be > a more .rst friendly markup you can use that will render better. Uhm... AFAICS the problem is the target (e.g. 'group') is outside the htmldoc section itself, I can't find any existing markup to serve this purpose well. What about sticking to quotes '' everywhere? FTR, I used @ following the kdoc style. [...] >> + - >> + name: group >> + doc: | >> + Group the specified input shapers under the specified >> + output shaper, eventually creating the latter, if needed. >> + Input shapers scope must be either @queue or @detached. > > It says above that you cannot create a detached shaper, so how do you > create one to use as an input shaper here? Is this group op more like a > multi-create op? The group operation has the main goal of configuring a single WRR or SP scheduling group atomically. It can creates the needed shapers as needed, see below. The need for such operation sparks from some H/W constraints: https://lore.kernel.org/netdev/9dd818dc-1fef-4633-b388-6ce7272f9cb4@lunn.ch/ >> + Output shaper scope must be either @detached or @netdev. >> + When using an output @detached scope shaper, if the >> + @handle @id is not specified, a new shaper of such scope >> + is created and, otherwise the specified output shaper >> + must be already existing. >> + The operation is atomic, on failures the extack is set >> + accordingly and no change is applied to the device >> + shaping configuration, otherwise the output shaper >> + handle is provided as reply. >> + attribute-set: net-shaper >> + flags: [ admin-perm ] > > Does there need to be a reciprocal 'ungroup' operation? Without it, > create / group / delete seems like they will have ambiguous semantics. I guess we need a better description. Can you please tell where/how the current one is ambiguous? Thanks, Paolo
On Thu, 1 Aug 2024 15:10:50 +0200 Jiri Pirko wrote: > >+ - > >+ name: netdev > >+ doc: The main shaper for the given network device. > >+ - > >+ name: queue > >+ doc: The shaper is attached to the given device queue. > >+ - > >+ name: detached > >+ doc: | > >+ The shaper is not attached to any user-visible network > >+ device component and allows nesting and grouping of > >+ queues or others detached shapers. > > What is the purpose of the "detached" thing? FWIW I also find the proposed model and naming confusing, but didn't want to object too hard in case it was just me. The model conflates attachment points with shapers themselves, and (which perhaps is more subjective) shapers with mux nodes. This is already visible in the first example of the cover letter: ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/shaper.yaml \ --do set --json '{"ifindex":'$IFINDEX', "shaper": {"handle": {"scope": "queue", "id":'$QUEUEID' }, "bw-max": 2000000 }}' We are "set"ting a shaper on a queue, not "add"ing it, because all attachment points implicitly have shapers(?) In my mind attachment points is where traffic comes from or goes to, shapers must be created to exist. And _every_ shaper has an ingress and egress, not just the ones in the middle (i.e. which aren't directly next to an attachment point) :(
On 8/1/24 15:10, Jiri Pirko wrote: > Tue, Jul 30, 2024 at 10:39:45PM CEST, pabeni@redhat.com wrote: >> + type: enum >> + name: scope >> + doc: the different scopes where a shaper can be attached >> + render-max: true >> + entries: >> + - name: unspec >> + doc: The scope is not specified >> + - >> + name: port >> + doc: The root for the whole H/W > > What is this "port"? ~ a wire plug. >> + - >> + name: netdev >> + doc: The main shaper for the given network device. >> + - >> + name: queue >> + doc: The shaper is attached to the given device queue. >> + - >> + name: detached >> + doc: | >> + The shaper is not attached to any user-visible network >> + device component and allows nesting and grouping of >> + queues or others detached shapers. > > What is the purpose of the "detached" thing? I fear I can't escape reusing most of the wording above. 'detached' nodes goal is to create groups of other shapers. i.e. queue groups, allowing multiple levels nesting, i.e. to implement this kind of hierarchy: q1 ----- \ q2 - \SP / RR ------ q3 - / \ q4 - \ SP -> (netdev) q5 - / / / q6 - \ RR q7 - / where q1..q7 are queue-level shapers and all the SP/RR are 'detached' one. The conf. does not necessary make any functional sense, just to describe the things. >> + - >> + name: group >> + doc: | >> + Group the specified input shapers under the specified >> + output shaper, eventually creating the latter, if needed. >> + Input shapers scope must be either @queue or @detached. >> + Output shaper scope must be either @detached or @netdev. >> + When using an output @detached scope shaper, if the >> + @handle @id is not specified, a new shaper of such scope >> + is created and, otherwise the specified output shaper >> + must be already existing. > > I'm lost. Could this designt be described in details in the doc I asked > in the cover letter? :/ Please. I'm unsure if the context information here and in the previous replies helped somehow. The group operation creates and configure a scheduling group, i.e. this q1 ----- \ q2 - \SP / RR ------ q3 - / \ q4 - \ SP -> (netdev) q5 - / / / q6 - \ RR q7 - / can be create with: group(inputs:[q6, q7], output:[detached,parent:netdev]) group(inputs:[q4, q5], output:[detached,parent:netdev]) group(inputs:[q1], output:[detached,parent:netdev]) group(inputs:[q2,q3], output:[detached,parent:<the detached shaper create above>]) I'm unsure if this the kind of info you are looking for? Thanks, Paolo
Thu, Aug 01, 2024 at 05:12:01PM CEST, pabeni@redhat.com wrote: >On 8/1/24 15:10, Jiri Pirko wrote: >> Tue, Jul 30, 2024 at 10:39:45PM CEST, pabeni@redhat.com wrote: >> > + type: enum >> > + name: scope >> > + doc: the different scopes where a shaper can be attached >> > + render-max: true >> > + entries: >> > + - name: unspec >> > + doc: The scope is not specified >> > + - >> > + name: port >> > + doc: The root for the whole H/W >> >> What is this "port"? > >~ a wire plug. What's "wire plug"? What of existing kernel objects this relates to? Is it a devlink port? > >> > + - >> > + name: netdev >> > + doc: The main shaper for the given network device. >> > + - >> > + name: queue >> > + doc: The shaper is attached to the given device queue. >> > + - >> > + name: detached >> > + doc: | >> > + The shaper is not attached to any user-visible network >> > + device component and allows nesting and grouping of >> > + queues or others detached shapers. >> >> What is the purpose of the "detached" thing? > >I fear I can't escape reusing most of the wording above. 'detached' nodes >goal is to create groups of other shapers. i.e. queue groups, >allowing multiple levels nesting, i.e. to implement this kind of hierarchy: > >q1 ----- \ >q2 - \SP / RR ------ >q3 - / \ > q4 - \ SP -> (netdev) > q5 - / / > / > q6 - \ RR > q7 - / > >where q1..q7 are queue-level shapers and all the SP/RR are 'detached' one. >The conf. does not necessary make any functional sense, just to describe the >things. Can you "attach" the "detached" ones? They are "detached" from what? > >> > + - >> > + name: group >> > + doc: | >> > + Group the specified input shapers under the specified >> > + output shaper, eventually creating the latter, if needed. >> > + Input shapers scope must be either @queue or @detached. >> > + Output shaper scope must be either @detached or @netdev. >> > + When using an output @detached scope shaper, if the >> > + @handle @id is not specified, a new shaper of such scope >> > + is created and, otherwise the specified output shaper >> > + must be already existing. >> >> I'm lost. Could this designt be described in details in the doc I asked >> in the cover letter? :/ Please. > >I'm unsure if the context information here and in the previous replies helped >somehow. > >The group operation creates and configure a scheduling group, i.e. this > >q1 ----- \ >q2 - \SP / RR ------ >q3 - / \ > q4 - \ SP -> (netdev) > q5 - / / > / > q6 - \ RR > q7 - / > >can be create with: > >group(inputs:[q6, q7], output:[detached,parent:netdev]) >group(inputs:[q4, q5], output:[detached,parent:netdev]) >group(inputs:[q1], output:[detached,parent:netdev]) >group(inputs:[q2,q3], output:[detached,parent:<the detached shaper create >above>]) So by "inputs" and "output" you are basically building a tree. In devlink rate, we have leaf and node, which is in sync with standard tree terminology. If what you are building is tree, why don't you use the same terminology? If you are building tree, you just need to have the link to upper noded (output in your terminology). Why you have "inputs"? Isn't that redundant? If this is not tree, what's that? Can for example q6 be input of 2 different groups? How is that supposed to work? > >I'm unsure if this the kind of info you are looking for? I'm trying to understand the design. Have to say I'm just confused :/ > >Thanks, > >Paolo >
Thu, Aug 01, 2024 at 04:31:04PM CEST, pabeni@redhat.com wrote: >On 7/31/24 23:13, Donald Hunter wrote: >> Paolo Abeni <pabeni@redhat.com> writes: >> >> > diff --git a/Documentation/netlink/specs/shaper.yaml b/Documentation/netlink/specs/shaper.yaml >> > new file mode 100644 >> > index 000000000000..7327f5596fdb >> > --- /dev/null >> > +++ b/Documentation/netlink/specs/shaper.yaml >> >> It's probably more user-friendly to use the same filename as the spec >> name, so net-shaper.yaml > >No big objection on my side, but if we enforce 'Name:' to be $(basename $file >.yaml), the 'Name' field becomes redundant. I agree with Donald, better to stay consistent. > >[...] >> > + render-max: true >> > + entries: >> > + - name: unspec >> > + doc: The scope is not specified >> >> What are the semantics of 'unspec' ? When can it be used? > >I guess at this point it can be dropped. It was introduced in a previous >incarnation to represent the port parent - the port does not have a parent, >being the root of the hierarchy. > >> > + - >> > + name: port >> > + doc: The root for the whole H/W >> > + - >> > + name: netdev >> > + doc: The main shaper for the given network device. >> >> What are the semantic differences between netdev and port? I'm happy that I'm not the only one in the dark :) > >netdev == Linux network device >port == wire plug > >> > + - >> > + name: queue >> > + doc: The shaper is attached to the given device queue. >> > + - >> > + name: detached >> > + doc: | >> > + The shaper is not attached to any user-visible network >> > + device component and allows nesting and grouping of >> > + queues or others detached shapers. >> >> I assume that shapers are always owned by the netdev regardless of >> attach status? But is it a "status"? It is a scope, can't change. I see you probably got the same confusion as I got, expecting that this can be attached somehow. > >If you mean that it's up to the netdev clean them up on (netdev) removal, >yes. > [...]
Paolo Abeni <pabeni@redhat.com> writes: > On 7/31/24 23:13, Donald Hunter wrote: >> Paolo Abeni <pabeni@redhat.com> writes: >> >>> + name: inputs >>> + type: nest >>> + multi-attr: true >>> + nested-attributes: ns-info >>> + doc: | >>> + Describes a set of inputs shapers for a @group operation >> The @group renders exactly as-is in the generated htmldocs. There may be >> a more .rst friendly markup you can use that will render better. > > Uhm... AFAICS the problem is the target (e.g. 'group') is outside the htmldoc section itself, I > can't find any existing markup to serve this purpose well. What about sticking to quotes '' > everywhere? > > FTR, I used @ following the kdoc style. Yeah, I was just thinking of using .rst markup like ``code`` or `italics`, but the meaning of @ is pretty obvious when reading the spec. If you stick with @ then we could always teach ynl-to-rst to render it as ``code``. > > [...] >>> + - >>> + name: group >>> + doc: | >>> + Group the specified input shapers under the specified >>> + output shaper, eventually creating the latter, if needed. >>> + Input shapers scope must be either @queue or @detached. >> It says above that you cannot create a detached shaper, so how do you >> create one to use as an input shaper here? Is this group op more like a >> multi-create op? > > The group operation has the main goal of configuring a single WRR or SP scheduling group > atomically. It can creates the needed shapers as needed, see below. > > The need for such operation sparks from some H/W constraints: > > https://lore.kernel.org/netdev/9dd818dc-1fef-4633-b388-6ce7272f9cb4@lunn.ch/ > >>> + Output shaper scope must be either @detached or @netdev. >>> + When using an output @detached scope shaper, if the >>> + @handle @id is not specified, a new shaper of such scope >>> + is created and, otherwise the specified output shaper >>> + must be already existing. >>> + The operation is atomic, on failures the extack is set >>> + accordingly and no change is applied to the device >>> + shaping configuration, otherwise the output shaper >>> + handle is provided as reply. >>> + attribute-set: net-shaper >>> + flags: [ admin-perm ] >> Does there need to be a reciprocal 'ungroup' operation? Without it, >> create / group / delete seems like they will have ambiguous semantics. > > I guess we need a better description. Can you please tell where/how the current one is > ambiguous? My expectation for 'group' would be to group existing things, with a reciprocal 'ungroup' operation. I think you intend 'group' to both be able to group existing shapers/groups and create a group of shapers. Am I right in saying that delete lets you delete something from a group (with side-effect of deleting group if it becomes empty), or delete a whole group? It feels a lot like each of 'set', 'group' and 'delete' are doing multiple things and the interaction between them all becomes challenging to describe, or to handle all the corner cases. I think part of the problem is the mixed terminology of input, output for groups, handle, parent for shapers and using detached to differentiate from 'implicitly attached to a resource'. Perhaps the API would be better if you had: - shaper-new - shaper-delete - shaper-get/dump - shaper-set - group-new - group-delete - group-get/dump - group-set If you went with Jakub's suggestion to give every shaper n x inputs and an output, then you could recombine groups and shapers and just have 4 ops. And you could rename 'detached' to 'shaper' so that an attachment is one of port, netdev, queue or shaper.
Tue, Jul 30, 2024 at 10:39:45PM CEST, pabeni@redhat.com wrote: [...] >+ - >+ name: inputs >+ type: nest >+ multi-attr: true >+ nested-attributes: ns-info Hmm, you sometimes use this "ns-" prefix. Why? Does it mean "net-shaper"? This results to: net_shaper_ns_info_nl_policy Wouldn't it be better to name it just "info": net_shaper_info_nl_policy And for ns-output-info, just "output-info": net_shaper_output_info_nl_policy ? >+ doc: | >+ Describes a set of inputs shapers for a @group operation >+ - >+ name: output >+ type: nest >+ nested-attributes: ns-output-info >+ doc: | >+ Describes the output shaper for a @group operation >+ Differently from @inputs and @shaper allow specifying >+ the shaper parent handle, too. >+ >+ - >+ name: shaper >+ type: nest >+ nested-attributes: ns-info >+ doc: | >+ Describes a single shaper for a @set operation >+ - >+ name: handle >+ subset-of: net-shaper >+ attributes: >+ - >+ name: scope >+ - >+ name: id >+ - >+ name: ns-info >+ subset-of: net-shaper >+ attributes: >+ - >+ name: handle >+ - >+ name: metric >+ - >+ name: bw-min >+ - >+ name: bw-max >+ - >+ name: burst >+ - >+ name: priority >+ - >+ name: weight >+ - >+ name: ns-output-info >+ subset-of: net-shaper >+ attributes: >+ - >+ name: parent >+ - >+ name: handle >+ - >+ name: metric >+ - >+ name: bw-min >+ - >+ name: bw-max >+ - >+ name: burst >+ - >+ name: priority >+ - >+ name: weight >+ [...]
Tue, Jul 30, 2024 at 10:39:45PM CEST, pabeni@redhat.com wrote: [...] >+const struct nla_policy net_shaper_ns_info_nl_policy[NET_SHAPER_A_WEIGHT + 1] = { >+ [NET_SHAPER_A_HANDLE] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy), >+ [NET_SHAPER_A_METRIC] = NLA_POLICY_MAX(NLA_U32, 1), >+ [NET_SHAPER_A_BW_MIN] = { .type = NLA_UINT, }, >+ [NET_SHAPER_A_BW_MAX] = { .type = NLA_UINT, }, >+ [NET_SHAPER_A_BURST] = { .type = NLA_UINT, }, >+ [NET_SHAPER_A_PRIORITY] = { .type = NLA_U32, }, >+ [NET_SHAPER_A_WEIGHT] = { .type = NLA_U32, }, >+}; >+ >+const struct nla_policy net_shaper_ns_output_info_nl_policy[NET_SHAPER_A_PARENT + 1] = { >+ [NET_SHAPER_A_PARENT] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy), >+ [NET_SHAPER_A_HANDLE] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy), >+ [NET_SHAPER_A_METRIC] = NLA_POLICY_MAX(NLA_U32, 1), >+ [NET_SHAPER_A_BW_MIN] = { .type = NLA_UINT, }, >+ [NET_SHAPER_A_BW_MAX] = { .type = NLA_UINT, }, >+ [NET_SHAPER_A_BURST] = { .type = NLA_UINT, }, >+ [NET_SHAPER_A_PRIORITY] = { .type = NLA_U32, }, >+ [NET_SHAPER_A_WEIGHT] = { .type = NLA_U32, }, Since this is the same set as above only extended by parent, wouldn't it be better to nest it and have it only once? [...]
Tue, Jul 30, 2024 at 10:39:45PM CEST, pabeni@redhat.com wrote:
[..]
>+ name: handle
I already commented (scope,id) passing to drivers. But in the uapi, why
this has to be a single u32 as well? Why this can't be 2 attributes?
[..]
Hi all, My replies this week will be delayed, please allow for some extra latency. On 8/2/24 13:15, Donald Hunter wrote: > Paolo Abeni <pabeni@redhat.com> writes: > >> On 7/31/24 23:13, Donald Hunter wrote: >>> Paolo Abeni <pabeni@redhat.com> writes: >>> >>>> + name: inputs >>>> + type: nest >>>> + multi-attr: true >>>> + nested-attributes: ns-info >>>> + doc: | >>>> + Describes a set of inputs shapers for a @group operation >>> The @group renders exactly as-is in the generated htmldocs. There may be >>> a more .rst friendly markup you can use that will render better. >> >> Uhm... AFAICS the problem is the target (e.g. 'group') is outside the htmldoc section itself, I >> can't find any existing markup to serve this purpose well. What about sticking to quotes '' >> everywhere? >> >> FTR, I used @ following the kdoc style. > > Yeah, I was just thinking of using .rst markup like ``code`` or > `italics`, but the meaning of @ is pretty obvious when reading the spec. > If you stick with @ then we could always teach ynl-to-rst to render it > as ``code``. I'm fine with using @ everywhere. >> [...] >>>> + - >>>> + name: group >>>> + doc: | >>>> + Group the specified input shapers under the specified >>>> + output shaper, eventually creating the latter, if needed. >>>> + Input shapers scope must be either @queue or @detached. >>> It says above that you cannot create a detached shaper, so how do you >>> create one to use as an input shaper here? Is this group op more like a >>> multi-create op? >> >> The group operation has the main goal of configuring a single WRR or SP scheduling group >> atomically. It can creates the needed shapers as needed, see below. >> >> The need for such operation sparks from some H/W constraints: >> >> https://lore.kernel.org/netdev/9dd818dc-1fef-4633-b388-6ce7272f9cb4@lunn.ch/ >> >>>> + Output shaper scope must be either @detached or @netdev. >>>> + When using an output @detached scope shaper, if the >>>> + @handle @id is not specified, a new shaper of such scope >>>> + is created and, otherwise the specified output shaper >>>> + must be already existing. >>>> + The operation is atomic, on failures the extack is set >>>> + accordingly and no change is applied to the device >>>> + shaping configuration, otherwise the output shaper >>>> + handle is provided as reply. >>>> + attribute-set: net-shaper >>>> + flags: [ admin-perm ] >>> Does there need to be a reciprocal 'ungroup' operation? Without it, >>> create / group / delete seems like they will have ambiguous semantics. >> >> I guess we need a better description. Can you please tell where/how the current one is >> ambiguous? > > My expectation for 'group' would be to group existing things, with a > reciprocal 'ungroup' operation. I think you intend 'group' to both be > able to group existing shapers/groups and create a group of shapers. > > Am I right in saying that delete lets you delete something from a group > (with side-effect of deleting group if it becomes empty), or delete a > whole group? In the current incarnation, delete() on the whole group is explicitly forbidden. Jakub suggested we should allow such behavior for the delegation use-case. > It feels a lot like each of 'set', 'group' and 'delete' are doing > multiple things and the interaction between them all becomes challenging > to describe, or to handle all the corner case > I think part of the > problem is the mixed terminology of input, output for groups, handle, > parent for shapers and using detached to differentiate from 'implicitly > attached to a resource'. > > Perhaps the API would be better if you had: > > - shaper-new > - shaper-delete > - shaper-get/dump > - shaper-set > - group-new > - group-delete > - group-get/dump > - group-set > > If you went with Jakub's suggestion to give every shaper n x inputs and > an output, then you could recombine groups and shapers and just have 4 > ops. And you could rename 'detached' to 'shaper' so that an attachment > is one of port, netdev, queue or shaper. I'm unsure I read the above correctly, and I'm unsure it's in the same direction of Jakub's suggestion. AFACS the above is basically the same interface we proposed in the past iteration and was explicitly nacked from Jakub, Additionally, one of the constraint to be addressed here is allowing to setup/configures all the nodes in a 'group' with a single operation, to deal with H/W limitations. How would the above address such constraint? Thanks, Paolo
Hi all, (same remark of my previous email). My replies this week will be delayed, please allow for some extra latency. On 8/2/24 12:49, Jiri Pirko wrote: > Thu, Aug 01, 2024 at 05:12:01PM CEST, pabeni@redhat.com wrote: >> On 8/1/24 15:10, Jiri Pirko wrote: >>> Tue, Jul 30, 2024 at 10:39:45PM CEST, pabeni@redhat.com wrote: >>>> + type: enum >>>> + name: scope >>>> + doc: the different scopes where a shaper can be attached >>>> + render-max: true >>>> + entries: >>>> + - name: unspec >>>> + doc: The scope is not specified >>>> + - >>>> + name: port >>>> + doc: The root for the whole H/W >>> >>> What is this "port"? >> >> ~ a wire plug. > > What's "wire plug"? What of existing kernel objects this relates to? Is > it a devlink port? I'm sorry, my hasty translation of my native language was really inaccurate. Let me re-phrase from scratch: that is actually the root of the whole scheduling tree (yes, it's a tree) for a given network device. One source of confusion is that in a previous iteration we intended to allow configuring even objects 'above' the network device level, but such feature has been dropped. We could probably drop this scope entirely. >>>> + - >>>> + name: netdev >>>> + doc: The main shaper for the given network device. >>>> + - >>>> + name: queue >>>> + doc: The shaper is attached to the given device queue. >>>> + - >>>> + name: detached >>>> + doc: | >>>> + The shaper is not attached to any user-visible network >>>> + device component and allows nesting and grouping of >>>> + queues or others detached shapers. >>> >>> What is the purpose of the "detached" thing? >> >> I fear I can't escape reusing most of the wording above. 'detached' nodes >> goal is to create groups of other shapers. i.e. queue groups, >> allowing multiple levels nesting, i.e. to implement this kind of hierarchy: >> >> q1 ----- \ >> q2 - \SP / RR ------ >> q3 - / \ >> q4 - \ SP -> (netdev) >> q5 - / / >> / >> q6 - \ RR >> q7 - / >> >> where q1..q7 are queue-level shapers and all the SP/RR are 'detached' one. >> The conf. does not necessary make any functional sense, just to describe the >> things. > > Can you "attach" the "detached" ones? They are "detached" from what? I see such name is very confusing. An alternative one could be 'group', but IIRC it was explicitly discarded while discussing a previous iteration. The 'detached' name comes from the fact the such shapers are not a direct representation of some well-known kernel object (queues, devices), >>>> + - >>>> + name: group >>>> + doc: | >>>> + Group the specified input shapers under the specified >>>> + output shaper, eventually creating the latter, if needed. >>>> + Input shapers scope must be either @queue or @detached. >>>> + Output shaper scope must be either @detached or @netdev. >>>> + When using an output @detached scope shaper, if the >>>> + @handle @id is not specified, a new shaper of such scope >>>> + is created and, otherwise the specified output shaper >>>> + must be already existing. >>> >>> I'm lost. Could this designt be described in details in the doc I asked >>> in the cover letter? :/ Please. >> >> I'm unsure if the context information here and in the previous replies helped >> somehow. >> >> The group operation creates and configure a scheduling group, i.e. this >> >> q1 ----- \ >> q2 - \SP / RR ------ >> q3 - / \ >> q4 - \ SP -> (netdev) >> q5 - / / >> / >> q6 - \ RR >> q7 - / >> >> can be create with: >> >> group(inputs:[q6, q7], output:[detached,parent:netdev]) >> group(inputs:[q4, q5], output:[detached,parent:netdev]) >> group(inputs:[q1], output:[detached,parent:netdev]) >> group(inputs:[q2,q3], output:[detached,parent:<the detached shaper create >> above>]) > > So by "inputs" and "output" you are basically building a tree. In > devlink rate, we have leaf and node, which is in sync with standard tree > terminology. > > If what you are building is tree, why don't you use the same > terminology? If you are building tree, you just need to have the link to > upper noded (output in your terminology). Why you have "inputs"? Isn't > that redundant? The idea behind the inputs/outputs naming is to represent the data flow towards the wire. I'm fine with the parent/children naming, but IIRC Jakub was not happy with it. Is there any intermediate ground that could satisfy both of you? Thanks, Paolo
On Mon, 5 Aug 2024 16:35:29 +0200 Paolo Abeni wrote: > > Perhaps the API would be better if you had: > > > > - shaper-new > > - shaper-delete > > - shaper-get/dump > > - shaper-set > > - group-new > > - group-delete > > - group-get/dump > > - group-set > > > > If you went with Jakub's suggestion to give every shaper n x inputs and > > an output, then you could recombine groups and shapers and just have 4 > > ops. And you could rename 'detached' to 'shaper' so that an attachment > > is one of port, netdev, queue or shaper. > > I'm unsure I read the above correctly, and I'm unsure it's in the same > direction of Jakub's suggestion. AFACS the above is basically the same > interface we proposed in the past iteration and was explicitly nacked > from Jakub, To be clear I was against the low level twiddling APIs, where one has to separately create a mux/group/scheduler and "move" all its children under it one by one. (due to the problems it creates with atomic transitions between configurations). Having shapers separate from the scheduling hierarchy doesn't seem bad, tho I haven't gone thru all the considerations in my head. > Additionally, one of the constraint to be addressed here is allowing to > setup/configures all the nodes in a 'group' with a single operation, to > deal with H/W limitations. How would the above address such constraint? FWIW I think the naming is the major source of confusion :(
Mon, Aug 05, 2024 at 05:11:09PM CEST, pabeni@redhat.com wrote: >Hi all, > >(same remark of my previous email). My replies this week will be delayed, >please allow for some extra latency. > >On 8/2/24 12:49, Jiri Pirko wrote: >> Thu, Aug 01, 2024 at 05:12:01PM CEST, pabeni@redhat.com wrote: >> > On 8/1/24 15:10, Jiri Pirko wrote: >> > > Tue, Jul 30, 2024 at 10:39:45PM CEST, pabeni@redhat.com wrote: >> > > > + type: enum >> > > > + name: scope >> > > > + doc: the different scopes where a shaper can be attached >> > > > + render-max: true >> > > > + entries: >> > > > + - name: unspec >> > > > + doc: The scope is not specified >> > > > + - >> > > > + name: port >> > > > + doc: The root for the whole H/W >> > > >> > > What is this "port"? >> > >> > ~ a wire plug. >> >> What's "wire plug"? What of existing kernel objects this relates to? Is >> it a devlink port? > > >I'm sorry, my hasty translation of my native language was really inaccurate. >Let me re-phrase from scratch: that is actually the root of the whole >scheduling tree (yes, it's a tree) for a given network device. > >One source of confusion is that in a previous iteration we intended to allow >configuring even objects 'above' the network device level, but such feature >has been dropped. > >We could probably drop this scope entirely. Drop for now, correct? I agree that your patchset now only works on top of netdev. But all infra should be ready to work on top of something else, devlink seems like good candidate. I mean, for devlink port function rate, we will definitelly need something like that. > >> > > > + - >> > > > + name: netdev >> > > > + doc: The main shaper for the given network device. >> > > > + - >> > > > + name: queue >> > > > + doc: The shaper is attached to the given device queue. >> > > > + - >> > > > + name: detached >> > > > + doc: | >> > > > + The shaper is not attached to any user-visible network >> > > > + device component and allows nesting and grouping of >> > > > + queues or others detached shapers. >> > > >> > > What is the purpose of the "detached" thing? >> > >> > I fear I can't escape reusing most of the wording above. 'detached' nodes >> > goal is to create groups of other shapers. i.e. queue groups, >> > allowing multiple levels nesting, i.e. to implement this kind of hierarchy: >> > >> > q1 ----- \ >> > q2 - \SP / RR ------ >> > q3 - / \ >> > q4 - \ SP -> (netdev) >> > q5 - / / >> > / >> > q6 - \ RR >> > q7 - / >> > >> > where q1..q7 are queue-level shapers and all the SP/RR are 'detached' one. >> > The conf. does not necessary make any functional sense, just to describe the >> > things. >> >> Can you "attach" the "detached" ones? They are "detached" from what? > >I see such name is very confusing. An alternative one could be 'group', but >IIRC it was explicitly discarded while discussing a previous iteration. > >The 'detached' name comes from the fact the such shapers are not a direct >representation of some well-known kernel object (queues, devices), Understand now. Maybe "node" would make more sense? Leaves are queues and root is the device? Aligns with the tree terminology... > >> > > > + - >> > > > + name: group >> > > > + doc: | >> > > > + Group the specified input shapers under the specified >> > > > + output shaper, eventually creating the latter, if needed. >> > > > + Input shapers scope must be either @queue or @detached. >> > > > + Output shaper scope must be either @detached or @netdev. >> > > > + When using an output @detached scope shaper, if the >> > > > + @handle @id is not specified, a new shaper of such scope >> > > > + is created and, otherwise the specified output shaper >> > > > + must be already existing. >> > > >> > > I'm lost. Could this designt be described in details in the doc I asked >> > > in the cover letter? :/ Please. >> > >> > I'm unsure if the context information here and in the previous replies helped >> > somehow. >> > >> > The group operation creates and configure a scheduling group, i.e. this >> > >> > q1 ----- \ >> > q2 - \SP / RR ------ >> > q3 - / \ >> > q4 - \ SP -> (netdev) >> > q5 - / / >> > / >> > q6 - \ RR >> > q7 - / >> > >> > can be create with: >> > >> > group(inputs:[q6, q7], output:[detached,parent:netdev]) >> > group(inputs:[q4, q5], output:[detached,parent:netdev]) >> > group(inputs:[q1], output:[detached,parent:netdev]) >> > group(inputs:[q2,q3], output:[detached,parent:<the detached shaper create >> > above>]) >> >> So by "inputs" and "output" you are basically building a tree. In >> devlink rate, we have leaf and node, which is in sync with standard tree >> terminology. >> >> If what you are building is tree, why don't you use the same >> terminology? If you are building tree, you just need to have the link to >> upper noded (output in your terminology). Why you have "inputs"? Isn't >> that redundant? > >The idea behind the inputs/outputs naming is to represent the data flow >towards the wire. >I'm fine with the parent/children naming, but IIRC Jakub was not happy with >it. Is there any intermediate ground that could satisfy both of you? It's a tree, so perhaps just stick with tree terminology, everyone is used to that. Makes sense? One way or another, this needs to be properly described in docs, all terminology. That would make things more clear, I believe. > >Thanks, > >Paolo >
On 8/6/24 09:06, Jiri Pirko wrote: > Mon, Aug 05, 2024 at 05:11:09PM CEST, pabeni@redhat.com wrote: >> Hi all, >> >> (same remark of my previous email). My replies this week will be delayed, >> please allow for some extra latency. >> >> On 8/2/24 12:49, Jiri Pirko wrote: >>> Thu, Aug 01, 2024 at 05:12:01PM CEST, pabeni@redhat.com wrote: >>>> On 8/1/24 15:10, Jiri Pirko wrote: >>>>> Tue, Jul 30, 2024 at 10:39:45PM CEST, pabeni@redhat.com wrote: >>>>>> + type: enum >>>>>> + name: scope >>>>>> + doc: the different scopes where a shaper can be attached >>>>>> + render-max: true >>>>>> + entries: >>>>>> + - name: unspec >>>>>> + doc: The scope is not specified >>>>>> + - >>>>>> + name: port >>>>>> + doc: The root for the whole H/W >>>>> >>>>> What is this "port"? >>>> >>>> ~ a wire plug. >>> >>> What's "wire plug"? What of existing kernel objects this relates to? Is >>> it a devlink port? >> >> >> I'm sorry, my hasty translation of my native language was really inaccurate. >> Let me re-phrase from scratch: that is actually the root of the whole >> scheduling tree (yes, it's a tree) for a given network device. >> >> One source of confusion is that in a previous iteration we intended to allow >> configuring even objects 'above' the network device level, but such feature >> has been dropped. >> >> We could probably drop this scope entirely. > > Drop for now, correct? I agree that your patchset now only works on top > of netdev. But all infra should be ready to work on top of something > else, devlink seems like good candidate. I mean, for devlink port > function rate, we will definitelly need something like that. > > >> >>>>>> + - >>>>>> + name: netdev >>>>>> + doc: The main shaper for the given network device. >>>>>> + - >>>>>> + name: queue >>>>>> + doc: The shaper is attached to the given device queue. >>>>>> + - >>>>>> + name: detached >>>>>> + doc: | >>>>>> + The shaper is not attached to any user-visible network >>>>>> + device component and allows nesting and grouping of >>>>>> + queues or others detached shapers. >>>>> >>>>> What is the purpose of the "detached" thing? >>>> >>>> I fear I can't escape reusing most of the wording above. 'detached' nodes >>>> goal is to create groups of other shapers. i.e. queue groups, >>>> allowing multiple levels nesting, i.e. to implement this kind of hierarchy: >>>> >>>> q1 ----- \ >>>> q2 - \SP / RR ------ >>>> q3 - / \ >>>> q4 - \ SP -> (netdev) >>>> q5 - / / >>>> / >>>> q6 - \ RR >>>> q7 - / >>>> >>>> where q1..q7 are queue-level shapers and all the SP/RR are 'detached' one. >>>> The conf. does not necessary make any functional sense, just to describe the >>>> things. >>> >>> Can you "attach" the "detached" ones? They are "detached" from what? >> >> I see such name is very confusing. An alternative one could be 'group', but >> IIRC it was explicitly discarded while discussing a previous iteration. >> >> The 'detached' name comes from the fact the such shapers are not a direct >> representation of some well-known kernel object (queues, devices), > > Understand now. Maybe "node" would make more sense? Leaves are queues > and root is the device? Aligns with the tree terminology... > >> >>>>>> + - >>>>>> + name: group >>>>>> + doc: | >>>>>> + Group the specified input shapers under the specified >>>>>> + output shaper, eventually creating the latter, if needed. >>>>>> + Input shapers scope must be either @queue or @detached. >>>>>> + Output shaper scope must be either @detached or @netdev. >>>>>> + When using an output @detached scope shaper, if the >>>>>> + @handle @id is not specified, a new shaper of such scope >>>>>> + is created and, otherwise the specified output shaper >>>>>> + must be already existing. >>>>> >>>>> I'm lost. Could this designt be described in details in the doc I asked >>>>> in the cover letter? :/ Please. >>>> >>>> I'm unsure if the context information here and in the previous replies helped >>>> somehow. >>>> >>>> The group operation creates and configure a scheduling group, i.e. this >>>> >>>> q1 ----- \ >>>> q2 - \SP / RR ------ >>>> q3 - / \ >>>> q4 - \ SP -> (netdev) >>>> q5 - / / >>>> / >>>> q6 - \ RR >>>> q7 - / >>>> >>>> can be create with: >>>> >>>> group(inputs:[q6, q7], output:[detached,parent:netdev]) >>>> group(inputs:[q4, q5], output:[detached,parent:netdev]) >>>> group(inputs:[q1], output:[detached,parent:netdev]) >>>> group(inputs:[q2,q3], output:[detached,parent:<the detached shaper create >>>> above>]) >>> >>> So by "inputs" and "output" you are basically building a tree. In >>> devlink rate, we have leaf and node, which is in sync with standard tree >>> terminology. >>> >>> If what you are building is tree, why don't you use the same >>> terminology? If you are building tree, you just need to have the link to >>> upper noded (output in your terminology). Why you have "inputs"? Isn't >>> that redundant? >> >> The idea behind the inputs/outputs naming is to represent the data flow >> towards the wire. >> I'm fine with the parent/children naming, but IIRC Jakub was not happy with >> it. Is there any intermediate ground that could satisfy both of you? > > It's a tree, so perhaps just stick with tree terminology, everyone is > used to that. Makes sense? One way or another, this needs to be > properly described in docs, all terminology. That would make things more > clear, I believe. @Jakub, would you be ok with: 'inputs' -> 'leaves' 'output' -> 'node' ? Also while at it, I think renaming the 'group()' operation as 'node_set()' could be clearer (or at least less unclear), WDYT? Note: I think it's would be more user-friendly to keep a single delete/get/dump operation for 'nodes' and leaves. Thanks, Paolo
On Mon, 12 Aug 2024 16:58:33 +0200 Paolo Abeni wrote: > > It's a tree, so perhaps just stick with tree terminology, everyone is > > used to that. Makes sense? One way or another, this needs to be > > properly described in docs, all terminology. That would make things more > > clear, I believe. > > @Jakub, would you be ok with: > > 'inputs' -> 'leaves' > 'output' -> 'node' > ? I think the confusion is primarily about the parent / child. input and output should be very clear, IMO. > Also while at it, I think renaming the 'group()' operation as > 'node_set()' could be clearer (or at least less unclear), WDYT? No idea how we arrived at node_set(), and how it can possibly represent a grouping operation. The operations is grouping inputs and creating a scheduler node. > Note: I think it's would be more user-friendly to keep a single > delete/get/dump operation for 'nodes' and leaves. Are you implying that nodes and leaves are different types of objects? Aren't leaves nodes without any inputs?
Mon, Aug 12, 2024 at 05:25:44PM CEST, kuba@kernel.org wrote: >On Mon, 12 Aug 2024 16:58:33 +0200 Paolo Abeni wrote: >> > It's a tree, so perhaps just stick with tree terminology, everyone is >> > used to that. Makes sense? One way or another, this needs to be >> > properly described in docs, all terminology. That would make things more >> > clear, I believe. >> >> @Jakub, would you be ok with: >> >> 'inputs' -> 'leaves' >> 'output' -> 'node' >> ? > >I think the confusion is primarily about the parent / child. >input and output should be very clear, IMO. For me, "inputs" and "output" in this context sounds very odd. It should be children and parent, isn't it. Confused... > >> Also while at it, I think renaming the 'group()' operation as >> 'node_set()' could be clearer (or at least less unclear), WDYT? > >No idea how we arrived at node_set(), and how it can possibly subtree_set() ? >represent a grouping operation. >The operations is grouping inputs and creating a scheduler node. > >> Note: I think it's would be more user-friendly to keep a single >> delete/get/dump operation for 'nodes' and leaves. > >Are you implying that nodes and leaves are different types of objects? >Aren't leaves nodes without any inputs? Agree. Same op would be nice for both.
On Mon, 12 Aug 2024 18:50:06 +0200 Jiri Pirko wrote: > Mon, Aug 12, 2024 at 05:25:44PM CEST, kuba@kernel.org wrote: > >I think the confusion is primarily about the parent / child. > >input and output should be very clear, IMO. > > For me, "inputs" and "output" in this context sounds very odd. It should > be children and parent, isn't it. Confused... Parent / child is completely confusing. Let's not. User will classify traffic based on 'leaf' attributes. Therefore in my mind traffic enters the tree at the "leaves", and travels towards the root (whether or not that's how HW evaluates the hierarchy). This is opposite to how trees as an data structure are normally traversed. Hence I find the tree analogy to be imperfect. But yes, root and leaf are definitely better than parent / child. > >> Also while at it, I think renaming the 'group()' operation as > >> 'node_set()' could be clearer (or at least less unclear), WDYT? > > > >No idea how we arrived at node_set(), and how it can possibly > > subtree_set() ? The operation is grouping inputs and creating a scheduler node. > >represent a grouping operation. > >The operations is grouping inputs and creating a scheduler node. > > > >> Note: I think it's would be more user-friendly to keep a single > >> delete/get/dump operation for 'nodes' and leaves. > > > >Are you implying that nodes and leaves are different types of objects? > >Aren't leaves nodes without any inputs? > > Agree. Same op would be nice for both.
Mon, Aug 12, 2024 at 07:42:21PM CEST, kuba@kernel.org wrote: >On Mon, 12 Aug 2024 18:50:06 +0200 Jiri Pirko wrote: >> Mon, Aug 12, 2024 at 05:25:44PM CEST, kuba@kernel.org wrote: >> >I think the confusion is primarily about the parent / child. >> >input and output should be very clear, IMO. >> >> For me, "inputs" and "output" in this context sounds very odd. It should >> be children and parent, isn't it. Confused... > >Parent / child is completely confusing. Let's not. > >User will classify traffic based on 'leaf' attributes. >Therefore in my mind traffic enters the tree at the "leaves", >and travels towards the root (whether or not that's how HW >evaluates the hierarchy). > >This is opposite to how trees as an data structure are normally >traversed. Hence I find the tree analogy to be imperfect. Normally? Tree as a datastructure could be traversed freely, why it can't? In this case, it is traversed from leaf to root. It's still a tree. Why the tree analogy is imperfect. From what I see, it fits 100%. >But yes, root and leaf are definitely better than parent / child. Node has 0-n children and 0-1 parents. In case it has 0 children, it's a leaf, in case it has 0 parents, it's a root. This is the common tree terminology, isn't it? > >> >> Also while at it, I think renaming the 'group()' operation as >> >> 'node_set()' could be clearer (or at least less unclear), WDYT? >> > >> >No idea how we arrived at node_set(), and how it can possibly >> >> subtree_set() ? > >The operation is grouping inputs and creating a scheduler node. Creating a node inside a tree, isn't it? Therefore subtree. But it could be unified to node_set() as Paolo suggested. That would work for any node, including leaf, tree, non-existent internal node. > >> >represent a grouping operation. >> >The operations is grouping inputs and creating a scheduler node. >> > >> >> Note: I think it's would be more user-friendly to keep a single >> >> delete/get/dump operation for 'nodes' and leaves. >> > >> >Are you implying that nodes and leaves are different types of objects? >> >Aren't leaves nodes without any inputs? >> >> Agree. Same op would be nice for both.
On Tue, 13 Aug 2024 07:38:46 +0200 Jiri Pirko wrote: > >Parent / child is completely confusing. Let's not. > > > >User will classify traffic based on 'leaf' attributes. > >Therefore in my mind traffic enters the tree at the "leaves", > >and travels towards the root (whether or not that's how HW > >evaluates the hierarchy). > > > >This is opposite to how trees as an data structure are normally > >traversed. Hence I find the tree analogy to be imperfect. > > Normally? Yes, normally, in sort and/or lookup algorithms the owner of the tree has a pointer to root, and walks from root. > Tree as a datastructure could be traversed freely, why it > can't? I didn't say it can't. > In this case, it is traversed from leaf to root. It's still a > tree. Why the tree analogy is imperfect. From what I see, it fits 100%. > > >But yes, root and leaf are definitely better than parent / child. > > Node has 0-n children and 0-1 parents. In case it has 0 children, it's a > leaf, in case it has 0 parents, it's a root. > This is the common tree terminology, isn't it? You're using tree terminology and then you're asking if it's the tree terminology. What are you trying to prove? To me using input / output is more intuitive, as it matches direction of traffic flow. I'm fine with root / leaf tho, as I said. > >> subtree_set() ? > > > >The operation is grouping inputs and creating a scheduler node. > > Creating a node inside a tree, isn't it? Therefore subtree. All nodes are inside the tree. > But it could be unified to node_set() as Paolo suggested. That would > work for any node, including leaf, tree, non-existent internal node. A "set" operation which creates a node.
On 8/13/24 16:12, Jakub Kicinski wrote: > To me using input / output is more intuitive, as it matches direction > of traffic flow. I'm fine with root / leaf tho, as I said. Can we converge on root / leaf ? >>>> subtree_set() ? >>> >>> The operation is grouping inputs and creating a scheduler node. >> >> Creating a node inside a tree, isn't it? Therefore subtree. > > All nodes are inside the tree. > >> But it could be unified to node_set() as Paolo suggested. That would >> work for any node, including leaf, tree, non-existent internal node. > > A "set" operation which creates a node. Here the outcome is unclear to me. My understanding is that group() does not fit Jiri nor Donald and and node_set() or subtree_set() do not fit Jakub. Did I misread something? As a trade-off, what about, group_set()? Thanks, Paolo
On Tue, 13 Aug 2024 16:47:34 +0200 Paolo Abeni wrote: > >> Creating a node inside a tree, isn't it? Therefore subtree. > > > > All nodes are inside the tree. > > > >> But it could be unified to node_set() as Paolo suggested. That would > >> work for any node, including leaf, tree, non-existent internal node. > > > > A "set" operation which creates a node. > > Here the outcome is unclear to me. My understanding is that group() does > not fit Jiri nor Donald and and node_set() or subtree_set() do not fit > Jakub. > > Did I misread something? As a trade-off, what about, group_set()? "set" is not a sensible verb for creating something. "group" in the original was the verb. Why are both saying "set" and not "create"? What am I missing?
On 8/13/24 16:58, Jakub Kicinski wrote: > On Tue, 13 Aug 2024 16:47:34 +0200 Paolo Abeni wrote: >>>> Creating a node inside a tree, isn't it? Therefore subtree. >>> >>> All nodes are inside the tree. >>> >>>> But it could be unified to node_set() as Paolo suggested. That would >>>> work for any node, including leaf, tree, non-existent internal node. >>> >>> A "set" operation which creates a node. >> >> Here the outcome is unclear to me. My understanding is that group() does >> not fit Jiri nor Donald and and node_set() or subtree_set() do not fit >> Jakub. >> >> Did I misread something? As a trade-off, what about, group_set()? > > "set" is not a sensible verb for creating something. "group" in > the original was the verb. > Why are both saying "set" and not "create"? What am I missing? Please, don't read too much in my limited English skills! I'm fine with group_create() - or create_group() Still WRT naming, I almost forgot about the much blamed 'detached' scope. Would 'node' or 'group' be a better name? (the latter only if we rename the homonymous operation) Thanks, Paolo
On Tue, 13 Aug 2024 17:31:17 +0200 Paolo Abeni wrote: > > "set" is not a sensible verb for creating something. "group" in > > the original was the verb. > > Why are both saying "set" and not "create"? What am I missing? > > Please, don't read too much in my limited English skills! > I'm fine with group_create() - or create_group() Again, group was a verb :) I don't think anyone suggested group as a noun / object. > Still WRT naming, I almost forgot about the much blamed 'detached' > scope. Would 'node' or 'group' be a better name? (the latter only if we > rename the homonymous operation) I vote 'node', given the above.
Jakub Kicinski <kuba@kernel.org> writes: > On Mon, 12 Aug 2024 16:58:33 +0200 Paolo Abeni wrote: >> > It's a tree, so perhaps just stick with tree terminology, everyone is >> > used to that. Makes sense? One way or another, this needs to be >> > properly described in docs, all terminology. That would make things more >> > clear, I believe. >> >> @Jakub, would you be ok with: >> >> 'inputs' -> 'leaves' >> 'output' -> 'node' >> ? > > I think the confusion is primarily about the parent / child. > input and output should be very clear, IMO. input / output seems the most intuitive of the different terms that have been suggested.
Paolo Abeni <pabeni@redhat.com> writes: > On 8/13/24 16:12, Jakub Kicinski wrote: >> To me using input / output is more intuitive, as it matches direction >> of traffic flow. I'm fine with root / leaf tho, as I said. > > Can we converge on root / leaf ? > >>>>> subtree_set() ? >>>> >>>> The operation is grouping inputs and creating a scheduler node. >>> >>> Creating a node inside a tree, isn't it? Therefore subtree. >> All nodes are inside the tree. >> >>> But it could be unified to node_set() as Paolo suggested. That would >>> work for any node, including leaf, tree, non-existent internal node. >> A "set" operation which creates a node. > > Here the outcome is unclear to me. My understanding is that group() does not fit Jiri nor Donald > and and node_set() or subtree_set() do not fit Jakub. I found group() confusing because it does not imply creation. So create-group() would be fine. But it seems like creating a group is a step towards creating a scheduler node?
On 8/13/24 19:12, Donald Hunter wrote: > Jakub Kicinski <kuba@kernel.org> writes: > >> On Mon, 12 Aug 2024 16:58:33 +0200 Paolo Abeni wrote: >>>> It's a tree, so perhaps just stick with tree terminology, everyone is >>>> used to that. Makes sense? One way or another, this needs to be >>>> properly described in docs, all terminology. That would make things more >>>> clear, I believe. >>> >>> @Jakub, would you be ok with: >>> >>> 'inputs' -> 'leaves' >>> 'output' -> 'node' >>> ? >> >> I think the confusion is primarily about th parent / child. >> input and output should be very clear, IMO. > > input / output seems the most intuitive of the different terms that have > been suggested. Since a sort of agreement was reached about using root / leaf instead, and (quoting someone smarter then me) a good compromise makes every party equally unhappy, would you consider such other option? Thanks, Paolo
Paolo Abeni <pabeni@redhat.com> writes: > On 8/13/24 19:12, Donald Hunter wrote: >> Jakub Kicinski <kuba@kernel.org> writes: >> >>> On Mon, 12 Aug 2024 16:58:33 +0200 Paolo Abeni wrote: >>>>> It's a tree, so perhaps just stick with tree terminology, everyone is >>>>> used to that. Makes sense? One way or another, this needs to be >>>>> properly described in docs, all terminology. That would make things more >>>>> clear, I believe. >>>> >>>> @Jakub, would you be ok with: >>>> >>>> 'inputs' -> 'leaves' >>>> 'output' -> 'node' >>>> ? >>> >>> I think the confusion is primarily about th parent / child. >>> input and output should be very clear, IMO. >> input / output seems the most intuitive of the different terms that have >> been suggested. > > Since a sort of agreement was reached about using root / leaf instead, and (quoting someone > smarter then me) a good compromise makes every party equally unhappy, would you consider such > other option? Sure, if that is the general consensus. Though I do find it confusing because there are not any tree-like ops. Does that mean you are shifting away from using 'inputs' and 'output' for the group op?
diff --git a/Documentation/netlink/specs/shaper.yaml b/Documentation/netlink/specs/shaper.yaml new file mode 100644 index 000000000000..7327f5596fdb --- /dev/null +++ b/Documentation/netlink/specs/shaper.yaml @@ -0,0 +1,262 @@ +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) + +name: net-shaper + +doc: Network device HW Rate Limiting offload + +definitions: + - + type: enum + name: scope + doc: the different scopes where a shaper can be attached + render-max: true + entries: + - name: unspec + doc: The scope is not specified + - + name: port + doc: The root for the whole H/W + - + name: netdev + doc: The main shaper for the given network device. + - + name: queue + doc: The shaper is attached to the given device queue. + - + name: detached + doc: | + The shaper is not attached to any user-visible network + device component and allows nesting and grouping of + queues or others detached shapers. + - + type: enum + name: metric + doc: different metric each shaper can support + entries: + - + name: bps + doc: Shaper operates on a bits per second basis + - + name: pps + doc: Shaper operates on a packets per second basis + +attribute-sets: + - + name: net-shaper + attributes: + - + name: ifindex + type: u32 + doc: Interface index owing the specified shaper[s] + - + name: handle + type: nest + nested-attributes: handle + doc: Unique identifier for the given 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 + - + name: bw-max + type: uint + doc: Shaping B/W for the given shaper or 0 when unlimited + - + name: burst + type: uint + doc: Maximum burst-size for bw-min and bw-max + - + name: priority + type: u32 + doc: Scheduling priority for the given shaper + - + name: weight + type: u32 + doc: | + Weighted round robin weight for given shaper. + The scheduling is applied to all the sibling + shapers with the same priority + - + name: scope + type: u32 + enum: scope + doc: The given handle scope + - + name: id + type: u32 + doc: | + The given handle id. The id semantic depends on the actual + scope, e.g. for 'queue' scope it's the queue id, for + 'detached' scope it's the shaper group identifier. + - + name: parent + type: nest + nested-attributes: handle + doc: | + Identifier for the parent of the affected shaper, + The parent handle value is implied by the shaper handle itself, + except for the output shaper in the 'group' operation. + - + name: inputs + type: nest + multi-attr: true + nested-attributes: ns-info + doc: | + Describes a set of inputs shapers for a @group operation + - + name: output + type: nest + nested-attributes: ns-output-info + doc: | + Describes the output shaper for a @group operation + Differently from @inputs and @shaper allow specifying + the shaper parent handle, too. + + - + name: shaper + type: nest + nested-attributes: ns-info + doc: | + Describes a single shaper for a @set operation + - + name: handle + subset-of: net-shaper + attributes: + - + name: scope + - + name: id + - + name: ns-info + subset-of: net-shaper + attributes: + - + name: handle + - + name: metric + - + name: bw-min + - + name: bw-max + - + name: burst + - + name: priority + - + name: weight + - + name: ns-output-info + subset-of: net-shaper + attributes: + - + name: parent + - + name: handle + - + name: metric + - + name: bw-min + - + name: bw-max + - + name: burst + - + name: priority + - + name: weight + +operations: + list: + - + name: get + doc: | + Get / Dump information about a/all the shaper for a given device + attribute-set: net-shaper + + do: + request: + attributes: + - ifindex + - handle + reply: + attributes: &ns-attrs + - parent + - handle + - metric + - bw-min + - bw-max + - burst + - priority + - weight + + dump: + request: + attributes: + - ifindex + reply: + attributes: *ns-attrs + - + name: set + doc: | + Create or configures the specified shaper. + On failures the extack is set accordingly. + Can't create @detached scope shaper, use + the @group operation instead. + attribute-set: net-shaper + flags: [ admin-perm ] + + do: + request: + attributes: + - ifindex + - shaper + + - + name: delete + doc: | + Clear (remove) the specified shaper. If after the removal + the parent shaper has no more children and the parent + shaper scope is @detached, even the parent is deleted, + recursively. + On failures the extack is set accordingly. + attribute-set: net-shaper + flags: [ admin-perm ] + + do: + request: + attributes: + - ifindex + - handle + + - + name: group + doc: | + Group the specified input shapers under the specified + output shaper, eventually creating the latter, if needed. + Input shapers scope must be either @queue or @detached. + Output shaper scope must be either @detached or @netdev. + When using an output @detached scope shaper, if the + @handle @id is not specified, a new shaper of such scope + is created and, otherwise the specified output shaper + must be already existing. + The operation is atomic, on failures the extack is set + accordingly and no change is applied to the device + shaping configuration, otherwise the output shaper + handle is provided as reply. + attribute-set: net-shaper + flags: [ admin-perm ] + + do: + request: + attributes: + - ifindex + - inputs + - output + reply: + attributes: + - handle diff --git a/MAINTAINERS b/MAINTAINERS index c0a3d9e93689..a1136f4dfa24 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15881,6 +15881,7 @@ F: include/linux/inetdevice.h F: include/linux/netdevice.h F: include/uapi/linux/cn_proc.h F: include/uapi/linux/if_* +F: include/uapi/linux/net_shaper.h F: include/uapi/linux/netdevice.h X: drivers/net/wireless/ diff --git a/include/uapi/linux/net_shaper.h b/include/uapi/linux/net_shaper.h new file mode 100644 index 000000000000..ab3d4eb0e1ab --- /dev/null +++ b/include/uapi/linux/net_shaper.h @@ -0,0 +1,74 @@ +/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */ +/* Do not edit directly, auto-generated from: */ +/* Documentation/netlink/specs/shaper.yaml */ +/* YNL-GEN uapi header */ + +#ifndef _UAPI_LINUX_NET_SHAPER_H +#define _UAPI_LINUX_NET_SHAPER_H + +#define NET_SHAPER_FAMILY_NAME "net-shaper" +#define NET_SHAPER_FAMILY_VERSION 1 + +/** + * enum net_shaper_scope - the different scopes where a shaper can be attached + * @NET_SHAPER_SCOPE_UNSPEC: The scope is not specified + * @NET_SHAPER_SCOPE_PORT: The root for the whole H/W + * @NET_SHAPER_SCOPE_NETDEV: The main shaper for the given network device. + * @NET_SHAPER_SCOPE_QUEUE: The shaper is attached to the given device queue. + * @NET_SHAPER_SCOPE_DETACHED: The shaper is not attached to any user-visible + * network device component and allows nesting and grouping of queues or + * others detached shapers. + */ +enum net_shaper_scope { + NET_SHAPER_SCOPE_UNSPEC, + NET_SHAPER_SCOPE_PORT, + NET_SHAPER_SCOPE_NETDEV, + NET_SHAPER_SCOPE_QUEUE, + NET_SHAPER_SCOPE_DETACHED, + + /* private: */ + __NET_SHAPER_SCOPE_MAX, + NET_SHAPER_SCOPE_MAX = (__NET_SHAPER_SCOPE_MAX - 1) +}; + +/** + * enum net_shaper_metric - different metric each shaper can support + * @NET_SHAPER_METRIC_BPS: Shaper operates on a bits per second basis + * @NET_SHAPER_METRIC_PPS: Shaper operates on a packets per second basis + */ +enum net_shaper_metric { + NET_SHAPER_METRIC_BPS, + NET_SHAPER_METRIC_PPS, +}; + +enum { + NET_SHAPER_A_IFINDEX = 1, + NET_SHAPER_A_HANDLE, + NET_SHAPER_A_METRIC, + NET_SHAPER_A_BW_MIN, + NET_SHAPER_A_BW_MAX, + NET_SHAPER_A_BURST, + NET_SHAPER_A_PRIORITY, + NET_SHAPER_A_WEIGHT, + NET_SHAPER_A_SCOPE, + NET_SHAPER_A_ID, + NET_SHAPER_A_PARENT, + NET_SHAPER_A_INPUTS, + NET_SHAPER_A_OUTPUT, + NET_SHAPER_A_SHAPER, + + __NET_SHAPER_A_MAX, + NET_SHAPER_A_MAX = (__NET_SHAPER_A_MAX - 1) +}; + +enum { + NET_SHAPER_CMD_GET = 1, + NET_SHAPER_CMD_SET, + NET_SHAPER_CMD_DELETE, + NET_SHAPER_CMD_GROUP, + + __NET_SHAPER_CMD_MAX, + NET_SHAPER_CMD_MAX = (__NET_SHAPER_CMD_MAX - 1) +}; + +#endif /* _UAPI_LINUX_NET_SHAPER_H */ diff --git a/net/Kconfig b/net/Kconfig index d27d0deac0bf..31fccfed04f7 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -66,6 +66,9 @@ config SKB_DECRYPTED config SKB_EXTENSIONS bool +config NET_SHAPER + bool + menu "Networking options" source "net/packet/Kconfig" diff --git a/net/Makefile b/net/Makefile index 65bb8c72a35e..60ed5190eda8 100644 --- a/net/Makefile +++ b/net/Makefile @@ -79,3 +79,4 @@ obj-$(CONFIG_XDP_SOCKETS) += xdp/ obj-$(CONFIG_MPTCP) += mptcp/ obj-$(CONFIG_MCTP) += mctp/ obj-$(CONFIG_NET_HANDSHAKE) += handshake/ +obj-$(CONFIG_NET_SHAPER) += shaper/ diff --git a/net/shaper/Makefile b/net/shaper/Makefile new file mode 100644 index 000000000000..13375884d60e --- /dev/null +++ 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. +# + +obj-y += shaper.o shaper_nl_gen.o + diff --git a/net/shaper/shaper.c b/net/shaper/shaper.c new file mode 100644 index 000000000000..49de88c68e2f --- /dev/null +++ b/net/shaper/shaper.c @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +#include <linux/kernel.h> +#include <linux/skbuff.h> + +#include "shaper_nl_gen.h" + +int net_shaper_nl_get_doit(struct sk_buff *skb, struct genl_info *info) +{ + return -EOPNOTSUPP; +} + +int net_shaper_nl_get_dumpit(struct sk_buff *skb, + struct netlink_callback *cb) +{ + return -EOPNOTSUPP; +} + +int net_shaper_nl_set_doit(struct sk_buff *skb, struct genl_info *info) +{ + return -EOPNOTSUPP; +} + +int net_shaper_nl_delete_doit(struct sk_buff *skb, struct genl_info *info) +{ + return -EOPNOTSUPP; +} + +static int __init shaper_init(void) +{ + return genl_register_family(&net_shaper_nl_family); +} + +subsys_initcall(shaper_init); diff --git a/net/shaper/shaper_nl_gen.c b/net/shaper/shaper_nl_gen.c new file mode 100644 index 000000000000..b444d1ff55a1 --- /dev/null +++ b/net/shaper/shaper_nl_gen.c @@ -0,0 +1,117 @@ +// SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) +/* Do not edit directly, auto-generated from: */ +/* Documentation/netlink/specs/shaper.yaml */ +/* YNL-GEN kernel source */ + +#include <net/netlink.h> +#include <net/genetlink.h> + +#include "shaper_nl_gen.h" + +#include <uapi/linux/net_shaper.h> + +/* Common nested types */ +const struct nla_policy net_shaper_handle_nl_policy[NET_SHAPER_A_ID + 1] = { + [NET_SHAPER_A_SCOPE] = NLA_POLICY_MAX(NLA_U32, 4), + [NET_SHAPER_A_ID] = { .type = NLA_U32, }, +}; + +const struct nla_policy net_shaper_ns_info_nl_policy[NET_SHAPER_A_WEIGHT + 1] = { + [NET_SHAPER_A_HANDLE] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy), + [NET_SHAPER_A_METRIC] = NLA_POLICY_MAX(NLA_U32, 1), + [NET_SHAPER_A_BW_MIN] = { .type = NLA_UINT, }, + [NET_SHAPER_A_BW_MAX] = { .type = NLA_UINT, }, + [NET_SHAPER_A_BURST] = { .type = NLA_UINT, }, + [NET_SHAPER_A_PRIORITY] = { .type = NLA_U32, }, + [NET_SHAPER_A_WEIGHT] = { .type = NLA_U32, }, +}; + +const struct nla_policy net_shaper_ns_output_info_nl_policy[NET_SHAPER_A_PARENT + 1] = { + [NET_SHAPER_A_PARENT] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy), + [NET_SHAPER_A_HANDLE] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy), + [NET_SHAPER_A_METRIC] = NLA_POLICY_MAX(NLA_U32, 1), + [NET_SHAPER_A_BW_MIN] = { .type = NLA_UINT, }, + [NET_SHAPER_A_BW_MAX] = { .type = NLA_UINT, }, + [NET_SHAPER_A_BURST] = { .type = NLA_UINT, }, + [NET_SHAPER_A_PRIORITY] = { .type = NLA_U32, }, + [NET_SHAPER_A_WEIGHT] = { .type = NLA_U32, }, +}; + +/* NET_SHAPER_CMD_GET - do */ +static const struct nla_policy net_shaper_get_do_nl_policy[NET_SHAPER_A_HANDLE + 1] = { + [NET_SHAPER_A_IFINDEX] = { .type = NLA_U32, }, + [NET_SHAPER_A_HANDLE] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy), +}; + +/* NET_SHAPER_CMD_GET - dump */ +static const struct nla_policy net_shaper_get_dump_nl_policy[NET_SHAPER_A_IFINDEX + 1] = { + [NET_SHAPER_A_IFINDEX] = { .type = NLA_U32, }, +}; + +/* NET_SHAPER_CMD_SET - do */ +static const struct nla_policy net_shaper_set_nl_policy[NET_SHAPER_A_SHAPER + 1] = { + [NET_SHAPER_A_IFINDEX] = { .type = NLA_U32, }, + [NET_SHAPER_A_SHAPER] = NLA_POLICY_NESTED(net_shaper_ns_info_nl_policy), +}; + +/* NET_SHAPER_CMD_DELETE - do */ +static const struct nla_policy net_shaper_delete_nl_policy[NET_SHAPER_A_HANDLE + 1] = { + [NET_SHAPER_A_IFINDEX] = { .type = NLA_U32, }, + [NET_SHAPER_A_HANDLE] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy), +}; + +/* NET_SHAPER_CMD_GROUP - do */ +static const struct nla_policy net_shaper_group_nl_policy[NET_SHAPER_A_OUTPUT + 1] = { + [NET_SHAPER_A_IFINDEX] = { .type = NLA_U32, }, + [NET_SHAPER_A_INPUTS] = NLA_POLICY_NESTED(net_shaper_ns_info_nl_policy), + [NET_SHAPER_A_OUTPUT] = NLA_POLICY_NESTED(net_shaper_ns_output_info_nl_policy), +}; + +/* Ops table for net_shaper */ +static const struct genl_split_ops net_shaper_nl_ops[] = { + { + .cmd = NET_SHAPER_CMD_GET, + .doit = net_shaper_nl_get_doit, + .policy = net_shaper_get_do_nl_policy, + .maxattr = NET_SHAPER_A_HANDLE, + .flags = GENL_CMD_CAP_DO, + }, + { + .cmd = NET_SHAPER_CMD_GET, + .dumpit = net_shaper_nl_get_dumpit, + .policy = net_shaper_get_dump_nl_policy, + .maxattr = NET_SHAPER_A_IFINDEX, + .flags = GENL_CMD_CAP_DUMP, + }, + { + .cmd = NET_SHAPER_CMD_SET, + .doit = net_shaper_nl_set_doit, + .policy = net_shaper_set_nl_policy, + .maxattr = NET_SHAPER_A_SHAPER, + .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO, + }, + { + .cmd = NET_SHAPER_CMD_DELETE, + .doit = net_shaper_nl_delete_doit, + .policy = net_shaper_delete_nl_policy, + .maxattr = NET_SHAPER_A_HANDLE, + .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO, + }, + { + .cmd = NET_SHAPER_CMD_GROUP, + .doit = net_shaper_nl_group_doit, + .policy = net_shaper_group_nl_policy, + .maxattr = NET_SHAPER_A_OUTPUT, + .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO, + }, +}; + +struct genl_family net_shaper_nl_family __ro_after_init = { + .name = NET_SHAPER_FAMILY_NAME, + .version = NET_SHAPER_FAMILY_VERSION, + .netnsok = true, + .parallel_ops = true, + .module = THIS_MODULE, + .split_ops = net_shaper_nl_ops, + .n_split_ops = ARRAY_SIZE(net_shaper_nl_ops), +}; diff --git a/net/shaper/shaper_nl_gen.h b/net/shaper/shaper_nl_gen.h new file mode 100644 index 000000000000..00cee4efd21e --- /dev/null +++ b/net/shaper/shaper_nl_gen.h @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */ +/* Do not edit directly, auto-generated from: */ +/* Documentation/netlink/specs/shaper.yaml */ +/* YNL-GEN kernel header */ + +#ifndef _LINUX_NET_SHAPER_GEN_H +#define _LINUX_NET_SHAPER_GEN_H + +#include <net/netlink.h> +#include <net/genetlink.h> + +#include <uapi/linux/net_shaper.h> + +/* Common nested types */ +extern const struct nla_policy net_shaper_handle_nl_policy[NET_SHAPER_A_ID + 1]; +extern const struct nla_policy net_shaper_ns_info_nl_policy[NET_SHAPER_A_WEIGHT + 1]; +extern const struct nla_policy net_shaper_ns_output_info_nl_policy[NET_SHAPER_A_PARENT + 1]; + +int net_shaper_nl_get_doit(struct sk_buff *skb, struct genl_info *info); +int net_shaper_nl_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb); +int net_shaper_nl_set_doit(struct sk_buff *skb, struct genl_info *info); +int net_shaper_nl_delete_doit(struct sk_buff *skb, struct genl_info *info); +int net_shaper_nl_group_doit(struct sk_buff *skb, struct genl_info *info); + +extern struct genl_family net_shaper_nl_family; + +#endif /* _LINUX_NET_SHAPER_GEN_H */
Define the user-space visible interface to query, configure and delete network shapers via yaml definition. Add dummy implementations for the relevant NL callbacks. set() and delete() operations touch a single shaper creating/updating or deleting it. The group() operation creates a shaper's group, nesting multiple input shapers under the specified output shaper. Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- RFC v1 -> RFC v2: - u64 -> uint - net_shapers -> net-shapers - documented all the attributes - dropped [ admin-perm ] for get() op - group op - set/delete touch a single shaper --- Documentation/netlink/specs/shaper.yaml | 262 ++++++++++++++++++++++++ MAINTAINERS | 1 + include/uapi/linux/net_shaper.h | 74 +++++++ net/Kconfig | 3 + net/Makefile | 1 + net/shaper/Makefile | 9 + net/shaper/shaper.c | 34 +++ net/shaper/shaper_nl_gen.c | 117 +++++++++++ net/shaper/shaper_nl_gen.h | 27 +++ 9 files changed, 528 insertions(+) create mode 100644 Documentation/netlink/specs/shaper.yaml create mode 100644 include/uapi/linux/net_shaper.h create mode 100644 net/shaper/Makefile create mode 100644 net/shaper/shaper.c create mode 100644 net/shaper/shaper_nl_gen.c create mode 100644 net/shaper/shaper_nl_gen.h