Re: [PATCH iwl-next v1 1/2] ixgbe: Refactor device operations to check whether netdev is available

[Simon Horman <horms@kernel.org>]

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.