Message-ID: <aBWHv6TAwLnbPhMd@LQ3V64L9R2>
Date: Fri, 2 May 2025 20:04:31 -0700
From: Joe Damato <jdamato@...tly.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>,
	Samiullah Khawaja <skhawaja@...gle.com>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	almasrymina@...gle.com, willemb@...gle.com, mkarsten@...terloo.ca,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next v5] Add support to set napi threaded for
 individual napi

On Fri, May 02, 2025 at 07:10:11PM -0700, Jakub Kicinski wrote:
> On Tue, 29 Apr 2025 17:16:03 -0700 Joe Damato wrote:
> > > The way I see it - the traditional permission model doesn't extend 
> > > in any meaningful way to network settings. All the settings are done 
> > > by some sort of helper or intermediary which implements its own
> > > privilege model.  
> > 
> > I agree that is how it is today, but maybe we are misunderstanding
> > each other? In my mind, today, the intermediary is something like a
> > script that runs a bunch of ethtool commands.
> > 
> > In my mind with the movement of more stuff to core and the existence
> > of YNL, it seems like the future is an app uses YNL and is able to
> > configure (for example) an RSS context and ntuple rules to steer
> > flows to queues it cares about... which in my mind is moving toward
> > eliminating the intermediary ?
> 
> Yes, AFAIU.
> 
> > > My thinking is more that the "global" settings are basically "system"
> > > settings. There is a high-perf app running which applied its own
> > > settings to its own queues. The remaining queues belong to the "system".
> > > Now if you for some reason want to modify the settings of the "system
> > > queues" you really don't want to override the app specific settings.  
> > 
> > Yea, I see what you are saying, I think.
> > 
> > Can I rephrase it to make sure I'm following?
> > 
> > An app uses YNL to set defer-hard-irqs to 100 for napis 2-4. napis 0
> > and 1 belong to the "system."
> > 
> > You are saying that writing to the NIC-wide sysfs path for
> > defer-hard-irqs wouldn't affect napis 0 and 1 because they don't
> > belong to the system anymore.
> > 
> > Is that right?
> 
> Typo - napis 2-4, right? If so - yes, exactly. 

Yes, a typo -- sorry for the added confusion.
 
> > If so... I think that's fairly interesting and maybe it implies a
> > tighter coupling of apps to queues than is possible with the API
> > that exists today? For example say an app does a bunch of config to
> > a few NAPIs and then suddenly crashes. I suppose core would need to
> > "know" about this so it can "release" those queues ?
> 
> Exactly, Stan also pointed out the config lifetime problem.
> My plan was to add the ability to tie the config to a netlink socket.
> App dies, socket goes away, clears the config. Same thing as we do for
> clean up of DEVMEM bindings. But I don't have full details.

Yea that makes sense. Right now, we are using YNL to configure NAPI
settings for specific queues but clearing them on graceful app
termination. Of course if the app dies unexpectedly, then the NAPIs
are in configured state with no app.

Having some sort of automatic cleanup thing (as you've described)
would be pretty handy.

> The thing I did add (in the rx-buf-len series) was a hook to the queue
> count changing code which wipes the configuration for queues which are
> explicitly disabled.
> So if you do some random reconfig (eg attach XDP) and driver recreates
> all NAPIs - the config should stay around. Same if you do ifdown ifup.
> But if you set NAPI count from 8 to 4 - the NAPIs 4..7 should get wiped.

Yea I see. I will go back and re-read that series because I think
missed that part.

IIRC, if you:
  1. set defer-hard-irqs on NAPIs 2 and 3
  2. resize down to 2 queues
  3. resize then back up to 4, the setting for NAPIs 2 and 3 should
     be restored.

I now wonder if I should change that to be more like what you
describe for rx-buf-len so we converge?

Powered by blists - more mailing lists