* [PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths
@ 2026-09-30 9:47 Zhang Yunfei
2026-09-30 9:47 ` [PATCH net v3 1/2] net: ngbe: propagate resume errors to the PM core Zhang Yunfei
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Zhang Yunfei @ 2026-09-30 9:47 UTC (permalink / raw)
To: netdev
Cc: jiawenwu, mengyuanlou, andrew+netdev, davem, edumazet, kuba,
pabeni, aleksandr.loktionov, u.kleine-koenig, weirongguang,
zhangyunfei1, linux-kernel, stable, leitao
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
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net v3 1/2] net: ngbe: propagate resume errors to the PM core 2026-09-30 9:47 [PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths Zhang Yunfei @ 2026-09-30 9:47 ` Zhang Yunfei 2026-09-30 20:00 ` Joe Damato 2026-10-04 10:20 ` netdev-bot+sashiko 2026-09-30 9:47 ` [PATCH net v3 2/2] net: ngbe: clear DRV_LOAD bit when ngbe_open() fails Zhang Yunfei ` (2 subsequent siblings) 3 siblings, 2 replies; 8+ messages in thread From: Zhang Yunfei @ 2026-09-30 9:47 UTC (permalink / raw) To: netdev Cc: jiawenwu, mengyuanlou, andrew+netdev, davem, edumazet, kuba, pabeni, aleksandr.loktionov, u.kleine-koenig, weirongguang, zhangyunfei1, linux-kernel, stable, leitao, Sashiko 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v3 1/2] net: ngbe: propagate resume errors to the PM core 2026-09-30 9:47 ` [PATCH net v3 1/2] net: ngbe: propagate resume errors to the PM core Zhang Yunfei @ 2026-09-30 20:00 ` Joe Damato 2026-10-04 10:20 ` netdev-bot+sashiko 1 sibling, 0 replies; 8+ messages in thread From: Joe Damato @ 2026-09-30 20:00 UTC (permalink / raw) To: Zhang Yunfei Cc: netdev, jiawenwu, mengyuanlou, andrew+netdev, davem, edumazet, kuba, pabeni, aleksandr.loktionov, u.kleine-koenig, weirongguang, linux-kernel, stable, leitao, Sashiko 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> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v3 1/2] net: ngbe: propagate resume errors to the PM core 2026-09-30 9:47 ` [PATCH net v3 1/2] net: ngbe: propagate resume errors to the PM core Zhang Yunfei 2026-09-30 20:00 ` Joe Damato @ 2026-10-04 10:20 ` netdev-bot+sashiko 1 sibling, 0 replies; 8+ messages in thread From: netdev-bot+sashiko @ 2026-10-04 10:20 UTC (permalink / raw) To: zhangyunfei1 Cc: netdev, jiawenwu, mengyuanlou, andrew+netdev, davem, edumazet, kuba, pabeni, aleksandr.loktionov, u.kleine-koenig, weirongguang, linux-kernel, stable, leitao, sashiko-bot Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider. Critical: 0 · High: 0 (2) · Medium: 2 · Low: 0 If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [Medium] The patch is tagged `Cc: stable@vger.kernel.org` and `Fixes: 6963e463256e` ("net: ngbe: add Wake on Lan support", July 2023). - [Medium] The patch marks the post-suspend freed state with `WX_STATE_RES_FREED` only when `ngbe_resume()` fails. Pre-existing issues: - [High] This problem wasn't introduced by this patch, but the shared Wangxun VF resume path has the same defect this patch fixes for ngbe,… - [High] This problem wasn't introduced by this patch, but the patch now relies on it. -- net: ngbe: propagate resume errors to the PM core This patch changes err in ngbe_resume() from u32 to int. It also passes failures from ngbe_reset_hw(), wx_init_interrupt_scheme() and ngbe_open() up to the PM core, instead of always returning 0. On every failing return it sets WX_STATE_RES_FREED, so a later ngbe_close() skips the teardown of state that was already freed at suspend. Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260917090050.1927999-1-zhangyunfei1%40kylinos.cn > diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c > index 855dc963c610c..8247f6c14be0f 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) [ ... ] > err = pci_enable_device_mem(pdev); > if (err) { > + set_bit(WX_STATE_RES_FREED, wx->state); [Severity: Medium] This patch has Fixes: 6963e463256e ("net: ngbe: add Wake on Lan support") and Cc: stable, so it will be picked up for every stable tree from v6.6.y on. Will it build in those trees? WX_STATE_RES_FREED comes from commit e73e4d187a1f52 ("net: wangxun: add pcie error handler"). So does the check in ngbe_close() that this fix depends on: if (test_bit(WX_STATE_RES_FREED, wx->state)) goto out; e73e4d187a1f52 is a feature commit with no Fixes: or stable tag. The commit message doesn't list it as a dependency. In stable trees that have 6963e463256e but not e73e4d187a1f52, WX_STATE_RES_FREED is undeclared. A backporter might drop the set_bit() calls to get it to build. In that case ngbe_close() still has no RES_FREED guard in those trees. The post-suspend "Trying to free already-free IRQ" double teardown described in the commit message would then still happen after a failed resume. Should e73e4d187a1f52 be listed as a stable prerequisite? > 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; > + } [Severity: Medium] The state being marked here is freed at suspend time, not in ngbe_resume(). ngbe_suspend()->ngbe_dev_shutdown() does: rtnl_lock(); netif_device_detach(netdev); if (netif_running(netdev)) ngbe_close(netdev); wx_clear_interrupt_scheme(wx); rtnl_unlock(); It never sets WX_STATE_RES_FREED, and __LINK_STATE_START stays set. Can something else close the netdev between suspend and the point where ngbe_resume() either succeeds or reaches one of these set_bit() calls? dev_close() doesn't check netif_device_present(). Take any RTNL holder, such as netns teardown on the non-freezable netns_wq: default_device_exit_net() dev_change_net_namespace() dev_close() ngbe_close() It would see RES_FREED clear and run ngbe_down() and wx_free_irq() again. MSI-X is already gone at that point, so wx_free_irq() takes this path: if (!(pdev->msix_enabled)) { if (!wx->misc_irq_domain) free_irq(pdev->irq, wx); return; } That gives the same "Trying to free already-free IRQ" warning the commit message describes. The locking is also inconsistent. This set_bit() and the one after the pci_enable_device_mem() failure run without rtnl_lock. ngbe_close() tests the bit under RTNL, and the third set_bit() below is also under RTNL. Would it work to set WX_STATE_RES_FREED in ngbe_dev_shutdown() under RTNL, right after ngbe_close() and wx_clear_interrupt_scheme()? That is what ngbe_down_suspend() does. A successful resume clears the bit again through ngbe_open()->ngbe_up_complete(), and the three set_bit() calls in ngbe_resume() would no longer be needed. > 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); [Severity: High] This is a pre-existing issue, but this patch now depends on it. When ngbe_open() fails here, does anything stop the RX DMA that ngbe_open() has already turned on? Before any step in ngbe_open() can fail, wx_configure() has already armed RX: wx_configure() wx_configure_rx() wx_configure_rx_ring() <- programs WX_PX_RR_BAL/BAH, sets RR_EN, calls wx_enable_rx_queue() and wx_alloc_rx_buffers() wx_enable_rx() <- sets WX_MAC_RX_CFG_RE and WX_RDB_PB_CTL_RXEN If ngbe_request_irq(), phylink_connect_phy() or netif_set_real_num_{tx,rx}_queues() fails afterwards, the unwind is: err_dis_phy: phylink_disconnect_phy(wx->phylink); err_free_irq: wx_free_irq(wx); err_free_resources: wx_free_isb_resources(wx); wx_free_resources(wx); err_control_hw: wx_control_hw(wx, false); return err; wx_free_rx_resources() calls dma_free_coherent() on the descriptor ring and destroys the page pool. Nothing writes the registers that disable the RX queues or the MAC receiver. On this resume path bus mastering is still on, because ngbe_reset_hw() called pci_set_master(). With the new set_bit(), a later ngbe_close() jumps straight to wx_control_hw(false) and skips ngbe_down()->ngbe_disable_device(). That is the only code that stops the queues. Even without the RES_FREED mark, ngbe_disable_device() would return early. wx_init_interrupt_scheme() set WX_STATE_DOWN, and nothing cleared it. If the link is up, for example kept up for Wake-on-LAN, can incoming frames be DMA-written into the freed descriptor memory and freed pages? A plain ndo_open failure reaches the same unwind. The patch in this series "net: ngbe: clear DRV_LOAD bit when ngbe_open() fails" adds wx_control_hw(wx, false) to this unwind. That covers the stale WX_CFG_PORT_CTL_DRV_LOAD bit. It still doesn't disable the RX rings or the MAC receiver, so the DMA part is still open at the end of the series. > rtnl_unlock(); > > - return 0; > + return err; > } [Severity: High] This isn't a bug introduced by this patch, but the shared Wangxun VF resume path has the same problem, and there it can crash. wxvf_resume() in drivers/net/ethernet/wangxun/libwx/wx_vf_common.c is used by both ngbevf and txgbevf: pci_set_master(pdev); wx_init_interrupt_scheme(wx); netif_device_attach(wx->netdev); return 0; If wx_init_interrupt_scheme() fails, the PM core is told resume succeeded and the netdev is attached with no q_vectors. wxvf_suspend() has already freed them: wx_free_q_vector() sets the wx->tx_ring[] and wx->rx_ring[] entries to NULL, and wx_reset_interrupt_capability() sets wx->msix_entry to NULL. A later "ip link set up" then passes the netif_device_present() check and runs wxvf_open(). Does that dereference NULL? If MSI-X allocation failed, the queue counts are left non-zero, so wx_setup_resources() walks NULL tx_ring[] entries. If q_vector allocation failed, wx_request_msix_irqs_vf() runs: err = request_irq(wx->msix_entry->vector, wx_msix_misc_vf, 0, netdev->name, wx); with msix_entry set to NULL. The commit message limits the fix to ngbe. Should wxvf_resume() get the same error handling? -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930094748.1198085-1-zhangyunfei1%40kylinos.cn ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net v3 2/2] net: ngbe: clear DRV_LOAD bit when ngbe_open() fails 2026-09-30 9:47 [PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths Zhang Yunfei 2026-09-30 9:47 ` [PATCH net v3 1/2] net: ngbe: propagate resume errors to the PM core Zhang Yunfei @ 2026-09-30 9:47 ` Zhang Yunfei 2026-09-30 19:58 ` Joe Damato 2026-09-30 20:01 ` [PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths Joe Damato 2026-10-06 0:50 ` patchwork-bot+netdevbpf 3 siblings, 1 reply; 8+ messages in thread From: Zhang Yunfei @ 2026-09-30 9:47 UTC (permalink / raw) To: netdev Cc: jiawenwu, mengyuanlou, andrew+netdev, davem, edumazet, kuba, pabeni, aleksandr.loktionov, u.kleine-koenig, weirongguang, zhangyunfei1, linux-kernel, stable, leitao 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v3 2/2] net: ngbe: clear DRV_LOAD bit when ngbe_open() fails 2026-09-30 9:47 ` [PATCH net v3 2/2] net: ngbe: clear DRV_LOAD bit when ngbe_open() fails Zhang Yunfei @ 2026-09-30 19:58 ` Joe Damato 0 siblings, 0 replies; 8+ messages in thread From: Joe Damato @ 2026-09-30 19:58 UTC (permalink / raw) To: Zhang Yunfei Cc: netdev, jiawenwu, mengyuanlou, andrew+netdev, davem, edumazet, kuba, pabeni, aleksandr.loktionov, u.kleine-koenig, weirongguang, linux-kernel, stable, leitao 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> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths 2026-09-30 9:47 [PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths Zhang Yunfei 2026-09-30 9:47 ` [PATCH net v3 1/2] net: ngbe: propagate resume errors to the PM core Zhang Yunfei 2026-09-30 9:47 ` [PATCH net v3 2/2] net: ngbe: clear DRV_LOAD bit when ngbe_open() fails Zhang Yunfei @ 2026-09-30 20:01 ` Joe Damato 2026-10-06 0:50 ` patchwork-bot+netdevbpf 3 siblings, 0 replies; 8+ messages in thread From: Joe Damato @ 2026-09-30 20:01 UTC (permalink / raw) To: Zhang Yunfei Cc: netdev, jiawenwu, mengyuanlou, andrew+netdev, davem, edumazet, kuba, pabeni, aleksandr.loktionov, u.kleine-koenig, weirongguang, linux-kernel, stable, leitao 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths 2026-09-30 9:47 [PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths Zhang Yunfei ` (2 preceding siblings ...) 2026-09-30 20:01 ` [PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths Joe Damato @ 2026-10-06 0:50 ` patchwork-bot+netdevbpf 3 siblings, 0 replies; 8+ messages in thread From: patchwork-bot+netdevbpf @ 2026-10-06 0:50 UTC (permalink / raw) To: Zhang Yunfei Cc: netdev, jiawenwu, mengyuanlou, andrew+netdev, davem, edumazet, kuba, pabeni, aleksandr.loktionov, u.kleine-koenig, weirongguang, linux-kernel, stable, leitao Hello: This series was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 30 Sep 2026 17:47:46 +0800 you wrote: > 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. > > [...] Here is the summary with links: - [net,v3,1/2] net: ngbe: propagate resume errors to the PM core https://git.kernel.org/netdev/net-next/c/6986ff35f5bb - [net,v3,2/2] net: ngbe: clear DRV_LOAD bit when ngbe_open() fails https://git.kernel.org/netdev/net-next/c/7b36b48049f4 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-06 0:50 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-30 9:47 [PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths Zhang Yunfei 2026-09-30 9:47 ` [PATCH net v3 1/2] net: ngbe: propagate resume errors to the PM core Zhang Yunfei 2026-09-30 20:00 ` Joe Damato 2026-10-04 10:20 ` netdev-bot+sashiko 2026-09-30 9:47 ` [PATCH net v3 2/2] net: ngbe: clear DRV_LOAD bit when ngbe_open() fails Zhang Yunfei 2026-09-30 19:58 ` Joe Damato 2026-09-30 20:01 ` [PATCH net v3 0/2] net: ngbe: fix error handling in resume and open paths Joe Damato 2026-10-06 0:50 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®