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

[Temerkhanov, Sergey]

> -----Original Message-----
> From: Simon Horman <horms at kernel.org>
> Sent: Thursday, July 16, 2026 3:03 PM
> To: Temerkhanov, Sergey <sergey.temerkhanov at intel.com>
> Cc: Simon Horman <horms at kernel.org>; intel-wired-lan at lists.osuosl.org;
> netdev at vger.kernel.org
> Subject: Re: [PATCH iwl-next v1 1/2] ixgbe: Refactor device operations to
> check whether netdev is available
> 
> 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.

This part is going to a separate patch in v2

Regards,
Sergey