Message-ID: <20240703142017.346ed0c1@kernel.org>
Date: Wed, 3 Jul 2024 14:20:17 -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>
Subject: Re: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec

On Wed, 03 Jul 2024 16:53:38 +0200 Paolo Abeni wrote:
> Note there is no stats support for the shapers, nor is planned, nor the
> H/W I know of have any support for it.
> 
> The destructive operations will be needed only when the configuration
> change is inherently destructive. Nothing prevents the user-space to
> push a direct configuration change when possible - which should be the
> most frequent case, in practice.
> 
> Regarding the entity responsible for control, I had in mind a single
> one, yes. I read the above as you are looking forward to e.g. different
> applications configuring their own shaping accessing directly the NL
> interface, am I correct? Why can’t such applications talk to that
> daemon, instead? 

We know such daemon did not materialize in other cases.
We can assume there is a daemon if we think that's a good design.
We shouldn't use a mirage of a third-party daemon to wave away
problems we didn't manage to solve.

> Anyway different applications must touch disjoint resources (e.g.
> disjoint queues sets) right? In such a case even multiple destructive
> configuration changes (on disjoint resources set) will not be
> problematic.
> 
> Still if we want to allow somewhat consistent, concurrent, destructive
> configuration changes on shared resource (why? It sounds a bit of
> overdesign at this point), we could extend the set() operation to
> additional support shapers deletion, e.g. adding an additional ‘delete’
> flag attribute to the ‘handle’ sub-set.

To judge whether it's an over-design we'd need to know what the user
scenarios are, those are not included in the series AFAICS.

> > For a fixed-layout scheduler where HW blocks have a fixed mapping 
> > with user's hierarchy - it's easier to program the shapers from 
> > the hierarchy directly. Since node maps to the same set of registers
> > reprogramming will be writing to a register a value it already has
> > - a noop. That's different than doing a full reset and reprogram 
> > each time, as two separate calls from user space.
> > 
> > For Intel's cases OTOH, when each command is a separate FW call
> > we can't actually enforce the atomic transitions, right?
> > Your code seems to handle returns smaller the number of commands,
> > from which I infer that we may execute half of the modification?  
> 
> Yes, the code and the NL API allows the NIC to do the update
> incrementally, and AFAICS Intel ICE has no support for complex
> transactions.
> Somewhat enforcing atomic transitions will be quite complex at best and
> is not need to accomplish the stated goal - allow reconfiguration even
> when the H/W does not support intermediate states.
> 
> Do we need to enforce atomicity? Why? NFT has proven that a
> transational model implementation is hard, and should be avoided if
> possible. 
> 
> > IOW for Andrew's HW - he'd probably prefer to look at the resulting
> > tree, no matter what previous state we were in. For Intel we _can't_
> > support atomic commands, if they span multiple cycles of FW exchanges?  
> 
> My understanding is that we can’t have atomic updates on Intel, from
> firmare perspective. As said, I don’t think it’s necessary to support
> them.
> 
> WRT the DSA H/W, do you mean the core should always set() the whole
> known tree to the driver, regardless of the specific changes asked by
> the user-space? If so, what about letting the driver expose some
> capability (or private flag) asking the core for such behavior? So that
> the driver will do the largish set() only with the H/W requiring that.
> 
> Anyway I'm not sure the mentioned DSA H/W would benefit from always
> receiving the whole configuration. e.g. changing the weight for a
> single queue shaper would not need the whole data-set.

To be blunt - what I'm getting at is that the API mirrors Intel's FW
API with an extra kludge to support the DSA H/W - in the end matching
neither what the DSA wants nor what Intel can do.

> > > In any case I think that the larger complexity to implement a full
> > > transactional model. nft had proven that to be very hard and bug
> > > prone. I really would avoid that option, if possible.  
> > 
> > Maybe instead of discussing the user space API it'd be more beneficial
> > to figure out a solid way of translating the existing APIs into the new
> > model?  
> 
> Could you please rephrase? I think all the arguments discussed here are
> related to the model - at some point that impact the user space API,
> too.

I was hoping that implementing the code that mirrors the existing APIs
into tree operations would teach us something about the primitives 
that operate on the tree. The proposed primitives are really low level,
which is why we need to "fuse" them into multi-change operations.

More specifically what I described a few emails up was a
group+schedule+limit paradigm. Instead of describing single moves 
and parameter setting - transformation would describe inputs,
scheduling across them, and rate limit to apply. But one transformation
would always operate on at most one MUX node.

IOW I'm trying to explore whether we can find a language of
transformations which will be more complex than single micro-operations
on the tree, but sufficiently expressive to provide atomic
transformations without transactions of micro-ops.

Powered by blists - more mailing lists