Skip to content

Commit c542b39

Browse files
Alex Pakhunovdavem330
authored andcommitted
tg3: Fix the TX ring stall
The TX ring maintained by the tg3 driver can end up in the state, when it has packets queued for sending but the NIC hardware is not informed, so no progress is made. This leads to a multi-second interruption in network traffic followed by dev_watchdog() firing and resetting the queue. The specific sequence of steps is: 1. tg3_start_xmit() is called at least once and queues packet(s) without updating tnapi->prodmbox (netdev_xmit_more() returns true) 2. tg3_start_xmit() is called with an SKB which causes tg3_tso_bug() to be called. 3. tg3_tso_bug() determines that the SKB is too large, ... if (unlikely(tg3_tx_avail(tnapi) <= frag_cnt_est)) { ... stops the queue, and returns NETDEV_TX_BUSY: netif_tx_stop_queue(txq); ... if (tg3_tx_avail(tnapi) <= frag_cnt_est) return NETDEV_TX_BUSY; 4. Since all tg3_tso_bug() call sites directly return, the code updating tnapi->prodmbox is skipped. 5. The queue is stuck now. tg3_start_xmit() is not called while the queue is stopped. The NIC is not processing new packets because tnapi->prodmbox wasn't updated. tg3_tx() is not called by tg3_poll_work() because the all TX descriptions that could be freed has been freed: /* run TX completion thread */ if (tnapi->hw_status->idx[0].tx_consumer != tnapi->tx_cons) { tg3_tx(tnapi); 6. Eventually, dev_watchdog() fires triggering a reset of the queue. This fix makes sure that the tnapi->prodmbox update happens regardless of the reason tg3_start_xmit() returned. Signed-off-by: Alex Pakhunov <alexey.pakhunov@spacex.com> Signed-off-by: Vincent Wong <vincent.wong2@spacex.com> Reviewed-by: Michael Chan <michael.chan@broadcom.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent dbc9e34 commit c542b39

File tree

1 file changed

+42
-11
lines changed
  • drivers/net/ethernet/broadcom

1 file changed

+42
-11
lines changed

drivers/net/ethernet/broadcom/tg3.c

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6647,9 +6647,9 @@ static void tg3_tx(struct tg3_napi *tnapi)
66476647

66486648
tnapi->tx_cons = sw_idx;
66496649

6650-
/* Need to make the tx_cons update visible to tg3_start_xmit()
6650+
/* Need to make the tx_cons update visible to __tg3_start_xmit()
66516651
* before checking for netif_queue_stopped(). Without the
6652-
* memory barrier, there is a small possibility that tg3_start_xmit()
6652+
* memory barrier, there is a small possibility that __tg3_start_xmit()
66536653
* will miss it and cause the queue to be stopped forever.
66546654
*/
66556655
smp_mb();
@@ -7889,7 +7889,7 @@ static bool tg3_tso_bug_gso_check(struct tg3_napi *tnapi, struct sk_buff *skb)
78897889
return skb_shinfo(skb)->gso_segs < tnapi->tx_pending / 3;
78907890
}
78917891

7892-
static netdev_tx_t tg3_start_xmit(struct sk_buff *, struct net_device *);
7892+
static netdev_tx_t __tg3_start_xmit(struct sk_buff *, struct net_device *);
78937893

78947894
/* Use GSO to workaround all TSO packets that meet HW bug conditions
78957895
* indicated in tg3_tx_frag_set()
@@ -7923,7 +7923,7 @@ static int tg3_tso_bug(struct tg3 *tp, struct tg3_napi *tnapi,
79237923

79247924
skb_list_walk_safe(segs, seg, next) {
79257925
skb_mark_not_on_list(seg);
7926-
tg3_start_xmit(seg, tp->dev);
7926+
__tg3_start_xmit(seg, tp->dev);
79277927
}
79287928

79297929
tg3_tso_bug_end:
@@ -7933,7 +7933,7 @@ static int tg3_tso_bug(struct tg3 *tp, struct tg3_napi *tnapi,
79337933
}
79347934

79357935
/* hard_start_xmit for all devices */
7936-
static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
7936+
static netdev_tx_t __tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
79377937
{
79387938
struct tg3 *tp = netdev_priv(dev);
79397939
u32 len, entry, base_flags, mss, vlan = 0;
@@ -8182,11 +8182,6 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
81828182
netif_tx_wake_queue(txq);
81838183
}
81848184

8185-
if (!netdev_xmit_more() || netif_xmit_stopped(txq)) {
8186-
/* Packets are ready, update Tx producer idx on card. */
8187-
tw32_tx_mbox(tnapi->prodmbox, entry);
8188-
}
8189-
81908185
return NETDEV_TX_OK;
81918186

81928187
dma_error:
@@ -8199,6 +8194,42 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
81998194
return NETDEV_TX_OK;
82008195
}
82018196

8197+
static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
8198+
{
8199+
struct netdev_queue *txq;
8200+
u16 skb_queue_mapping;
8201+
netdev_tx_t ret;
8202+
8203+
skb_queue_mapping = skb_get_queue_mapping(skb);
8204+
txq = netdev_get_tx_queue(dev, skb_queue_mapping);
8205+
8206+
ret = __tg3_start_xmit(skb, dev);
8207+
8208+
/* Notify the hardware that packets are ready by updating the TX ring
8209+
* tail pointer. We respect netdev_xmit_more() thus avoiding poking
8210+
* the hardware for every packet. To guarantee forward progress the TX
8211+
* ring must be drained when it is full as indicated by
8212+
* netif_xmit_stopped(). This needs to happen even when the current
8213+
* skb was dropped or rejected with NETDEV_TX_BUSY. Otherwise packets
8214+
* queued by previous __tg3_start_xmit() calls might get stuck in
8215+
* the queue forever.
8216+
*/
8217+
if (!netdev_xmit_more() || netif_xmit_stopped(txq)) {
8218+
struct tg3_napi *tnapi;
8219+
struct tg3 *tp;
8220+
8221+
tp = netdev_priv(dev);
8222+
tnapi = &tp->napi[skb_queue_mapping];
8223+
8224+
if (tg3_flag(tp, ENABLE_TSS))
8225+
tnapi++;
8226+
8227+
tw32_tx_mbox(tnapi->prodmbox, tnapi->tx_prod);
8228+
}
8229+
8230+
return ret;
8231+
}
8232+
82028233
static void tg3_mac_loopback(struct tg3 *tp, bool enable)
82038234
{
82048235
if (enable) {
@@ -17729,7 +17760,7 @@ static int tg3_init_one(struct pci_dev *pdev,
1772917760
* device behind the EPB cannot support DMA addresses > 40-bit.
1773017761
* On 64-bit systems with IOMMU, use 40-bit dma_mask.
1773117762
* On 64-bit systems without IOMMU, use 64-bit dma_mask and
17732-
* do DMA address check in tg3_start_xmit().
17763+
* do DMA address check in __tg3_start_xmit().
1773317764
*/
1773417765
if (tg3_flag(tp, IS_5788))
1773517766
persist_dma_mask = dma_mask = DMA_BIT_MASK(32);

0 commit comments

Comments
 (0)