Skip to content

Commit 0c9547e

Browse files
nikita-youshkuba-moo
authored andcommitted
net: renesas: rswitch: fix race window between tx start and complete
If hardware is already transmitting, it can start handling the descriptor being written to immediately after it observes updated DT field, before the queue is kicked by a write to GWTRC. If the start_xmit() execution is preempted at unfortunate moment, this transmission can complete, and interrupt handled, before gq->cur gets updated. With the current implementation of completion, this will cause the last entry not completed. Fix that by changing completion loop to check DT values directly, instead of depending on gq->cur. Fixes: 3590918 ("net: ethernet: renesas: Add support for "Ethernet Switch"") Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Link: https://patch.msgid.link/20241208095004.69468-3-nikita.yoush@cogentembedded.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent 5cb0999 commit 0c9547e

File tree

1 file changed

+8
-8
lines changed

1 file changed

+8
-8
lines changed

drivers/net/ethernet/renesas/rswitch.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -862,13 +862,10 @@ static void rswitch_tx_free(struct net_device *ndev)
862862
struct rswitch_ext_desc *desc;
863863
struct sk_buff *skb;
864864

865-
for (; rswitch_get_num_cur_queues(gq) > 0;
866-
gq->dirty = rswitch_next_queue_index(gq, false, 1)) {
867-
desc = &gq->tx_ring[gq->dirty];
868-
if ((desc->desc.die_dt & DT_MASK) != DT_FEMPTY)
869-
break;
870-
865+
desc = &gq->tx_ring[gq->dirty];
866+
while ((desc->desc.die_dt & DT_MASK) == DT_FEMPTY) {
871867
dma_rmb();
868+
872869
skb = gq->skbs[gq->dirty];
873870
if (skb) {
874871
rdev->ndev->stats.tx_packets++;
@@ -879,7 +876,10 @@ static void rswitch_tx_free(struct net_device *ndev)
879876
dev_kfree_skb_any(gq->skbs[gq->dirty]);
880877
gq->skbs[gq->dirty] = NULL;
881878
}
879+
882880
desc->desc.die_dt = DT_EEMPTY;
881+
gq->dirty = rswitch_next_queue_index(gq, false, 1);
882+
desc = &gq->tx_ring[gq->dirty];
883883
}
884884
}
885885

@@ -1685,6 +1685,8 @@ static netdev_tx_t rswitch_start_xmit(struct sk_buff *skb, struct net_device *nd
16851685
gq->skbs[(gq->cur + nr_desc - 1) % gq->ring_size] = skb;
16861686
gq->unmap_addrs[(gq->cur + nr_desc - 1) % gq->ring_size] = dma_addr_orig;
16871687

1688+
dma_wmb();
1689+
16881690
/* DT_FSTART should be set at last. So, this is reverse order. */
16891691
for (i = nr_desc; i-- > 0; ) {
16901692
desc = &gq->tx_ring[rswitch_next_queue_index(gq, true, i)];
@@ -1695,8 +1697,6 @@ static netdev_tx_t rswitch_start_xmit(struct sk_buff *skb, struct net_device *nd
16951697
goto err_unmap;
16961698
}
16971699

1698-
wmb(); /* gq->cur must be incremented after die_dt was set */
1699-
17001700
gq->cur = rswitch_next_queue_index(gq, true, nr_desc);
17011701
rswitch_modify(rdev->addr, GWTRC(gq->index), 0, BIT(gq->index % 32));
17021702

0 commit comments

Comments
 (0)