[patch 00/14] E1000 bugfix series for 2.6.20

[patch 00/14] E1000 bugfix series for 2.6.20

[Arjan van de Ven]

Hi,

this patch series contains critical bugfixes to the e1000 driver for
2.6.20 (and 3 very low intrusive features that help in testing). Without
these patches e1000 is totally unusable on my test machine.

Greetings,
   Arjan van de Ven

[patch 01/14] e1000: The user-supplied itr setting needs the lower 2 bits masked off

[Arjan van de Ven]

Subject: e1000: The user-supplied itr setting needs the lower 2 bits masked off
From: Jesse Brandeburg <jesse.brandeburg@intel.com>

The lower 2 bits of a user-supplied itr setting (via ethtool) need to be
masked off: These lower two bits are used as control bits.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

---
 drivers/net/e1000/e1000_param.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/net/e1000/e1000_param.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_param.c
+++ linux-2.6/drivers/net/e1000/e1000_param.c
@@ -487,7 +487,9 @@ e1000_check_options(struct e1000_adapter
 				e1000_validate_option(&adapter->itr, &opt,
 				        adapter);
 				/* save the setting, because the dynamic bits change itr */
-				adapter->itr_setting = adapter->itr;
+				/* clear the lower two bits because they are
+				 * used as control */
+				adapter->itr_setting = adapter->itr & ~3;
 				break;
 			}
 		} else {

[patch 02/14] e1000: dynamic itr: take TSO and jumbo into account

[Arjan van de Ven]

Subject: e1000: dynamic itr: take TSO and jumbo into account
From: Jesse Brandeburg <jesse.brandeburg@intel.com>

The dynamic interrupt rate control patches omitted proper counting
for jumbo's and TSO resulting in suboptimal interrupt mitigation strategies.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---

 drivers/net/e1000/e1000_main.c |   43 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -2628,29 +2628,34 @@ static unsigned int e1000_update_itr(str
 	if (packets == 0)
 		goto update_itr_done;
 
-
 	switch (itr_setting) {
 	case lowest_latency:
-		if ((packets < 5) && (bytes > 512))
+		/* jumbo frames get bulk treatment*/
+		if (bytes/packets > 8000)
+			retval = bulk_latency;
+		else if ((packets < 5) && (bytes > 512))
 			retval = low_latency;
 		break;
 	case low_latency:  /* 50 usec aka 20000 ints/s */
 		if (bytes > 10000) {
-			if ((packets < 10) ||
-			     ((bytes/packets) > 1200))
+			/* jumbo frames need bulk latency setting */
+			if (bytes/packets > 8000)
+				retval = bulk_latency;
+			else if ((packets < 10) || ((bytes/packets) > 1200))
 				retval = bulk_latency;
 			else if ((packets > 35))
 				retval = lowest_latency;
-		} else if (packets <= 2 && bytes < 512)
+		} else if (bytes/packets > 2000)
+			retval = bulk_latency;
+		else if (packets <= 2 && bytes < 512)
 			retval = lowest_latency;
 		break;
 	case bulk_latency: /* 250 usec aka 4000 ints/s */
 		if (bytes > 25000) {
 			if (packets > 35)
 				retval = low_latency;
-		} else {
-			if (bytes < 6000)
-				retval = low_latency;
+		} else if (bytes < 6000) {
+			retval = low_latency;
 		}
 		break;
 	}
@@ -2679,17 +2684,20 @@ static void e1000_set_itr(struct e1000_a
 	                            adapter->tx_itr,
 	                            adapter->total_tx_packets,
 	                            adapter->total_tx_bytes);
+	/* conservative mode (itr 3) eliminates the lowest_latency setting */
+	if (adapter->itr_setting == 3 && adapter->tx_itr == lowest_latency)
+		adapter->tx_itr = low_latency;
+
 	adapter->rx_itr = e1000_update_itr(adapter,
 	                            adapter->rx_itr,
 	                            adapter->total_rx_packets,
 	                            adapter->total_rx_bytes);
+	/* conservative mode (itr 3) eliminates the lowest_latency setting */
+	if (adapter->itr_setting == 3 && adapter->rx_itr == lowest_latency)
+		adapter->rx_itr = low_latency;
 
 	current_itr = max(adapter->rx_itr, adapter->tx_itr);
 
-	/* conservative mode eliminates the lowest_latency setting */
-	if (current_itr == lowest_latency && (adapter->itr_setting == 3))
-		current_itr = low_latency;
-
 	switch (current_itr) {
 	/* counts and packets in update_itr are dependent on these numbers */
 	case lowest_latency:
@@ -3868,11 +3876,11 @@ e1000_clean_tx_irq(struct e1000_adapter 
 			cleaned = (i == eop);
 
 			if (cleaned) {
-				/* this packet count is wrong for TSO but has a
-				 * tendency to make dynamic ITR change more
-				 * towards bulk */
+				struct sk_buff *skb = buffer_info->skb;
+				unsigned int segs = skb_shinfo(skb)->gso_segs;
+				total_tx_packets += segs;
 				total_tx_packets++;
-				total_tx_bytes += buffer_info->skb->len;
+				total_tx_bytes += skb->len;
 			}
 			e1000_unmap_and_free_tx_resource(adapter, buffer_info);
 			tx_desc->upper.data = 0;

[patch 03/14] e1000: omit stats for broken counter in 82543

[Arjan van de Ven]

Subject: e1000: omit stats for broken counter in 82543
From: Jesse Brandeburg <jesse.brandeburg@intel.com>

The 82543 chip does not count tx_carrier_errors properly in FD mode;
report zeros instead of garbage.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---

 drivers/net/e1000/e1000_main.c |    5 +++++
 1 file changed, 5 insertions(+), 0 deletion(-)

Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -3580,7 +3580,12 @@ e1000_update_stats(struct e1000_adapter 
 	adapter->net_stats.tx_errors = adapter->stats.txerrc;
 	adapter->net_stats.tx_aborted_errors = adapter->stats.ecol;
 	adapter->net_stats.tx_window_errors = adapter->stats.latecol;
 	adapter->net_stats.tx_carrier_errors = adapter->stats.tncrs;
+	if (adapter->hw.mac_type == e1000_82543 &&
+	    adapter->link_duplex == FULL_DUPLEX) {
+		adapter->net_stats.tx_carrier_errors = 0;
+		adapter->stats.tncrs = 0;
+	}
 
 	/* Tx Dropped needs to be maintained elsewhere */
 

Re: [patch 03/14] e1000: omit stats for broken counter in 82543

[Jeff Garzik]

Arjan van de Ven wrote:
> Subject: e1000: omit stats for broken counter in 82543
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> The 82543 chip does not count tx_carrier_errors properly in FD mode;
> report zeros instead of garbage.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> ---
> 
>  drivers/net/e1000/e1000_main.c |    5 +++++
>  1 file changed, 5 insertions(+), 0 deletion(-)
> 
> Index: linux-2.6/drivers/net/e1000/e1000_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/e1000/e1000_main.c
> +++ linux-2.6/drivers/net/e1000/e1000_main.c
> @@ -3580,7 +3580,12 @@ e1000_update_stats(struct e1000_adapter 
>  	adapter->net_stats.tx_errors = adapter->stats.txerrc;
>  	adapter->net_stats.tx_aborted_errors = adapter->stats.ecol;
>  	adapter->net_stats.tx_window_errors = adapter->stats.latecol;
>  	adapter->net_stats.tx_carrier_errors = adapter->stats.tncrs;
> +	if (adapter->hw.mac_type == e1000_82543 &&
> +	    adapter->link_duplex == FULL_DUPLEX) {
> +		adapter->net_stats.tx_carrier_errors = 0;
> +		adapter->stats.tncrs = 0;
> +	}

Needs to use an "i have broken stats" feature flag, rather than adding 
yet another mac_type test into the code.  This testing of MAC type 
rather than feature flags is a major e1000 problem, and it bloats the 
driver quite a bit.  Intel has been told for /months/ this is a problem, 
yet I still see patches like this.

This is one of the comments I sent to Auke.

	Jeff

Re: [patch 03/14] e1000: omit stats for broken counter in 82543

[Arjan van de Ven]

Jeff Garzik wrote:
> Needs to use an "i have broken stats" feature flag, rather than adding 
> yet another mac_type test into the code.  This testing of MAC type 
> rather than feature flags is a major e1000 problem, and it bloats the 
> driver quite a bit.  Intel has been told for /months/ this is a problem, 
> yet I still see patches like this.
> 
it is "nice" that you say this, and Intel is working on a "flags" 
based driver. However that is, as you state yourself here, a major 
invasive change, and thus not suitable for 2.6.20 inclusion. Yet these 
fixes are important bugfixes; I don't think it's fair to hold these 
hostage..

Greetings,
   Arjan van de Ven

Re: [patch 03/14] e1000: omit stats for broken counter in 82543

[Jeff Garzik]

Arjan van de Ven wrote:
> Jeff Garzik wrote:
>> Needs to use an "i have broken stats" feature flag, rather than adding 
>> yet another mac_type test into the code.  This testing of MAC type 
>> rather than feature flags is a major e1000 problem, and it bloats the 
>> driver quite a bit.  Intel has been told for /months/ this is a 
>> problem, yet I still see patches like this.
>>
> it is "nice" that you say this, and Intel is working on a "flags" based 
> driver. However that is, as you state yourself here, a major invasive 
> change, and thus not suitable for 2.6.20 inclusion. Yet these fixes are 
> important bugfixes; I don't think it's fair to hold these hostage..

Completely false.  I /never/ said it was a major invasive change.

The following is obviously /not/ an invasive change, but rather a simple 
incremental approach:

1) Define "unsigned long flags" in your adapter struct
(only has to be done once)

2) For the management patch (patch #3?), define a flag 
E1000_FLG_I_HAVE_MGMT.

3) Set this flag everywhere patch #3 does a mac_type test.  Appears to 
be 2-4 locations TOUCHED BY THE PATCH ANYWAY.

4) Watch the patch get applied.

Shall I do this for Intel, since this has been explained multiple times 
without success?  This is not an invasive approach, it only touches what 
code you were touching anyway.


A WORD OF WARNING:  I am also highly suspicious of impending driver 
rewrites.  Such massive events inevitably break 'git bisect'.  A far 
better approach is the Al Viro equivalent-transformation approach, which 
does not break 'git bisect' and can be easily verified by a human.

	Jeff

Re: [patch 03/14] e1000: omit stats for broken counter in 82543

[Jeff Garzik]

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

Arjan van de Ven wrote:
> Subject: e1000: omit stats for broken counter in 82543
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> The 82543 chip does not count tx_carrier_errors properly in FD mode;
> report zeros instead of garbage.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

I rewrote your patch #3 into the attached patch.  Man, that was hard. 
It was a MAJOR REVISION, trust me.

See attached.  (e1000 even ALREADY HAS some feature flags for this sort 
of thing)

	Jeff



[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1389 bytes --]

diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
index 0201ca5..7376609 100644
--- a/drivers/net/e1000/e1000_hw.c
+++ b/drivers/net/e1000/e1000_hw.c
@@ -442,6 +442,9 @@ e1000_set_mac_type(struct e1000_hw *hw)
 		break;
 	}
 
+	if (hw->mac_type == e1000_82543)
+		hw->bad_tx_carr_stats_fd = TRUE;
+
 	return E1000_SUCCESS;
 }
 
diff --git a/drivers/net/e1000/e1000_hw.h b/drivers/net/e1000/e1000_hw.h
index 28cdfe3..fef1f7b 100644
--- a/drivers/net/e1000/e1000_hw.h
+++ b/drivers/net/e1000/e1000_hw.h
@@ -1460,6 +1460,7 @@ struct e1000_hw {
 	boolean_t		mng_reg_access_disabled;
 	boolean_t		leave_av_bit_off;
 	boolean_t		kmrn_lock_loss_workaround_disabled;
+	boolean_t		bad_tx_carr_stats_fd;
 };
 
 
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 62ef267..0816de2 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -3581,6 +3581,11 @@ e1000_update_stats(struct e1000_adapter *adapter)
 	adapter->net_stats.tx_aborted_errors = adapter->stats.ecol;
 	adapter->net_stats.tx_window_errors = adapter->stats.latecol;
 	adapter->net_stats.tx_carrier_errors = adapter->stats.tncrs;
+	if (adapter->hw.bad_tx_carr_stats_fd &&
+	    adapter->link_duplex == FULL_DUPLEX) {
+		adapter->net_stats.tx_carrier_errors = 0;
+		adapter->stats.tncrs = 0;
+	}
 
 	/* Tx Dropped needs to be maintained elsewhere */
 

[patch 04/14] e1000: consolidate managability enabling/disabling

[Arjan van de Ven]

Subject: e1000: consolidate managability enabling/disabling
From: Jesse Brandeburg <jesse.brandeburg@intel.com>

Several bugs existed in how we handle manageability issues all over the driver.
This patch consolidates all the managability release and init code in two single
functions and call them from appropriate locations. This fixes several
BMC packet redirect issues and powerup/down hiccups.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---

 drivers/net/e1000/e1000_hw.c   |    5 -
 drivers/net/e1000/e1000_hw.h   |    1 
 drivers/net/e1000/e1000_main.c |  127 +++++++++++++++++++++--------------------
 3 files changed, 71 insertions(+), 62 deletions(-)

Index: linux-2.6/drivers/net/e1000/e1000_hw.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_hw.c
+++ linux-2.6/drivers/net/e1000/e1000_hw.c
@@ -7817,9 +7817,8 @@ e1000_enable_mng_pass_thru(struct e1000_
             fwsm = E1000_READ_REG(hw, FWSM);
             factps = E1000_READ_REG(hw, FACTPS);
 
-            if (((fwsm & E1000_FWSM_MODE_MASK) ==
-                (e1000_mng_mode_pt << E1000_FWSM_MODE_SHIFT)) &&
-                (factps & E1000_FACTPS_MNGCG))
+            if ((((fwsm & E1000_FWSM_MODE_MASK) >> E1000_FWSM_MODE_SHIFT) ==
+                   e1000_mng_mode_pt) && !(factps & E1000_FACTPS_MNGCG))
                 return TRUE;
         } else
             if ((manc & E1000_MANC_SMBUS_EN) && !(manc & E1000_MANC_ASF_EN))
Index: linux-2.6/drivers/net/e1000/e1000_hw.h
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_hw.h
+++ linux-2.6/drivers/net/e1000/e1000_hw.h
@@ -1301,6 +1301,7 @@ struct e1000_ffvt_entry {
 #define E1000_82542_RSSIR       E1000_RSSIR
 #define E1000_82542_KUMCTRLSTA E1000_KUMCTRLSTA
 #define E1000_82542_SW_FW_SYNC E1000_SW_FW_SYNC
+#define E1000_82542_MANC2H      E1000_MANC2H
 
 /* Statistics counters collected by the MAC */
 struct e1000_hw_stats {
Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -464,6 +464,51 @@ e1000_get_hw_control(struct e1000_adapte
 	}
 }
 
+static void
+e1000_init_manageability(struct e1000_adapter *adapter)
+{
+	if (adapter->en_mng_pt) {
+		uint32_t manc2h = E1000_READ_REG(&adapter->hw, MANC2H);
+		uint32_t manc = E1000_READ_REG(&adapter->hw, MANC);
+
+		/* disable hardware interception of ARP */
+		manc &= ~(E1000_MANC_ARP_EN);
+
+		/* enable receiving management packets to the host */
+		/* this will probably generate destination unreachable messages
+		 * from the host OS, but the packets will be handled on SMBUS */
+		if (adapter->hw.mac_type >= e1000_82571) {
+			manc |= E1000_MANC_EN_MNG2HOST;
+#define E1000_MNG2HOST_PORT_623 (1 << 5)
+#define E1000_MNG2HOST_PORT_664 (1 << 6)
+			manc2h |= E1000_MNG2HOST_PORT_623;
+			manc2h |= E1000_MNG2HOST_PORT_664;
+			E1000_WRITE_REG(&adapter->hw, MANC2H, manc2h);
+		}
+
+		E1000_WRITE_REG(&adapter->hw, MANC, manc);
+	}
+}
+
+static void
+e1000_release_manageability(struct e1000_adapter *adapter)
+{
+	if (adapter->en_mng_pt) {
+		uint32_t manc = E1000_READ_REG(&adapter->hw, MANC);
+
+		/* re-enable hardware interception of ARP */
+		manc |= E1000_MANC_ARP_EN;
+
+		if (adapter->hw.mac_type >= e1000_82571)
+			manc &= ~E1000_MANC_EN_MNG2HOST;
+
+		/* don't explicitly have to mess with MANC2H since
+		 * MANC has an enable disable that gates MANC2H */
+
+		E1000_WRITE_REG(&adapter->hw, MANC, manc);
+	}
+}
+
 int
 e1000_up(struct e1000_adapter *adapter)
 {
@@ -475,6 +520,7 @@ e1000_up(struct e1000_adapter *adapter)
 	e1000_set_multi(netdev);
 
 	e1000_restore_vlan(adapter);
+	e1000_init_manageability(adapter);
 
 	e1000_configure_tx(adapter);
 	e1000_setup_rctl(adapter);
@@ -614,7 +660,7 @@ e1000_reinit_locked(struct e1000_adapter
 void
 e1000_reset(struct e1000_adapter *adapter)
 {
-	uint32_t pba, manc;
+	uint32_t pba;
 	uint16_t fc_high_water_mark = E1000_FC_HIGH_DIFF;
 
 	/* Repartition Pba for greater than 9k mtu
@@ -705,14 +751,7 @@ e1000_reset(struct e1000_adapter *adapte
 		                    phy_data);
 	}
 
-	if ((adapter->en_mng_pt) &&
-	    (adapter->hw.mac_type >= e1000_82540) &&
-	    (adapter->hw.mac_type < e1000_82571) &&
-	    (adapter->hw.media_type == e1000_media_type_copper)) {
-		manc = E1000_READ_REG(&adapter->hw, MANC);
-		manc |= (E1000_MANC_ARP_EN | E1000_MANC_EN_MNG2HOST);
-		E1000_WRITE_REG(&adapter->hw, MANC, manc);
-	}
+	e1000_release_manageability(adapter);
 }
 
 /**
@@ -1078,22 +1117,13 @@ e1000_remove(struct pci_dev *pdev)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	uint32_t manc;
 #ifdef CONFIG_E1000_NAPI
 	int i;
 #endif
 
 	flush_scheduled_work();
 
-	if (adapter->hw.mac_type >= e1000_82540 &&
-	    adapter->hw.mac_type < e1000_82571 &&
-	    adapter->hw.media_type == e1000_media_type_copper) {
-		manc = E1000_READ_REG(&adapter->hw, MANC);
-		if (manc & E1000_MANC_SMBUS_EN) {
-			manc |= E1000_MANC_ARP_EN;
-			E1000_WRITE_REG(&adapter->hw, MANC, manc);
-		}
-	}
+	e1000_release_manageability(adapter);
 
 	/* Release control of h/w to f/w.  If f/w is AMT enabled, this
 	 * would have already happened in close and is redundant. */
@@ -5014,7 +5044,7 @@ e1000_suspend(struct pci_dev *pdev, pm_m
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	uint32_t ctrl, ctrl_ext, rctl, manc, status;
+	uint32_t ctrl, ctrl_ext, rctl, status;
 	uint32_t wufc = adapter->wol;
 #ifdef CONFIG_PM
 	int retval = 0;
@@ -5083,16 +5113,12 @@ e1000_suspend(struct pci_dev *pdev, pm_m
 		pci_enable_wake(pdev, PCI_D3cold, 0);
 	}
 
-	if (adapter->hw.mac_type >= e1000_82540 &&
-	    adapter->hw.mac_type < e1000_82571 &&
-	    adapter->hw.media_type == e1000_media_type_copper) {
-		manc = E1000_READ_REG(&adapter->hw, MANC);
-		if (manc & E1000_MANC_SMBUS_EN) {
-			manc |= E1000_MANC_ARP_EN;
-			E1000_WRITE_REG(&adapter->hw, MANC, manc);
-			pci_enable_wake(pdev, PCI_D3hot, 1);
-			pci_enable_wake(pdev, PCI_D3cold, 1);
-		}
+	e1000_release_manageability(adapter);
+
+	/* make sure adapter isn't asleep if manageability is enabled */
+	if (adapter->en_mng_pt) {
+		pci_enable_wake(pdev, PCI_D3hot, 1);
+		pci_enable_wake(pdev, PCI_D3cold, 1);
 	}
 
 	if (adapter->hw.phy_type == e1000_phy_igp_3)
@@ -5118,7 +5144,7 @@ e1000_resume(struct pci_dev *pdev)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	uint32_t manc, err;
+	uint32_t err;
 
 	pci_set_power_state(pdev, PCI_D0);
 	e1000_pci_restore_state(adapter);
@@ -5138,19 +5164,13 @@ e1000_resume(struct pci_dev *pdev)
 	e1000_reset(adapter);
 	E1000_WRITE_REG(&adapter->hw, WUS, ~0);
 
+	e1000_init_manageability(adapter);
+
 	if (netif_running(netdev))
 		e1000_up(adapter);
 
 	netif_device_attach(netdev);
 
-	if (adapter->hw.mac_type >= e1000_82540 &&
-	    adapter->hw.mac_type < e1000_82571 &&
-	    adapter->hw.media_type == e1000_media_type_copper) {
-		manc = E1000_READ_REG(&adapter->hw, MANC);
-		manc &= ~(E1000_MANC_ARP_EN);
-		E1000_WRITE_REG(&adapter->hw, MANC, manc);
-	}
-
 	/* If the controller is 82573 and f/w is AMT, do not set
 	 * DRV_LOAD until the interface is up.  For all other cases,
 	 * let the f/w know that the h/w is now under the control
@@ -5251,7 +5271,8 @@ static void e1000_io_resume(struct pci_d
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev->priv;
-	uint32_t manc, swsm;
+
+	e1000_init_manageability(adapter);
 
 	if (netif_running(netdev)) {
 		if (e1000_up(adapter)) {
@@ -5262,26 +5283,14 @@ static void e1000_io_resume(struct pci_d
 
 	netif_device_attach(netdev);
 
-	if (adapter->hw.mac_type >= e1000_82540 &&
-	    adapter->hw.mac_type < e1000_82571 &&
-	    adapter->hw.media_type == e1000_media_type_copper) {
-		manc = E1000_READ_REG(&adapter->hw, MANC);
-		manc &= ~(E1000_MANC_ARP_EN);
-		E1000_WRITE_REG(&adapter->hw, MANC, manc);
-	}
-
-	switch (adapter->hw.mac_type) {
-	case e1000_82573:
-		swsm = E1000_READ_REG(&adapter->hw, SWSM);
-		E1000_WRITE_REG(&adapter->hw, SWSM,
-				swsm | E1000_SWSM_DRV_LOAD);
-		break;
-	default:
-		break;
-	}
+	/* If the controller is 82573 and f/w is AMT, do not set
+	 * DRV_LOAD until the interface is up.  For all other cases,
+	 * let the f/w know that the h/w is now under the control
+	 * of the driver. */
+	if (adapter->hw.mac_type != e1000_82573 ||
+	    !e1000_check_mng_mode(&adapter->hw))
+		e1000_get_hw_control(adapter);
 
-	if (netif_running(netdev))
-		mod_timer(&adapter->watchdog_timer, jiffies);
 }
 
 /* e1000_main.c */

Re: [patch 04/14] e1000: consolidate managability enabling/disabling

[Jeff Garzik]

Arjan van de Ven wrote:
> +static void
> +e1000_init_manageability(struct e1000_adapter *adapter)
> +{
> +	if (adapter->en_mng_pt) {
> +		uint32_t manc2h = E1000_READ_REG(&adapter->hw, MANC2H);
> +		uint32_t manc = E1000_READ_REG(&adapter->hw, MANC);
> +
> +		/* disable hardware interception of ARP */
> +		manc &= ~(E1000_MANC_ARP_EN);
> +
> +		/* enable receiving management packets to the host */
> +		/* this will probably generate destination unreachable messages
> +		 * from the host OS, but the packets will be handled on SMBUS */
> +		if (adapter->hw.mac_type >= e1000_82571) {
> +			manc |= E1000_MANC_EN_MNG2HOST;
> +#define E1000_MNG2HOST_PORT_623 (1 << 5)
> +#define E1000_MNG2HOST_PORT_664 (1 << 6)
> +			manc2h |= E1000_MNG2HOST_PORT_623;
> +			manc2h |= E1000_MNG2HOST_PORT_664;
> +			E1000_WRITE_REG(&adapter->hw, MANC2H, manc2h);
> +		}
> +
> +		E1000_WRITE_REG(&adapter->hw, MANC, manc);
> +	}
> +}
> +
> +static void
> +e1000_release_manageability(struct e1000_adapter *adapter)
> +{
> +	if (adapter->en_mng_pt) {
> +		uint32_t manc = E1000_READ_REG(&adapter->hw, MANC);
> +
> +		/* re-enable hardware interception of ARP */
> +		manc |= E1000_MANC_ARP_EN;
> +
> +		if (adapter->hw.mac_type >= e1000_82571)
> +			manc &= ~E1000_MANC_EN_MNG2HOST;
> +
> +		/* don't explicitly have to mess with MANC2H since
> +		 * MANC has an enable disable that gates MANC2H */
> +
> +		E1000_WRITE_REG(&adapter->hw, MANC, manc);
> +	}
> +}

This patch adds more error-probe mac_type tests, rather than a simple 
and tough-to-screw-up E1000_HAS_MGMT feature bit.


> +	/* If the controller is 82573 and f/w is AMT, do not set
> +	 * DRV_LOAD until the interface is up.  For all other cases,
> +	 * let the f/w know that the h/w is now under the control
> +	 * of the driver. */
> +	if (adapter->hw.mac_type != e1000_82573 ||
> +	    !e1000_check_mng_mode(&adapter->hw))
> +		e1000_get_hw_control(adapter);
>  
> -	if (netif_running(netdev))
> -		mod_timer(&adapter->watchdog_timer, jiffies);

ditto

Re: [patch 04/14] e1000: consolidate managability enabling/disabling

[Jeff Garzik]

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

Arjan van de Ven wrote:
> Subject: e1000: consolidate managability enabling/disabling
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> Several bugs existed in how we handle manageability issues all over the driver.
> This patch consolidates all the managability release and init code in two single
> functions and call them from appropriate locations. This fixes several
> BMC packet redirect issues and powerup/down hiccups.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

The revisions for this patch were "major revisions", as you had warned. 
  See attached.

	Jeff



[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 9248 bytes --]

commit fa359ae5b1c4ddbd5a5ba3bc72d4276efbc6654a
Author: Jeff Garzik <jeff@garzik.org>
Date:   Fri Dec 15 10:56:10 2006 -0500

    e1000: consolidate managability enabling/disabling
    
    Several bugs existed in how we handle manageability issues all
    over the driver.  This patch consolidates all the managability
    release and init code in two single functions and call them from
    appropriate locations. This fixes several BMC packet redirect issues
    and powerup/down hiccups.
    
    Originally from Jesse Brandeburg <jesse.brandeburg@intel.com>, rewritten
    to use feature flags by me.
    
    Signed-off-by: Jeff Garzik <jeff@garzik.org>

fa359ae5b1c4ddbd5a5ba3bc72d4276efbc6654a
diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
index ce82eb5..1ea556e 100644
--- a/drivers/net/e1000/e1000_hw.c
+++ b/drivers/net/e1000/e1000_hw.c
@@ -448,6 +448,10 @@ e1000_set_mac_type(struct e1000_hw *hw)
 	if (hw->mac_type == e1000_82543)
 		hw->bad_tx_carr_stats_fd = TRUE;
 
+	/* capable of receiving management packets to the host */
+	if (hw->mac_type >= e1000_82571)
+		hw->has_manc2h = TRUE;
+
 	return E1000_SUCCESS;
 }
 
@@ -7823,9 +7827,8 @@ e1000_enable_mng_pass_thru(struct e1000_hw *hw)
             fwsm = E1000_READ_REG(hw, FWSM);
             factps = E1000_READ_REG(hw, FACTPS);
 
-            if (((fwsm & E1000_FWSM_MODE_MASK) ==
-                (e1000_mng_mode_pt << E1000_FWSM_MODE_SHIFT)) &&
-                (factps & E1000_FACTPS_MNGCG))
+            if ((((fwsm & E1000_FWSM_MODE_MASK) >> E1000_FWSM_MODE_SHIFT) ==
+                   e1000_mng_mode_pt) && !(factps & E1000_FACTPS_MNGCG))
                 return TRUE;
         } else
             if ((manc & E1000_MANC_SMBUS_EN) && !(manc & E1000_MANC_ASF_EN))
diff --git a/drivers/net/e1000/e1000_hw.h b/drivers/net/e1000/e1000_hw.h
index fef1f7b..18a4ae4 100644
--- a/drivers/net/e1000/e1000_hw.h
+++ b/drivers/net/e1000/e1000_hw.h
@@ -1301,6 +1301,7 @@ struct e1000_ffvt_entry {
 #define E1000_82542_RSSIR       E1000_RSSIR
 #define E1000_82542_KUMCTRLSTA E1000_KUMCTRLSTA
 #define E1000_82542_SW_FW_SYNC E1000_SW_FW_SYNC
+#define E1000_82542_MANC2H      E1000_MANC2H
 
 /* Statistics counters collected by the MAC */
 struct e1000_hw_stats {
@@ -1461,6 +1462,7 @@ struct e1000_hw {
 	boolean_t		leave_av_bit_off;
 	boolean_t		kmrn_lock_loss_workaround_disabled;
 	boolean_t		bad_tx_carr_stats_fd;
+	boolean_t		has_manc2h;
 };
 
 
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 0816de2..7a51283 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -464,6 +464,52 @@ e1000_get_hw_control(struct e1000_adapter *adapter)
 	}
 }
 
+static void
+e1000_init_manageability(struct e1000_adapter *adapter)
+{
+	if (adapter->en_mng_pt) {
+		uint32_t manc = E1000_READ_REG(&adapter->hw, MANC);
+
+		/* disable hardware interception of ARP */
+		manc &= ~(E1000_MANC_ARP_EN);
+
+		/* enable receiving management packets to the host */
+		/* this will probably generate destination unreachable messages
+		 * from the host OS, but the packets will be handled on SMBUS */
+		if (adapter->hw.has_manc2h) {
+			uint32_t manc2h = E1000_READ_REG(&adapter->hw, MANC2H);
+
+			manc |= E1000_MANC_EN_MNG2HOST;
+#define E1000_MNG2HOST_PORT_623 (1 << 5)
+#define E1000_MNG2HOST_PORT_664 (1 << 6)
+			manc2h |= E1000_MNG2HOST_PORT_623;
+			manc2h |= E1000_MNG2HOST_PORT_664;
+			E1000_WRITE_REG(&adapter->hw, MANC2H, manc2h);
+		}
+
+		E1000_WRITE_REG(&adapter->hw, MANC, manc);
+	}
+}
+
+static void
+e1000_release_manageability(struct e1000_adapter *adapter)
+{
+	if (adapter->en_mng_pt) {
+		uint32_t manc = E1000_READ_REG(&adapter->hw, MANC);
+
+		/* re-enable hardware interception of ARP */
+		manc |= E1000_MANC_ARP_EN;
+
+		if (adapter->hw.has_manc2h)
+			manc &= ~E1000_MANC_EN_MNG2HOST;
+
+		/* don't explicitly have to mess with MANC2H since
+		 * MANC has an enable disable that gates MANC2H */
+
+		E1000_WRITE_REG(&adapter->hw, MANC, manc);
+	}
+}
+
 int
 e1000_up(struct e1000_adapter *adapter)
 {
@@ -475,6 +521,7 @@ e1000_up(struct e1000_adapter *adapter)
 	e1000_set_multi(netdev);
 
 	e1000_restore_vlan(adapter);
+	e1000_init_manageability(adapter);
 
 	e1000_configure_tx(adapter);
 	e1000_setup_rctl(adapter);
@@ -614,7 +661,7 @@ e1000_reinit_locked(struct e1000_adapter *adapter)
 void
 e1000_reset(struct e1000_adapter *adapter)
 {
-	uint32_t pba, manc;
+	uint32_t pba;
 	uint16_t fc_high_water_mark = E1000_FC_HIGH_DIFF;
 
 	/* Repartition Pba for greater than 9k mtu
@@ -705,14 +752,7 @@ e1000_reset(struct e1000_adapter *adapter)
 		                    phy_data);
 	}
 
-	if ((adapter->en_mng_pt) &&
-	    (adapter->hw.mac_type >= e1000_82540) &&
-	    (adapter->hw.mac_type < e1000_82571) &&
-	    (adapter->hw.media_type == e1000_media_type_copper)) {
-		manc = E1000_READ_REG(&adapter->hw, MANC);
-		manc |= (E1000_MANC_ARP_EN | E1000_MANC_EN_MNG2HOST);
-		E1000_WRITE_REG(&adapter->hw, MANC, manc);
-	}
+	e1000_release_manageability(adapter);
 }
 
 /**
@@ -1078,22 +1118,13 @@ e1000_remove(struct pci_dev *pdev)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	uint32_t manc;
 #ifdef CONFIG_E1000_NAPI
 	int i;
 #endif
 
 	flush_scheduled_work();
 
-	if (adapter->hw.mac_type >= e1000_82540 &&
-	    adapter->hw.mac_type < e1000_82571 &&
-	    adapter->hw.media_type == e1000_media_type_copper) {
-		manc = E1000_READ_REG(&adapter->hw, MANC);
-		if (manc & E1000_MANC_SMBUS_EN) {
-			manc |= E1000_MANC_ARP_EN;
-			E1000_WRITE_REG(&adapter->hw, MANC, manc);
-		}
-	}
+	e1000_release_manageability(adapter);
 
 	/* Release control of h/w to f/w.  If f/w is AMT enabled, this
 	 * would have already happened in close and is redundant. */
@@ -5011,7 +5042,7 @@ e1000_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	uint32_t ctrl, ctrl_ext, rctl, manc, status;
+	uint32_t ctrl, ctrl_ext, rctl, status;
 	uint32_t wufc = adapter->wol;
 #ifdef CONFIG_PM
 	int retval = 0;
@@ -5080,16 +5111,12 @@ e1000_suspend(struct pci_dev *pdev, pm_message_t state)
 		pci_enable_wake(pdev, PCI_D3cold, 0);
 	}
 
-	if (adapter->hw.mac_type >= e1000_82540 &&
-	    adapter->hw.mac_type < e1000_82571 &&
-	    adapter->hw.media_type == e1000_media_type_copper) {
-		manc = E1000_READ_REG(&adapter->hw, MANC);
-		if (manc & E1000_MANC_SMBUS_EN) {
-			manc |= E1000_MANC_ARP_EN;
-			E1000_WRITE_REG(&adapter->hw, MANC, manc);
-			pci_enable_wake(pdev, PCI_D3hot, 1);
-			pci_enable_wake(pdev, PCI_D3cold, 1);
-		}
+	e1000_release_manageability(adapter);
+
+	/* make sure adapter isn't asleep if manageability is enabled */
+	if (adapter->en_mng_pt) {
+		pci_enable_wake(pdev, PCI_D3hot, 1);
+		pci_enable_wake(pdev, PCI_D3cold, 1);
 	}
 
 	if (adapter->hw.phy_type == e1000_phy_igp_3)
@@ -5115,7 +5142,7 @@ e1000_resume(struct pci_dev *pdev)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	uint32_t manc, err;
+	uint32_t err;
 
 	pci_set_power_state(pdev, PCI_D0);
 	e1000_pci_restore_state(adapter);
@@ -5135,19 +5162,13 @@ e1000_resume(struct pci_dev *pdev)
 	e1000_reset(adapter);
 	E1000_WRITE_REG(&adapter->hw, WUS, ~0);
 
+	e1000_init_manageability(adapter);
+
 	if (netif_running(netdev))
 		e1000_up(adapter);
 
 	netif_device_attach(netdev);
 
-	if (adapter->hw.mac_type >= e1000_82540 &&
-	    adapter->hw.mac_type < e1000_82571 &&
-	    adapter->hw.media_type == e1000_media_type_copper) {
-		manc = E1000_READ_REG(&adapter->hw, MANC);
-		manc &= ~(E1000_MANC_ARP_EN);
-		E1000_WRITE_REG(&adapter->hw, MANC, manc);
-	}
-
 	/* If the controller is 82573 and f/w is AMT, do not set
 	 * DRV_LOAD until the interface is up.  For all other cases,
 	 * let the f/w know that the h/w is now under the control
@@ -5248,7 +5269,8 @@ static void e1000_io_resume(struct pci_dev *pdev)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev->priv;
-	uint32_t manc, swsm;
+
+	e1000_init_manageability(adapter);
 
 	if (netif_running(netdev)) {
 		if (e1000_up(adapter)) {
@@ -5259,26 +5281,14 @@ static void e1000_io_resume(struct pci_dev *pdev)
 
 	netif_device_attach(netdev);
 
-	if (adapter->hw.mac_type >= e1000_82540 &&
-	    adapter->hw.mac_type < e1000_82571 &&
-	    adapter->hw.media_type == e1000_media_type_copper) {
-		manc = E1000_READ_REG(&adapter->hw, MANC);
-		manc &= ~(E1000_MANC_ARP_EN);
-		E1000_WRITE_REG(&adapter->hw, MANC, manc);
-	}
-
-	switch (adapter->hw.mac_type) {
-	case e1000_82573:
-		swsm = E1000_READ_REG(&adapter->hw, SWSM);
-		E1000_WRITE_REG(&adapter->hw, SWSM,
-				swsm | E1000_SWSM_DRV_LOAD);
-		break;
-	default:
-		break;
-	}
+	/* If the controller is 82573 and f/w is AMT, do not set
+	 * DRV_LOAD until the interface is up.  For all other cases,
+	 * let the f/w know that the h/w is now under the control
+	 * of the driver. */
+	if (adapter->hw.mac_type != e1000_82573 ||
+	    !e1000_check_mng_mode(&adapter->hw))
+		e1000_get_hw_control(adapter);
 
-	if (netif_running(netdev))
-		mod_timer(&adapter->watchdog_timer, jiffies);
 }
 
 /* e1000_main.c */

[patch 05/14] e1000: Fix Wake-on-Lan with forced gigabit speed

[Arjan van de Ven]

Subject: e1000: Fix Wake-on-Lan with forced gigabit speed
From: Jesse Brandeburg <jesse.brandeburg@intel.com>

If the user has forced gigabit speed, phy power management must be disabled;
otherwise the NIC would try to negotiate to a linkspeed of 10/100 mbit on
shutdown, which would lead to a total loss of link. This loss of link breaks
Wake-on-Lan and IPMI.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---

 drivers/net/e1000/e1000_main.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -732,6 +732,20 @@ e1000_reset(struct e1000_adapter *adapte
 	if (e1000_init_hw(&adapter->hw))
 		DPRINTK(PROBE, ERR, "Hardware Error\n");
 	e1000_update_mng_vlan(adapter);
+
+	/* if (adapter->hwflags & HWFLAGS_PHY_PWR_BIT) { */
+	if (adapter->hw.mac_type >= e1000_82544 &&
+	    adapter->hw.mac_type <= e1000_82547_rev_2 &&
+	    adapter->hw.autoneg == 1 &&
+	    adapter->hw.autoneg_advertised == ADVERTISE_1000_FULL) {
+		uint32_t ctrl = E1000_READ_REG(&adapter->hw, CTRL);
+		/* clear phy power management bit if we are in gig only mode,
+		 * which if enabled will attempt negotiation to 100Mb, which
+		 * can cause a loss of link at power off or driver unload */
+		ctrl &= ~E1000_CTRL_SWDPIN3;
+		E1000_WRITE_REG(&adapter->hw, CTRL, ctrl);
+	}
+
 	/* Enable h/w to recognize an 802.1Q VLAN Ethernet packet */
 	E1000_WRITE_REG(&adapter->hw, VET, ETHERNET_IEEE_VLAN_TYPE);
 

Re: [patch 05/14] e1000: Fix Wake-on-Lan with forced gigabit speed

[Jeff Garzik]

Arjan van de Ven wrote:
> Subject: e1000: Fix Wake-on-Lan with forced gigabit speed
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> If the user has forced gigabit speed, phy power management must be disabled;
> otherwise the NIC would try to negotiate to a linkspeed of 10/100 mbit on
> shutdown, which would lead to a total loss of link. This loss of link breaks
> Wake-on-Lan and IPMI.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> ---
> 
>  drivers/net/e1000/e1000_main.c |   14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> Index: linux-2.6/drivers/net/e1000/e1000_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/e1000/e1000_main.c
> +++ linux-2.6/drivers/net/e1000/e1000_main.c
> @@ -732,6 +732,20 @@ e1000_reset(struct e1000_adapter *adapte
>  	if (e1000_init_hw(&adapter->hw))
>  		DPRINTK(PROBE, ERR, "Hardware Error\n");
>  	e1000_update_mng_vlan(adapter);
> +
> +	/* if (adapter->hwflags & HWFLAGS_PHY_PWR_BIT) { */
> +	if (adapter->hw.mac_type >= e1000_82544 &&
> +	    adapter->hw.mac_type <= e1000_82547_rev_2 &&
> +	    adapter->hw.autoneg == 1 &&
> +	    adapter->hw.autoneg_advertised == ADVERTISE_1000_FULL) {
> +		uint32_t ctrl = E1000_READ_REG(&adapter->hw, CTRL);
> +		/* clear phy power management bit if we are in gig only mode,
> +		 * which if enabled will attempt negotiation to 100Mb, which
> +		 * can cause a loss of link at power off or driver unload */
> +		ctrl &= ~E1000_CTRL_SWDPIN3;
> +		E1000_WRITE_REG(&adapter->hw, CTRL, ctrl);

NAK, for same reason as other patches.

I have been telling Intel this for months, yet keep seeing the same patches.

Re: [patch 05/14] e1000: Fix Wake-on-Lan with forced gigabit speed

[Jeff Garzik]

Arjan van de Ven wrote:
> Subject: e1000: Fix Wake-on-Lan with forced gigabit speed
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> If the user has forced gigabit speed, phy power management must be disabled;
> otherwise the NIC would try to negotiate to a linkspeed of 10/100 mbit on
> shutdown, which would lead to a total loss of link. This loss of link breaks
> Wake-on-Lan and IPMI.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

ACK patches 5 and 6

[patch 06/14] e1000: disable TSO on the 82544 with slab debugging

[Arjan van de Ven]

Subject: e1000: disable TSO on the 82544 with slab debugging
From: Jesse Brandeburg <jesse.brandeburg@intel.com>

CONFIG_DEBUG_SLAB changes alignments of the data structures the slab
allocators return. These break certain workarounds for TSO on the 82544.
Since DEBUG_SLAB is relatively rare and not used for performance sensitive
cases, the simplest fix is to disable TSO in this special situation.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---

 drivers/net/e1000/e1000_main.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -911,6 +911,12 @@ e1000_probe(struct pci_dev *pdev,
 	   (adapter->hw.mac_type != e1000_82547))
 		netdev->features |= NETIF_F_TSO;
 
+#ifdef CONFIG_DEBUG_SLAB
+	/* 82544's work arounds do not play nicely with DEBUG SLAB */
+	if (adapter->hw.mac_type == e1000_82544)
+		netdev->features &= ~NETIF_F_TSO;
+#endif
+
 #ifdef NETIF_F_TSO6
 	if (adapter->hw.mac_type > e1000_82547_rev_2)
 		netdev->features |= NETIF_F_TSO6;

Re: [patch 06/14] e1000: disable TSO on the 82544 with slab debugging

[Jeff Garzik]

Arjan van de Ven wrote:
> Subject: e1000: disable TSO on the 82544 with slab debugging
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> CONFIG_DEBUG_SLAB changes alignments of the data structures the slab
> allocators return. These break certain workarounds for TSO on the 82544.
> Since DEBUG_SLAB is relatively rare and not used for performance sensitive
> cases, the simplest fix is to disable TSO in this special situation.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> ---
> 
>  drivers/net/e1000/e1000_main.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> Index: linux-2.6/drivers/net/e1000/e1000_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/e1000/e1000_main.c
> +++ linux-2.6/drivers/net/e1000/e1000_main.c
> @@ -911,6 +911,12 @@ e1000_probe(struct pci_dev *pdev,
>  	   (adapter->hw.mac_type != e1000_82547))
>  		netdev->features |= NETIF_F_TSO;
>  
> +#ifdef CONFIG_DEBUG_SLAB
> +	/* 82544's work arounds do not play nicely with DEBUG SLAB */
> +	if (adapter->hw.mac_type == e1000_82544)
> +		netdev->features &= ~NETIF_F_TSO;
> +#endif

NAK, same reason as the others

Re: [patch 06/14] e1000: disable TSO on the 82544 with slab debugging

[Herbert Xu]

Jeff Garzik <jeff@garzik.org> wrote:
>  
>> +#ifdef CONFIG_DEBUG_SLAB
>> +     /* 82544's work arounds do not play nicely with DEBUG SLAB */
>> +     if (adapter->hw.mac_type == e1000_82544)
>> +             netdev->features &= ~NETIF_F_TSO;
>> +#endif
> 
> NAK, same reason as the others

Any chance you could apply this patch instead? I've verified that this
does resolve the problem on 82544.

[NETDRV] e1000: Do not truncate TSO TCP header with 82544 workaround

The e1000 driver has a workaround for 82544 on PCI-X where if the
terminating byte of a buffer is at addresses 0-3 mod 8, then 4 bytes
are shaved off it and defered to a new segment.  This is due to an
erratum that could otherwise cause TX hangs.

Unfortunately this breaks TSO because it may cause the TCP header to
be split over two segments which itself causes TX hangs.  The solution
is to pull 4 bytes of data up from the next segment rather than pushing
4 bytes off.  This ensures the TCP header remains in one piece and
works around the PCI-X hang.

This patch is based on one from Jesse Brandeburg.

This bug has been trigered by both CONFIG_DEBUG_SLAB as well as Xen.

Note that the only reason we don't see this normally is because the
TCP stack starts writing from the end, i.e., it writes the TCP header
first then slaps on the IP header, etc.  So the end of the TCP header
(skb->tail - 1 here) is always aligned correctly.

Had we made the start of the IP header (e.g., IPv6) 8-byte aligned
instead, this would happen for normal TCP traffic as well.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 73f3a85..2c6ba42 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -3168,6 +3168,16 @@ #ifdef NETIF_F_TSO
 		if (skb->data_len && (hdr_len == (skb->len - skb->data_len))) {
 			switch (adapter->hw.mac_type) {
 				unsigned int pull_size;
+			case e1000_82544:
+				/* Make sure we have room to chop off 4 bytes,
+				 * and that the end alignment will work out to
+				 * this hardware's requirements
+				 * NOTE: this is a TSO only workaround
+				 * if end byte alignment not correct move us
+				 * into the next dword */
+				if ((unsigned long)(skb->tail - 1) & 4)
+					break;
+				/* fall through */
 			case e1000_82571:
 			case e1000_82572:
 			case e1000_82573:

Re: [patch 06/14] e1000: disable TSO on the 82544 with slab debugging

[Jesse Brandeburg]

On 12/15/06, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Any chance you could apply this patch instead? I've verified that this
> does resolve the problem on 82544.
>
> [NETDRV] e1000: Do not truncate TSO TCP header with 82544 workaround

Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

Re: [patch 06/14] e1000: disable TSO on the 82544 with slab debugging

[Jeff Garzik]

Herbert Xu wrote:
> Jeff Garzik <jeff@garzik.org> wrote:
>>  
>>> +#ifdef CONFIG_DEBUG_SLAB
>>> +     /* 82544's work arounds do not play nicely with DEBUG SLAB */
>>> +     if (adapter->hw.mac_type == e1000_82544)
>>> +             netdev->features &= ~NETIF_F_TSO;
>>> +#endif
>> NAK, same reason as the others
> 
> Any chance you could apply this patch instead? I've verified that this
> does resolve the problem on 82544.
> 
> [NETDRV] e1000: Do not truncate TSO TCP header with 82544 workaround

applied

[patch 07/14] e1000: workaround for the ESB2 NIC RX unit issue

[Arjan van de Ven]

Subject: e1000: workaround for the ESB2 NIC RX unit issue
From: Jesse Brandeburg <jesse.brandeburg@intel.com>

In rare occasions, ESB2 systems would end up started without the RX
unit being turned on. Add a check that runs post-init to work around this
issue.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---

 drivers/net/e1000/e1000_main.c |    7 +++++++
 1 file changed, 7 insertions(+)

Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -2579,6 +2579,13 @@ e1000_watchdog(unsigned long data)
 			netif_wake_queue(netdev);
 			mod_timer(&adapter->phy_info_timer, jiffies + 2 * HZ);
 			adapter->smartspeed = 0;
+		} else {
+			/* make sure the receive unit is started */
+			if (adapter->hw.mac_type == e1000_80003es2lan) {
+				struct e1000_hw *hw = &adapter->hw;
+				uint32_t rctl = E1000_READ_REG(hw, RCTL);
+				E1000_WRITE_REG(hw, RCTL, rctl | E1000_RCTL_EN);
+			}
 		}
 	} else {
 		if (netif_carrier_ok(netdev)) {

Re: [patch 07/14] e1000: workaround for the ESB2 NIC RX unit issue

[Jeff Garzik]

Arjan van de Ven wrote:
> Subject: e1000: workaround for the ESB2 NIC RX unit issue
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> In rare occasions, ESB2 systems would end up started without the RX
> unit being turned on. Add a check that runs post-init to work around this
> issue.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> ---
> 
>  drivers/net/e1000/e1000_main.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> Index: linux-2.6/drivers/net/e1000/e1000_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/e1000/e1000_main.c
> +++ linux-2.6/drivers/net/e1000/e1000_main.c
> @@ -2579,6 +2579,13 @@ e1000_watchdog(unsigned long data)
>  			netif_wake_queue(netdev);
>  			mod_timer(&adapter->phy_info_timer, jiffies + 2 * HZ);
>  			adapter->smartspeed = 0;
> +		} else {
> +			/* make sure the receive unit is started */
> +			if (adapter->hw.mac_type == e1000_80003es2lan) {
> +				struct e1000_hw *hw = &adapter->hw;
> +				uint32_t rctl = E1000_READ_REG(hw, RCTL);
> +				E1000_WRITE_REG(hw, RCTL, rctl | E1000_RCTL_EN);
> +			}

NAK, ditto

[patch 08/14] e1000: fix to set the new max frame size before resetting the adapter

[Arjan van de Ven]

Subject: e1000: fix to set the new max frame size before resetting the adapter
From: Bruce Allan <bruce.w.allan@intel.com>

This bugfix makes sure that the driver data reflects the full new situation
before the adapter is reinitialized.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---

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

Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -3485,12 +3485,11 @@ e1000_change_mtu(struct net_device *netd
 		adapter->rx_buffer_len = MAXIMUM_ETHERNET_VLAN_SIZE;
 
 	netdev->mtu = new_mtu;
+	adapter->hw.max_frame_size = max_frame;
 
 	if (netif_running(netdev))
 		e1000_reinit_locked(adapter);
 
-	adapter->hw.max_frame_size = max_frame;
-
 	return 0;
 }
 

[patch 09/14] e1000: fix ethtool reported bus type for older adapters

[Arjan van de Ven]

Subject: e1000: fix ethtool reported bus type for older adapters
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

For older adapters we know that they are of the PCI bus type, so we can
just set this.

Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---

 drivers/net/e1000/e1000_hw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/drivers/net/e1000/e1000_hw.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_hw.c
+++ linux-2.6/drivers/net/e1000/e1000_hw.c
@@ -6575,7 +6575,7 @@ e1000_get_bus_info(struct e1000_hw *hw)
     switch (hw->mac_type) {
     case e1000_82542_rev2_0:
     case e1000_82542_rev2_1:
-        hw->bus_type = e1000_bus_type_unknown;
+        hw->bus_type = e1000_bus_type_pci;
         hw->bus_speed = e1000_bus_speed_unknown;
         hw->bus_width = e1000_bus_width_unknown;
         break;

Re: [patch 09/14] e1000: fix ethtool reported bus type for older adapters

[Jeff Garzik]

Arjan van de Ven wrote:
> Subject: e1000: fix ethtool reported bus type for older adapters
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> 
> For older adapters we know that they are of the PCI bus type, so we can
> just set this.
> 
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

ACK patches 1-2, 8-9

[patch 10/14] e1000: narrow down the scope of the tipg timer tweak

[Arjan van de Ven]

Subject: e1000: narrow down the scope of the tipg timer tweak
From: Jesse Brandeburg <jesse.brandeburg@intel.com>

the driver has (ancient) code for messing with TIPG from the 82542 days.
Unfortunately this code was running on our current adapters and setting
TIPG for fiber to be +1 over the copper value.  This caused 1.45Mpps
to be sent instead of 1.487Mpps.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---

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

Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -1582,9 +1582,9 @@ e1000_configure_tx(struct e1000_adapter 
 	}
 
 	/* Set the default values for the Tx Inter Packet Gap timer */
-
-	if (hw->media_type == e1000_media_type_fiber ||
-	    hw->media_type == e1000_media_type_internal_serdes)
+	if (adapter->hw.mac_type <= e1000_82547_rev_2 &&
+	    (hw->media_type == e1000_media_type_fiber ||
+	     hw->media_type == e1000_media_type_internal_serdes))
 		tipg = DEFAULT_82543_TIPG_IPGT_FIBER;
 	else
 		tipg = DEFAULT_82543_TIPG_IPGT_COPPER;

Re: [patch 10/14] e1000: narrow down the scope of the tipg timer tweak

[Jeff Garzik]

Arjan van de Ven wrote:
> Subject: e1000: narrow down the scope of the tipg timer tweak
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> the driver has (ancient) code for messing with TIPG from the 82542 days.
> Unfortunately this code was running on our current adapters and setting
> TIPG for fiber to be +1 over the copper value.  This caused 1.45Mpps
> to be sent instead of 1.487Mpps.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> ---
> 
>  drivers/net/e1000/e1000_main.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6/drivers/net/e1000/e1000_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/e1000/e1000_main.c
> +++ linux-2.6/drivers/net/e1000/e1000_main.c
> @@ -1582,9 +1582,9 @@ e1000_configure_tx(struct e1000_adapter 
>  	}
>  
>  	/* Set the default values for the Tx Inter Packet Gap timer */
> -
> -	if (hw->media_type == e1000_media_type_fiber ||
> -	    hw->media_type == e1000_media_type_internal_serdes)
> +	if (adapter->hw.mac_type <= e1000_82547_rev_2 &&
> +	    (hw->media_type == e1000_media_type_fiber ||
> +	     hw->media_type == e1000_media_type_internal_serdes))
>  		tipg = DEFAULT_82543_TIPG_IPGT_FIBER;
>  	else
>  		tipg = DEFAULT_82543_TIPG_IPGT_COPPER;

NAK, ditto

[patch 11/14] e1000: Fix PBA allocation calculations

[Arjan van de Ven]

Subject: e1000: Fix PBA allocation calculations
From: Bruce Allan <bruce.w.allan@intel.com>

Assign the PBA to be large enough to contain at least 2 jumbo frames on
all adapters. This dramatically increases performance on several adapters
and fixes TX performance degradation issues where the PBA was misallocated
in the old algorithm.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---

 drivers/net/e1000/e1000_hw.h   |    1 
 drivers/net/e1000/e1000_main.c |   99 +++++++++++++++++++++++++++++++++++------
 2 files changed, 86 insertions(+), 14 deletions(-)

Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -661,16 +661,34 @@ e1000_reinit_locked(struct e1000_adapter
 void
 e1000_reset(struct e1000_adapter *adapter)
 {
-	uint32_t pba;
+	uint32_t pba = 0, tx_space, min_tx_space, min_rx_space;
 	uint16_t fc_high_water_mark = E1000_FC_HIGH_DIFF;
+	boolean_t legacy_pba_adjust = FALSE;
 
 	/* Repartition Pba for greater than 9k mtu
 	 * To take effect CTRL.RST is required.
 	 */
 
 	switch (adapter->hw.mac_type) {
+	case e1000_82542_rev2_0:
+	case e1000_82542_rev2_1:
+	case e1000_82543:
+	case e1000_82544:
+	case e1000_82540:
+	case e1000_82541:
+	case e1000_82541_rev_2:
+		legacy_pba_adjust = TRUE;
+		pba = E1000_PBA_48K;
+		break;
+	case e1000_82545:
+	case e1000_82545_rev_3:
+	case e1000_82546:
+	case e1000_82546_rev_3:
+		pba = E1000_PBA_48K;
+		break;
 	case e1000_82547:
 	case e1000_82547_rev_2:
+		legacy_pba_adjust = TRUE;
 		pba = E1000_PBA_30K;
 		break;
 	case e1000_82571:
@@ -679,27 +697,80 @@ e1000_reset(struct e1000_adapter *adapte
 		pba = E1000_PBA_38K;
 		break;
 	case e1000_82573:
-		pba = E1000_PBA_12K;
+		pba = E1000_PBA_20K;
 		break;
 	case e1000_ich8lan:
 		pba = E1000_PBA_8K;
-		break;
-	default:
-		pba = E1000_PBA_48K;
+	case e1000_undefined:
+	case e1000_num_macs:
 		break;
 	}
 
-	if ((adapter->hw.mac_type != e1000_82573) &&
-	   (adapter->netdev->mtu > E1000_RXBUFFER_8192))
-		pba -= 8; /* allocate more FIFO for Tx */
+	if (legacy_pba_adjust == TRUE) {
+		if (adapter->netdev->mtu > E1000_RXBUFFER_8192)
+			pba -= 8; /* allocate more FIFO for Tx */
 
+		if (adapter->hw.mac_type == e1000_82547) {
+			adapter->tx_fifo_head = 0;
+			adapter->tx_head_addr = pba << E1000_TX_HEAD_ADDR_SHIFT;
+			adapter->tx_fifo_size =
+				(E1000_PBA_40K - pba) << E1000_PBA_BYTES_SHIFT;
+			atomic_set(&adapter->tx_fifo_stall, 0);
+		}
+	} else if (adapter->hw.max_frame_size > MAXIMUM_ETHERNET_FRAME_SIZE) {
+		/* adjust PBA for jumbo frames */
+		E1000_WRITE_REG(&adapter->hw, PBA, pba);
+
+		/* To maintain wire speed transmits, the Tx FIFO should be
+		 * large enough to accomodate two full transmit packets,
+		 * rounded up to the next 1KB and expressed in KB.  Likewise,
+		 * the Rx FIFO should be large enough to accomodate at least
+		 * one full receive packet and is similarly rounded up and
+		 * expressed in KB. */
+		pba = E1000_READ_REG(&adapter->hw, PBA);
+		/* upper 16 bits has Tx packet buffer allocation size in KB */
+		tx_space = pba >> 16;
+		/* lower 16 bits has Rx packet buffer allocation size in KB */
+		pba &= 0xffff;
+		/* don't include ethernet FCS because hardware appends/strips */
+		min_rx_space = adapter->netdev->mtu + ENET_HEADER_SIZE +
+		               VLAN_TAG_SIZE;
+		min_tx_space = min_rx_space;
+		min_tx_space *= 2;
+		E1000_ROUNDUP(min_tx_space, 1024);
+		min_tx_space >>= 10;
+		E1000_ROUNDUP(min_rx_space, 1024);
+		min_rx_space >>= 10;
+
+		/* If current Tx allocation is less than the min Tx FIFO size,
+		 * and the min Tx FIFO size is less than the current Rx FIFO
+		 * allocation, take space away from current Rx allocation */
+		if (tx_space < min_tx_space &&
+		    ((min_tx_space - tx_space) < pba)) {
+			pba = pba - (min_tx_space - tx_space);
 
-	if (adapter->hw.mac_type == e1000_82547) {
-		adapter->tx_fifo_head = 0;
-		adapter->tx_head_addr = pba << E1000_TX_HEAD_ADDR_SHIFT;
-		adapter->tx_fifo_size =
-			(E1000_PBA_40K - pba) << E1000_PBA_BYTES_SHIFT;
-		atomic_set(&adapter->tx_fifo_stall, 0);
+			/* PCI/PCIx hardware has PBA alignment constraints */
+			switch (adapter->hw.mac_type) {
+			case e1000_82545 ... e1000_82546_rev_3:
+				pba &= ~(E1000_PBA_8K - 1);
+				break;
+			default:
+				break;
+			}
+
+			/* if short on rx space, rx wins and must trump tx
+			 * adjustment or use Early Receive if available */
+			if (pba < min_rx_space) {
+				switch (adapter->hw.mac_type) {
+				case e1000_82573:
+					/* ERT enabled in e1000_configure_rx */
+					break;
+				default:
+					pba = min_rx_space;
+					break;
+				}
+			}
+		}
 	}
 
 	E1000_WRITE_REG(&adapter->hw, PBA, pba);
Index: linux-2.6/drivers/net/e1000/e1000_hw.h
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_hw.h
+++ linux-2.6/drivers/net/e1000/e1000_hw.h
@@ -2419,6 +2419,7 @@ struct e1000_host_command_info {
 #define E1000_PBA_8K 0x0008    /* 8KB, default Rx allocation */
 #define E1000_PBA_12K 0x000C    /* 12KB, default Rx allocation */
 #define E1000_PBA_16K 0x0010    /* 16KB, default TX allocation */
+#define E1000_PBA_20K 0x0014
 #define E1000_PBA_22K 0x0016
 #define E1000_PBA_24K 0x0018
 #define E1000_PBA_30K 0x001E

[patch 12/14] e1000: Make the copybreak value a module parameter

[Arjan van de Ven]

Subject: e1000: Make the copybreak value a module parameter
From: Jesse Brandeburg <jesse.brandeburg@intel.com>

Allow the user to vary the size that copybreak works. Currently cb is enabled
for packets < 256 bytes, but various tests indicate that this should be
configurable for specific use cases. In addition, this parameter allows us
to force never/always during testing to get full and predictable coverage of 
both code paths.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---

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

Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -213,6 +213,12 @@ static void e1000_netpoll (struct net_de
 
 extern void e1000_check_options(struct e1000_adapter *adapter);
 
+#define COPYBREAK_DEFAULT 256
+static unsigned int copybreak __read_mostly = COPYBREAK_DEFAULT;
+module_param(copybreak, uint, 0644);
+MODULE_PARM_DESC(copybreak,
+	"Maximum size of packet that is copied to a new buffer on receive");
+
 static pci_ers_result_t e1000_io_error_detected(struct pci_dev *pdev,
                      pci_channel_state_t state);
 static pci_ers_result_t e1000_io_slot_reset(struct pci_dev *pdev);
@@ -264,7 +271,13 @@ e1000_init_module(void)
 	printk(KERN_INFO "%s\n", e1000_copyright);
 
 	ret = pci_register_driver(&e1000_driver);
-
+	if (copybreak != COPYBREAK_DEFAULT) {
+		if (copybreak == 0)
+			printk(KERN_INFO "e1000: copybreak disabled\n");
+		else
+			printk(KERN_INFO "e1000: copybreak enabled for "
+			       "packets <= %u bytes\n", copybreak);
+	}
 	return ret;
 }
 
@@ -4245,8 +4258,7 @@ e1000_clean_rx_irq(struct e1000_adapter 
 		/* code added for copybreak, this should improve
 		 * performance for small packets with large amounts
 		 * of reassembly being done in the stack */
-#define E1000_CB_LENGTH 256
-		if (length < E1000_CB_LENGTH) {
+		if (length < copybreak) {
 			struct sk_buff *new_skb =
 			    netdev_alloc_skb(netdev, length + NET_IP_ALIGN);
 			if (new_skb) {
@@ -4404,7 +4416,7 @@ e1000_clean_rx_irq_ps(struct e1000_adapt
 
 		/* page alloc/put takes too long and effects small packet
 		 * throughput, so unsplit small packets and save the alloc/put*/
-		if (l1 && ((length + l1) <= adapter->rx_ps_bsize0)) {
+		if (l1 && (l1 <= copybreak) && ((length + l1) <= adapter->rx_ps_bsize0)) {
 			u8 *vaddr;
 			/* there is no documentation about how to call
 			 * kmap_atomic, so we can't hold the mapping

Re: [patch 12/14] e1000: Make the copybreak value a module parameter

[Jeff Garzik]

Arjan van de Ven wrote:
> Subject: e1000: Make the copybreak value a module parameter
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> Allow the user to vary the size that copybreak works. Currently cb is enabled
> for packets < 256 bytes, but various tests indicate that this should be
> configurable for specific use cases. In addition, this parameter allows us
> to force never/always during testing to get full and predictable coverage of 
> both code paths.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

ACK patches 11-12

Re: [patch 12/14] e1000: Make the copybreak value a module parameter

[Jeff Garzik]

Arjan van de Ven wrote:
> Subject: e1000: Make the copybreak value a module parameter
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> Allow the user to vary the size that copybreak works. Currently cb is enabled
> for packets < 256 bytes, but various tests indicate that this should be
> configurable for specific use cases. In addition, this parameter allows us
> to force never/always during testing to get full and predictable coverage of 
> both code paths.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

ACK 8-12

[patch 13/14] e1000: 3 new driver stats for managability testing

[Arjan van de Ven]

Subject: e1000: 3 new driver stats for managability testing
From: Jesse Brandeburg <jesse.brandeburg@intel.com>

Add 3 extra packet redirect counters for tracking purposes to make sure we
can test that all packets arrive properly.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---

 drivers/net/e1000/e1000_ethtool.c |    3 +++
 drivers/net/e1000/e1000_main.c    |    7 +++++++
 2 files changed, 10 insertions(+)

Index: linux-2.6/drivers/net/e1000/e1000_ethtool.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_ethtool.c
+++ linux-2.6/drivers/net/e1000/e1000_ethtool.c
@@ -100,6 +100,9 @@ static const struct e1000_stats e1000_gs
 	{ "rx_csum_offload_errors", E1000_STAT(hw_csum_err) },
 	{ "rx_header_split", E1000_STAT(rx_hdr_split) },
 	{ "alloc_rx_buff_failed", E1000_STAT(alloc_rx_buff_failed) },
+	{ "tx_smbus", E1000_STAT(stats.mgptc) },
+	{ "rx_smbus", E1000_STAT(stats.mgprc) },
+	{ "dropped_smbus", E1000_STAT(stats.mgpdc) },
 };
 
 #define E1000_QUEUE_STATS_LEN 0
Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -3633,6 +3633,13 @@ e1000_update_stats(struct e1000_adapter 
 			adapter->phy_stats.receive_errors += phy_tmp;
 	}
 
+	/* Management Stats */
+	if (adapter->hw.mac_type > e1000_82544) {
+		adapter->stats.mgptc += E1000_READ_REG(hw, MGTPTC);
+		adapter->stats.mgprc += E1000_READ_REG(hw, MGTPRC);
+		adapter->stats.mgpdc += E1000_READ_REG(hw, MGTPDC);
+	}
+
 	spin_unlock_irqrestore(&adapter->stats_lock, flags);
 }
 #ifdef CONFIG_PCI_MSI

Re: [patch 13/14] e1000: 3 new driver stats for managability testing

[Jeff Garzik]

Arjan van de Ven wrote:
> Subject: e1000: 3 new driver stats for managability testing
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> Add 3 extra packet redirect counters for tracking purposes to make sure we
> can test that all packets arrive properly.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> ---
> 
>  drivers/net/e1000/e1000_ethtool.c |    3 +++
>  drivers/net/e1000/e1000_main.c    |    7 +++++++
>  2 files changed, 10 insertions(+)
> 
> Index: linux-2.6/drivers/net/e1000/e1000_ethtool.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/e1000/e1000_ethtool.c
> +++ linux-2.6/drivers/net/e1000/e1000_ethtool.c
> @@ -100,6 +100,9 @@ static const struct e1000_stats e1000_gs
>  	{ "rx_csum_offload_errors", E1000_STAT(hw_csum_err) },
>  	{ "rx_header_split", E1000_STAT(rx_hdr_split) },
>  	{ "alloc_rx_buff_failed", E1000_STAT(alloc_rx_buff_failed) },
> +	{ "tx_smbus", E1000_STAT(stats.mgptc) },
> +	{ "rx_smbus", E1000_STAT(stats.mgprc) },
> +	{ "dropped_smbus", E1000_STAT(stats.mgpdc) },
>  };
>  
>  #define E1000_QUEUE_STATS_LEN 0
> Index: linux-2.6/drivers/net/e1000/e1000_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/e1000/e1000_main.c
> +++ linux-2.6/drivers/net/e1000/e1000_main.c
> @@ -3633,6 +3633,13 @@ e1000_update_stats(struct e1000_adapter 
>  			adapter->phy_stats.receive_errors += phy_tmp;
>  	}
>  
> +	/* Management Stats */
> +	if (adapter->hw.mac_type > e1000_82544) {
> +		adapter->stats.mgptc += E1000_READ_REG(hw, MGTPTC);
> +		adapter->stats.mgprc += E1000_READ_REG(hw, MGTPRC);
> +		adapter->stats.mgpdc += E1000_READ_REG(hw, MGTPDC);
> +	}
> +

NAK, ditto

Re: [patch 13/14] e1000: 3 new driver stats for managability testing

[Jeff Garzik]

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

Arjan van de Ven wrote:
> Subject: e1000: 3 new driver stats for managability testing
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> Add 3 extra packet redirect counters for tracking purposes to make sure we
> can test that all packets arrive properly.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

as revised...


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 2491 bytes --]

commit 979b575f7666d12157d8b975f022bf8fb6d23c64
Author: Jeff Garzik <jeff@garzik.org>
Date:   Fri Dec 15 11:16:33 2006 -0500

    e1000: 3 new driver stats for managability testing
    
    Add 3 extra packet redirect counters for tracking purposes to make sure
    we can test that all packets arrive properly.
    
    Originally from Jesse Brandeburg <jesse.brandeburg@intel.com>,
    rewritten to use feature flags by me.
    
    Signed-off-by: Jeff Garzik <jeff@garzik.org>

979b575f7666d12157d8b975f022bf8fb6d23c64
diff --git a/drivers/net/e1000/e1000_ethtool.c b/drivers/net/e1000/e1000_ethtool.c
index da459f7..fb96c87 100644
--- a/drivers/net/e1000/e1000_ethtool.c
+++ b/drivers/net/e1000/e1000_ethtool.c
@@ -100,6 +100,9 @@ static const struct e1000_stats e1000_gstrings_stats[] = {
 	{ "rx_csum_offload_errors", E1000_STAT(hw_csum_err) },
 	{ "rx_header_split", E1000_STAT(rx_hdr_split) },
 	{ "alloc_rx_buff_failed", E1000_STAT(alloc_rx_buff_failed) },
+	{ "tx_smbus", E1000_STAT(stats.mgptc) },
+	{ "rx_smbus", E1000_STAT(stats.mgprc) },
+	{ "dropped_smbus", E1000_STAT(stats.mgpdc) },
 };
 
 #define E1000_QUEUE_STATS_LEN 0
diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
index 5a6a61e..9be4469 100644
--- a/drivers/net/e1000/e1000_hw.c
+++ b/drivers/net/e1000/e1000_hw.c
@@ -458,6 +458,9 @@ e1000_set_mac_type(struct e1000_hw *hw)
 	if (hw->mac_type == e1000_80003es2lan)
 		hw->rx_needs_kicking = TRUE;
 
+	if (hw->mac_type > e1000_82544)
+		hw->has_smbus = TRUE;
+
 	return E1000_SUCCESS;
 }
 
diff --git a/drivers/net/e1000/e1000_hw.h b/drivers/net/e1000/e1000_hw.h
index 15b8625..d671058 100644
--- a/drivers/net/e1000/e1000_hw.h
+++ b/drivers/net/e1000/e1000_hw.h
@@ -1464,6 +1464,7 @@ struct e1000_hw {
 	boolean_t		bad_tx_carr_stats_fd;
 	boolean_t		has_manc2h;
 	boolean_t		rx_needs_kicking;
+	boolean_t		has_smbus;
 };
 
 
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 3f40a90..b06b51a 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -3743,6 +3743,13 @@ e1000_update_stats(struct e1000_adapter *adapter)
 			adapter->phy_stats.receive_errors += phy_tmp;
 	}
 
+	/* Management Stats */
+	if (adapter->hw.has_smbus) {
+		adapter->stats.mgptc += E1000_READ_REG(hw, MGTPTC);
+		adapter->stats.mgprc += E1000_READ_REG(hw, MGTPRC);
+		adapter->stats.mgpdc += E1000_READ_REG(hw, MGTPDC);
+	}
+
 	spin_unlock_irqrestore(&adapter->stats_lock, flags);
 }
 #ifdef CONFIG_PCI_MSI

[patch 14/14] e1000: No-delay link detection at interface up

[Arjan van de Ven]

Subject: e1000: No-delay link detection at interface up
From: Jesse Brandeburg <jesse.brandeburg@intel.com>

Currently after an interface up, the link state is detected 2 seconds later
when the first watchdog timer runs. This patch changes that by triggering
the hardware to generate a link-change interrupt from the up() function
instead. This has the result that the link state gets detected immediately
and without races. This has the potential to speed up booting since a normal
distribution boot process waits for a link before DHCP is attempted.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---

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

Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6/drivers/net/e1000/e1000_main.c
@@ -543,7 +543,8 @@ e1000_up(struct e1000_adapter *adapter)
 
 	clear_bit(__E1000_DOWN, &adapter->flags);
 
-	mod_timer(&adapter->watchdog_timer, jiffies + 2 * HZ);
+	/* fire a link change interrupt to start the watchdog */
+	E1000_WRITE_REG(&adapter->hw, ICS, E1000_ICS_LSC);
 	return 0;
 }
 

Re: [patch 14/14] e1000: No-delay link detection at interface up

[Jeff Garzik]

Arjan van de Ven wrote:
> Subject: e1000: No-delay link detection at interface up
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> Currently after an interface up, the link state is detected 2 seconds later
> when the first watchdog timer runs. This patch changes that by triggering
> the hardware to generate a link-change interrupt from the up() function
> instead. This has the result that the link state gets detected immediately
> and without races. This has the potential to speed up booting since a normal
> distribution boot process waits for a link before DHCP is attempted.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

ACK, though you can also just schedule the timer to run immediately, or 
even run it yourself inside spin_lock_bh()

	Jeff

Re: [patch 14/14] e1000: No-delay link detection at interface up

[Arjan van de Ven]

Jeff Garzik wrote:
> ACK, though you can also just schedule the timer to run immediately, or 
> even run it yourself inside spin_lock_bh()

sure but this is very simple, clean and obvious ;)

Re: [patch 14/14] e1000: No-delay link detection at interface up

[Jeff Garzik]

Arjan van de Ven wrote:
> Jeff Garzik wrote:
>> ACK, though you can also just schedule the timer to run immediately, 
>> or even run it yourself inside spin_lock_bh()
> 
> sure but this is very simple, clean and obvious ;)

Hence the ACK...

	Jeff

Re: [patch 00/14] E1000 bugfix series for 2.6.20

[Jeff Garzik]

Arjan van de Ven wrote:
> Hi,
> 
> this patch series contains critical bugfixes to the e1000 driver for
> 2.6.20 (and 3 very low intrusive features that help in testing). Without
> these patches e1000 is totally unusable on my test machine.

Didn't Auke just submit these, and have them ACKd/NAKd as appropriate?

	Jeff

Re: [patch 00/14] E1000 bugfix series for 2.6.20

[Arjan van de Ven]

Jeff Garzik wrote:
> Arjan van de Ven wrote:
>> Hi,
>>
>> this patch series contains critical bugfixes to the e1000 driver for
>> 2.6.20 (and 3 very low intrusive features that help in testing). Without
>> these patches e1000 is totally unusable on my test machine.
> 
> Didn't Auke just submit these, and have them ACKd/NAKd as appropriate?
> 

they're not in 2.6.20-rc1 so I suspect they didn't go anywhere...
(and Auke is on holiday for a bit afaik)

Re: [patch 00/14] E1000 bugfix series for 2.6.20

[Jeff Garzik]

Arjan van de Ven wrote:
> Jeff Garzik wrote:
>> Arjan van de Ven wrote:
>>> Hi,
>>>
>>> this patch series contains critical bugfixes to the e1000 driver for
>>> 2.6.20 (and 3 very low intrusive features that help in testing). Without
>>> these patches e1000 is totally unusable on my test machine.
>>
>> Didn't Auke just submit these, and have them ACKd/NAKd as appropriate?
>>
> 
> they're not in 2.6.20-rc1 so I suspect they didn't go anywhere...
> (and Auke is on holiday for a bit afaik)

They didn't get applied because the patches need the revisions I requested.

	Jeff

Re: [patch 00/14] E1000 bugfix series for 2.6.20

[Arjan van de Ven]

Jeff Garzik wrote:
> 
> They didn't get applied because the patches need the revisions I requested.

the patches I posted are cleaned up subset...