Re: [PATCH net-next] tsnep: Link queues to NAPIs

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 13 Jan 2025 13:48:14 -0800 Joe Damato wrote:
> > > 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.