partial resend: e1000 fixes and updates

PATCH In review Kok, Auke 2 participants 12 messages Jan 13, 2007 – Jan 23, 2007

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Kok, Auke

author

2007-01-13 00:54 UTC #0 lore raw

cover partial resend: e1000 fixes and updates

Hi,

This patch series contains exclusively fixes for e1000. Some of these patches were already sent in december, but didn't make it into any usptream tree yet. Most importantly, it addresses two issues in the recently merged msi interrupt handler and dynamic itr code. A performance fix and some minor cleanups are also added. This brings the driver up to version 7.3.20-k2.

The summary below lists all patches. Once that were previously acked are annotated with (*)

These patches apply against netdev-2.6 #upstream-linus commit 77aab8bf22042d1658d4adbca8b71779e7f2d0ff. Please pull:

git pull git://lost.foo-projects.org/~ahkok/git/netdev-2.6 upstream-linus

to receive them.

Cheers,

Auke

---
Bruce Allan <[email protected]>:
e1000: clear ip csum info from context descriptor (*)

Jesse Brandeburg <[email protected]>:
e1000: simplify case handling gigabit at half duplex (*)
e1000: Fix MSI only interrupt handler routine
e1000: fix NAPI performance on 4-port adapters (*)
e1000: tune our dynamic itr transmit packet accounting

Auke Kok <[email protected]>:
e1000: clean up debug output defines
e1000: display flow control of link status at link up (*)
e1000: update version to 7.3.20-k2

---
 drivers/net/e1000/e1000.h       |    1
 drivers/net/e1000/e1000_main.c  |  102 ++++++++++++++++++----------------------
 drivers/net/e1000/e1000_osdep.h |    4 -
 drivers/net/e1000/e1000_param.c |   15 +----
 4 files changed, 51 insertions(+), 71 deletions(-)

---
Auke Kok <[email protected]>

Kok, Auke

author

2007-01-13 00:54 UTC #1 lore raw

PATCH 1/8 e1000: simplify case handling gigabit at half duplex

Remvoe duplicate code handling erraneous user supplied wrong case of gigabit speed with half duplex.

Signed-off-by Jesse Brandeburg <[email protected]>

Signed-off-by Auke Kok <[email protected]>

---

 drivers/net/e1000/e1000_param.c |   15 +++------------
 1 files changed, 3 insertions(+), 12 deletions(-)

drivers/net/e1000/e1000_param.c +3 ~~-12~~

index cf2a279..f8862e2 100644
@@ -760,22 +760,13 @@ e1000_check_copper_options(struct e1000_adapter *adapter)
 	case SPEED_1000:
 		DPRINTK(PROBE, INFO, "1000 Mbps Speed specified without "
 			"Duplex\n");
-		DPRINTK(PROBE, INFO,
-			"Using Autonegotiation at 1000 Mbps "
-			"Full Duplex only\n");
-		adapter->hw.autoneg = adapter->fc_autoneg = 1;
-		adapter->hw.autoneg_advertised = ADVERTISE_1000_FULL;
-		break;
+		goto full_duplex_only;
 	case SPEED_1000 + HALF_DUPLEX:
 		DPRINTK(PROBE, INFO,
 			"Half Duplex is not supported at 1000 Mbps\n");
-		DPRINTK(PROBE, INFO,
-			"Using Autonegotiation at 1000 Mbps "
-			"Full Duplex only\n");
-		adapter->hw.autoneg = adapter->fc_autoneg = 1;
-		adapter->hw.autoneg_advertised = ADVERTISE_1000_FULL;
-		break;
+		/* fall through */
 	case SPEED_1000 + FULL_DUPLEX:
+full_duplex_only:
 		DPRINTK(PROBE, INFO,
 		       "Using Autonegotiation at 1000 Mbps Full Duplex only\n");
 		adapter->hw.autoneg = adapter->fc_autoneg = 1;

---
Auke Kok <[email protected]>

Kok, Auke

author

2007-01-13 00:54 UTC #2 lore raw

PATCH 2/8 e1000: clean up debug output defines

In reply to Kok, Auke

Remove unused MSGOUT macro and add "\n" to function debug output.

Signed-off-by Auke Kok <[email protected]>

---

 drivers/net/e1000/e1000_osdep.h |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

drivers/net/e1000/e1000_osdep.h +1 -3

index 18afc0c..10af742 100644
@@ -48,8 +48,6 @@ typedef enum {
     TRUE = 1
 } boolean_t;
 
-#define MSGOUT(S, A, B)	printk(KERN_DEBUG S "\n", A, B)
-
 #ifdef DBG
 #define DEBUGOUT(S)		printk(KERN_DEBUG S "\n")
 #define DEBUGOUT1(S, A...)	printk(KERN_DEBUG S "\n", A)

@@ -58,7 +56,7 @@ typedef enum {
 #define DEBUGOUT1(S, A...)
 #endif
 
-#define DEBUGFUNC(F) DEBUGOUT(F)
+#define DEBUGFUNC(F) DEBUGOUT(F "\n")
 #define DEBUGOUT2 DEBUGOUT1
 #define DEBUGOUT3 DEBUGOUT2
 #define DEBUGOUT7 DEBUGOUT3

---
Auke Kok <[email protected]>

Kok, Auke

author

2007-01-13 00:54 UTC #3 lore raw

PATCH 3/8 e1000: Fix MSI only interrupt handler routine

In reply to Kok, Auke

Unfortunately the read-free MSI interrupt handler needs to flush write the icr register and thus we can't be read-free. Our MSI irq routine thus becomes a lot more simpler since we don't need to track link state anymore.

Signed-off-by Jesse Brandeburg <[email protected]>

Signed-off-by Auke Kok <[email protected]>

---

 drivers/net/e1000/e1000.h      |    1 -
 drivers/net/e1000/e1000_main.c |   60 +++++++++++++---------------------------
 2 files changed, 19 insertions(+), 42 deletions(-)

drivers/net/e1000/e1000.h +0 -1

index f091042..8e7acb0 100644
@@ -257,7 +257,6 @@ struct e1000_adapter {
 	spinlock_t tx_queue_lock;
 #endif
 	atomic_t irq_sem;
-	unsigned int detect_link;
 	unsigned int total_tx_bytes;
 	unsigned int total_tx_packets;
 	unsigned int total_rx_bytes;

drivers/net/e1000/e1000_main.c +19 ~~-41~~

76 lines not on this page · raw message

---
Auke Kok <[email protected]>

Kok, Auke

author

2007-01-13 00:54 UTC #4 lore raw

PATCH 4/8 e1000: fix NAPI performance on 4-port adapters

In reply to Kok, Auke

This fix attempts to solve a customer (IBM) reported issue with NAPI enabled e1000 having bad performance when transmitting simultaneously on four ports. The issue comes down to an interaction between NAPI, hardware interrupt balancing, and the driver rescheduling poll on the same processor. Try to fix by allowing the driver to re-enable interrupts sooner instead of polling one more time, when there was recently all the work completed in cleanup.

Signed-off-by Jesse Brandeburg <[email protected]>

Signed-off-by Auke Kok <[email protected]>

---

 drivers/net/e1000/e1000_main.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

drivers/net/e1000/e1000_main.c +8 -5

index d408949..ab1b40f 100644
@@ -3814,7 +3814,7 @@ e1000_intr_msi(int irq, void *data)
 
 	for (i = 0; i < E1000_MAX_INTR; i++)
 		if (unlikely(!adapter->clean_rx(adapter, adapter->rx_ring) &
-		   !e1000_clean_tx_irq(adapter, adapter->tx_ring)))
+		   e1000_clean_tx_irq(adapter, adapter->tx_ring)))
 			break;
 
 	if (likely(adapter->itr_setting & 3))

@@ -3917,7 +3917,7 @@ e1000_intr(int irq, void *data)
 
 	for (i = 0; i < E1000_MAX_INTR; i++)
 		if (unlikely(!adapter->clean_rx(adapter, adapter->rx_ring) &
-		   !e1000_clean_tx_irq(adapter, adapter->tx_ring)))
+		   e1000_clean_tx_irq(adapter, adapter->tx_ring)))
 			break;
 
 	if (likely(adapter->itr_setting & 3))

@@ -3967,7 +3967,7 @@ e1000_clean(struct net_device *poll_dev, int *budget)
 	poll_dev->quota -= work_done;
 
 	/* If no Tx and not enough Rx work done, exit the polling mode */
-	if ((!tx_cleaned && (work_done == 0)) ||
+	if ((tx_cleaned && (work_done < work_to_do)) ||
 	   !netif_running(poll_dev)) {
 quit_polling:
 		if (likely(adapter->itr_setting & 3))

@@ -3997,7 +3997,7 @@ e1000_clean_tx_irq(struct e1000_adapter *adapter,
 #ifdef CONFIG_E1000_NAPI
 	unsigned int count = 0;
 #endif
-	boolean_t cleaned = FALSE;
+	boolean_t cleaned = TRUE;
 	unsigned int total_tx_bytes=0, total_tx_packets=0;
 
 	i = tx_ring->next_to_clean;

@@ -4028,7 +4028,10 @@ e1000_clean_tx_irq(struct e1000_adapter *adapter,
 #ifdef CONFIG_E1000_NAPI
 #define E1000_TX_WEIGHT 64
 		/* weight of a sort for tx, to avoid endless transmit cleanup */
-		if (count++ == E1000_TX_WEIGHT) break;
+		if (count++ == E1000_TX_WEIGHT) {
+			cleaned = FALSE;
+			break;
+		}
 #endif
 	}

---
Auke Kok <[email protected]>

Kok, Auke

author

2007-01-13 00:54 UTC #5 lore raw

PATCH 5/8 e1000: display flow control of link status at link up

In reply to Kok, Auke

Print RX/TX flow control setting at link up time to display the actual link FC properties instead of the advertised values.

Signed-off-by Auke Kok <[email protected]>

---

 drivers/net/e1000/e1000_main.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

drivers/net/e1000/e1000_main.c +11 -4

index ab1b40f..43dde27 100644
@@ -2583,15 +2583,22 @@ e1000_watchdog(unsigned long data)
 
 	if (link) {
 		if (!netif_carrier_ok(netdev)) {
+			uint32_t ctrl;
 			boolean_t txb2b = 1;
 			e1000_get_speed_and_duplex(&adapter->hw,
 			                           &adapter->link_speed,
 			                           &adapter->link_duplex);
 
-			DPRINTK(LINK, INFO, "NIC Link is Up %d Mbps %s\n",
-			       adapter->link_speed,
-			       adapter->link_duplex == FULL_DUPLEX ?
-			       "Full Duplex" : "Half Duplex");
+			ctrl = E1000_READ_REG(&adapter->hw, CTRL);
+			DPRINTK(LINK, INFO, "NIC Link is Up %d Mbps %s, "
+			        "Flow Control: %s\n",
+			        adapter->link_speed,
+			        adapter->link_duplex == FULL_DUPLEX ?
+			        "Full Duplex" : "Half Duplex",
+			        ((ctrl & E1000_CTRL_TFCE) && (ctrl &
+			        E1000_CTRL_RFCE)) ? "RX/TX" : ((ctrl &
+			        E1000_CTRL_RFCE) ? "RX" : ((ctrl &
+			        E1000_CTRL_TFCE) ? "TX" : "None" )));
 
 			/* tweak tx_queue_len according to speed/duplex
 			 * and adjust the timeout factor */

---
Auke Kok <[email protected]>

Kok, Auke

author

2007-01-13 00:54 UTC #6 lore raw

PATCH 6/8 e1000: clear ip csum info from context descriptor

In reply to Kok, Auke

Since the driver sets the IP checksum insertion bit (IXSM in Status field) in transmit context descriptors, it should clear the IP checksum bits of any garbage so as not to confuse the hardware.

Signed-off-by Bruce Allan <[email protected]>

Signed-off-by Auke Kok <[email protected]>

---

 drivers/net/e1000/e1000_main.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

drivers/net/e1000/e1000_main.c +2 -1

index 43dde27..8c6c74d 100644
@@ -2975,8 +2975,9 @@ e1000_tx_csum(struct e1000_adapter *adapter, struct e1000_tx_ring *tx_ring,
 		buffer_info = &tx_ring->buffer_info[i];
 		context_desc = E1000_CONTEXT_DESC(*tx_ring, i);
 
+		context_desc->lower_setup.ip_config = 0;
 		context_desc->upper_setup.tcp_fields.tucss = css;
-		context_desc->upper_setup.tcp_fields.tucso = css + skb->csum_offset;
+		context_desc->upper_setup.tcp_fields.tucso = css + skb->csum;
 		context_desc->upper_setup.tcp_fields.tucse = 0;
 		context_desc->tcp_seg_setup.data = 0;
 		context_desc->cmd_and_length = cpu_to_le32(E1000_TXD_CMD_DEXT);

---
Auke Kok <[email protected]>

Kok, Auke

author

2007-01-13 00:54 UTC #7 lore raw

PATCH 7/8 e1000: tune our dynamic itr transmit packet accounting

In reply to Kok, Auke

The driver was still mis-calculating the number of bytes sent during transmit, now the driver computes what appears to be exactly 100% correct byte counts (not including CRC) when figuring out how many bytes and frames were sent during the current transmit packet.
---

 drivers/net/e1000/e1000_main.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

drivers/net/e1000/e1000_main.c +6 -3

index 8c6c74d..ae76479 100644
@@ -4020,10 +4020,13 @@ e1000_clean_tx_irq(struct e1000_adapter *adapter,
 
 			if (cleaned) {
 				struct sk_buff *skb = buffer_info->skb;
-				unsigned int segs = skb_shinfo(skb)->gso_segs;
+				unsigned int segs, bytecount;
+				segs = skb_shinfo(skb)->gso_segs ?: 1;
+				/* multiply data chunks by size of headers */
+				bytecount = ((segs - 1) * skb_headlen(skb)) +
+				            skb->len;
 				total_tx_packets += segs;
-				total_tx_packets++;
-				total_tx_bytes += skb->len;
+				total_tx_bytes += bytecount;
 			}
 			e1000_unmap_and_free_tx_resource(adapter, buffer_info);
 			tx_desc->upper.data = 0;

---
Auke Kok <[email protected]>

Kok, Auke

author

2007-01-13 00:54 UTC #8 lore raw

PATCH 8/8 e1000: update version to 7.3.20-k2

In reply to Kok, Auke

Signed-off-by Auke Kok <[email protected]>

---

 drivers/net/e1000/e1000_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/net/e1000/e1000_main.c +1 -1

index ae76479..2035ca4 100644
@@ -36,7 +36,7 @@ static char e1000_driver_string[] = "Intel(R) PRO/1000 Network Driver";
 #else
 #define DRIVERNAPI "-NAPI"
 #endif
-#define DRV_VERSION "7.3.15-k2"DRIVERNAPI
+#define DRV_VERSION "7.3.20-k2"DRIVERNAPI
 char e1000_driver_version[] = DRV_VERSION;
 static char e1000_copyright[] = "Copyright (c) 1999-2006 Intel Corporation.";

---
Auke Kok <[email protected]>

Jeff Garzik

2007-01-18 16:56 UTC #9 lore raw

In reply to Kok, Auke

Quoted 8 lines · Auke

Hi,
This patch series contains exclusively fixes for e1000. Some of these patches were already sent in december, but didn't make it into any usptream tree yet. Most importantly, it addresses two issues in the recently merged msi interrupt handler and dynamic itr code. A performance fix and some minor cleanups are also added. This brings the driver up to version 7.3.20-k2.

Quoted 7 lines

The summary below lists all patches. Once that were previously acked are annotated with (*)
These patches apply against netdev-2.6 #upstream-linus commit 77aab8bf22042d1658d4adbca8b71779e7f2d0ff. Please pull:
git pull git://lost.foo-projects.org/~ahkok/git/netdev-2.6 upstream-linus

Sigh. I /know/ I've told you this before, but let's review the branches again. NEVER EVER use branch 'upstream-linus'. That is for Linus only.

There are three branches you should care about:

master Vanilla upstream Linus tree, as of my last pull
upstream-fixes netdev fixes being sent to Linus for -rc
upstream Everything else pending, but not yet merged

The decision tree is as follows:

* If there are no dependent e1000 changes, send patches against master (or torvalds/linux-2.6.git, if you prefer)
* If there are dependent e1000 changes in #upstream-fixes, send patches against #upstream-fixes
* If there are dependent e1000 changes in #upstream, send patches against #upstream

In the current tree there are no e1000 changes, so please regenerate your tree against vanilla linux-2.6.git and resubmit.

I ACK patches 1-8.

Jeff

Auke Kok

author

2007-01-18 17:16 UTC #10 lore raw

Quoted 12 lines · Jeff Garzik

Quoted 11 lines · Auke
Hi,
This patch series contains exclusively fixes for e1000. Some of these patches were already sent in december, but didn't make it into any usptream tree yet. Most importantly, it addresses two issues in the recently merged msi interrupt handler and dynamic itr code. A performance fix and some minor cleanups are also added. This brings the driver up to version 7.3.20-k2.
The summary below lists all patches. Once that were previously acked are annotated with (*)

Quoted 6 lines

Quoted 4 lines
These patches apply against netdev-2.6 #upstream-linus commit 77aab8bf22042d1658d4adbca8b71779e7f2d0ff. Please pull:
git pull git://lost.foo-projects.org/~ahkok/git/netdev-2.6 upstream-linus
Sigh. I /know/ I've told you this before, but let's review the branches again. NEVER EVER use branch 'upstream-linus'. That is for Linus only.

gah, sorry about that.

I've moved it all over to #master-e1000 which applies against bf81b46482c0fa8ea638e409d39768ea92a6b0f0 ("Linux 2.6.20-rc4 --Linus Torvalds").

Please pull:

git pull git://lost.foo-projects.org/~ahkok/git/netdev-2.6 master-e1000

Thanks,

Auke

Quoted 3 lines

* If there are no dependent e1000 changes, send patches against master (or torvalds/linux-2.6.git, if you prefer)
* If there are dependent e1000 changes in #upstream-fixes, send patches against #upstream-fixes
* If there are dependent e1000 changes in #upstream, send patches against #upstream

*writes cheatsheet*

Jeff Garzik

2007-01-23 21:31 UTC #11 lore raw

Quoted 20 lines · Auke Kok

Quoted 12 lines · Jeff Garzik
Quoted 11 lines · Auke
Hi,
This patch series contains exclusively fixes for e1000. Some of these patches were already sent in december, but didn't make it into any usptream tree yet. Most importantly, it addresses two issues in the recently merged msi interrupt handler and dynamic itr code. A performance fix and some minor cleanups are also added. This brings the driver up to version 7.3.20-k2.
The summary below lists all patches. Once that were previously acked are annotated with (*)
Quoted 6 lines
Quoted 4 lines
These patches apply against netdev-2.6 #upstream-linus commit 77aab8bf22042d1658d4adbca8b71779e7f2d0ff. Please pull:
git pull git://lost.foo-projects.org/~ahkok/git/netdev-2.6 upstream-linus
Sigh. I /know/ I've told you this before, but let's review the branches again. NEVER EVER use branch 'upstream-linus'. That is for Linus only.

Quoted 7 lines

gah, sorry about that.
I've moved it all over to #master-e1000 which applies against bf81b46482c0fa8ea638e409d39768ea92a6b0f0 ("Linux 2.6.20-rc4 --Linus Torvalds").
Please pull:
git pull git://lost.foo-projects.org/~ahkok/git/netdev-2.6 master-e1000

Strike my last email.

Pulled into #upstream.

Jeff