Subject: Re: [net-next PATCH v8 02/10] net: Add queue and napi association

From: Jakub Kicinski <[email-masked]>
Date: Tue, 21 Nov 2023 17:15:00 -0800

On Tue, 21 Nov 2023 16:08:07 -0800 Nambiar, Amritha wrote:
> > To reiterate - the thing I find odd about the current situation is that
> > we hide the queues if they get disabled by lowering ethtool -L, but we
> > don't hide them when the entire interface is down. When the entire
> > interface is down there should be no queues, right?
> 
> "When the entire interface is down there should be no queues" - 
> currently, 'ethtool --show-channels' reports all the available queues 
> when interface is DOWN

That's not the same. ethtool -l shows the configuration not 
the instantiated objects. ethtool -a will also show you the
pause settings even when cable is not plugged in.
sysfs objects of the queues are still exposed for devices which 
are down, that's true. But again, that's to expose the config.

> > Differently put - what logic that'd make sense to the user do we apply
> > when trying to decide if the queue is visible? < real_num_queues is
> > an implementation detail.
> > 
> > We can list all the queues, always, too. No preference. I just want to
> > make sure that the rules are clear and not very dependent on current
> > implementation and not different driver to driver.  
> 
> I think currently, the queue dump results when the device is down aligns 
> for both APIs (netdev-genl queue-get and ethtool show-channels) for all 
> the drivers. If we decide to NOT show queues/NAPIs (with netdev-genl) 
> when the device is down, the user would see conflicting results, the 
> dump results with netdev-genl APIs would be different from what 'ethtool 
> --show-channels' and 'ps -aef | grep napi' reports.

We should make the distinction between configuration and state of
instantiated objects clear before we get too far. Say we support
setting ring length for a specific queue. Global setting is 512,
queue X wants 256. How do we remove the override for queue X?
By setting it to 512? What if we want 512, and the default shifts
to something else? We'll need an explicit "reset" command.

I think it may be cleaner to keep queue-get as state of queues,
and configuration / settings / rules completely separate.

Am I wrong? Willem?