On Mon, Jan 27, 2025 at 02:24:00PM -0800, Jakub Kicinski wrote:

On Mon, 27 Jan 2025 17:07:54 -0500 Joe Damato wrote:

quoted

quoted

Tx NAPIs are one aspect, whether they have ID or not we may want direct
access to the struct somewhere in the core, via txq, at some point, and
then people may forget the linking has an unintended effect of also
changing the netlink attrs. The other aspect is that driver may link
queue to a Rx NAPI instance before napi_enable(), so before ID is
assigned. Again, we don't want to report ID of 0 in that case.  

I'm sorry I'm not sure I'm following what you are saying here; I
think there might be separate threads concurrently and I'm probably
just confused :)

I think you are saying that netdev_nl_napi_fill_one should not
report 0, which I think is fine but probably a separate patch?

I think, but am not sure, that Jason was asking for guidance on
TX-only NAPIs and linking them with calls to netif_queue_set_napi.
It seems that Jason may be suggesting that the driver shouldn't have
to know that TX-only NAPIs have a NAPI ID of 0 and thus should call
netif_queue_set_napi for all NAPIs and not have to deal think about
TX-only NAPIs at all.

From you've written, Jakub, I think you are suggesting you agree
with that, but with the caveat that netdev_nl_napi_fill_one should
not report 0.

Right up to this point.

quoted

Then, one day in the future, if TX-only NAPIs get an ID they will
magically start to show up.

Is that right?

Sort of. I was trying to point out corner cases which would also
benefit from netdev_nl_queue_fill_one() being more careful about 
the NAPI IDs it reports. But the conclusion is the same.

quoted

If so, I'll re-spin the RFC to call netif_queue_set_napi for all
NAPIs in virtio_net, including TX-only NAPIs and see about including
a patch to tweak netdev_nl_napi_fill_one, if necessary.

netdev_nl_queue_fill_one(), not netdev_nl_napi_fill_one()

Right, sorry for the typo/added confusion.
 

Otherwise SG.

After net-next reopens I think the patch to netdev_nl_queue_fill_one()
could be posted separately. There may be drivers out there which already
link Tx NAPIs, we shouldn't delay making the reporting more careful.

OK, I'll start with that when net-next reopens while waiting on the
locking changes to come later and do the actual linking.