igb_xdp_setup() calls igb_close() + igb_open() when transitioning
between XDP and non-XDP mode on a running device. This has two issues:
1. ndo_xsk_wakeup() runs under rcu_read_lock() and may still access
the rings while igb_xdp_setup() removes the XDP program. Without
waiting for an RCU grace period, igb_close() can tear down the
rings while ndo_xsk_wakeup() is still executing. Add
synchronize_rcu() before igb_close() when removing an XDP program
to ensure all in-flight RCU readers complete first.
2. The igb_close()/igb_open() window leaves trans_start stale from
before the close: the TX watchdog can fire a spurious timeout and
queue a reset_task that races with igb_open(). Add
netif_trans_update() after igb_open() to refresh the timestamp, and
cancel_work() to cancel any reset_task that may have been queued
while the device was down.
Note: cancel_work_sync() cannot be used here because igb_reset_task()
takes rtnl_lock, which is already held by the ndo_bpf caller. Plain
cancel_work() is sufficient: if reset_task is already running, it blocks
on rtnl_lock and will check __IGB_DOWN when it acquires it.
Fixes: 9cbc948b5a20 ("igb: add XDP support")
Cc: [email protected]
Signed-off-by: Alex Dvoretsky <[email protected]>
---
drivers/net/ethernet/intel/igb/igb_main.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
b/drivers/net/ethernet/intel/igb/igb_main.c
index ddb7ce9e97bf..9ba944bf67b4 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2913,6 +2913,9 @@ static int igb_xdp_setup(struct net_device *dev, struct
netdev_bpf *bpf)
/* device is up and bpf is added/removed, must setup the RX queues */
if (need_reset && running) {
+ if (!prog)
+ /* Wait for RCU readers (e.g. ndo_xsk_wakeup). */
+ synchronize_rcu();
igb_close(dev);
} else {
for (i = 0; i < adapter->num_rx_queues; i++)
@@ -2936,6 +2939,14 @@ static int igb_xdp_setup(struct net_device *dev, struct
netdev_bpf *bpf)
if (running)
igb_open(dev);
+ /* Refresh watchdog timestamp after reopen and cancel any
+ * reset task queued while the device was down.
+ */
+ if (need_reset && running) {
+ netif_trans_update(dev);
+ cancel_work(&adapter->reset_task);
+ }
+
return 0;
}
--
2.51.0
