From: Alexander Lobakin <aleksander.loba...@intel.com>
Date: Tue, 8 Apr 2025 15:22:48 +0200
> From: Maciej Fijalkowski <maciej.fijalkow...@intel.com>
> Date: Tue, 11 Mar 2025 15:05:38 +0100
>
>> On Wed, Mar 05, 2025 at 05:21:19PM +0100, Alexander Lobakin wrote:
>>> "Couple" is a bit humbly... Add the following functionality to libeth:
[...]
>>> +/**
>>> + * libeth_xdp_xmit_do_bulk - implement full .ndo_xdp_xmit() in driver
>>> + * @dev: target &net_device
>>> + * @n: number of frames to send
>>> + * @fr: XDP frames to send
>>> + * @f: flags passed by the stack
>>> + * @xqs: array of XDPSQs driver structs
>>> + * @nqs: number of active XDPSQs, the above array length
>>> + * @fl: driver callback to flush an XDP xmit bulk
>>> + * @fin: driver cabback to finalize the queue
>>> + *
>>> + * If the driver has active XDPSQs, perform common checks and send the
>>> frames.
>>> + * Finalize the queue, if requested.
>>> + *
>>> + * Return: number of frames sent or -errno on error.
>>> + */
>>> +#define libeth_xdp_xmit_do_bulk(dev, n, fr, f, xqs, nqs, fl, fin) \
>>> + _libeth_xdp_xmit_do_bulk(dev, n, fr, f, xqs, nqs, fl, fin, \
>>> + __UNIQUE_ID(bq_), __UNIQUE_ID(ret_), \
>>> + __UNIQUE_ID(nqs_))
>>
>> why __UNIQUE_ID() is needed?
>
> As above, variable shadowing.
>
>>
>>> +
>>> +#define _libeth_xdp_xmit_do_bulk(d, n, fr, f, xqs, nqs, fl, fin, ub, ur,
>>> un) \
>>
>> why single underscore? usually we do __ for internal funcs as you did
>> somewhere above.
>
> Double-underscored is defined above already :D
> So it would be either like this or __ + ___
>
>>
>> also, why define and not inlined func?
>
> I'll double check, but if you look at its usage in idpf/xdp.c, you'll
> see that some arguments are non-trivial to obtain, IOW they cost some
> cycles. Macro ensures they won't be fetched prior to
> `likely(number_of_xdpsqs)`.
> I'll convert to an inline and check if the compiler handles this itself.
> It didn't behave in {,__}libeth_xdp_tx_fill_stats() unfortunately, hence
> macro there as well =\
UPD: it can't be an inline func since it's meant to be called like that
from the driver:
return libeth_xdp_xmit_do_bulk(dev, n, frames, flags,
&vport->txqs[vport->xdp_txq_offset],
vport->num_xdp_txq,
idpf_xdp_xmit_flush_bulk,
idpf_xdp_tx_finalize);
The type of `&vport->txqs[vport->xdp_txq_offset]` is undefined from
libeth's perspective. libeth_xdp_xmit_init_bulk() embedded into it picks
the appropriate queue right away in the driver and it's a macro itself.
>
>>
>>> +({ \
>>> + u32 un = (nqs); \
>>> + int ur; \
>>> + \
>>> + if (likely(un)) { \
>>> + struct libeth_xdp_tx_bulk ub; \
>>> + \
>>> + libeth_xdp_xmit_init_bulk(&ub, d, xqs, un); \
>>> + ur = __libeth_xdp_xmit_do_bulk(&ub, fr, n, f, fl, fin); \
>>> + } else { \
>>> + ur = -ENXIO; \
>>> + } \
>>> + \
>>> + ur; \
>>> +})
Thanks,
Olek
