Re: [PATCH v4 net-next 02/12] netlink: spec: add shaper YAML spec

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 23 Aug 2024 10:35:05 +0200 Paolo Abeni wrote:

> > 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.  
> 
> How about re-phrasing the previous paragraph as:
> 
>    Each @shaper is identified within the given device, by an @handle,
>    comprising both a @scope and an @id.
> 
>    Depending on the @scope value, the shapers are attached to specific
>    HW objects (queues, devices) or, for @node scope, represent a
>    scheduling group that can be placed in an arbitrary location of
>    the scheduling tree.

s/that can be placed in an arbitrary location of/which is an inner node/

So:

    Depending on the @scope value, the shapers are attached to specific
    HW objects (queues, devices) or, for @node scope, represent a
    scheduling group which is an inner node the scheduling tree with 
    multiple inputs.

>    Shapers can be created with two different operations: the @set
>    operation, to create and update single "attached" shaper, and
>    the @group operation, to create and update a scheduling
>    group. Only the @group operation can create @node scope shapers.

> >> +    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.  
> 
> What about:
> 
>    Can be placed in any arbitrary location of
>    the scheduling tree, except leaves and root.

Oh, I was thinking along the same lines above.
Whether "except leaves or root" or "inner node" is clearer is up to you.

> >> +      -
> >> +        name: weight
> >> +        type: u32
> >> +        doc: |
> >> +          Weighted round robin weight for given shaper.  
> > 
> > Relative weight of the input into a round robin node.  
> 
> I would avoid mentioning 'input' unless we rolls back to the previous 
> naming scheme.

Okay, how about:

	Relative weight used by a parent round robin node.

> >> +           Differently from @leaves and @shaper allow specifying
> >> +           the shaper parent handle, too.  
> > 
> > Maybe this attr is better called "node", after all.  
> 
> Fine by me, but would that cause some confusion with the alias scope 
> value?

But to be clear, the "root" describes the node we're creating, right?
Alternatively maybe we should remove the level of nesting and let 
the attributes live at the command level?