Re: BQL-related tg3 transmit timeout on 5720 / Dell R720

Re: BQL-related tg3 transmit timeout on 5720 / Dell R720

[Roland Dreier]

On Wed, May 22, 2013 at 3:02 PM, Roland Dreier <roland@kernel.org> wrote:
> I'll try to find a kernel where tg3 works on this system so I can bisect.

So I finally was able to successfully bisect our problem with tg3
transmit timeouts with recent kernels.  Recall this was on on _some_
of our Dell R720 systems with 4X tg3 ethernet with devices like:

    tg3 0000:02:00.0: eth0: Tigon3 [partno(BCM95720) rev 5720000] (PCI
Express) MAC address 90:b1:1c:3f:46:b8
    tg3 0000:02:00.0: eth0: attached PHY is 5720C (10/100/1000Base-T
Ethernet) (WireSpeed[1], EEE[1])

The bisection came down to

    commit 298376d3e8f00147548c426959ce79efc47b669a
    Author: Tom Herbert <therbert@google.com>
    Date:   Mon Nov 28 08:33:30 2011

        tg3: Support for byte queue limits

        Changes to tg3 to use byte queue limits.

and indeed reverting that patch (and the follow-on fix
5cb917bc4f38/"tg3: Fix to use multi queue BQL interfaces") makes
things work on our 3.6.11 kernel and on net-next.

The mysterious thing is that after running a "working" (no TX timeout)
kernel, doing a warm reboot into any version of tg3 continues to
work.  However, I think I have the beginnings of an explanation:

initially, BQL stops the queue after the first TX packet is queued
up.  For "bad" systems/devices (the ones that hit the TX timeout),
that first TX never completes.  However, if I get rid of the
netdev_tx_sent_queue() call (so BQL never stops the queue), the net
stack goes on ahead and queues up 3 more packets, and at that point,
all 4 sends complete.

Then, once we've gotten that first send completion, the hardware state
seems to be "fixed" so that even after a warm reboot, the next kernel
gets its first send to complete (so BQL works OK).

I turned the BQL-related calls into just trace_printk(), and got the
following trace for a system in the cold-rebooted state:

#                              _-----=> irqs-off
#                             / _----=> need-resched
#                            | / _---=> hardirq/softirq
#                            || / _--=> preempt-depth
#                            ||| /     delay
#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
#              | |       |   ||||       |         |
          <idle>-0     [001] .Ns.    21.094020: tg3_start_xmit: start
xmit for queue 3
          <idle>-0     [001] .Ns.    21.094023: tg3_start_xmit:
queueing 90 bytes
          <idle>-0     [001] ..s.    21.762540: tg3_start_xmit: start
xmit for queue 3
          <idle>-0     [001] ..s.    21.762554: tg3_start_xmit:
queueing 78 bytes
           <...>-10    [001] ..s.    22.760736: tg3_start_xmit: start
xmit for queue 3
           <...>-10    [001] ..s.    22.760737: tg3_start_xmit:
queueing 70 bytes
          <idle>-0     [001] ..s.    26.761697: tg3_start_xmit: start
xmit for queue 3
          <idle>-0     [001] ..s.    26.761699: tg3_start_xmit:
queueing 70 bytes
          <idle>-0     [001] ..s.    26.761817: tg3_poll_work: TX comp
for index 3
          <idle>-0     [001] ..s.    26.761820: tg3_poll_work: sw_idx
0 hw_idx 4 pkts_compl 0 bytes_compl 0
          <idle>-0     [001] ..s.    26.761823: tg3_poll_work: sw_idx
1 hw_idx 4 pkts_compl 1 bytes_compl 90
          <idle>-0     [001] ..s.    26.761825: tg3_poll_work: sw_idx
2 hw_idx 4 pkts_compl 2 bytes_compl 168
          <idle>-0     [001] ..s.    26.761826: tg3_poll_work: sw_idx
3 hw_idx 4 pkts_compl 3 bytes_compl 238
          <idle>-0     [001] ..s.    26.761828: tg3_poll_work:
completing 4 packets, 308 bytes

So you can see that it isn't until 3 more packets and 600+ msec that
the first send completes.

After a warm reboot, the same system gives the following trace:

#                              _-----=> irqs-off
#                             / _----=> need-resched
#                            | / _---=> hardirq/softirq
#                            || / _--=> preempt-depth
#                            ||| /     delay
#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
#              | |       |   ||||       |         |
          <idle>-0     [001] .Ns.    16.813248: tg3_start_xmit: start
xmit for queue 0
          <idle>-0     [001] .Ns.    16.813252: tg3_start_xmit:
queueing 78 bytes
          <idle>-0     [001] ..s.    16.813370: tg3_poll_work: TX comp
for index 0
          <idle>-0     [001] ..s.    16.813372: tg3_poll_work: sw_idx
0 hw_idx 1 pkts_compl 0 bytes_compl 0
          <idle>-0     [001] ..s.    16.813375: tg3_poll_work:
completing 1 packets, 78 bytes
          <idle>-0     [001] .Ns.    16.823231: tg3_start_xmit: start
xmit for queue 0
          <idle>-0     [001] .Ns.    16.823231: tg3_start_xmit:
queueing 90 bytes
           <...>-869   [001] ..s.    16.823310: tg3_poll_work: TX comp
for index 0
           <...>-869   [001] ..s.    16.823310: tg3_poll_work: sw_idx
1 hw_idx 2 pkts_compl 0 bytes_compl 0
           <...>-869   [001] ..s.    16.823311: tg3_poll_work:
completing 1 packets, 90 bytes

and each send completes in turn.

For now I can work around the issue by hacking BQL out of tg3 in our
kernel, but I guess it would be good to understand this tg3-specific
issue of sends not completing and handle that in the tg3 driver.

I have a system that reproduces this very reliably, so let me know if
there is any further logging or other info that would help understand
this further.

Thanks,
  Roland

Re: BQL-related tg3 transmit timeout on 5720 / Dell R720

[Nithin Nayak Sujir]

On 5/30/2013 2:05 AM, Roland Dreier wrote:
> On Wed, May 22, 2013 at 3:02 PM, Roland Dreier <roland@kernel.org> wrote:
>> I'll try to find a kernel where tg3 works on this system so I can bisect.
>
> So I finally was able to successfully bisect our problem with tg3
> transmit timeouts with recent kernels.  Recall this was on on _some_
> of our Dell R720 systems with 4X tg3 ethernet with devices like:
>
>      tg3 0000:02:00.0: eth0: Tigon3 [partno(BCM95720) rev 5720000] (PCI
> Express) MAC address 90:b1:1c:3f:46:b8
>      tg3 0000:02:00.0: eth0: attached PHY is 5720C (10/100/1000Base-T
> Ethernet) (WireSpeed[1], EEE[1])
>
> The bisection came down to
>
>      commit 298376d3e8f00147548c426959ce79efc47b669a
>      Author: Tom Herbert <therbert@google.com>
>      Date:   Mon Nov 28 08:33:30 2011
>
>          tg3: Support for byte queue limits
>
>          Changes to tg3 to use byte queue limits.
>
[...]

> and each send completes in turn.
>
> For now I can work around the issue by hacking BQL out of tg3 in our
> kernel, but I guess it would be good to understand this tg3-specific
> issue of sends not completing and handle that in the tg3 driver.
>

Thanks for the bisect and detailed analysis. I will investigate this 
further.

> I have a system that reproduces this very reliably, so let me know if
> there is any further logging or other info that would help understand
> this further.
>

Is the 5720 a NIC or a LOM? If it's a NIC would it be possible to try it 
on a different system to see if the behaviour depends on the system at all?

> Thanks,
>    Roland
>

Re: BQL-related tg3 transmit timeout on 5720 / Dell R720

[Roland Dreier]

On Thu, May 30, 2013 at 7:34 AM, Nithin Nayak Sujir <nsujir@broadcom.com> wrote:
> Is the 5720 a NIC or a LOM? If it's a NIC would it be possible to try it on
> a different system to see if the behaviour depends on the system at all?

This is the LOM.  On the Dell it's a daughtercard but I don't think
it's user-replaceable.

In other words, sorry, I don't think I can try it on a different system.

 - R.

Re: BQL-related tg3 transmit timeout on 5720 / Dell R720

[Nithin Nayak Sujir]

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]


On 05/30/2013 09:19 AM, Roland Dreier wrote:
> On Thu, May 30, 2013 at 7:34 AM, Nithin Nayak Sujir <nsujir@broadcom.com> wrote:
>> Is the 5720 a NIC or a LOM? If it's a NIC would it be possible to try it on
>> a different system to see if the behaviour depends on the system at all?
>
> This is the LOM.  On the Dell it's a daughtercard but I don't think
> it's user-replaceable.
>
> In other words, sorry, I don't think I can try it on a different system.
>
>   - R.
>

Could you test the attached patch on top of net-next?

Nithin.


[-- Attachment #2: 0001-tg3-Add-read-dma-workaround-for-5720.patch --]
[-- Type: text/plain, Size: 3585 bytes --]

>From cdcf043fccebfe57ae6f1c04f9b15dbc8046ac69 Mon Sep 17 00:00:00 2001
From: Nithin Nayak Sujir <nsujir@broadcom.com>
Date: Thu, 30 May 2013 16:16:43 -0700
Subject: [PATCH] tg3: Add read dma workaround for 5720

Commit 091f0ea30074bc43f9250961b3247af713024bc6 "tg3: Add New 5719 Read
DMA workaround" added a workaround for TX DMA stall on the 5719. This
workaround needs to be applied to the 5720 as well.

Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 21 +++++++++++++++------
 drivers/net/ethernet/broadcom/tg3.h |  5 +++--
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index d7755d8..7e24137 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -9568,6 +9568,14 @@ static void tg3_rss_write_indir_tbl(struct tg3 *tp)
 	}
 }
 
+static inline u32 tg3_lso_rd_dma_workaround_bit(struct tg3 *tp)
+{
+	if (tg3_asic_rev(tp) == ASIC_REV_5719)
+		return TG3_LSO_RD_DMA_TX_LENGTH_WA_5719;
+	else
+		return TG3_LSO_RD_DMA_TX_LENGTH_WA_5720;
+}
+
 /* tp->lock is held. */
 static int tg3_reset_hw(struct tg3 *tp, bool reset_phy)
 {
@@ -10224,16 +10232,17 @@ static int tg3_reset_hw(struct tg3 *tp, bool reset_phy)
 	tw32_f(RDMAC_MODE, rdmac_mode);
 	udelay(40);
 
-	if (tg3_asic_rev(tp) == ASIC_REV_5719) {
+	if (tg3_asic_rev(tp) == ASIC_REV_5719 ||
+	    tg3_asic_rev(tp) == ASIC_REV_5720) {
 		for (i = 0; i < TG3_NUM_RDMA_CHANNELS; i++) {
 			if (tr32(TG3_RDMA_LENGTH + (i << 2)) > TG3_MAX_MTU(tp))
 				break;
 		}
 		if (i < TG3_NUM_RDMA_CHANNELS) {
 			val = tr32(TG3_LSO_RD_DMA_CRPTEN_CTRL);
-			val |= TG3_LSO_RD_DMA_TX_LENGTH_WA;
+			val |= tg3_lso_rd_dma_workaround_bit(tp);
 			tw32(TG3_LSO_RD_DMA_CRPTEN_CTRL, val);
-			tg3_flag_set(tp, 5719_RDMA_BUG);
+			tg3_flag_set(tp, 5719_5720_RDMA_BUG);
 		}
 	}
 
@@ -10597,15 +10606,15 @@ static void tg3_periodic_fetch_stats(struct tg3 *tp)
 	TG3_STAT_ADD32(&sp->tx_ucast_packets, MAC_TX_STATS_UCAST);
 	TG3_STAT_ADD32(&sp->tx_mcast_packets, MAC_TX_STATS_MCAST);
 	TG3_STAT_ADD32(&sp->tx_bcast_packets, MAC_TX_STATS_BCAST);
-	if (unlikely(tg3_flag(tp, 5719_RDMA_BUG) &&
+	if (unlikely(tg3_flag(tp, 5719_5720_RDMA_BUG) &&
 		     (sp->tx_ucast_packets.low + sp->tx_mcast_packets.low +
 		      sp->tx_bcast_packets.low) > TG3_NUM_RDMA_CHANNELS)) {
 		u32 val;
 
 		val = tr32(TG3_LSO_RD_DMA_CRPTEN_CTRL);
-		val &= ~TG3_LSO_RD_DMA_TX_LENGTH_WA;
+		val &= ~tg3_lso_rd_dma_workaround_bit(tp);
 		tw32(TG3_LSO_RD_DMA_CRPTEN_CTRL, val);
-		tg3_flag_clear(tp, 5719_RDMA_BUG);
+		tg3_flag_clear(tp, 5719_5720_RDMA_BUG);
 	}
 
 	TG3_STAT_ADD32(&sp->rx_octets, MAC_RX_STATS_OCTETS);
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index 2530c20..cd63d11 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -1423,7 +1423,8 @@
 #define TG3_LSO_RD_DMA_CRPTEN_CTRL	0x00004910
 #define TG3_LSO_RD_DMA_CRPTEN_CTRL_BLEN_BD_4K	 0x00030000
 #define TG3_LSO_RD_DMA_CRPTEN_CTRL_BLEN_LSO_4K	 0x000c0000
-#define TG3_LSO_RD_DMA_TX_LENGTH_WA	 0x02000000
+#define TG3_LSO_RD_DMA_TX_LENGTH_WA_5719	 0x02000000
+#define TG3_LSO_RD_DMA_TX_LENGTH_WA_5720	 0x00200000
 /* 0x4914 --> 0x4be0 unused */
 
 #define TG3_NUM_RDMA_CHANNELS		4
@@ -3060,7 +3061,7 @@ enum TG3_FLAGS {
 	TG3_FLAG_APE_HAS_NCSI,
 	TG3_FLAG_TX_TSTAMP_EN,
 	TG3_FLAG_4K_FIFO_LIMIT,
-	TG3_FLAG_5719_RDMA_BUG,
+	TG3_FLAG_5719_5720_RDMA_BUG,
 	TG3_FLAG_RESET_TASK_PENDING,
 	TG3_FLAG_PTP_CAPABLE,
 	TG3_FLAG_5705_PLUS,
-- 
1.8.1.4

Re: BQL-related tg3 transmit timeout on 5720 / Dell R720

[Roland Dreier]

> Could you test the attached patch on top of net-next?

Yep, that works.

Tested-by: Roland Dreier <roland@purestorage.com>

Thanks!

Re: BQL-related tg3 transmit timeout on 5720 / Dell R720

[Nithin Nayak Sujir]

On 6/3/2013 9:40 AM, Roland Dreier wrote:
>> Could you test the attached patch on top of net-next?
>
> Yep, that works.
>
> Tested-by: Roland Dreier <roland@purestorage.com>
>

Thanks, Roland.
I'll submit this.


> Thanks!
>