> -----Original Message----- > From: Simon Horman <[email protected]> > Sent: Thursday, July 16, 2026 3:03 PM > To: Temerkhanov, Sergey <[email protected]> > Cc: Simon Horman <[email protected]>; [email protected]; > [email protected] > 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
