RE: [PATCH net-next v5 04/11] net: ngbe: improve the reset flow

["Jiawen Wu" <jiawenwu@trustnetic.com>]

On Thu, Mar 19, 2026 7:26 PM, Paolo Abeni wrote:
> On 3/17/26 8:38 AM, Jiawen Wu wrote:
> > @@ -383,6 +384,12 @@ static void ngbe_disable_device(struct wx *wx)
> >
> >  static void ngbe_reset(struct wx *wx)
> >  {
> > +	int err;
> > +
> > +	err = ngbe_reset_hw(wx);
> > +	if (err)
> > +		wx_err(wx, "Hardware Error: %d\n", err);
> 
> AI review says:
> 
> """
> Andrew Lunn suggested in v1 review that ngbe_reset() should return int
> to propagate error codes to callers, rather than just logging errors
> and continuing. While the response mentioned hardware errors are
> handled in another work queue, should the callers (ngbe_down,
> ngbe_setup_tc, ngbe_do_reset, ngbe_resume) be able to detect and
> handle reset failures?
> 
> Reference:
> https://lore.kernel.org/netdev/4ddc6f7d-ee6f-48a9-857f-a10448815675@lunn.ch/
> """
> 
> There are a few problems with the workqueue approach:
> - AFAICS the workqueue is scheduled on xmit timeouts, which are
> apparenly not directly related to ngbe_reset_hw() failures
> - the worker try to reset again the H/W and unconditionally clears the
> error status, even if such reset fails.
> - ngbe_reset_hw() is supposed to stop interrupt generation; if it fails,
> it looks like that the sequence:
> 
> 	ngbe_down(wx);
> 		// ngbe_reset()
> 	ngbe_up()
> 
> can have very bad consequences (like ring reconf with IRQ enabled).
> 
> I think explicitly propagating and handling the failure here is needed.

To be clear, when ngbe_reset_hw() fails, the hardware is already in a PCIe
error state. In patch set v1, this error will be detected in watchdog, and
do PCIe recovery. But it was removed in v2, so the sequence seems weird.

In the current flow, I'll fix it to return int.