Re: [RFC PATCH net-next 03/13] net: core: add XDP_MODE_HW offload hook for knod

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The patch introduces an order-dependent state violation where attaching a memory provider and a hardware-offloaded XDP program succeeds or fails depending purely on the order of operations.

Pre-existing issues:
- [High] Blocking XDP uninstalls when a memory provider is active leads to a resource leak of the BPF program and a permanently active 'zombie' XDP program on slave devices.
--

--- Patch [3]: [RFC PATCH net-next 03/13] net: core: add XDP_MODE_HW offload hook for knod ---
commit 2719ee9dd236296c86e09fa87d5113250098ff78
Author: Taehee Yoo <ap420073@gmail.com>

net: core: add XDP_MODE_HW offload hook for knod

Route XDP program install/uninstall to the knod accelerator when a
device is attached, so BPF programs run on the accelerator instead of
the host.

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 7c21bc0a1e349..5ba1c969029bc 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -10334,7 +10334,7 @@ static int dev_xdp_install(struct net_device *dev, enum bpf_xdp_mode mode,
>  		return -EBUSY;
>  	}
>  
> -	if (dev_get_min_mp_channel_count(dev)) {
> +	if (dev_get_min_mp_channel_count(dev) && mode != XDP_MODE_HW) {

[Severity: High]
This is a pre-existing issue, but blocking XDP uninstalls when a memory
provider is active appears to lead to a resource leak and an inconsistent
state on slave devices.

If we attach an XDP program to a bonding device, it propagates to slave
devices without updating their individual dev->xdp_state. If we then bind a
memory provider to the slave device (which succeeds since its local prog count
is 0), what happens when we try to destroy the bond device?

netif_xdp_propagate() will fail because of the memory provider:

net/core/dev.c:netif_xdp_propagate() {
    ...
	if (dev_get_min_mp_channel_count(dev)) {
		NL_SET_ERR_MSG(bpf->extack, "unable to propagate XDP...");
		return -EBUSY;
	}
    ...
}

In dev_xdp_uninstall(), this failure is ignored:

net/core/dev.c:dev_xdp_uninstall() {
    ...
		WARN_ON(dev_xdp_install(dev, mode, bpf_op, NULL, 0, NULL));

		/* auto-detach link from net device */
		link = dev_xdp_link(dev, mode);
		if (link)
			link->dev = NULL;
		else
			bpf_prog_put(prog);
    ...
}

Doesn't this result in the bond dropping its reference and clearing its state,
while the slave driver never unloads the program because ndo_bpf is not
called? Will this leave a zombie XDP program permanently executing on the
slave device and leak the BPF program memory?


[Severity: Medium]
Does modifying this check create an order-dependent state regression?

If a memory provider is attached first, this modified check allows an
XDP_MODE_HW program to be installed successfully.

However, if an XDP_MODE_HW program is installed first, a subsequent attempt
to bind a memory provider fails in __netif_mp_open_rxq() because it
unconditionally checks the program count:

net/core/netdev_rx_queue.c:__netif_mp_open_rxq() {
    ...
	if (dev_xdp_prog_count(dev)) {
		NL_SET_ERR_MSG(extack, "unable to custom memory provider...");
		return -EEXIST;
	}
    ...
}

Should __netif_mp_open_rxq() also check if the existing programs are hardware
offloaded to ensure the API behaves consistently regardless of the operation
order?

>  		NL_SET_ERR_MSG(extack, "unable to install XDP to device using memory provider");
>  		return -EBUSY;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719175857.4071636-1-ap420073@gmail.com?part=3