This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
https://netdev-ai.bots.linux.dev/sashiko/
---
ixgbe: Refactor device operations to check whether netdev is available
This commit refactors several ixgbe driver operations to check whether
the netdev they operate on is enabled by introducing an ixgbe_netif_running()
helper and adding netif_device_present() guards across the ethtool, DCB,
FCoE and XDP code paths.
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 8873a8cc4a18..12361ed58a0f 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
[ ... ]
> @@ -10954,8 +10958,10 @@ static int ixgbe_xdp_setup(struct net_device *dev,
> struct bpf_prog *prog)
> synchronize_rcu();
> err = ixgbe_setup_tc(dev, adapter->hw_tcs);
>
> - if (err)
> + if (err) {
> + xchg(&adapter->xdp_prog, old_prog);
> return -EINVAL;
> + }
> if (!prog)
> xdp_features_clear_redirect_target(dev);
> } else {
[Medium]
The xchg(&adapter->xdp_prog, old_prog) rollback on the ixgbe_setup_tc()
error path looks like a semantic fix rather than a refactor.
Before this patch the error path left adapter->xdp_prog pointing at prog:
old_prog = xchg(&adapter->xdp_prog, prog);
...
err = ixgbe_setup_tc(dev, adapter->hw_tcs);
if (err)
return -EINVAL;
while the ndo_bpf caller dev_xdp_install() drops its reference on prog
when the driver returns an error:
if (err) {
if (prog)
bpf_prog_put(prog);
return err;
}
Can this leave adapter->xdp_prog as a dangling pointer to a freed
bpf_prog on the pre-patch code, which is what the new xchg() restores?
The commit message describes only a refactor and does not mention this
change.
Should the xdp_prog rollback be split into its own patch with a Fixes:
tag pointing at the commit that introduced the missing rollback, per
Documentation/process/submitting-patches.rst which asks for one logical
change per patch?
The new behavior also skips the subsequent if (old_prog) bpf_prog_put(old_prog)
in the failure path, which is a separate semantic worth calling out in the
commit log.
