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

[PATCH net v2 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 v2:
- also propagate the ngbe_reset_hw() failure, so the whole tail of
  ngbe_resume() reports errors to the PM core (Sashiko review);
- drop the inaccurate "device can be re-probed" claim: the PM core
  records and logs the failure, there is no re-probe (Sashiko review);
- patch 2/2 unchanged.

Link: https://lore.kernel.org/netdev/20260917090050.1927999-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 | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)


base-commit: 93f51579e7df248780214094418f205253383cc5
-- 
2.25.1

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

[Zhang Yunfei]

ngbe_resume() declares err as u32 and unconditionally returns 0, so
failures of wx_init_interrupt_scheme() or ngbe_open() are silently
swallowed, and the return value of ngbe_reset_hw() is ignored
entirely. The device stays in netif_device_detach() state with a
broken interrupt scheme, the PM core is told the resume succeeded,
and the netdev never appears in the networking stack again: the
reset task also bails out early on the missing
netif_device_present() check, so the device cannot self-heal.

Fix the type to int and propagate the errors instead, making the
whole tail of the resume path consistent with the
pci_enable_device_mem() failure path at the top, which already
propagates its error. If the hardware reset fails, the remaining
resume steps cannot succeed, so return early instead of continuing
with a broken device. A failed resume is then reported to the PM
core, which records and logs the failure, instead of being silently
swallowed.

Fixes: 6963e463256e ("net: ngbe: add Wake on Lan support")
Cc: stable@vger.kernel.org
Signed-off-by: Zhang Yunfei <zhangyunfei1@kylinos.cn>
---

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 | 8 +++++---
 1 file changed, 5 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..e8cabcc84a41 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
@@ -954,7 +954,7 @@ 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;
@@ -968,7 +968,9 @@ 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)
+		return err;
 	rtnl_lock();
 	err = wx_init_interrupt_scheme(wx);
 	if (!err && netif_running(netdev))
@@ -977,7 +979,7 @@ static int ngbe_resume(struct pci_dev *pdev)
 		netif_device_attach(netdev);
 	rtnl_unlock();
 
-	return 0;
+	return err;
 }
 
 static struct pci_driver ngbe_driver = {
-- 
2.25.1

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

[Zhang Yunfei]

ngbe_open() sets the WX_CFG_PORT_CTL_DRV_LOAD bit via
wx_control_hw(wx, true) to tell the management firmware that the
host driver has taken over the port (NCSI/OOB firmware stops using
its management channel). Every error path of ngbe_open() returns
without clearing it, leaving rings, IRQs and the PHY torn down
while the firmware still believes the host owns the port, an
inconsistent driver-firmware handshake state that persists until
the next successful ifup.

Roll the bit back on all open error paths, matching ngbe_close()
and ngbe_dev_shutdown(), which already clear it.

Fixes: e7956139a6cf ("net: ngbe: Add irqs request flow")
Cc: stable@vger.kernel.org
Signed-off-by: Zhang Yunfei <zhangyunfei1@kylinos.cn>
---
 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 e8cabcc84a41..7e2cc69fe8f8 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 v2 2/2] net: ngbe: clear DRV_LOAD bit when ngbe_open() fails

[Breno Leitao]

On Tue, Sep 22, 2026 at 06:08:36PM +0800, Zhang Yunfei wrote:
> ngbe_open() sets the WX_CFG_PORT_CTL_DRV_LOAD bit via
> wx_control_hw(wx, true) to tell the management firmware that the
> host driver has taken over the port (NCSI/OOB firmware stops using
> its management channel). Every error path of ngbe_open() returns
> without clearing it, leaving rings, IRQs and the PHY torn down
> while the firmware still believes the host owns the port, an
> inconsistent driver-firmware handshake state that persists until
> the next successful ifup.
> 
> Roll the bit back on all open error paths, matching ngbe_close()
> and ngbe_dev_shutdown(), which already clear it.

silly question, why unwind, rather than not taking hw control until the
port is actually up, as the last thing in ngbe_open()?

Looking at txgbe, it does something like:

	static int txgbe_open(struct net_device *netdev) {
	.....
		txgbe_up_complete(wx);

		return 0;
	}

Would ngbe_open() be able to follow a similar approach?

--breno

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

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Zhang Yunfei <zhangyunfei1@kylinos.cn>
> Sent: Tuesday, September 22, 2026 12:09 PM
> To: netdev@vger.kernel.org
> Cc: jiawenwu@trustnetic.com; mengyuanlou@net-swift.com;
> andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; leitao@debian.org;
> weirongguang@kylinos.cn; u.kleine-koenig@baylibre.com; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: [PATCH net v2 1/2] net: ngbe: propagate resume errors to the
> PM core
> 
> ngbe_resume() declares err as u32 and unconditionally returns 0, so
> failures of wx_init_interrupt_scheme() or ngbe_open() are silently
> swallowed, and the return value of ngbe_reset_hw() is ignored
> entirely. The device stays in netif_device_detach() state with a
> broken interrupt scheme, the PM core is told the resume succeeded, and
> the netdev never appears in the networking stack again: the reset task
> also bails out early on the missing
> netif_device_present() check, so the device cannot self-heal.
> 
> Fix the type to int and propagate the errors instead, making the whole
> tail of the resume path consistent with the
> pci_enable_device_mem() failure path at the top, which already
> propagates its error. If the hardware reset fails, the remaining
> resume steps cannot succeed, so return early instead of continuing
> with a broken device. A failed resume is then reported to the PM core,
> which records and logs the failure, instead of being silently
> swallowed.
> 
> Fixes: 6963e463256e ("net: ngbe: add Wake on Lan support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhang Yunfei <zhangyunfei1@kylinos.cn>
> ---
> 
> 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 | 8 +++++---
>  1 file changed, 5 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..e8cabcc84a41 100644
> --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> @@ -954,7 +954,7 @@ 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;
> @@ -968,7 +968,9 @@ 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)
> +		return err;
>  	rtnl_lock();
>  	err = wx_init_interrupt_scheme(wx);
>  	if (!err && netif_running(netdev))
> @@ -977,7 +979,7 @@ static int ngbe_resume(struct pci_dev *pdev)
>  		netif_device_attach(netdev);
>  	rtnl_unlock();
> 
> -	return 0;
> +	return err;
>  }
> 
>  static struct pci_driver ngbe_driver = {
> --
> 2.25.1


Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

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

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Zhang Yunfei <zhangyunfei1@kylinos.cn>
> Sent: Tuesday, September 22, 2026 12:09 PM
> To: netdev@vger.kernel.org
> Cc: jiawenwu@trustnetic.com; mengyuanlou@net-swift.com;
> andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; leitao@debian.org;
> weirongguang@kylinos.cn; u.kleine-koenig@baylibre.com; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: [PATCH net v2 2/2] net: ngbe: clear DRV_LOAD bit when
> ngbe_open() fails
> 
> ngbe_open() sets the WX_CFG_PORT_CTL_DRV_LOAD bit via
> wx_control_hw(wx, true) to tell the management firmware that the host
> driver has taken over the port (NCSI/OOB firmware stops using its
> management channel). Every error path of ngbe_open() returns without
> clearing it, leaving rings, IRQs and the PHY torn down while the
> firmware still believes the host owns the port, an inconsistent
> driver-firmware handshake state that persists until the next
> successful ifup.
> 
> Roll the bit back on all open error paths, matching ngbe_close() and
> ngbe_dev_shutdown(), which already clear it.
> 
> Fixes: e7956139a6cf ("net: ngbe: Add irqs request flow")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhang Yunfei <zhangyunfei1@kylinos.cn>
> ---
>  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 e8cabcc84a41..7e2cc69fe8f8 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

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

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

[Breno Leitao]

On Tue, Sep 22, 2026 at 06:08:35PM +0800, Zhang Yunfei wrote:
> ngbe_resume() declares err as u32 and unconditionally returns 0, so
> failures of wx_init_interrupt_scheme() or ngbe_open() are silently
> swallowed, and the return value of ngbe_reset_hw() is ignored
> entirely. The device stays in netif_device_detach() state with a
> broken interrupt scheme, the PM core is told the resume succeeded,
> and the netdev never appears in the networking stack again: the
> reset task also bails out early on the missing
> netif_device_present() check, so the device cannot self-heal.
> 
> Fix the type to int and propagate the errors instead, making the
> whole tail of the resume path consistent with the
> pci_enable_device_mem() failure path at the top, which already
> propagates its error. If the hardware reset fails, the remaining
> resume steps cannot succeed, so return early instead of continuing
> with a broken device. A failed resume is then reported to the PM
> core, which records and logs the failure, instead of being silently
> swallowed.
> 
> Fixes: 6963e463256e ("net: ngbe: add Wake on Lan support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhang Yunfei <zhangyunfei1@kylinos.cn>

Reviewed-by: Breno Leitao <leitao@debian.org>