[PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths

[PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths

[Zhang Yunfei]

Two error-handling fixes for the ngbe PM/open paths.

ngbe_resume() declared err as u32 and returned 0 unconditionally, so a
failed ngbe_reset_hw(), wx_init_interrupt_scheme() or ngbe_open() left
the device in netif_device_detach() state with a broken interrupt
scheme while the PM core was told the resume succeeded; the reset task
bails out on the missing netif_device_present() check, so the device
cannot self-heal. Patch 1 fixes the type and propagates all of these
errors, making the whole tail of the resume path consistent with the
pci_enable_device_mem() failure path at the top, which already reports
its error.

ngbe_open() sets the WX_CFG_PORT_CTL_DRV_LOAD bit to tell the
management firmware the host has taken over the port, but no error
path cleared it, leaving the firmware owning a port whose rings and
IRQs are gone. Patch 2 rolls the bit back on all open error paths,
matching ngbe_close() and ngbe_dev_shutdown().

---
Changes in v3:
- patch 1: set WX_STATE_RES_FREED on every failing return of
  ngbe_resume() (the pci_enable_device_mem() failure, the hardware
  reset failure early return and the wx_init_interrupt_scheme()/
  ngbe_open() failures), so that a later ngbe_close() (ndo_stop or
  unregister_netdev()) skips re-running the teardown on the
  already-freed post-suspend state instead of freeing IRQs that are no
  longer requested (Sashiko review of v2);
- patch 2: correct the Fixes tag to a1cf597b99a7, the commit that
  introduced the bug (the first ngbe_open() failure path after the
  DRV_LOAD bit is set; v2 pointed at e7956139a6cf, which added more
  failing returns but not the first one); no code change.

Changes in v2:
- also propagate the ngbe_reset_hw() failure, so the whole tail of
  ngbe_resume() reports errors to the PM core (Sashiko review of v1);
- drop the inaccurate "device can be re-probed" claim: the PM core
  records and logs the failure, there is no re-probe (Sashiko review
  of v1);
- patch 2/2 unchanged.

Link: https://lore.kernel.org/netdev/20260922100836.1147718-1-zhangyunfei1@kylinos.cn/T/#u/


Zhang Yunfei (2):
  net: ngbe: propagate resume errors to the PM core
  net: ngbe: clear DRV_LOAD bit when ngbe_open() fails

 drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)


base-commit: 93f51579e7df248780214094418f205253383cc5
-- 
2.25.1

[PATCH net v3 1/2] net: ngbe: propagate resume errors to the PM core

[Zhang Yunfei]

After a failed resume from suspend, the device stays detached from
the networking stack: every attempt to bring the interface up fails
the netif_device_present() check in __dev_open() with -ENODEV,
while the PM core is told that resume succeeded.

ngbe_resume() declares err as u32 and unconditionally returns 0:
the return value of ngbe_reset_hw() is ignored entirely, and
failures of wx_init_interrupt_scheme() and ngbe_open() are silently
swallowed. The interrupt scheme torn down at suspend is never
rebuilt, so the device cannot self-heal.

Fix the type to int and propagate the errors, making the whole tail
of the resume path consistent with the pci_enable_device_mem()
failure path at the top. If the hardware reset fails, return early:
the remaining resume steps cannot succeed. The suspend path may
already have torn the interface down (ngbe_close() and
wx_clear_interrupt_scheme()), leaving freed rings and IRQs behind
while netif_running() still reports true, so set WX_STATE_RES_FREED
on every failing return, the same mark ngbe_down_suspend() uses for
the PCI error recovery path: a later ngbe_close() skips the
teardown of the already-freed state, and ngbe_up_complete() clears
the bit again so later opens are unaffected. This is safe on the
wx_init_interrupt_scheme() failure path too, as it cleans up after
itself.

Found by manual code inspection of the PM error paths; the missing
ngbe_reset_hw() propagation was reported by Sashiko in its review
of v1. The failure paths are unreachable without fault injection:
a loadable test module injects failures into ngbe_reset_hw(),
wx_init_interrupt_scheme() and ngbe_open(). In a QEMU VM the
unfixed driver hits the kernel "Trying to free already-free IRQ"
warning on the post-suspend close; with the fix, the error is
reported and no warning appears. No physical ngbe device is
involved.

Fixes: 6963e463256e ("net: ngbe: add Wake on Lan support")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260917090050.1927999-1-zhangyunfei1%40kylinos.cn
Cc: stable@vger.kernel.org
Signed-off-by: Zhang Yunfei <zhangyunfei1@kylinos.cn>
---
Changes in v3:
- set WX_STATE_RES_FREED on every failing return of ngbe_resume()
  (the pci_enable_device_mem() failure, the reset failure early return
  and the wx_init_interrupt_scheme()/ngbe_open() failures), so that a
  later ngbe_close() skips re-running the teardown on the already-freed
  post-suspend state (Sashiko review of v2);

Changes in v2:
- also propagate the ngbe_reset_hw() failure, so the whole tail of
  ngbe_resume() reports errors to the PM core;
- drop the inaccurate "device can be re-probed" claim: the PM core
  records and logs the failure, there is no re-probe.

 drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
index 855dc963c610..8247f6c14be0 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
@@ -954,13 +954,14 @@ static int ngbe_resume(struct pci_dev *pdev)
 {
 	struct net_device *netdev;
 	struct wx *wx;
-	u32 err;
+	int err;
 
 	wx = pci_get_drvdata(pdev);
 	netdev = wx->netdev;
 
 	err = pci_enable_device_mem(pdev);
 	if (err) {
+		set_bit(WX_STATE_RES_FREED, wx->state);
 		wx_err(wx, "Cannot enable PCI device from suspend\n");
 		return err;
 	}
@@ -968,16 +969,23 @@ static int ngbe_resume(struct pci_dev *pdev)
 	pci_set_master(pdev);
 	device_wakeup_disable(&pdev->dev);
 
-	ngbe_reset_hw(wx);
+	err = ngbe_reset_hw(wx);
+	if (err) {
+		set_bit(WX_STATE_RES_FREED, wx->state);
+		wx_err(wx, "Hardware reset failed: %d\n", err);
+		return err;
+	}
 	rtnl_lock();
 	err = wx_init_interrupt_scheme(wx);
 	if (!err && netif_running(netdev))
 		err = ngbe_open(netdev);
 	if (!err)
 		netif_device_attach(netdev);
+	else
+		set_bit(WX_STATE_RES_FREED, wx->state);
 	rtnl_unlock();
 
-	return 0;
+	return err;
 }
 
 static struct pci_driver ngbe_driver = {
-- 
2.25.1

[PATCH net v3 2/2] net: ngbe: clear DRV_LOAD bit when ngbe_open() fails

[Zhang Yunfei]

On NCSI-managed systems, a failed ifup - for example when IRQ or
ring allocation fails under memory pressure - leaves the port
without either host or firmware driving it: the firmware already
handed the port over on open, so out-of-band management of the NIC
through that port stops until the next successful ifup.

ngbe_open() sets the WX_CFG_PORT_CTL_DRV_LOAD bit to tell the
management firmware the host has taken over the port, but every
error path returns without clearing it, leaving rings, IRQs and the
PHY torn down while the firmware still believes the host owns the
port.

Roll the bit back on all open error paths, matching ngbe_close()
and ngbe_dev_shutdown(), so a failed ifup leaves the same
firmware-visible state as if the interface had never been opened.

Found by manual code inspection of the open error paths. The
deterministic reproduction uses a loadable test module injecting a
wx_setup_resources() failure: in a QEMU VM the unfixed driver
leaves DRV_LOAD set after a failed open, and with the fix the bit
is cleared. No physical ngbe device is involved.

Fixes: a1cf597b99a7 ("net: ngbe: Add ngbe mdio bus driver.")
Cc: stable@vger.kernel.org
Signed-off-by: Zhang Yunfei <zhangyunfei1@kylinos.cn>
---
Changes in v3:
- correct the Fixes tag to a1cf597b99a7, the commit that introduced
  the bug (the first ngbe_open() failure path after the DRV_LOAD bit
  is set; v2 pointed at e7956139a6cf, which added more failing returns
  but not the first one); no code change.

 drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
index 8247f6c14be0..0aea5a99a1e2 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
@@ -494,7 +494,7 @@ static int ngbe_open(struct net_device *netdev)
 
 	err = wx_setup_resources(wx);
 	if (err)
-		return err;
+		goto err_control_hw;
 
 	wx_configure(wx);
 
@@ -526,6 +526,8 @@ static int ngbe_open(struct net_device *netdev)
 err_free_resources:
 	wx_free_isb_resources(wx);
 	wx_free_resources(wx);
+err_control_hw:
+	wx_control_hw(wx, false);
 	return err;
 }
 
-- 
2.25.1

Re: [PATCH net v3 2/2] net: ngbe: clear DRV_LOAD bit when ngbe_open() fails

[Joe Damato]

On Wed, Sep 30, 2026 at 05:47:48PM +0800, Zhang Yunfei wrote:
> On NCSI-managed systems, a failed ifup - for example when IRQ or
> ring allocation fails under memory pressure - leaves the port
> without either host or firmware driving it: the firmware already
> handed the port over on open, so out-of-band management of the NIC
> through that port stops until the next successful ifup.
> 
> ngbe_open() sets the WX_CFG_PORT_CTL_DRV_LOAD bit to tell the
> management firmware the host has taken over the port, but every
> error path returns without clearing it, leaving rings, IRQs and the
> PHY torn down while the firmware still believes the host owns the
> port.
> 
> Roll the bit back on all open error paths, matching ngbe_close()
> and ngbe_dev_shutdown(), so a failed ifup leaves the same
> firmware-visible state as if the interface had never been opened.
> 
> Found by manual code inspection of the open error paths. The
> deterministic reproduction uses a loadable test module injecting a
> wx_setup_resources() failure: in a QEMU VM the unfixed driver
> leaves DRV_LOAD set after a failed open, and with the fix the bit
> is cleared. No physical ngbe device is involved.
> 
> Fixes: a1cf597b99a7 ("net: ngbe: Add ngbe mdio bus driver.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhang Yunfei <zhangyunfei1@kylinos.cn>
> ---
> Changes in v3:
> - correct the Fixes tag to a1cf597b99a7, the commit that introduced
>   the bug (the first ngbe_open() failure path after the DRV_LOAD bit
>   is set; v2 pointed at e7956139a6cf, which added more failing returns
>   but not the first one); no code change.
> 
>  drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

idk much about these devices but the reasoning and code looks right to me
after reading it so:

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net v3 1/2] net: ngbe: propagate resume errors to the PM core

[Joe Damato]

On Wed, Sep 30, 2026 at 05:47:47PM +0800, Zhang Yunfei wrote:
> After a failed resume from suspend, the device stays detached from
> the networking stack: every attempt to bring the interface up fails
> the netif_device_present() check in __dev_open() with -ENODEV,
> while the PM core is told that resume succeeded.
> 
> ngbe_resume() declares err as u32 and unconditionally returns 0:
> the return value of ngbe_reset_hw() is ignored entirely, and
> failures of wx_init_interrupt_scheme() and ngbe_open() are silently
> swallowed. The interrupt scheme torn down at suspend is never
> rebuilt, so the device cannot self-heal.
> 
> Fix the type to int and propagate the errors, making the whole tail
> of the resume path consistent with the pci_enable_device_mem()
> failure path at the top. If the hardware reset fails, return early:
> the remaining resume steps cannot succeed. The suspend path may
> already have torn the interface down (ngbe_close() and
> wx_clear_interrupt_scheme()), leaving freed rings and IRQs behind
> while netif_running() still reports true, so set WX_STATE_RES_FREED
> on every failing return, the same mark ngbe_down_suspend() uses for
> the PCI error recovery path: a later ngbe_close() skips the
> teardown of the already-freed state, and ngbe_up_complete() clears
> the bit again so later opens are unaffected. This is safe on the
> wx_init_interrupt_scheme() failure path too, as it cleans up after
> itself.
> 
> Found by manual code inspection of the PM error paths; the missing
> ngbe_reset_hw() propagation was reported by Sashiko in its review
> of v1. The failure paths are unreachable without fault injection:
> a loadable test module injects failures into ngbe_reset_hw(),
> wx_init_interrupt_scheme() and ngbe_open(). In a QEMU VM the
> unfixed driver hits the kernel "Trying to free already-free IRQ"
> warning on the post-suspend close; with the fix, the error is
> reported and no warning appears. No physical ngbe device is
> involved.
> 
> Fixes: 6963e463256e ("net: ngbe: add Wake on Lan support")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260917090050.1927999-1-zhangyunfei1%40kylinos.cn
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhang Yunfei <zhangyunfei1@kylinos.cn>
> ---
> Changes in v3:
> - set WX_STATE_RES_FREED on every failing return of ngbe_resume()
>   (the pci_enable_device_mem() failure, the reset failure early return
>   and the wx_init_interrupt_scheme()/ngbe_open() failures), so that a
>   later ngbe_close() skips re-running the teardown on the already-freed
>   post-suspend state (Sashiko review of v2);
> 
> Changes in v2:
> - also propagate the ngbe_reset_hw() failure, so the whole tail of
>   ngbe_resume() reports errors to the PM core;
> - drop the inaccurate "device can be re-probed" claim: the PM core
>   records and logs the failure, there is no re-probe.
> 
>  drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)

propagating the error through seems sensible, so:

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths

[Joe Damato]

On Wed, Sep 30, 2026 at 05:47:46PM +0800, Zhang Yunfei wrote:
> Two error-handling fixes for the ngbe PM/open paths.

[...]

whole series feels very AI-y but from my reading of the proposed patches they
seem reasonable, hence my tags.

idk if these failure modes are real enough to be fixes or not though.