On Mon, 13 Jan 2025 13:48:14 -0800 Joe Damato wrote:

quoted

quoted

The changes generally look OK to me (it seems RTNL is held on all
paths where this code can be called from as far as I can tell), but
there was one thing that stood out to me.

AFAIU, drivers avoid marking XDP queues as NETDEV_QUEUE_TYPE_RX
or NETDEV_QUEUE_TYPE_TX. I could be wrong, but that was my
understanding and I submit patches to several drivers with this
assumption.

For example, in commit b65969856d4f ("igc: Link queues to NAPI
instances"), I unlinked/linked the NAPIs and queue IDs when XDP was
enabled/disabled. Likewise, in commit 64b62146ba9e ("net/mlx4: link
NAPI instances to queues and IRQs"), I avoided the XDP queues.

If drivers are to avoid marking XDP queues as NETDEV_QUEUE_TYPE_RX
or NETDEV_QUEUE_TYPE_TX, perhaps tsnep needs to be modified
similarly?  

With 5ef44b3cb4 ("xsk: Bring back busy polling support") the linking of
the NAPIs is required for XDP/XSK. So it is strange to me if for XDP/XSK
the NAPIs should be unlinked. But I'm not an expert, so maybe there is
a reason why.

I added Magnus, maybe he knows if XSK queues shall still be linked to
NAPIs.  

OK, so I think I was probably just wrong?

I looked at bnxt and it seems to mark XDP queues, which means
probably my patches for igc, ena, and mlx4 need to be fixed and the
proposed patch I have for virtio_net needs to be adjusted.

I can't remember now why I thought XDP queues should be avoided. I
feel like I read that or got that as feedback at some point, but I
can't remember now. Maybe it was just one driver or something I was
working on and I accidentally thought it should be avoided
everywhere? Not sure.

Hopefully some one can give a definitive answer on this one before I
go through and try to fix all the drivers I modified :|

XDP and AF_XDP are different things. The XDP part of AF_XDP is to some
extent for advertising purposes :) If memory serves me well:

XDP Tx -> these are additional queues automatically allocated for
          in-kernel XDP, allocated when XDP is attached on Rx.
          These should _not_ be listed in netlink queue, or NAPI;
          IOW should not be linked to NAPI instances.
XDP Rx -> is not a thing, XDP attaches to stack queues, there are no
          dedicated XDP Rx queues
AF_XDP -> AF_XDP "takes over" stack queues. It's a bit of a gray area.
          I don't recall if we made a call on these being linked, but
          they could probably be listed like devmem as a queue with
          an extra attribute, not a completely separate queue type.