On Mon, Jan 27, 2025 at 01:33:04PM -0800, Jakub Kicinski wrote:

On Mon, 27 Jan 2025 14:31:21 -0500 Joe Damato wrote:

quoted

Actually, I missed a patch Jakub submit to net [1], which prevents
dumping TX-only NAPIs.

That patch only addresses NAPI ops, here I think we're talking about
attributes of the queue object.

Yea, that's true.

quoted

So, I think this RFC as-is (only calling netif_queue_set_napi
for RX NAPIs) should be fine without changes.

Weak preference towards making netdev_nl_queue_fill_one() "do the right
thing" when NAPI does not have ID assigned. And right thing IMO would
be to skip reporting the NAPI_ID attribute.

Ah, right. Your patch was for netdev_nl_napi_fill_one, so presumably
a similar patch would need to be written for
netdev_nl_queue_fill_one.

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.

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

Is that right?

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.