Fix for tg3 networking lockup

I'm sending this on mostly because it was a bit of a pain to track down, and hopefully it will save time if anyone else hits this while playing with the -rt kernel. It is NOT the right way to fix things, so please don't even think of applying this patch (unless you need it, in your own local tree :-).

One of these days when we have time to breath we'll look into fixing this the right way, if someone doesn't beat us to it first. :-)

- Ted

Fix for tg3 networking lockup (LTC BZ# 25487)

The tg3 networking card is a finicky beast, and needs to be tickled every tenth of a second or so or it will go deaf and dumb until it is reset via "ifdown eth0; ifup eth0".

The problem comes if a workload consumes enough CPU power so that tg3_timer() doesn't run frequently enough. Unfortunately, the softirq_timer doesn't run at a sufficiently high priority to guarantee this, and boosting softirq_timer to prio 90 or so causes the customer benchmark to fail due to a decrease in determinism due to running all timers at high priority.

The right fix is to change the timer code to dynamically change priorities, but this is a non-trivial fix. This is a quick hack driven by code freeze deadlines. :-{

What we do is hack in a special case hook into kernel/clockevents.c to enforce tg3_timer getting called approximately every 0.128 seconds (assuming HZ=250). This appears to be sufficient to stop the networking hardware from going insane.

Index: linux-rtj12.7/drivers/net/tg3.c
===================================================================
@@ -342,6 +342,36 @@
 	{ "interrupt test (offline)" },
 };
 
+/*
+ * TG3 timer section
+ *
+ * HACK HACK HACK --- we need to make sure the timer runs at a high
+ * priority, but unfortunately there's no good way to do this given
+ * the current timer infrastructure.  So we kludge it, by hacking the
+ * clockevents code.  I wish there were a better a way, but changing
+ * all of the timer infrastructure to work like hrtimers is far too
+ * invasive, and the hrtimer infrastructure assumes that priority
+ * boosting only happens to in response to a process running at a
+ * particular priority depending on an hrtimer, so in order to use
+ * hrtimers I would have needed to create a kernel thread and then
+ * export all of the hrtimer functions.  This is ugly as sin, but it
+ * involves the smallest possible changes for ease of auditing. ---
+ * [tytso:20060803.0217EDT]
+ */
+
+extern void (*tg3_hack_hook)(void); /* defined in kernel/clockevents.c */
+static void tg3_timer(unsigned long __opaque); /* defined below */
+static LIST_HEAD(tg3_timer_list); /* list of all active tg3 interfaces */
+
+static void tg3_run_timers(void)
+{
+	struct tg3 *tp;
+
+	list_for_each_entry(tp, &tg3_timer_list, timer_hack_list) {
+		tg3_timer((unsigned long) tp);
+	}
+}
+
 static void tg3_write32(struct tg3 *tp, u32 off, u32 val)
 {
 	writel(val, tp->regs + off);

@@ -3480,7 +3510,6 @@
 static void tg3_reset_task(void *_data)
 {
 	struct tg3 *tp = _data;
-	unsigned int restart_timer;
 
 	tg3_full_lock(tp, 0);
 	tp->tg3_flags |= TG3_FLAG_IN_RESET_TASK;

@@ -3497,7 +3526,6 @@
 
 	tg3_full_lock(tp, 1);
 
-	restart_timer = tp->tg3_flags2 & TG3_FLG2_RESTART_TIMER;
 	tp->tg3_flags2 &= ~TG3_FLG2_RESTART_TIMER;
 
 	tg3_halt(tp, RESET_KIND_SHUTDOWN, 0);

@@ -3505,9 +3533,6 @@
 
 	tg3_netif_start(tp);
 
-	if (restart_timer)
-		mod_timer(&tp->timer, jiffies + 1);
-
 	tp->tg3_flags &= ~TG3_FLAG_IN_RESET_TASK;
 
 	tg3_full_unlock(tp);

@@ -6328,7 +6353,6 @@
 	spin_unlock(&tp->lock);
 
 	tp->timer.expires = jiffies + tp->timer_offset;
-	add_timer(&tp->timer);
 }
 
 static int tg3_test_interrupt(struct tg3 *tp)

@@ -6570,7 +6594,7 @@
 
 	tg3_full_lock(tp, 0);
 
-	add_timer(&tp->timer);
+	list_add(&tp->timer_hack_list, &tg3_timer_list);
 	tp->tg3_flags |= TG3_FLAG_INIT_COMPLETE;
 	tg3_enable_ints(tp);
 

@@ -6825,7 +6849,7 @@
 
 	netif_stop_queue(dev);
 
-	del_timer_sync(&tp->timer);
+	list_del(&tp->timer_hack_list);
 
 	tg3_full_lock(tp, 1);
 #if 0

@@ -10751,6 +10775,7 @@
 	spin_lock_init(&tp->tx_lock);
 	spin_lock_init(&tp->indirect_lock);
 	INIT_WORK(&tp->reset_task, tg3_reset_task, tp);
+	INIT_LIST_HEAD(&tp->timer_hack_list);
 
 	tp->regs = ioremap_nocache(tg3reg_base, tg3reg_len);
 	if (tp->regs == 0UL) {

@@ -10945,6 +10970,7 @@
 	       (pdev->dma_mask == DMA_32BIT_MASK) ? 32 :
 	        (((u64) pdev->dma_mask == DMA_40BIT_MASK) ? 40 : 64));
 
+	tg3_hack_hook = tg3_run_timers;
 	return 0;
 
 err_out_iounmap:

@@ -10997,7 +11023,7 @@
 	flush_scheduled_work();
 	tg3_netif_stop(tp);
 
-	del_timer_sync(&tp->timer);
+	list_del(&tp->timer_hack_list);
 
 	tg3_full_lock(tp, 1);
 	tg3_disable_ints(tp);

@@ -11018,7 +11044,7 @@
 		tg3_init_hw(tp);
 
 		tp->timer.expires = jiffies + tp->timer_offset;
-		add_timer(&tp->timer);
+		list_add(&tp->timer_hack_list, &tg3_timer_list);
 
 		netif_device_attach(dev);
 		tg3_netif_start(tp);

@@ -11052,7 +11078,7 @@
 	tg3_init_hw(tp);
 
 	tp->timer.expires = jiffies + tp->timer_offset;
-	add_timer(&tp->timer);
+	list_add(&tp->timer_hack_list, &tg3_timer_list);
 
 	tg3_netif_start(tp);
 

@@ -11077,6 +11103,7 @@
 
 static void __exit tg3_cleanup(void)
 {
+	tg3_hack_hook = 0;
 	pci_unregister_driver(&tg3_driver);
 }
 

Index: linux-rtj12.7/drivers/net/tg3.h
===================================================================
@@ -2208,6 +2208,7 @@
 	u32				timer_offset;
 	u16				asf_counter;
 	u16				asf_multiplier;
+	struct list_head		timer_hack_list;
 
 	struct tg3_link_config		link_config;
 	struct tg3_bufmgr_config	bufmgr_config;

Index: linux-rtj12.7/kernel/time/clockevents.c
===================================================================
@@ -91,6 +91,24 @@
 	return IRQ_HANDLED;
 }
 
+/* I am so ashamed... */
+void (*tg3_hack_hook)(void);
+EXPORT_SYMBOL_GPL(tg3_hack_hook);
+
+#if (HZ == 1000)
+#define TG3_HACK_MASK (127)
+#elif (HZ == 250)
+#define TG3_HACK_MASK (31)
+#else /* HZ == 100 */
+#define TG3_HACK_MASK (7)
+#endif
+
+static inline void check_tg3_hack(void)
+{
+	if (tg3_hack_hook && ((jiffies & TG3_HACK_MASK) == 0))
+		(*tg3_hack_hook)();
+}
+
 /*
  * Handle tick
  */

@@ -99,6 +117,8 @@
 	write_seqlock(&xtime_lock);
 	do_timer(regs);
 	write_sequnlock(&xtime_lock);
+
+	check_tg3_hack();
 	timeofday_overflow_protection();
 }
 

@@ -111,6 +131,7 @@
 	do_timer(regs);
 	write_sequnlock(&xtime_lock);
 
+	check_tg3_hack();
 	update_process_times(user_mode(regs));
 }
 

@@ -123,6 +144,7 @@
 	do_timer(regs);
 	write_sequnlock(&xtime_lock);
 
+	check_tg3_hack();
 	update_process_times(user_mode(regs));
 	profile_tick(CPU_PROFILING, regs);
 }

You probably should resend the patch to netdev and Michael Chan <[email protected]>. He might have ideas on how this could be avoided.

Cheers,

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

This only shows up with the real-time kernel where timer softirq's run in their own processes, and a high priority process preempts the timer softirq. I don't really consider this a networking bug, or even driver bug, although it does seem unfortunate that Broadcom hardware locks up and goes unresponsive if the OS doesn't tickle it every tenth of a second or so. (Definitely a bad idea if the tg3 gets used on any laptops, from a power usage perspective.) But that seems like a (lame) hardware bug, not a driver bug....

- Ted

Interesting. On my Dell D610 notebook with tg3 and vpn, I have to ping a server on the vpn to keep it alive, otherwise it disappears soon and I have to restart the vpn. Of course, this could just be the vpn or some other software problem instead of a tg3 problem.

---
~Randy

There is some form of priority inheritance on the timer softirq. It said in the patch header that the right fix was for the timer softirq to change priorities. Which Real Time patch are you using? Or is the current system not sufficient ?

Daniel

On Thursday 03 August 2006 17:49, Randy.Dunlap wrote:
[snip]

Probably. I have an NC6000 with a tg3 and have never experienced link failure problems, even under -rt.

Cheers,
Alistair.

Final year Computer Science undergraduate.
1F2 55 South Clerk Street, Edinburgh, UK.

We're using a someone older version of the CONFIG_PREEMPT_RT patches (2.6.16-rt22, with various bug fixes pulled up to what we are running.)

There is priority inheritance on the hrtimers, but not on normal timers, and conversations with Thomas and Stephen at OLS indicated this is on the wishlist, but it has not happened yet. As I mentioned in the patch comments, I looked at hacking hrtimers into the tg3 driver code, but (a) the hrtimers code assume that the priority inheritance happens when a process is associated with the hrtimer and the process is a high priority process, and (b) the hrtimers code aren't exported for use by modules. So I went with a very quick hack, since we have a hard code freeze for a customer deliverable.

In the long term, we're going to need something a bit more sophisticated than what we have in the hrtimers code, since not all code which requests timers are necessarily associated with a process. The tg3_timer() code, for example, is trigger by the device driver but isn't associated with a process for boosting purposes, and creating a process just so that tg3_timer() can be boosted seems like the Wrong Thing.

In addition, the timer wheel code has a *large* number of timers that get added and then removed without ever getting expired by the TCP networking code, and I'm not at all convinced that the technique used for doing prio boosting for the hrtimers will scale to what is needed for normal timers.

- Ted

That sounds almost certainly like a VPN problem. The tg3_timer() code wakes up every second or tenth of a second (depending on which mode you're in) and takes care of keeping the tg3 hardware mollified. On a standard kernel, this shouldn't ever be an issue. For the -rt kernel, this problem only shows up if you have enough tasks running at rtprio's above the rtprio of the softirq-timer for long enough that tg3 chip gets angry....

- Ted

Ted, what tg3 hardware is having this timer related problem? Can you send me the tg3 probing output?

tg3.c:v3.49 (Feb 2, 2006)
ACPI: PCI Interrupt 0000:02:01.0[A] -> GSI 24 (level, low) -> IRQ 17
eth0: Tigon3 [partno(BCM95704s) rev 2100 PHY(serdes)] (PCIX:100MHz:64-bit) 10/100/1000BaseT Ethernet 00:14:5e:86:44:24
eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[1] Split[0] WireSpeed[0] TSOcap[0]
eth0: dma_rwctrl[769f4000] dma_mask[64-bit]

02:01.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5704S Gigabit Ethernet (rev 10)
Subsystem: IBM: Unknown device 0301
Flags: bus master, 66Mhz, medium devsel, latency 64, IRQ 17
Memory at efff0000 (64-bit, non-prefetchable) [size=64K]
Capabilities: [40] PCI-X non-bridge device.
Capabilities: [48] Power Management version 2
Capabilities: [50] Vital Product Data
Capabilities: [58] Message Signalled Interrupts: 64bit+ Queue=0/3 Enable-

The other interesting bit of information is that after networking card goes dead and I do the "ifdown eth0; ifup eth0", the following printk shows up:

tg3: tg3_abort_hw timed out for eth0, TX_MODE_ENABLE will not clear MAC_TX_MODE=ffffffff

This is from an IBM LS-20 blade.

Is this helpful?

- Ted

From: Alistair John Strachan <[email protected]>
Date: Thu, 3 Aug 2006 18:04:55 +0100

And note that the "poke the chip N times a second to avoid lockup" issue only matters on very very old tg3 chips.

Therefore I think Ted fixed his problem by accident, as very few people in the world actually have tg3 chips old enough to need that periodic poking.

From: Theodore Tso <[email protected]>
Date: Thu, 3 Aug 2006 13:17:31 -0400

Ted please make sure the tg3 chips you have actually do need that periodic poking code that tg3_timer() has, most chips do not.

You don't need the periodic poke unless TG3_FLAG_TAGGED_STATUS is cleared, and that is only the case for two chips 1) 5700 and 2) 5788.

The only thing left is the link status and that is not so concerned about mild forms of latency in the timer firing.

From: Theodore Tso <[email protected]>
Date: Thu, 3 Aug 2006 16:17:41 -0400

The 5704 chip will set TG3_FLAG_TAGGED_STATUS, and therefore doesn't need the periodic poking done by tg3_timer().

True. But they also have ASF enabled which requires tg3_timer() to send the heartbeat periodically. If the heartbeat is late, ASF may reset the chip believing that the system has crashed.

We'll see if we can do away with the timer-based heartbeat. That's probably the best solution.

From: "Michael Chan" <[email protected]>
Date: Thu, 03 Aug 2006 16:28:19 -0700

The tg3 driver is not the only device in the world that requires a timer based "ping" to work. The watchdog drivers and the softlockup detector are other instances which require a timer to not be delayed an unreasonable amount of time.

Therefore TG3 is not unique in this regard, and I thus don't think it's worthwhile to change tg3 just to accomodate this broken behavior of the RT patches.

Hmm.... all I can say is that I could reliably knock the box off the network by running a four processes that tied up all CPU's at high real-time priorities, and after I applied the horrible hack that guaranteed that tg3_timer() was run every 0.128 seconds, the system stayed on the network. I'm not sure why, but it did fix the problem.

Any suggestions on how I could figure out what was really going on and what would be a better fix would be greatly appreciated.

- Ted

From: Theodore Tso <[email protected]>
Date: Thu, 3 Aug 2006 19:53:26 -0400

As Michael explained, it's the ASF heartbeat sent by tg3_timer() that must be delivered to the chip within certain timing constraints.

If you had any watchdog devices on this machine, they would likely trigger too and reset your machine :)

Watchdogs usually require one heartbeat every 30 seconds or so. Does the ASF heartbeat need to be that frequent?

Cheers,

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

From: Herbert Xu <[email protected]>
Date: Fri, 4 Aug 2006 09:59:27 +1000

The ASF heartbeat needs to be sent every 2 seconds.

That's not broken behavior in RT .. That's just plain old task priorities. Some high priority task (SCHED_FIFO prio 99) is sucking up a lot of the CPU. But that's 100% legal in SCHED_FIFO.

Daniel

Removing the timer-based "ping" might be a good thing to do from the point of view of reducing power utilization of laptops (but hey, I don't have a tg3 in my laptop, so I won't worry about it a whole lot :-), but I agree that in general the RT patches need to be able to call functions such as tg3_timer() reliably even when under a high real-time process workload, without needing to use the blunt hammer of "chrt -f 95 pidof softirq-timer". (Since not all timer callbacks need to be run at rt prio 95.)

- Ted

Yep, we send it every 2 seconds and it will reset in 5 seconds after the last heartbeat. So the margin is 3 seconds. These numbers are somewhat arbitrary and the goal is to allow ASF to function properly without too much delay after the system has crashed.

From: Theodore Tso <[email protected]>
Date: Thu, 3 Aug 2006 20:07:07 -0400

You won't have "ASF" in your laptop tg3. ASF, as the second character it's acronym implies is for "Servers".

I suppose the timer subsystem needs a "I'd like to have this timer called at time X, but it's ok to call it later until time X+Y" option; that's useful for RT like stuff but also for power savings... (eg you can batch timer firings that way a lot better)

Parden me for asking a dumb question, but what's being accomplished by resetting the chip if the system has crashed? Why not reset the chip when the system reboots and it sees the PCI bus reset? I guess I'm missing the purpose of the ASF heartbeat; why does the networking chip need a chip-specific watchdog?

- Ted

ASF is firmware that monitors the system and sends out alerts whenever certain events happen. So it needs to run before the OS boots or after it has crashed. When the driver is up and running, the driver and ASF run independently sending and receiving traffic on the same wire. Of course, the bandwidth that is used by ASF is a very tiny fraction of the host traffic. If the system crashes, the FIFO and other resources on the NIC will be backed up and ASF can no longer function without resetting
the chip.

As David explained, ASF is only used on servers.

Thanks, that description was very helpful. Would you accept a patch with adding a comment describing this? I couldn't figure it out from looking at the source and googling "ASF" turned up lots of other uses for that particular acronym.

It appears that there is no way of disabling ASF; is that a true statement?

Thanks, regards,

- Ted

Ted,

I took a quick look at the tg3 driver and that timer interrupt as well have read this thread. My suggestion would be to separate that tg3_timer into 4 different timers, which is what it actually looks like. One I believe (the first one) is an actual timeout and the other three are timers that are expected to be triggered everytime (watchdog like: at 1 second, 2 seconds and 3 seconds) and thus should be converted into a hrtimers that goes off when expected then having some crazy accounting thing in one timer.

I don't fully understand the ASF part here, and if it definitely needs to go off then set that timer with the highest prio. I've ask Thomas to add a way to add a hrtimer with a given prio instead of always taking the current->normal_prio. But until he does that, you can do a hack of changing the current prio to something very high (like 99) then start the timer, and then lower the prio back to what it was. This is a hack, but it might lead to a better solution in the future.

-- Steve

From: Steven Rostedt <[email protected]>
Date: Mon, 7 Aug 2006 01:34:56 -0400 (EDT)

Timers have non-trivial cost. It's cheaper to have one and vector off to the necessary operations each tick internalls.

That's why it's implemented as one timer.

I will put it on my queue to add some comments for ASF.

Turning off ASF is just a matter of changing some bits in NVRAM and recalculating the checksum. If you need the tool to do this, I'll have someone send it to you.

Note that on some of the blade servers, I believe ASF is vital and should not be disabled.

hrtimers don't have the cost of a normal timer. And that's why I suggested to convert them. There's a much bigger cost in a single timer that always times out than 3 hrtimers. hrtimers are expected to timeout, but timers are not.

Of the 4 timers, only one is a timeout. The other three expire every time, forcing the timer wheel into effect. Even though it's one timer implementing 4, it's expensive to use it as a watchdog.

-- Steve

I just got a chance to look a little more deeper at what the tg3 timer is doing, and I was wrong. The timeout is not a timeout but some messing around when the network card doesn't use tagged status (whatever that is). Which just pushes the point that this should _not_ be a timer, but a hrtimer (expected to expire).

So you can keep this as one timer, but I would still switch it to a hrtimer regardless, since it is expected to timeout. (maybe separate out the ASF if that still needs to be special?).

Ted,

I don't know what the max latency of that timer is, (I'm sure it wouldn't be too hard to measuer, just add some timings around the timer handler, let it run for a while and keep account of the max time). But, since the user that opens this network card is the one that initializes the timer, if you simply switch the timer to be a hrtimer (that should also go in mainline) and then have a really high prio task start up the network, that timer would then run at the prio of the task that started the network.

-- Steve

From: Steven Rostedt <[email protected]>
Date: Tue, 8 Aug 2006 08:24:10 -0400 (EDT)

It's not a watchdog, the timer continually fires.

It is the on-chip ASF firmware that "times out" if it does not see the heartbeat message from the driver within 5 seconds.

The driver timer in question runs every 2 seconds to write this heartbeat message to the chip.

Right, I should have used the term "heartbeat" instead :)

OK, but still, if it is expected to expire, which this one is, then it should be converted to a hrtimer, instead of a normal timer. The hrtimers were introduced to make it more efficient for expiring timers. Well, they really were introduced for high resolution, but in order to do that the expiring timer implementation needed to be improved. The timer wheel is most efficient for those timers that are not expected to time out (probably what a watchdog timer would actually do, so my terminology was really incorrect).

The timer wheel is O(1) in removing and adding a timer, but can be O(n) if the timers cascade (which would happen when a timer moves to expire).

The hrtimers are O(1) in expiring but O(log n) on adding and removing. So they are better to use when you know that the timer will expire.

This also help's Ted's case for the -rt patch since the hrtimers there have a dynamic priority.

-- Steve

Hi,

Can we please, please, please stop with this? While this is correct, it's only rarely relevant for most users. The cascading is really not as bad it's always represented here.
You need a huge amount of timers pending to run into trouble and at this point hrtimers won't behave much better. In this case it's better to do what Dave suggested - use a single continous timer and vector off the needed work from there.
The primary reason to use hrtimers should only be whether or not the high resolution is needed. hrtimer can have a significant higher fixed cost (due to a possibly expensive get_time() operation), so one should be a little careful with them. Possible problems in any of the timer systems should hardly be a concern for most users of it, at the point the timer system can't handle this many timers, we should rather look at the user and fix the problem there.

bye, Roman

Still, it would be great if ASF could be disabled, because I have noticed that when ASF is enabled, the tg3 driver automatically disables TSO (TCP Segmentation Offloading). Here is a dmesg output from a server where I am seeing that behavior:

  eth0: Tigon3 [partno(BCM95704A6) rev 2100 PHY(5704)] (PCIX:133MHz:64-bit) \
    10/100/1000BaseT Ethernet 00:30:48:59:c4:94
  eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[1] Split[0] WireSpeed[1] TSOcap[0]
  [...]
  eth1: Tigon3 [partno(BCM95704A6) rev 2100 PHY(5704)] (PCIX:133MHz:64-bit) \
    10/100/1000BaseT Ethernet 00:30:48:59:c4:95
  eth1: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] Split[0] WireSpeed[1] TSOcap[1]

Both interfaces are fundamentally TSO-capable, but since ASF is enabled on eth0, tg3 disables TSO on this interface. Of course at this point it is not even possible to use ethtool to re-enable it because the driver considers eth0 as not TSO-capable at all.

As far as I know, the tg3 driver has been doing that since one of your patches shipped with 2.6.11-rc2-bk3, Michael, see [1]. Here is the relevant code snippet (line numbers are for 2.6.16):

  10835   if (tp->tg3_flags2 & TG3_FLG2_HW_TSO) {
  10836           tp->tg3_flags2 |= TG3_FLG2_TSO_CAPABLE;
  10837   }
  10838   else if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5700 ||
  10839       GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5701 ||
  10840       tp->pci_chip_rev_id == CHIPREV_ID_5705_A0 ||
  10841       (tp->tg3_flags & TG3_FLAG_ENABLE_ASF) != 0) {
  10842           tp->tg3_flags2 &= ~TG3_FLG2_TSO_CAPABLE;
  10843   } else {
  10844           tp->tg3_flags2 |= TG3_FLG2_TSO_CAPABLE;
  10845   }

The culprit is line 10841. Why is that done ?

[1] ftp://ftp.us.kernel.org:/pub/linux/kernel/v2.6/snapshots/old/

    patch-2.6.11-rc2-bk3.log, patch-2.6.11-rc2-bk3.bz2

--
Marc Bevand

The reason is that TSO on 5704 and older chips is done by firmware. ASF is also implemented by firmware. If ASF is enabled, there is no room to do TSO and ASF at the same time.

Firmware-based TSO is actually slower than no TSO. The only benefit is a little better CPU utilization.

On the newer chips, TSO is done by hardware which solves both the ASF and throughput problems.

Just for test purpose, I have applied the following patch to my tg3.c. I now seem to be able to enable/disable TSO, but I admit don't know whether ASF is still functional or not.

       else if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5700 ||
           GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5701 ||
-           tp->pci_chip_rev_id == CHIPREV_ID_5705_A0 ||
-           (tp->tg3_flags & TG3_FLAG_ENABLE_ASF) != 0) {
+           tp->pci_chip_rev_id == CHIPREV_ID_5705_A0) {
               tp->tg3_flags2 &= ~TG3_FLG2_TSO_CAPABLE;
       } else {

Then tg3 considered my interface as TSO-capable ("TSOcap[1]" in dmesg). TSO was still disabled by default, which is normal because there is this other check a couple of lines below:

       /* TSO is on by default on chips that support hardware TSO.
        * Firmware TSO on older chips gives lower performance, so it
        * is off by default, but can be enabled using ethtool.
        */
       if (tp->tg3_flags2 & TG3_FLG2_HW_TSO)
               dev->features |= NETIF_F_TSO;

But I was able to turn TSO on via ethtool -K. This is exactly the behavior I would like to see in tg3. So are you saying the patch I applied actually breaks ASF ?

I know, in one of my test-cases, firmware TSO reduces the max achievable TCP bandwidth from 930 to 840 Mbit/s on a GigE network while reducing the CPU utilization from 44% to 22%. I think firmware TSO still makes sense in some cases.

Marc Bevand

Yes, the TSO firmware code that tg3 downloads to the chip using your patch will overwrite the ASF code. The ASF code will get reloaded again at the next chip reset once your patch is reverted.