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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DESS6PMQCC80.L2KUOYYDCWQW@bootlin.com>
Date: Mon, 08 Dec 2025 11:59:23 +0100
From: Théo Lebrun <theo.lebrun@...tlin.com>
To: "Paolo Valerio" <pvalerio@...hat.com>, Théo Lebrun
 <theo.lebrun@...tlin.com>, <netdev@...r.kernel.org>
Cc: "Nicolas Ferre" <nicolas.ferre@...rochip.com>, "Claudiu Beznea"
 <claudiu.beznea@...on.dev>, "Andrew Lunn" <andrew+netdev@...n.ch>, "David
 S. Miller" <davem@...emloft.net>, "Eric Dumazet" <edumazet@...gle.com>,
 "Jakub Kicinski" <kuba@...nel.org>, "Paolo Abeni" <pabeni@...hat.com>,
 "Lorenzo Bianconi" <lorenzo@...nel.org>, "Thomas Petazzoni"
 <thomas.petazzoni@...tlin.com>, Grégory Clement
 <gregory.clement@...tlin.com>, Benoît Monin
 <benoit.monin@...tlin.com>
Subject: Re: [PATCH RFC net-next 4/6] cadence: macb/gem: add XDP support for
 gem

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@...tlin.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