Skip to content

Commit 413f027

Browse files
kuba-mookuba-moo
committed
net: protect NAPI enablement with netdev_lock()
Wrap napi_enable() / napi_disable() with netdev_lock(). Provide the "already locked" flavor of the API. iavf needs the usual adjustment. A number of drivers call napi_enable() under a spin lock, so they have to be modified to take netdev_lock() first, then spin lock then call napi_enable_locked(). Protecting napi_enable() implies that napi->napi_id is protected by netdev_lock(). Acked-by: Francois Romieu <[email protected]> # via-velocity Reviewed-by: Eric Dumazet <[email protected]> Reviewed-by: Kuniyuki Iwashima <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 1b23cdb commit 413f027

File tree

6 files changed

+56
-22
lines changed

6 files changed

+56
-22
lines changed

‎drivers/net/ethernet/amd/pcnet32.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ static void pcnet32_netif_start(struct net_device *dev)
462462
val = lp->a->read_csr(ioaddr, CSR3);
463463
val &= 0x00ff;
464464
lp->a->write_csr(ioaddr, CSR3, val);
465-
napi_enable(&lp->napi);
465+
napi_enable_locked(&lp->napi);
466466
}
467467

468468
/*
@@ -889,6 +889,7 @@ static int pcnet32_set_ringparam(struct net_device *dev,
889889
if (netif_running(dev))
890890
pcnet32_netif_stop(dev);
891891

892+
netdev_lock(dev);
892893
spin_lock_irqsave(&lp->lock, flags);
893894
lp->a->write_csr(ioaddr, CSR0, CSR0_STOP); /* stop the chip */
894895

@@ -920,6 +921,7 @@ static int pcnet32_set_ringparam(struct net_device *dev,
920921
}
921922

922923
spin_unlock_irqrestore(&lp->lock, flags);
924+
netdev_unlock(dev);
923925

924926
netif_info(lp, drv, dev, "Ring Param Settings: RX: %d, TX: %d\n",
925927
lp->rx_ring_size, lp->tx_ring_size);
@@ -985,6 +987,7 @@ static int pcnet32_loopback_test(struct net_device *dev, uint64_t * data1)
985987
if (netif_running(dev))
986988
pcnet32_netif_stop(dev);
987989

990+
netdev_lock(dev);
988991
spin_lock_irqsave(&lp->lock, flags);
989992
lp->a->write_csr(ioaddr, CSR0, CSR0_STOP); /* stop the chip */
990993

@@ -1122,6 +1125,7 @@ static int pcnet32_loopback_test(struct net_device *dev, uint64_t * data1)
11221125
lp->a->write_bcr(ioaddr, 20, 4); /* return to 16bit mode */
11231126
}
11241127
spin_unlock_irqrestore(&lp->lock, flags);
1128+
netdev_unlock(dev);
11251129

11261130
return rc;
11271131
} /* end pcnet32_loopback_test */
@@ -2101,6 +2105,7 @@ static int pcnet32_open(struct net_device *dev)
21012105
return -EAGAIN;
21022106
}
21032107

2108+
netdev_lock(dev);
21042109
spin_lock_irqsave(&lp->lock, flags);
21052110
/* Check for a valid station address */
21062111
if (!is_valid_ether_addr(dev->dev_addr)) {
@@ -2266,7 +2271,7 @@ static int pcnet32_open(struct net_device *dev)
22662271
goto err_free_ring;
22672272
}
22682273

2269-
napi_enable(&lp->napi);
2274+
napi_enable_locked(&lp->napi);
22702275

22712276
/* Re-initialize the PCNET32, and start it when done. */
22722277
lp->a->write_csr(ioaddr, 1, (lp->init_dma_addr & 0xffff));
@@ -2300,6 +2305,7 @@ static int pcnet32_open(struct net_device *dev)
23002305
lp->a->read_csr(ioaddr, CSR0));
23012306

23022307
spin_unlock_irqrestore(&lp->lock, flags);
2308+
netdev_unlock(dev);
23032309

23042310
return 0; /* Always succeed */
23052311

@@ -2315,6 +2321,7 @@ static int pcnet32_open(struct net_device *dev)
23152321

23162322
err_free_irq:
23172323
spin_unlock_irqrestore(&lp->lock, flags);
2324+
netdev_unlock(dev);
23182325
free_irq(dev->irq, dev);
23192326
return rc;
23202327
}

‎drivers/net/ethernet/intel/iavf/iavf_main.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,7 +1180,7 @@ static void iavf_napi_enable_all(struct iavf_adapter *adapter)
11801180

11811181
q_vector = &adapter->q_vectors[q_idx];
11821182
napi = &q_vector->napi;
1183-
napi_enable(napi);
1183+
napi_enable_locked(napi);
11841184
}
11851185
}
11861186

@@ -1196,7 +1196,7 @@ static void iavf_napi_disable_all(struct iavf_adapter *adapter)
11961196

11971197
for (q_idx = 0; q_idx < q_vectors; q_idx++) {
11981198
q_vector = &adapter->q_vectors[q_idx];
1199-
napi_disable(&q_vector->napi);
1199+
napi_disable_locked(&q_vector->napi);
12001200
}
12011201
}
12021202

‎drivers/net/ethernet/marvell/mvneta.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4392,6 +4392,7 @@ static int mvneta_cpu_online(unsigned int cpu, struct hlist_node *node)
43924392
if (pp->neta_armada3700)
43934393
return 0;
43944394

4395+
netdev_lock(port->napi.dev);
43954396
spin_lock(&pp->lock);
43964397
/*
43974398
* Configuring the driver for a new CPU while the driver is
@@ -4418,7 +4419,7 @@ static int mvneta_cpu_online(unsigned int cpu, struct hlist_node *node)
44184419

44194420
/* Mask all ethernet port interrupts */
44204421
on_each_cpu(mvneta_percpu_mask_interrupt, pp, true);
4421-
napi_enable(&port->napi);
4422+
napi_enable_locked(&port->napi);
44224423

44234424
/*
44244425
* Enable per-CPU interrupts on the CPU that is
@@ -4439,6 +4440,8 @@ static int mvneta_cpu_online(unsigned int cpu, struct hlist_node *node)
44394440
MVNETA_CAUSE_LINK_CHANGE);
44404441
netif_tx_start_all_queues(pp->dev);
44414442
spin_unlock(&pp->lock);
4443+
netdev_unlock(port->napi.dev);
4444+
44424445
return 0;
44434446
}
44444447

‎drivers/net/ethernet/via/via-velocity.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2320,7 +2320,8 @@ static int velocity_change_mtu(struct net_device *dev, int new_mtu)
23202320
if (ret < 0)
23212321
goto out_free_tmp_vptr_1;
23222322

2323-
napi_disable(&vptr->napi);
2323+
netdev_lock(dev);
2324+
napi_disable_locked(&vptr->napi);
23242325

23252326
spin_lock_irqsave(&vptr->lock, flags);
23262327

@@ -2342,12 +2343,13 @@ static int velocity_change_mtu(struct net_device *dev, int new_mtu)
23422343

23432344
velocity_give_many_rx_descs(vptr);
23442345

2345-
napi_enable(&vptr->napi);
2346+
napi_enable_locked(&vptr->napi);
23462347

23472348
mac_enable_int(vptr->mac_regs);
23482349
netif_start_queue(dev);
23492350

23502351
spin_unlock_irqrestore(&vptr->lock, flags);
2352+
netdev_unlock(dev);
23512353

23522354
velocity_free_rings(tmp_vptr);
23532355

‎include/linux/netdevice.h

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ struct napi_struct {
382382
struct sk_buff *skb;
383383
struct list_head rx_list; /* Pending GRO_NORMAL skbs */
384384
int rx_count; /* length of rx_list */
385-
unsigned int napi_id;
385+
unsigned int napi_id; /* protected by netdev_lock */
386386
struct hrtimer timer;
387387
struct task_struct *thread;
388388
unsigned long gro_flush_timeout;
@@ -570,16 +570,11 @@ static inline bool napi_complete(struct napi_struct *n)
570570

571571
int dev_set_threaded(struct net_device *dev, bool threaded);
572572

573-
/**
574-
* napi_disable - prevent NAPI from scheduling
575-
* @n: NAPI context
576-
*
577-
* Stop NAPI from being scheduled on this context.
578-
* Waits till any outstanding processing completes.
579-
*/
580573
void napi_disable(struct napi_struct *n);
574+
void napi_disable_locked(struct napi_struct *n);
581575

582576
void napi_enable(struct napi_struct *n);
577+
void napi_enable_locked(struct napi_struct *n);
583578

584579
/**
585580
* napi_synchronize - wait until NAPI is not running

‎net/core/dev.c

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6958,11 +6958,13 @@ void netif_napi_add_weight_locked(struct net_device *dev,
69586958
}
69596959
EXPORT_SYMBOL(netif_napi_add_weight_locked);
69606960

6961-
void napi_disable(struct napi_struct *n)
6961+
void napi_disable_locked(struct napi_struct *n)
69626962
{
69636963
unsigned long val, new;
69646964

69656965
might_sleep();
6966+
netdev_assert_locked(n->dev);
6967+
69666968
set_bit(NAPI_STATE_DISABLE, &n->state);
69676969

69686970
val = READ_ONCE(n->state);
@@ -6985,16 +6987,25 @@ void napi_disable(struct napi_struct *n)
69856987

69866988
clear_bit(NAPI_STATE_DISABLE, &n->state);
69876989
}
6988-
EXPORT_SYMBOL(napi_disable);
6990+
EXPORT_SYMBOL(napi_disable_locked);
69896991

69906992
/**
6991-
* napi_enable - enable NAPI scheduling
6992-
* @n: NAPI context
6993+
* napi_disable() - prevent NAPI from scheduling
6994+
* @n: NAPI context
69936995
*
6994-
* Resume NAPI from being scheduled on this context.
6995-
* Must be paired with napi_disable.
6996+
* Stop NAPI from being scheduled on this context.
6997+
* Waits till any outstanding processing completes.
6998+
* Takes netdev_lock() for associated net_device.
69966999
*/
6997-
void napi_enable(struct napi_struct *n)
7000+
void napi_disable(struct napi_struct *n)
7001+
{
7002+
netdev_lock(n->dev);
7003+
napi_disable_locked(n);
7004+
netdev_unlock(n->dev);
7005+
}
7006+
EXPORT_SYMBOL(napi_disable);
7007+
7008+
void napi_enable_locked(struct napi_struct *n)
69987009
{
69997010
unsigned long new, val = READ_ONCE(n->state);
70007011

@@ -7011,6 +7022,22 @@ void napi_enable(struct napi_struct *n)
70117022
new |= NAPIF_STATE_THREADED;
70127023
} while (!try_cmpxchg(&n->state, &val, new));
70137024
}
7025+
EXPORT_SYMBOL(napi_enable_locked);
7026+
7027+
/**
7028+
* napi_enable() - enable NAPI scheduling
7029+
* @n: NAPI context
7030+
*
7031+
* Enable scheduling of a NAPI instance.
7032+
* Must be paired with napi_disable().
7033+
* Takes netdev_lock() for associated net_device.
7034+
*/
7035+
void napi_enable(struct napi_struct *n)
7036+
{
7037+
netdev_lock(n->dev);
7038+
napi_enable_locked(n);
7039+
netdev_unlock(n->dev);
7040+
}
70147041
EXPORT_SYMBOL(napi_enable);
70157042

70167043
static void flush_gro_hash(struct napi_struct *napi)

0 commit comments

Comments
 (0)