Re: [xdp-hints] [PATCH bpf-next v2 6/8] mlx4: Introduce mlx4_xdp_buff wrapper for xdp_buff

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 23 Nov 2022 10:26:41 -0800 Stanislav Fomichev wrote:
> > This embedding trick works for drivers that put xdp_buff on the stack,
> > but mlx5 supports XSK zerocopy, which uses the xsk_buff_pool for
> > allocating them. This makes it a bit awkward to do the same thing there;
> > and since it's probably going to be fairly common to do something like
> > this, how about we just add a 'void *drv_priv' pointer to struct
> > xdp_buff that the drivers can use? The xdp_buff already takes up a full
> > cache line anyway, so any data stuffed after it will spill over to a new
> > one; so I don't think there's much difference performance-wise.  
> 
> I guess the alternative is to extend xsk_buff_pool with some new
> argument for xdp_buff tailroom? (so it can kmalloc(sizeof(xdp_buff) +
> xdp_buff_tailroom))
> But it seems messy because there is no way of knowing what the target
> device's tailroom is, so it has to be a user setting :-/
> I've started with a priv pointer in xdp_buff initially, it seems fine
> to go back. I'll probably convert veth/mlx4 to the same mode as well
> to avoid having different approaches in different places..

Can we not do this please? Add 16B of "private driver space" after
the xdp_buff in xdp_buff_xsk (we have 16B to full cacheline), the
drivers decide how they use it. Drivers can do BUILD_BUG_ON() for their
expected size and cast that to whatever struct they want. This is how
various offloads work, the variable size tailroom would be an over
design IMO.

And this way non XSK paths can keep its normal typing.

> > I'll send my patch to add support to mlx5 (using the drv_priv pointer
> > approach) separately.  
> 
> Saw them, thanks! Will include them in v3+.