[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240905173857.588f2578@kernel.org>
Date: Thu, 5 Sep 2024 17:38:57 -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>,
anthony.l.nguyen@...el.com, przemyslaw.kitszel@...el.com,
intel-wired-lan@...ts.osuosl.org, edumazet@...gle.com
Subject: Re: [PATCH v6 net-next 02/15] netlink: spec: add shaper YAML spec
On Thu, 5 Sep 2024 18:17:42 +0200 Paolo Abeni wrote:
> > I don't see example uses in the cover letter or the test so there's
> > a good chance I'm missing something, but... why node_parent?
> > The only thing you need to know about the parent is its handle,
> > so just "parent", right?
> >
> > Also why node_handle? Just "handle", and other attrs of the node can
> > live in the main scope.
>
> I added the 'node_' prefix in the list to stress that such attributes
> belong to the node.
>
> In the yaml/command line will be only 'handle', 'parent'.
And the scope inside parent is 'handle', not subset of 'net-shaper'?
Just to be 100% sure :)
> > Unless you have a strong reason to do this to simplify the code -
> > "from netlink perspective" it looks like unnecessary nesting.
> > The operation arguments describe the node, there's no need to nest
> > things in another layer.
>
> Ok, the code complexity should not change much. Side question: currently
> the node() operation allows specifying all the b/w related attributes
> for the 'node' shaper, should I keep them? (and move them in the main
> yaml scope)
Up to you, I was surprised they were there (I expected @group to
be solely about creation of the RR node, and rate limit would have
to be set via a separate @set). But I don't expect providing rate
limit params in @group to be problematic and user space may find it
convenient. So I'm neutral.
And yes, they should sit directly at the message level, not in any
nest.
Powered by blists - more mailing lists