[PATCH net v3] net: libwx: fix PM suspend/resume flow for VF drivers

Rework wxvf_suspend() and wxvf_resume() to handle interface lifecycle
transitions safely under rtnl_lock() and eliminate resource races.

In wxvf_suspend(), if the netdevice is running, invoke wxvf_close() to
quiesce DMA engines, disable NAPI, and release IRQs/rings cleanly prior
to clearing PCI bus mastering and disabling the device.

In wxvf_resume(), avoid partial hardware re-initialization on active
queues. If the device was running before suspend, defer complete queue
and interrupt setup to a new reset subtask flag WX_FLAG_NEED_REOPEN,
which safely invokes wxvf_open() under rtnl_lock(). Provide error
unwinding inside wxvf_reopen_subtask() if re-opening fails.

Fixes: 377d180bd71c ("net: wangxun: add txgbevf build")
Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>
---
Changelogs:
v3:
- Fix DMA-after-free and UAF races in wxvf_suspend() by invoking
  wxvf_close() when running, ensuring DMA engines are stopped and
  NAPI/IRQs/rings are cleanly freed before clearing PCI state.
- Replace manual resource allocation in wxvf_resume() with deferred
  reopening via WX_FLAG_NEED_REOPEN, scheduling wxvf_reopen_subtask() to
  run full wxvf_open() sequence under rtnl_lock().
- Move timer/work synchronization after rtnl_lock() release in
  wxvf_suspend() to prevent service tasks from re-arming the timer.
- Add error unwinding in wxvf_reopen_subtask() to set WX_STATE_DOWN,
  clear bus mastering, and log errors if reopen fails.
- Skip wxvf_link_config_subtask() when WX_STATE_DOWN is set to prevent
  link updates on downed interfaces.
v2: https://lore.kernel.org/netdev/20260829091423.83097-1-mengyuanlou@net-swift.com/
- Refactored the suspend and resume logic to eliminate full netdevice close/open
  cycles in favor of lightweight interrupt and queue manipulation:
  * In wxvf_suspend(), replaced wxvf_close() with explicit queue stopping
    (netif_tx_disable), carrier drop, NAPI disabling (wx_napi_disable_all), IRQ
    releasing (wx_free_irq), and resource freeing (wx_free_resources).
  * In wxvf_resume(), replaced wxvf_open() with granular resource allocation
    (wx_setup_resources), MSI-X IRQ requesting (wx_request_msix_irqs_vf), and deferred
    hardware reconfiguration via WX_FLAG_NEED_DO_RESET flag.
- Dropped the addition of device_link_add() to parent PF in ngbevf and txgbevf probe()
  paths to keep the patch focused strictly on libwx PM suspend/resume flow.
v1: https://lore.kernel.org/netdev/20260826095243.16939-1-mengyuanlou@net-swift.com/
---
 drivers/net/ethernet/wangxun/libwx/wx_type.h  |  1 +
 .../net/ethernet/wangxun/libwx/wx_vf_common.c | 57 ++++++++++++++++++-
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
index 2eba5ab59925..1b18b069decc 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h

@@ -1288,6 +1288,7 @@ enum wx_pf_flags {
 	WX_FLAG_NEED_MODULE_RESET,
 	WX_FLAG_NEED_UPDATE_LINK,
 	WX_FLAG_NEED_DO_RESET,
+	WX_FLAG_NEED_REOPEN,
 	WX_FLAG_RX_MERGE_ENABLED,
 	WX_FLAG_TXHEAD_WB_ENABLED,
 	WX_FLAG_NEED_PCIE_RECOVERY,

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c b/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c
index 26de78e9a69e..3e55738e2096 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c

@@ -16,9 +16,16 @@ int wxvf_suspend(struct device *dev_d)
 	struct pci_dev *pdev = to_pci_dev(dev_d);
 	struct wx *wx = pci_get_drvdata(pdev);
 
+	rtnl_lock();
 	netif_device_detach(wx->netdev);
+	if (netif_running(wx->netdev))
+		wxvf_close(wx->netdev);
 	wx_clear_interrupt_scheme(wx);
+	pci_clear_master(pdev);
 	pci_disable_device(pdev);
+	rtnl_unlock();
+	timer_delete_sync(&wx->service_timer);
+	cancel_work_sync(&wx->service_task);
 
 	return 0;
 }

@@ -34,11 +41,25 @@ int wxvf_resume(struct device *dev_d)
 {
 	struct pci_dev *pdev = to_pci_dev(dev_d);
 	struct wx *wx = pci_get_drvdata(pdev);
+	int err;
 
-	pci_set_master(pdev);
-	wx_init_interrupt_scheme(wx);
-	netif_device_attach(wx->netdev);
+	err = pci_enable_device_mem(pdev);
+	if (err) {
+		dev_err(&pdev->dev, "Cannot enable PCI device from suspend\n");
+		return err;
+	}
 
+	rtnl_lock();
+	if (netif_running(wx->netdev)) {
+		clear_bit(WX_STATE_DOWN, wx->state);
+		clear_bit(WX_STATE_SERVICE_SCHED, wx->state);
+		mod_timer(&wx->service_timer, jiffies + HZ);
+		set_bit(WX_FLAG_NEED_REOPEN, wx->flags);
+	} else {
+		pci_set_master(pdev);
+		netif_device_attach(wx->netdev);
+	}
+	rtnl_unlock();
 	return 0;
 }
 EXPORT_SYMBOL(wxvf_resume);

@@ -388,6 +409,9 @@ static void wxvf_link_config_subtask(struct wx *wx)
 {
 	struct net_device *netdev = wx->netdev;
 
+	if (test_bit(WX_STATE_DOWN, wx->state))
+		return;
+
 	wxvf_watchdog_update_link(wx);
 	if (wx->link) {
 		if (netif_carrier_ok(netdev))

@@ -403,10 +427,37 @@ static void wxvf_link_config_subtask(struct wx *wx)
 	}
 }
 
+static void wxvf_reopen_subtask(struct wx *wx)
+{
+	if (!test_bit(WX_FLAG_NEED_REOPEN, wx->flags))
+		return;
+
+	rtnl_lock();
+	pci_set_master(wx->pdev);
+	if (wx_init_interrupt_scheme(wx))
+		goto out;
+	if (wxvf_open(wx->netdev))
+		goto out_clear_scheme;
+	clear_bit(WX_FLAG_NEED_REOPEN, wx->flags);
+	netif_device_attach(wx->netdev);
+	rtnl_unlock();
+	return;
+
+out_clear_scheme:
+	wx_clear_interrupt_scheme(wx);
+out:
+	pci_clear_master(wx->pdev);
+	clear_bit(WX_FLAG_NEED_REOPEN, wx->flags);
+	set_bit(WX_STATE_DOWN, wx->state);
+	rtnl_unlock();
+	dev_err(&wx->pdev->dev, "Failed to reopen device\n");
+}
+
 static void wxvf_service_task(struct work_struct *work)
 {
 	struct wx *wx = container_of(work, struct wx, service_task);
 
+	wxvf_reopen_subtask(wx);
 	wxvf_link_config_subtask(wx);
 	wxvf_reset_subtask(wx);
 	wx_service_event_complete(wx);

-- 
2.30.1

On Fri, Sep 4, 2026 at 1:52 AM Mengyuan Lou [off-list ref] wrote:

Is the rtnl_lock() protection needed around pci_clear_master() and
pci_disable_device()?

+       timer_delete_sync(&wx->service_timer);
+       cancel_work_sync(&wx->service_task);

These should probably be moved up before
pci_clear_master/pci_disable_device and after the rtnl_unlock().

By clearing this on the error path, the VF tries once to establish
communication with the PF. What if the PF is not back up by that time?

+       set_bit(WX_STATE_DOWN, wx->state);

And by setting this, the service_task which calls
wxvf_reopen_subtask() won't run again. So essentially on a 1 time
failure of the VF trying to talk to the PF, the VF gives up. I think
it is better for the VF to retry continuously(or at least a maximum
amount of times) to talk to the PF before giving up.

Also, it probably would be better to split up the patch into:
- rtnl_lock() protections
- implement a proper retry mechanism for establishing VF-PF communication.

+       rtnl_unlock();
+       dev_err(&wx->pdev->dev, "Failed to reopen device\n");
+}
+
 static void wxvf_service_task(struct work_struct *work)
 {
        struct wx *wx = container_of(work, struct wx, service_task);

+       wxvf_reopen_subtask(wx);
        wxvf_link_config_subtask(wx);
        wxvf_reset_subtask(wx);
        wx_service_event_complete(wx);
--
2.30.1

2026年9月5日 09:56，Harshitha Ramamurthy [off-list ref] 写道：

On Fri, Sep 4, 2026 at 1:52 AM Mengyuan Lou [off-list ref] wrote:

quoted

Rework wxvf_suspend() and wxvf_resume() to handle interface lifecycle
transitions safely under rtnl_lock() and eliminate resource races.

In wxvf_suspend(), if the netdevice is running, invoke wxvf_close() to
quiesce DMA engines, disable NAPI, and release IRQs/rings cleanly prior
to clearing PCI bus mastering and disabling the device.

In wxvf_resume(), avoid partial hardware re-initialization on active
queues. If the device was running before suspend, defer complete queue
and interrupt setup to a new reset subtask flag WX_FLAG_NEED_REOPEN,
which safely invokes wxvf_open() under rtnl_lock(). Provide error
unwinding inside wxvf_reopen_subtask() if re-opening fails.

Fixes: 377d180bd71c ("net: wangxun: add txgbevf build")
Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>
---
Changelogs:
v3:
- Fix DMA-after-free and UAF races in wxvf_suspend() by invoking
 wxvf_close() when running, ensuring DMA engines are stopped and
 NAPI/IRQs/rings are cleanly freed before clearing PCI state.
- Replace manual resource allocation in wxvf_resume() with deferred
 reopening via WX_FLAG_NEED_REOPEN, scheduling wxvf_reopen_subtask() to
 run full wxvf_open() sequence under rtnl_lock().
- Move timer/work synchronization after rtnl_lock() release in
 wxvf_suspend() to prevent service tasks from re-arming the timer.
- Add error unwinding in wxvf_reopen_subtask() to set WX_STATE_DOWN,
 clear bus mastering, and log errors if reopen fails.
- Skip wxvf_link_config_subtask() when WX_STATE_DOWN is set to prevent
 link updates on downed interfaces.
v2: https://lore.kernel.org/netdev/20260829091423.83097-1-mengyuanlou@net-swift.com/
- Refactored the suspend and resume logic to eliminate full netdevice close/open
 cycles in favor of lightweight interrupt and queue manipulation:
 * In wxvf_suspend(), replaced wxvf_close() with explicit queue stopping
   (netif_tx_disable), carrier drop, NAPI disabling (wx_napi_disable_all), IRQ
   releasing (wx_free_irq), and resource freeing (wx_free_resources).
 * In wxvf_resume(), replaced wxvf_open() with granular resource allocation
   (wx_setup_resources), MSI-X IRQ requesting (wx_request_msix_irqs_vf), and deferred
   hardware reconfiguration via WX_FLAG_NEED_DO_RESET flag.
- Dropped the addition of device_link_add() to parent PF in ngbevf and txgbevf probe()
 paths to keep the patch focused strictly on libwx PM suspend/resume flow.
v1: https://lore.kernel.org/netdev/20260826095243.16939-1-mengyuanlou@net-swift.com/
---
drivers/net/ethernet/wangxun/libwx/wx_type.h  |  1 +
.../net/ethernet/wangxun/libwx/wx_vf_common.c | 57 ++++++++++++++++++-
2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
index 2eba5ab59925..1b18b069decc 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h

@@ -1288,6 +1288,7 @@ enum wx_pf_flags {
       WX_FLAG_NEED_MODULE_RESET,
       WX_FLAG_NEED_UPDATE_LINK,
       WX_FLAG_NEED_DO_RESET,
+       WX_FLAG_NEED_REOPEN,
       WX_FLAG_RX_MERGE_ENABLED,
       WX_FLAG_TXHEAD_WB_ENABLED,
       WX_FLAG_NEED_PCIE_RECOVERY,

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c b/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c
index 26de78e9a69e..3e55738e2096 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c

@@ -16,9 +16,16 @@ int wxvf_suspend(struct device *dev_d)
       struct pci_dev *pdev = to_pci_dev(dev_d);
       struct wx *wx = pci_get_drvdata(pdev);

+       rtnl_lock();
       netif_device_detach(wx->netdev);
+       if (netif_running(wx->netdev))
+               wxvf_close(wx->netdev);
       wx_clear_interrupt_scheme(wx);
+       pci_clear_master(pdev);
       pci_disable_device(pdev);
+       rtnl_unlock();

Is the rtnl_lock() protection needed around pci_clear_master() and
pci_disable_device()?

quoted

+       timer_delete_sync(&wx->service_timer);
+       cancel_work_sync(&wx->service_task);

These should probably be moved up before
pci_clear_master/pci_disable_device and after the rtnl_unlock().

ok

quoted

       return 0;
}

@@ -34,11 +41,25 @@ int wxvf_resume(struct device *dev_d)

{
       struct pci_dev *pdev = to_pci_dev(dev_d);
       struct wx *wx = pci_get_drvdata(pdev);
+       int err;

-       pci_set_master(pdev);
-       wx_init_interrupt_scheme(wx);
-       netif_device_attach(wx->netdev);
+       err = pci_enable_device_mem(pdev);
+       if (err) {
+               dev_err(&pdev->dev, "Cannot enable PCI device from suspend\n");
+               return err;
+       }

+       rtnl_lock();
+       if (netif_running(wx->netdev)) {
+               clear_bit(WX_STATE_DOWN, wx->state);
+               clear_bit(WX_STATE_SERVICE_SCHED, wx->state);
+               mod_timer(&wx->service_timer, jiffies + HZ);
+               set_bit(WX_FLAG_NEED_REOPEN, wx->flags);
+       } else {
+               pci_set_master(pdev);
+               netif_device_attach(wx->netdev);
+       }
+       rtnl_unlock();
       return 0;
}
EXPORT_SYMBOL(wxvf_resume);

@@ -388,6 +409,9 @@ static void wxvf_link_config_subtask(struct wx *wx)

{
       struct net_device *netdev = wx->netdev;

+       if (test_bit(WX_STATE_DOWN, wx->state))
+               return;
+
       wxvf_watchdog_update_link(wx);
       if (wx->link) {
               if (netif_carrier_ok(netdev))

@@ -403,10 +427,37 @@ static void wxvf_link_config_subtask(struct wx *wx)
       }

}

+static void wxvf_reopen_subtask(struct wx *wx)
+{
+       if (!test_bit(WX_FLAG_NEED_REOPEN, wx->flags))
+               return;
+
+       rtnl_lock();
+       pci_set_master(wx->pdev);
+       if (wx_init_interrupt_scheme(wx))
+               goto out;
+       if (wxvf_open(wx->netdev))
+               goto out_clear_scheme;
+       clear_bit(WX_FLAG_NEED_REOPEN, wx->flags);
+       netif_device_attach(wx->netdev);
+       rtnl_unlock();
+       return;
+
+out_clear_scheme:
+       wx_clear_interrupt_scheme(wx);
+out:
+       pci_clear_master(wx->pdev);
+       clear_bit(WX_FLAG_NEED_REOPEN, wx->flags);

By clearing this on the error path, the VF tries once to establish
communication with the PF. What if the PF is not back up by that time?

quoted

+       set_bit(WX_STATE_DOWN, wx->state);

And by setting this, the service_task which calls
wxvf_reopen_subtask() won't run again. So essentially on a 1 time
failure of the VF trying to talk to the PF, the VF gives up. I think
it is better for the VF to retry continuously(or at least a maximum
amount of times) to talk to the PF before giving up.

Also, it probably would be better to split up the patch into:
- rtnl_lock() protections
- implement a proper retry mechanism for establishing VF-PF communication.

Error handling is indeed very important and requires careful consideration.
Thanks.

quoted

+       rtnl_unlock();
+       dev_err(&wx->pdev->dev, "Failed to reopen device\n");
+}
+
static void wxvf_service_task(struct work_struct *work)
{
       struct wx *wx = container_of(work, struct wx, service_task);

+       wxvf_reopen_subtask(wx);
       wxvf_link_config_subtask(wx);
       wxvf_reset_subtask(wx);
       wx_service_event_complete(wx);
--
2.30.1