Skip to content

Commit 3f1baa9

Browse files
Sankararaman Jayaramankuba-moo
authored andcommitted
vmxnet3: Fix tx queue race condition with XDP
If XDP traffic runs on a CPU which is greater than or equal to the number of the Tx queues of the NIC, then vmxnet3_xdp_get_tq() always picks up queue 0 for transmission as it uses reciprocal scale instead of simple modulo operation. vmxnet3_xdp_xmit() and vmxnet3_xdp_xmit_frame() use the above returned queue without any locking which can lead to race conditions when multiple XDP xmits run in parallel on different CPU's. This patch uses a simple module scheme when the current CPU equals or exceeds the number of Tx queues on the NIC. It also adds locking in vmxnet3_xdp_xmit() and vmxnet3_xdp_xmit_frame() functions. Fixes: 54f00cc ("vmxnet3: Add XDP support.") Signed-off-by: Sankararaman Jayaraman <[email protected]> Signed-off-by: Ronak Doshi <[email protected]> Reviewed-by: Simon Horman <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent a8aa6a6 commit 3f1baa9

File tree

1 file changed

+12
-2
lines changed

1 file changed

+12
-2
lines changed

drivers/net/vmxnet3/vmxnet3_xdp.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ vmxnet3_xdp_get_tq(struct vmxnet3_adapter *adapter)
2828
if (likely(cpu < tq_number))
2929
tq = &adapter->tx_queue[cpu];
3030
else
31-
tq = &adapter->tx_queue[reciprocal_scale(cpu, tq_number)];
31+
tq = &adapter->tx_queue[cpu % tq_number];
3232

3333
return tq;
3434
}
@@ -124,6 +124,7 @@ vmxnet3_xdp_xmit_frame(struct vmxnet3_adapter *adapter,
124124
u32 buf_size;
125125
u32 dw2;
126126

127+
spin_lock_irq(&tq->tx_lock);
127128
dw2 = (tq->tx_ring.gen ^ 0x1) << VMXNET3_TXD_GEN_SHIFT;
128129
dw2 |= xdpf->len;
129130
ctx.sop_txd = tq->tx_ring.base + tq->tx_ring.next2fill;
@@ -134,6 +135,7 @@ vmxnet3_xdp_xmit_frame(struct vmxnet3_adapter *adapter,
134135

135136
if (vmxnet3_cmd_ring_desc_avail(&tq->tx_ring) == 0) {
136137
tq->stats.tx_ring_full++;
138+
spin_unlock_irq(&tq->tx_lock);
137139
return -ENOSPC;
138140
}
139141

@@ -142,8 +144,10 @@ vmxnet3_xdp_xmit_frame(struct vmxnet3_adapter *adapter,
142144
tbi->dma_addr = dma_map_single(&adapter->pdev->dev,
143145
xdpf->data, buf_size,
144146
DMA_TO_DEVICE);
145-
if (dma_mapping_error(&adapter->pdev->dev, tbi->dma_addr))
147+
if (dma_mapping_error(&adapter->pdev->dev, tbi->dma_addr)) {
148+
spin_unlock_irq(&tq->tx_lock);
146149
return -EFAULT;
150+
}
147151
tbi->map_type |= VMXNET3_MAP_SINGLE;
148152
} else { /* XDP buffer from page pool */
149153
page = virt_to_page(xdpf->data);
@@ -182,6 +186,7 @@ vmxnet3_xdp_xmit_frame(struct vmxnet3_adapter *adapter,
182186
dma_wmb();
183187
gdesc->dword[2] = cpu_to_le32(le32_to_cpu(gdesc->dword[2]) ^
184188
VMXNET3_TXD_GEN);
189+
spin_unlock_irq(&tq->tx_lock);
185190

186191
/* No need to handle the case when tx_num_deferred doesn't reach
187192
* threshold. Backend driver at hypervisor side will poll and reset
@@ -225,6 +230,7 @@ vmxnet3_xdp_xmit(struct net_device *dev,
225230
{
226231
struct vmxnet3_adapter *adapter = netdev_priv(dev);
227232
struct vmxnet3_tx_queue *tq;
233+
struct netdev_queue *nq;
228234
int i;
229235

230236
if (unlikely(test_bit(VMXNET3_STATE_BIT_QUIESCED, &adapter->state)))
@@ -236,13 +242,17 @@ vmxnet3_xdp_xmit(struct net_device *dev,
236242
if (tq->stopped)
237243
return -ENETDOWN;
238244

245+
nq = netdev_get_tx_queue(adapter->netdev, tq->qid);
246+
247+
__netif_tx_lock(nq, smp_processor_id());
239248
for (i = 0; i < n; i++) {
240249
if (vmxnet3_xdp_xmit_frame(adapter, frames[i], tq, true)) {
241250
tq->stats.xdp_xmit_err++;
242251
break;
243252
}
244253
}
245254
tq->stats.xdp_xmit += i;
255+
__netif_tx_unlock(nq);
246256

247257
return i;
248258
}

0 commit comments

Comments
 (0)