Skip to content

Commit 88e69af

Browse files
rkannoth1davem330
authored andcommitted
octeontx2-pf: Fix page pool cache index corruption.
The access to page pool `cache' array and the `count' variable is not locked. Page pool cache access is fine as long as there is only one consumer per pool. octeontx2 driver fills in rx buffers from page pool in NAPI context. If system is stressed and could not allocate buffers, refiiling work will be delegated to a delayed workqueue. This means that there are two cosumers to the page pool cache. Either workqueue or IRQ/NAPI can be run on other CPU. This will lead to lock less access, hence corruption of cache pool indexes. To fix this issue, NAPI is rescheduled from workqueue context to refill rx buffers. Fixes: b2e3406 ("octeontx2-pf: Add support for page pool") Signed-off-by: Ratheesh Kannoth <[email protected]> Reported-by: Sebastian Andrzej Siewior <[email protected]> Reviewed-by: Sebastian Andrzej Siewior <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 281f65d commit 88e69af

File tree

7 files changed

+44
-51
lines changed

7 files changed

+44
-51
lines changed

drivers/net/ethernet/marvell/octeontx2/nic/cn10k.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,13 @@ int cn10k_sq_aq_init(void *dev, u16 qidx, u16 sqb_aura)
107107
}
108108

109109
#define NPA_MAX_BURST 16
110-
void cn10k_refill_pool_ptrs(void *dev, struct otx2_cq_queue *cq)
110+
int cn10k_refill_pool_ptrs(void *dev, struct otx2_cq_queue *cq)
111111
{
112112
struct otx2_nic *pfvf = dev;
113+
int cnt = cq->pool_ptrs;
113114
u64 ptrs[NPA_MAX_BURST];
114-
int num_ptrs = 1;
115115
dma_addr_t bufptr;
116+
int num_ptrs = 1;
116117

117118
/* Refill pool with new buffers */
118119
while (cq->pool_ptrs) {
@@ -131,6 +132,7 @@ void cn10k_refill_pool_ptrs(void *dev, struct otx2_cq_queue *cq)
131132
num_ptrs = 1;
132133
}
133134
}
135+
return cnt - cq->pool_ptrs;
134136
}
135137

136138
void cn10k_sqe_flush(void *dev, struct otx2_snd_queue *sq, int size, int qidx)

drivers/net/ethernet/marvell/octeontx2/nic/cn10k.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ static inline int mtu_to_dwrr_weight(struct otx2_nic *pfvf, int mtu)
2424
return weight;
2525
}
2626

27-
void cn10k_refill_pool_ptrs(void *dev, struct otx2_cq_queue *cq);
27+
int cn10k_refill_pool_ptrs(void *dev, struct otx2_cq_queue *cq);
2828
void cn10k_sqe_flush(void *dev, struct otx2_snd_queue *sq, int size, int qidx);
2929
int cn10k_sq_aq_init(void *dev, u16 qidx, u16 sqb_aura);
3030
int cn10k_lmtst_init(struct otx2_nic *pfvf);

drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c

Lines changed: 6 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -574,20 +574,8 @@ int otx2_alloc_rbuf(struct otx2_nic *pfvf, struct otx2_pool *pool,
574574
int otx2_alloc_buffer(struct otx2_nic *pfvf, struct otx2_cq_queue *cq,
575575
dma_addr_t *dma)
576576
{
577-
if (unlikely(__otx2_alloc_rbuf(pfvf, cq->rbpool, dma))) {
578-
struct refill_work *work;
579-
struct delayed_work *dwork;
580-
581-
work = &pfvf->refill_wrk[cq->cq_idx];
582-
dwork = &work->pool_refill_work;
583-
/* Schedule a task if no other task is running */
584-
if (!cq->refill_task_sched) {
585-
cq->refill_task_sched = true;
586-
schedule_delayed_work(dwork,
587-
msecs_to_jiffies(100));
588-
}
577+
if (unlikely(__otx2_alloc_rbuf(pfvf, cq->rbpool, dma)))
589578
return -ENOMEM;
590-
}
591579
return 0;
592580
}
593581

@@ -1082,39 +1070,20 @@ static int otx2_cq_init(struct otx2_nic *pfvf, u16 qidx)
10821070
static void otx2_pool_refill_task(struct work_struct *work)
10831071
{
10841072
struct otx2_cq_queue *cq;
1085-
struct otx2_pool *rbpool;
10861073
struct refill_work *wrk;
1087-
int qidx, free_ptrs = 0;
10881074
struct otx2_nic *pfvf;
1089-
dma_addr_t bufptr;
1075+
int qidx;
10901076

10911077
wrk = container_of(work, struct refill_work, pool_refill_work.work);
10921078
pfvf = wrk->pf;
10931079
qidx = wrk - pfvf->refill_wrk;
10941080
cq = &pfvf->qset.cq[qidx];
1095-
rbpool = cq->rbpool;
1096-
free_ptrs = cq->pool_ptrs;
10971081

1098-
while (cq->pool_ptrs) {
1099-
if (otx2_alloc_rbuf(pfvf, rbpool, &bufptr)) {
1100-
/* Schedule a WQ if we fails to free atleast half of the
1101-
* pointers else enable napi for this RQ.
1102-
*/
1103-
if (!((free_ptrs - cq->pool_ptrs) > free_ptrs / 2)) {
1104-
struct delayed_work *dwork;
1105-
1106-
dwork = &wrk->pool_refill_work;
1107-
schedule_delayed_work(dwork,
1108-
msecs_to_jiffies(100));
1109-
} else {
1110-
cq->refill_task_sched = false;
1111-
}
1112-
return;
1113-
}
1114-
pfvf->hw_ops->aura_freeptr(pfvf, qidx, bufptr + OTX2_HEAD_ROOM);
1115-
cq->pool_ptrs--;
1116-
}
11171082
cq->refill_task_sched = false;
1083+
1084+
local_bh_disable();
1085+
napi_schedule(wrk->napi);
1086+
local_bh_enable();
11181087
}
11191088

11201089
int otx2_config_nix_queues(struct otx2_nic *pfvf)

drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,7 @@ struct flr_work {
302302
struct refill_work {
303303
struct delayed_work pool_refill_work;
304304
struct otx2_nic *pf;
305+
struct napi_struct *napi;
305306
};
306307

307308
/* PTPv2 originTimestamp structure */
@@ -370,7 +371,7 @@ struct dev_hw_ops {
370371
int (*sq_aq_init)(void *dev, u16 qidx, u16 sqb_aura);
371372
void (*sqe_flush)(void *dev, struct otx2_snd_queue *sq,
372373
int size, int qidx);
373-
void (*refill_pool_ptrs)(void *dev, struct otx2_cq_queue *cq);
374+
int (*refill_pool_ptrs)(void *dev, struct otx2_cq_queue *cq);
374375
void (*aura_freeptr)(void *dev, int aura, u64 buf);
375376
};
376377

drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1943,16 +1943,17 @@ int otx2_stop(struct net_device *netdev)
19431943

19441944
netif_tx_disable(netdev);
19451945

1946+
for (wrk = 0; wrk < pf->qset.cq_cnt; wrk++)
1947+
cancel_delayed_work_sync(&pf->refill_wrk[wrk].pool_refill_work);
1948+
devm_kfree(pf->dev, pf->refill_wrk);
1949+
19461950
otx2_free_hw_resources(pf);
19471951
otx2_free_cints(pf, pf->hw.cint_cnt);
19481952
otx2_disable_napi(pf);
19491953

19501954
for (qidx = 0; qidx < netdev->num_tx_queues; qidx++)
19511955
netdev_tx_reset_queue(netdev_get_tx_queue(netdev, qidx));
19521956

1953-
for (wrk = 0; wrk < pf->qset.cq_cnt; wrk++)
1954-
cancel_delayed_work_sync(&pf->refill_wrk[wrk].pool_refill_work);
1955-
devm_kfree(pf->dev, pf->refill_wrk);
19561957

19571958
kfree(qset->sq);
19581959
kfree(qset->cq);

drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -424,9 +424,10 @@ static int otx2_rx_napi_handler(struct otx2_nic *pfvf,
424424
return processed_cqe;
425425
}
426426

427-
void otx2_refill_pool_ptrs(void *dev, struct otx2_cq_queue *cq)
427+
int otx2_refill_pool_ptrs(void *dev, struct otx2_cq_queue *cq)
428428
{
429429
struct otx2_nic *pfvf = dev;
430+
int cnt = cq->pool_ptrs;
430431
dma_addr_t bufptr;
431432

432433
while (cq->pool_ptrs) {
@@ -435,6 +436,8 @@ void otx2_refill_pool_ptrs(void *dev, struct otx2_cq_queue *cq)
435436
otx2_aura_freeptr(pfvf, cq->cq_idx, bufptr + OTX2_HEAD_ROOM);
436437
cq->pool_ptrs--;
437438
}
439+
440+
return cnt - cq->pool_ptrs;
438441
}
439442

440443
static int otx2_tx_napi_handler(struct otx2_nic *pfvf,
@@ -521,6 +524,7 @@ int otx2_napi_handler(struct napi_struct *napi, int budget)
521524
struct otx2_cq_queue *cq;
522525
struct otx2_qset *qset;
523526
struct otx2_nic *pfvf;
527+
int filled_cnt = -1;
524528

525529
cq_poll = container_of(napi, struct otx2_cq_poll, napi);
526530
pfvf = (struct otx2_nic *)cq_poll->dev;
@@ -541,7 +545,7 @@ int otx2_napi_handler(struct napi_struct *napi, int budget)
541545
}
542546

543547
if (rx_cq && rx_cq->pool_ptrs)
544-
pfvf->hw_ops->refill_pool_ptrs(pfvf, rx_cq);
548+
filled_cnt = pfvf->hw_ops->refill_pool_ptrs(pfvf, rx_cq);
545549
/* Clear the IRQ */
546550
otx2_write64(pfvf, NIX_LF_CINTX_INT(cq_poll->cint_idx), BIT_ULL(0));
547551

@@ -561,9 +565,25 @@ int otx2_napi_handler(struct napi_struct *napi, int budget)
561565
otx2_config_irq_coalescing(pfvf, i);
562566
}
563567

564-
/* Re-enable interrupts */
565-
otx2_write64(pfvf, NIX_LF_CINTX_ENA_W1S(cq_poll->cint_idx),
566-
BIT_ULL(0));
568+
if (unlikely(!filled_cnt)) {
569+
struct refill_work *work;
570+
struct delayed_work *dwork;
571+
572+
work = &pfvf->refill_wrk[cq->cq_idx];
573+
dwork = &work->pool_refill_work;
574+
/* Schedule a task if no other task is running */
575+
if (!cq->refill_task_sched) {
576+
work->napi = napi;
577+
cq->refill_task_sched = true;
578+
schedule_delayed_work(dwork,
579+
msecs_to_jiffies(100));
580+
}
581+
} else {
582+
/* Re-enable interrupts */
583+
otx2_write64(pfvf,
584+
NIX_LF_CINTX_ENA_W1S(cq_poll->cint_idx),
585+
BIT_ULL(0));
586+
}
567587
}
568588
return workdone;
569589
}

drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,6 @@ void cn10k_sqe_flush(void *dev, struct otx2_snd_queue *sq,
170170
int size, int qidx);
171171
void otx2_sqe_flush(void *dev, struct otx2_snd_queue *sq,
172172
int size, int qidx);
173-
void otx2_refill_pool_ptrs(void *dev, struct otx2_cq_queue *cq);
174-
void cn10k_refill_pool_ptrs(void *dev, struct otx2_cq_queue *cq);
173+
int otx2_refill_pool_ptrs(void *dev, struct otx2_cq_queue *cq);
174+
int cn10k_refill_pool_ptrs(void *dev, struct otx2_cq_queue *cq);
175175
#endif /* OTX2_TXRX_H */

0 commit comments

Comments
 (0)