Re: [PATCH RFC net-next 4/6] cadence: macb/gem: add XDP support for gem

["Théo Lebrun" <theo.lebrun@bootlin.com>]

On Tue Dec 2, 2025 at 6:32 PM CET, Paolo Valerio wrote:
> On 27 Nov 2025 at 03:41:56 PM, Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>> On Wed Nov 19, 2025 at 2:53 PM CET, Paolo Valerio wrote:
>>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>>> index 5829c1f773dd..53ea1958b8e4 100644
>>> --- a/drivers/net/ethernet/cadence/macb_main.c
>>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>>> +static u32 gem_xdp_run(struct macb_queue *queue, struct xdp_buff *xdp,
>>> +		       struct net_device *dev)

[...]

>>> +{
>>> +	struct bpf_prog *prog;
>>> +	u32 act = XDP_PASS;
>>> +
>>> +	rcu_read_lock();
>>> +
>>> +	prog = rcu_dereference(queue->bp->prog);
>>> +	if (!prog)
>>> +		goto out;
>>> +
>>> +	act = bpf_prog_run_xdp(prog, xdp);
>>> +	switch (act) {
>>> +	case XDP_PASS:
>>> +		goto out;
>>> +	case XDP_REDIRECT:
>>> +		if (unlikely(xdp_do_redirect(dev, xdp, prog))) {
>>> +			act = XDP_DROP;
>>> +			break;
>>> +		}
>>> +		goto out;
>>
>> Why the `unlikely()`?
>
> just expecting the err path to be the exception, although this is not
> consistend with the XDP_TX path.
> Do you prefer to remove it?

No we can keep it, I had missed this was the error case.
Sorry about that.

>
>>> +	default:
>>> +		bpf_warn_invalid_xdp_action(dev, prog, act);
>>> +		fallthrough;
>>> +	case XDP_ABORTED:
>>> +		trace_xdp_exception(dev, prog, act);
>>> +		fallthrough;
>>> +	case XDP_DROP:
>>> +		break;
>>> +	}
>>> +
>>> +	page_pool_put_full_page(queue->page_pool,
>>> +				virt_to_head_page(xdp->data), true);
>>
>> Maybe move that to the XDP_DROP, it is the only `break` in the above
>> switch statement. It will be used by the default and XDP_ABORTED cases
>> through fallthrough. We can avoid the out label and its two gotos that
>> way.
>
> We'd not put to page pool in case the redirect fails or am I missing
> something?

Ah yes there are two break statements. I missed it.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com