[PATCH 00/10] e1000: Driver fixes and update to 7.0.38-k2

[PATCH 00/10] e1000: Driver fixes and update to 7.0.38-k2

[Kok, Auke]

Hi,

This patch series implements two fixes, a series of cleanups and
two optimization patches.

The version of the driver was bumped to 7.0.38-k2. Year changes to 2006.

As I'm taking over patch duties from Jeff Kirsher, I added myself
to the MAINTAINERS file.

These changes are available through git.

Jeff, please pull from:

git://63.64.152.142/~ahkok/git/netdev-2.6 e1000-7.0.38-k2


Cheers,

Auke Kok


---
01/10: [FIX] Remove PM warning DPRINTKs breaking 2.4.x kernels
02/10: [FIX] Esb2 wol link cycle bug and uninitialized registers
03/10: [PATCH] De-inline functions to benefit from compiler smartness
04/10: [PATCH] Made an adapter struct variable into a local (txb2b)
05/10: [PATCH] Update truesize with the length of the packet for
                      packet split
06/10: [PATCH] Dead variable cleanup
07/10: [PATCH] Buffer optimizations for small MTU
08/10: [PATCH] implement more efficient tx queue locking
09/10: [DOC] Version bump, contact fix, year string change
10/10: [MAINTAINERS] {e100{,0},ixgb}: Add Auke Kok as new patch maintainer
                     for e{100,1000} and ixgb


---
 MAINTAINERS                       |    6 +
 drivers/net/e1000/Makefile        |    1 
 drivers/net/e1000/e1000.h         |    6 -
 drivers/net/e1000/e1000_ethtool.c |    3 
 drivers/net/e1000/e1000_hw.c      |    3 
 drivers/net/e1000/e1000_hw.h      |    3 
 drivers/net/e1000/e1000_main.c    |  198 ++++++++++++++++++--------------------
 drivers/net/e1000/e1000_osdep.h   |    3 
 drivers/net/e1000/e1000_param.c   |    3 
 9 files changed, 118 insertions(+), 108 deletions(-)


--
Auke Kok <auke-jan.h.kok@intel.com>
Intel Pro Ethernet Driver Group
LAN Access Division / Digital Enterprise Group 

[PATCH 01/10] e1000: Remove PM warning DPRINTKs breaking 2.4.x kernels

[Kok, Auke]

remove DPRINTKs that were printing warnings about power management on
2.4 kernels.  Since we really don't react differently these printk
statements are not needed.  This code was originally added to fix
some compile time warnings that got fixed by newer kernels.


Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: John Ronciak <john.ronciak@intel.com>
---

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

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index add8dc4..ac1e838 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -4515,21 +4515,13 @@ e1000_suspend(struct pci_dev *pdev, pm_m
 
 		E1000_WRITE_REG(&adapter->hw, WUC, E1000_WUC_PME_EN);
 		E1000_WRITE_REG(&adapter->hw, WUFC, wufc);
-		retval = pci_enable_wake(pdev, PCI_D3hot, 1);
-		if (retval)
-			DPRINTK(PROBE, ERR, "Error enabling D3 wake\n");
-		retval = pci_enable_wake(pdev, PCI_D3cold, 1);
-		if (retval)
-			DPRINTK(PROBE, ERR, "Error enabling D3 cold wake\n");
+		pci_enable_wake(pdev, PCI_D3hot, 1);
+		pci_enable_wake(pdev, PCI_D3cold, 1);
 	} else {
 		E1000_WRITE_REG(&adapter->hw, WUC, 0);
 		E1000_WRITE_REG(&adapter->hw, WUFC, 0);
-		retval = pci_enable_wake(pdev, PCI_D3hot, 0);
-		if (retval)
-			DPRINTK(PROBE, ERR, "Error enabling D3 wake\n");
-		retval = pci_enable_wake(pdev, PCI_D3cold, 0);
-		if (retval)
-			DPRINTK(PROBE, ERR, "Error enabling D3 cold wake\n");
+		pci_enable_wake(pdev, PCI_D3hot, 0);
+		pci_enable_wake(pdev, PCI_D3cold, 0);
 	}
 
 	if (adapter->hw.mac_type >= e1000_82540 &&
@@ -4538,13 +4530,8 @@ e1000_suspend(struct pci_dev *pdev, pm_m
 		if (manc & E1000_MANC_SMBUS_EN) {
 			manc |= E1000_MANC_ARP_EN;
 			E1000_WRITE_REG(&adapter->hw, MANC, manc);
-			retval = pci_enable_wake(pdev, PCI_D3hot, 1);
-			if (retval)
-				DPRINTK(PROBE, ERR, "Error enabling D3 wake\n");
-			retval = pci_enable_wake(pdev, PCI_D3cold, 1);
-			if (retval)
-				DPRINTK(PROBE, ERR,
-				        "Error enabling D3 cold wake\n");
+			pci_enable_wake(pdev, PCI_D3hot, 1);
+			pci_enable_wake(pdev, PCI_D3cold, 1);
 		}
 	}
 
@@ -4554,9 +4541,7 @@ e1000_suspend(struct pci_dev *pdev, pm_m
 
 	pci_disable_device(pdev);
 
-	retval = pci_set_power_state(pdev, pci_choose_state(pdev, state));
-	if (retval)
-		DPRINTK(PROBE, ERR, "Error in setting power state\n");
+	pci_set_power_state(pdev, pci_choose_state(pdev, state));
 
 	return 0;
 }
@@ -4567,22 +4552,15 @@ e1000_resume(struct pci_dev *pdev)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	int retval;
 	uint32_t manc, ret_val;
 
-	retval = pci_set_power_state(pdev, PCI_D0);
-	if (retval)
-		DPRINTK(PROBE, ERR, "Error in setting power state\n");
+	pci_set_power_state(pdev, PCI_D0);
 	e1000_pci_restore_state(adapter);
 	ret_val = pci_enable_device(pdev);
 	pci_set_master(pdev);
 
-	retval = pci_enable_wake(pdev, PCI_D3hot, 0);
-	if (retval)
-		DPRINTK(PROBE, ERR, "Error enabling D3 wake\n");
-	retval = pci_enable_wake(pdev, PCI_D3cold, 0);
-	if (retval)
-		DPRINTK(PROBE, ERR, "Error enabling D3 cold wake\n");
+	pci_enable_wake(pdev, PCI_D3hot, 0);
+	pci_enable_wake(pdev, PCI_D3cold, 0);
 
 	e1000_reset(adapter);
 	E1000_WRITE_REG(&adapter->hw, WUS, ~0);



--
Auke Kok <auke-jan.h.kok@intel.com>
Intel Pro Ethernet Driver Group
LAN Access Division / Digital Enterprise Group 

[PATCH 02/10] e1000: Esb2 wol link cycle bug and uninitialized registers

[Kok, Auke]

Esb2 link didn't return after wol disable. The code previously assumed
that writing reset to PHY_CTRL phy register turned the phy back on.
In the ESB2 phy case that didn't occur.

Add ESB2 to acquire/release_hw functions upon review it was
discovered that esb2 was skipped on these functions


Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: John Ronciak <john.ronciak@intel.com>
---

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

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index ac1e838..8de9f9a 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -359,6 +359,7 @@ e1000_release_hw_control(struct e1000_ad
 	switch (adapter->hw.mac_type) {
 	case e1000_82571:
 	case e1000_82572:
+	case e1000_80003es2lan:
 		ctrl_ext = E1000_READ_REG(&adapter->hw, CTRL_EXT);
 		E1000_WRITE_REG(&adapter->hw, CTRL_EXT,
 				ctrl_ext & ~E1000_CTRL_EXT_DRV_LOAD);
@@ -392,6 +393,7 @@ e1000_get_hw_control(struct e1000_adapte
 	switch (adapter->hw.mac_type) {
 	case e1000_82571:
 	case e1000_82572:
+	case e1000_80003es2lan:
 		ctrl_ext = E1000_READ_REG(&adapter->hw, CTRL_EXT);
 		E1000_WRITE_REG(&adapter->hw, CTRL_EXT,
 				ctrl_ext | E1000_CTRL_EXT_DRV_LOAD);
@@ -419,7 +421,7 @@ e1000_up(struct e1000_adapter *adapter)
 		uint16_t mii_reg;
 		e1000_read_phy_reg(&adapter->hw, PHY_CTRL, &mii_reg);
 		if (mii_reg & MII_CR_POWER_DOWN)
-			e1000_phy_reset(&adapter->hw);
+			e1000_phy_hw_reset(&adapter->hw);
 	}
 
 	e1000_set_multi(netdev);



--
Auke Kok <auke-jan.h.kok@intel.com>
Intel Pro Ethernet Driver Group
LAN Access Division / Digital Enterprise Group 

[PATCH 03/10] e1000: De-inline functions to benefit from compiler smartness

[Kok, Auke]

De-inline functions to benefit from compiler smartness


Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: John Ronciak <john.ronciak@intel.com>
---

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

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 8de9f9a..5e7b4ee 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -208,8 +208,8 @@ static void e1000_leave_82542_rst(struct
 static void e1000_tx_timeout(struct net_device *dev);
 static void e1000_reset_task(struct net_device *dev);
 static void e1000_smartspeed(struct e1000_adapter *adapter);
-static inline int e1000_82547_fifo_workaround(struct e1000_adapter *adapter,
-					      struct sk_buff *skb);
+static int e1000_82547_fifo_workaround(struct e1000_adapter *adapter,
+                                       struct sk_buff *skb);
 
 static void e1000_vlan_rx_register(struct net_device *netdev, struct vlan_group *grp);
 static void e1000_vlan_rx_add_vid(struct net_device *netdev, uint16_t vid);
@@ -291,7 +291,7 @@ module_exit(e1000_exit_module);
  * @adapter: board private structure
  **/
 
-static inline void
+static void
 e1000_irq_disable(struct e1000_adapter *adapter)
 {
 	atomic_inc(&adapter->irq_sem);
@@ -305,7 +305,7 @@ e1000_irq_disable(struct e1000_adapter *
  * @adapter: board private structure
  **/
 
-static inline void
+static void
 e1000_irq_enable(struct e1000_adapter *adapter)
 {
 	if (likely(atomic_dec_and_test(&adapter->irq_sem))) {
@@ -349,7 +349,7 @@ e1000_update_mng_vlan(struct e1000_adapt
  * 
  **/
 
-static inline void 
+static void
 e1000_release_hw_control(struct e1000_adapter *adapter)
 {
 	uint32_t ctrl_ext;
@@ -384,7 +384,7 @@ e1000_release_hw_control(struct e1000_ad
  * 
  **/
 
-static inline void 
+static void
 e1000_get_hw_control(struct e1000_adapter *adapter)
 {
 	uint32_t ctrl_ext;
@@ -1181,7 +1181,7 @@ e1000_close(struct net_device *netdev)
  * @start: address of beginning of memory
  * @len: length of memory
  **/
-static inline boolean_t
+static boolean_t
 e1000_check_64k_bound(struct e1000_adapter *adapter,
 		      void *start, unsigned long len)
 {
@@ -1807,7 +1807,7 @@ e1000_free_all_tx_resources(struct e1000
 		e1000_free_tx_resources(adapter, &adapter->tx_ring[i]);
 }
 
-static inline void
+static void
 e1000_unmap_and_free_tx_resource(struct e1000_adapter *adapter,
 			struct e1000_buffer *buffer_info)
 {
@@ -2400,7 +2400,7 @@ e1000_watchdog_task(struct e1000_adapter
 #define E1000_TX_FLAGS_VLAN_MASK	0xffff0000
 #define E1000_TX_FLAGS_VLAN_SHIFT	16
 
-static inline int
+static int
 e1000_tso(struct e1000_adapter *adapter, struct e1000_tx_ring *tx_ring,
           struct sk_buff *skb)
 {
@@ -2480,7 +2480,7 @@ e1000_tso(struct e1000_adapter *adapter,
 	return FALSE;
 }
 
-static inline boolean_t
+static boolean_t
 e1000_tx_csum(struct e1000_adapter *adapter, struct e1000_tx_ring *tx_ring,
               struct sk_buff *skb)
 {
@@ -2516,7 +2516,7 @@ e1000_tx_csum(struct e1000_adapter *adap
 #define E1000_MAX_TXD_PWR	12
 #define E1000_MAX_DATA_PER_TXD	(1<<E1000_MAX_TXD_PWR)
 
-static inline int
+static int
 e1000_tx_map(struct e1000_adapter *adapter, struct e1000_tx_ring *tx_ring,
              struct sk_buff *skb, unsigned int first, unsigned int max_per_txd,
              unsigned int nr_frags, unsigned int mss)
@@ -2625,7 +2625,7 @@ e1000_tx_map(struct e1000_adapter *adapt
 	return count;
 }
 
-static inline void
+static void
 e1000_tx_queue(struct e1000_adapter *adapter, struct e1000_tx_ring *tx_ring,
                int tx_flags, int count)
 {
@@ -2689,7 +2689,7 @@ e1000_tx_queue(struct e1000_adapter *ada
 #define E1000_FIFO_HDR			0x10
 #define E1000_82547_PAD_LEN		0x3E0
 
-static inline int
+static int
 e1000_82547_fifo_workaround(struct e1000_adapter *adapter, struct sk_buff *skb)
 {
 	uint32_t fifo_space = adapter->tx_fifo_size - adapter->tx_fifo_head;
@@ -2716,7 +2716,7 @@ no_fifo_stall_required:
 }
 
 #define MINIMUM_DHCP_PACKET_SIZE 282
-static inline int
+static int
 e1000_transfer_dhcp_info(struct e1000_adapter *adapter, struct sk_buff *skb)
 {
 	struct e1000_hw *hw =  &adapter->hw;
@@ -3445,7 +3445,7 @@ e1000_clean_tx_irq(struct e1000_adapter 
  * @sk_buff:     socket buffer with received data
  **/
 
-static inline void
+static void
 e1000_rx_checksum(struct e1000_adapter *adapter,
 		  uint32_t status_err, uint32_t csum,
 		  struct sk_buff *skb)



--
Auke Kok <auke-jan.h.kok@intel.com>
Intel Pro Ethernet Driver Group
LAN Access Division / Digital Enterprise Group 

[PATCH 04/10] e1000: Made an adapter struct variable into a local (txb2b)

[Kok, Auke]

Made an adapter struct variable into a local (txb2b)


Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: John Ronciak <john.ronciak@intel.com>
---

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

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index 281de41..764808f 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -334,7 +334,6 @@ struct e1000_adapter {
 	boolean_t have_msi;
 #endif
 	/* to not mess up cache alignment, always add to the bottom */
-	boolean_t txb2b;
 #ifdef NETIF_F_TSO
 	boolean_t tso_force;
 #endif
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 5e7b4ee..2cc9955 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -2247,6 +2247,7 @@ e1000_watchdog_task(struct e1000_adapter
 
 	if (link) {
 		if (!netif_carrier_ok(netdev)) {
+			boolean_t txb2b = 1;
 			e1000_get_speed_and_duplex(&adapter->hw,
 			                           &adapter->link_speed,
 			                           &adapter->link_duplex);
@@ -2260,23 +2261,22 @@ e1000_watchdog_task(struct e1000_adapter
 			 * and adjust the timeout factor */
 			netdev->tx_queue_len = adapter->tx_queue_len;
 			adapter->tx_timeout_factor = 1;
-			adapter->txb2b = 1;
 			switch (adapter->link_speed) {
 			case SPEED_10:
-				adapter->txb2b = 0;
+				txb2b = 0;
 				netdev->tx_queue_len = 10;
 				adapter->tx_timeout_factor = 8;
 				break;
 			case SPEED_100:
-				adapter->txb2b = 0;
+				txb2b = 0;
 				netdev->tx_queue_len = 100;
 				/* maybe add some timeout factor ? */
 				break;
 			}
 
-			if ((adapter->hw.mac_type == e1000_82571 || 
+			if ((adapter->hw.mac_type == e1000_82571 ||
 			     adapter->hw.mac_type == e1000_82572) &&
-			    adapter->txb2b == 0) {
+			    txb2b == 0) {
 #define SPEED_MODE_BIT (1 << 21)
 				uint32_t tarc0;
 				tarc0 = E1000_READ_REG(&adapter->hw, TARC0);



--
Auke Kok <auke-jan.h.kok@intel.com>
Intel Pro Ethernet Driver Group
LAN Access Division / Digital Enterprise Group 

[PATCH 05/10] e1000: Update truesize with the length of the packet for packet split

[Kok, Auke]

Update skb with the real packet size.


Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: John Ronciak <john.ronciak@intel.com>
---

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

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 2cc9955..7ba1b16 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -3770,6 +3770,7 @@ e1000_clean_rx_irq_ps(struct e1000_adapt
 			ps_page->ps_page[j] = NULL;
 			skb->len += length;
 			skb->data_len += length;
+			skb->truesize += length;
 		}
 
 copydone:



--
Auke Kok <auke-jan.h.kok@intel.com>
Intel Pro Ethernet Driver Group
LAN Access Division / Digital Enterprise Group 

[PATCH 06/10] e1000: Dead variable cleanup

[Kok, Auke]

Removal of unused rx_dropped counter.

Removed reference to E1000_CTRL_EXT_CANC which is no longer valid,
replaced with E1000_CTRL_EXT_INT_TIMER_CLR


Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: John Ronciak <john.ronciak@intel.com>
---

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

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 7ba1b16..6d7ba0b 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -1715,7 +1715,7 @@ e1000_configure_rx(struct e1000_adapter 
 	if (hw->mac_type >= e1000_82571) {
 		ctrl_ext = E1000_READ_REG(hw, CTRL_EXT);
 		/* Reset delay timers after every interrupt */
-		ctrl_ext |= E1000_CTRL_EXT_CANC;
+		ctrl_ext |= E1000_CTRL_EXT_INT_TIMER_CLR;
 #ifdef CONFIG_E1000_NAPI
 		/* Auto-Mask interrupts upon ICR read. */
 		ctrl_ext |= E1000_CTRL_EXT_IAME;
@@ -3165,7 +3165,6 @@ e1000_update_stats(struct e1000_adapter 
 		adapter->stats.crcerrs + adapter->stats.algnerrc +
 		adapter->stats.ruc + adapter->stats.roc +
 		adapter->stats.cexterr;
-	adapter->net_stats.rx_dropped = 0;
 	adapter->net_stats.rx_length_errors = adapter->stats.ruc +
 	                                      adapter->stats.roc;
 	adapter->net_stats.rx_crc_errors = adapter->stats.crcerrs;



--
Auke Kok <auke-jan.h.kok@intel.com>
Intel Pro Ethernet Driver Group
LAN Access Division / Digital Enterprise Group 

[PATCH 07/10] e1000: Buffer optimizations for small MTU

[Kok, Auke]

Remove multi-descriptor support from legacy recieve path

Add memory usage efficiency by using more correct size descriptors for
small MTU sizes and optimize using LPE for <= 1522 byte frame sizes

An extra performance fix that effected our TCP window size growth
as a receiver.  Set our initial buffer to be 128 bytes instead of 256
to prevent over-socket charge when truesize is computed in the stack.
old way: truesize = 256 + l1 = 256 + 1460 = 1716
new way: truesize = 128 + l1 = 128 + 1460 = 1588
The magic value that we can't cross is 1648.


Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: John Ronciak <john.ronciak@intel.com>
---

 drivers/net/e1000/e1000.h      |    2 +
 drivers/net/e1000/e1000_main.c |   87 +++++++++++++++++++++++-----------------
 2 files changed, 52 insertions(+), 37 deletions(-)

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index 764808f..2dfabdb 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -114,6 +114,8 @@ struct e1000_adapter;
 /* Supported Rx Buffer Sizes */
 #define E1000_RXBUFFER_128   128    /* Used for packet split */
 #define E1000_RXBUFFER_256   256    /* Used for packet split */
+#define E1000_RXBUFFER_512   512
+#define E1000_RXBUFFER_1024  1024
 #define E1000_RXBUFFER_2048  2048
 #define E1000_RXBUFFER_4096  4096
 #define E1000_RXBUFFER_8192  8192
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 6d7ba0b..f604a1d 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -972,8 +972,8 @@ e1000_sw_init(struct e1000_adapter *adap
 
 	pci_read_config_word(pdev, PCI_COMMAND, &hw->pci_cmd_word);
 
-	adapter->rx_buffer_len = E1000_RXBUFFER_2048;
-	adapter->rx_ps_bsize0 = E1000_RXBUFFER_256;
+	adapter->rx_buffer_len = MAXIMUM_ETHERNET_FRAME_SIZE;
+	adapter->rx_ps_bsize0 = E1000_RXBUFFER_128;
 	hw->max_frame_size = netdev->mtu +
 			     ENET_HEADER_SIZE + ETHERNET_FCS_SIZE;
 	hw->min_frame_size = MINIMUM_ETHERNET_FRAME_SIZE;
@@ -1599,14 +1599,21 @@ e1000_setup_rctl(struct e1000_adapter *a
 		rctl |= E1000_RCTL_LPE;
 
 	/* Setup buffer sizes */
-	if (adapter->hw.mac_type >= e1000_82571) {
-		/* We can now specify buffers in 1K increments.
-		 * BSIZE and BSEX are ignored in this case. */
-		rctl |= adapter->rx_buffer_len << 0x11;
-	} else {
-		rctl &= ~E1000_RCTL_SZ_4096;
-		rctl |= E1000_RCTL_BSEX; 
-		switch (adapter->rx_buffer_len) {
+	rctl &= ~E1000_RCTL_SZ_4096;
+	rctl |= E1000_RCTL_BSEX;
+	switch (adapter->rx_buffer_len) {
+		case E1000_RXBUFFER_256:
+			rctl |= E1000_RCTL_SZ_256;
+			rctl &= ~E1000_RCTL_BSEX;
+			break;
+		case E1000_RXBUFFER_512:
+			rctl |= E1000_RCTL_SZ_512;
+			rctl &= ~E1000_RCTL_BSEX;
+			break;
+		case E1000_RXBUFFER_1024:
+			rctl |= E1000_RCTL_SZ_1024;
+			rctl &= ~E1000_RCTL_BSEX;
+			break;
 		case E1000_RXBUFFER_2048:
 		default:
 			rctl |= E1000_RCTL_SZ_2048;
@@ -1621,7 +1628,6 @@ e1000_setup_rctl(struct e1000_adapter *a
 		case E1000_RXBUFFER_16384:
 			rctl |= E1000_RCTL_SZ_16384;
 			break;
-		}
 	}
 
 #ifndef CONFIG_E1000_DISABLE_PACKET_SPLIT
@@ -2982,8 +2988,7 @@ e1000_change_mtu(struct net_device *netd
 
 	/* Adapter-specific max frame size limits. */
 	switch (adapter->hw.mac_type) {
-	case e1000_82542_rev2_0:
-	case e1000_82542_rev2_1:
+	case e1000_undefined ... e1000_82542_rev2_1:
 		if (max_frame > MAXIMUM_ETHERNET_FRAME_SIZE) {
 			DPRINTK(PROBE, ERR, "Jumbo Frames not supported.\n");
 			return -EINVAL;
@@ -3017,27 +3022,32 @@ e1000_change_mtu(struct net_device *netd
 		break;
 	}
 
-
-	if (adapter->hw.mac_type > e1000_82547_rev_2) {
-		adapter->rx_buffer_len = max_frame;
-		E1000_ROUNDUP(adapter->rx_buffer_len, 1024);
-	} else {
-		if(unlikely((adapter->hw.mac_type < e1000_82543) &&
-		   (max_frame > MAXIMUM_ETHERNET_FRAME_SIZE))) {
-			DPRINTK(PROBE, ERR, "Jumbo Frames not supported "
-					    "on 82542\n");
-			return -EINVAL;
-		} else {
-			if(max_frame <= E1000_RXBUFFER_2048)
-				adapter->rx_buffer_len = E1000_RXBUFFER_2048;
-			else if(max_frame <= E1000_RXBUFFER_4096)
-				adapter->rx_buffer_len = E1000_RXBUFFER_4096;
-			else if(max_frame <= E1000_RXBUFFER_8192)
-				adapter->rx_buffer_len = E1000_RXBUFFER_8192;
-			else if(max_frame <= E1000_RXBUFFER_16384)
-				adapter->rx_buffer_len = E1000_RXBUFFER_16384;
-		}
-	}
+	/* NOTE: dev_alloc_skb reserves 16 bytes, and typically NET_IP_ALIGN
+	 * means we reserve 2 more, this pushes us to allocate from the next
+	 * larger slab size
+	 * i.e. RXBUFFER_2048 --> size-4096 slab */
+
+	if (max_frame <= E1000_RXBUFFER_256)
+		adapter->rx_buffer_len = E1000_RXBUFFER_256;
+	else if (max_frame <= E1000_RXBUFFER_512)
+		adapter->rx_buffer_len = E1000_RXBUFFER_512;
+	else if (max_frame <= E1000_RXBUFFER_1024)
+		adapter->rx_buffer_len = E1000_RXBUFFER_1024;
+	else if (max_frame <= E1000_RXBUFFER_2048)
+		adapter->rx_buffer_len = E1000_RXBUFFER_2048;
+	else if (max_frame <= E1000_RXBUFFER_4096)
+		adapter->rx_buffer_len = E1000_RXBUFFER_4096;
+	else if (max_frame <= E1000_RXBUFFER_8192)
+		adapter->rx_buffer_len = E1000_RXBUFFER_8192;
+	else if (max_frame <= E1000_RXBUFFER_16384)
+		adapter->rx_buffer_len = E1000_RXBUFFER_16384;
+
+	/* adjust allocation if LPE protects us, and we aren't using SBP */
+#define MAXIMUM_ETHERNET_VLAN_SIZE 1522
+	if (!adapter->hw.tbi_compatibility_on &&
+	    ((max_frame == MAXIMUM_ETHERNET_FRAME_SIZE) ||
+	     (max_frame == MAXIMUM_ETHERNET_VLAN_SIZE)))
+		adapter->rx_buffer_len = MAXIMUM_ETHERNET_VLAN_SIZE;
 
 	netdev->mtu = new_mtu;
 
@@ -3568,10 +3578,12 @@ e1000_clean_rx_irq(struct e1000_adapter 
 				                       flags);
 				length--;
 			} else {
-				dev_kfree_skb_irq(skb);
+				/* recycle */
+				buffer_info->skb = skb;
 				goto next_desc;
 			}
-		}
+		} else
+			skb_put(skb, length);
 
 		/* code added for copybreak, this should improve
 		 * performance for small packets with large amounts
@@ -3676,6 +3688,7 @@ e1000_clean_rx_irq_ps(struct e1000_adapt
 	i = rx_ring->next_to_clean;
 	rx_desc = E1000_RX_DESC_PS(*rx_ring, i);
 	staterr = le32_to_cpu(rx_desc->wb.middle.status_error);
+	buffer_info = &rx_ring->buffer_info[i];
 
 	while (staterr & E1000_RXD_STAT_DD) {
 		buffer_info = &rx_ring->buffer_info[i];
@@ -3736,7 +3749,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) < E1000_CB_LENGTH)) {
+		if (l1 && ((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



--
Auke Kok <auke-jan.h.kok@intel.com>
Intel Pro Ethernet Driver Group
LAN Access Division / Digital Enterprise Group 

[PATCH 08/10] e1000: implement more efficient tx queue locking

[Kok, Auke]

Implement more efficient locking (avoid the lock) when checking for
a stopped queue.  Also don't wake the queue unless the threshold is
reached to avoid queue on/off thrash.


Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: John Ronciak <john.ronciak@intel.com>
---

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

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index f604a1d..3c37c40 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -3400,13 +3400,15 @@ e1000_clean_tx_irq(struct e1000_adapter 
 
 	tx_ring->next_to_clean = i;
 
-	spin_lock(&tx_ring->tx_lock);
-
+#define TX_WAKE_THRESHOLD 32
 	if (unlikely(cleaned && netif_queue_stopped(netdev) &&
-		    netif_carrier_ok(netdev)))
-		netif_wake_queue(netdev);
-
-	spin_unlock(&tx_ring->tx_lock);
+	             netif_carrier_ok(netdev))) {
+		spin_lock(&tx_ring->tx_lock);
+		if (netif_queue_stopped(netdev) &&
+		    (E1000_DESC_UNUSED(tx_ring) >= TX_WAKE_THRESHOLD))
+			netif_wake_queue(netdev);
+		spin_unlock(&tx_ring->tx_lock);
+	}
 
 	if (adapter->detect_tx_hung) {
 		/* Detect a transmit hang in hardware, this serializes the



--
Auke Kok <auke-jan.h.kok@intel.com>
Intel Pro Ethernet Driver Group
LAN Access Division / Digital Enterprise Group 

[PATCH 09/10] e1000: Version bump, contact fix, year string change

[Kok, Auke]

Add the sourceforge project mailinglist to the contact information.

Bump version to 7.0.38-k2

Update copyright string with the new year.


Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: John Ronciak <john.ronciak@intel.com>
---

 drivers/net/e1000/Makefile        |    1 +
 drivers/net/e1000/e1000.h         |    3 ++-
 drivers/net/e1000/e1000_ethtool.c |    3 ++-
 drivers/net/e1000/e1000_hw.c      |    3 ++-
 drivers/net/e1000/e1000_hw.h      |    3 ++-
 drivers/net/e1000/e1000_main.c    |    7 ++++---
 drivers/net/e1000/e1000_osdep.h   |    3 ++-
 drivers/net/e1000/e1000_param.c   |    3 ++-
 8 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/net/e1000/Makefile b/drivers/net/e1000/Makefile
index ca9f895..92823ac 100644
--- a/drivers/net/e1000/Makefile
+++ b/drivers/net/e1000/Makefile
@@ -22,6 +22,7 @@
 # 
 # Contact Information:
 # Linux NICS <linux.nics@intel.com>
+# e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
 # Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
 #
 ################################################################################
diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index 2dfabdb..2bc34fb 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -1,7 +1,7 @@
 /*******************************************************************************
 
   
-  Copyright(c) 1999 - 2005 Intel Corporation. All rights reserved.
+  Copyright(c) 1999 - 2006 Intel Corporation. All rights reserved.
   
   This program is free software; you can redistribute it and/or modify it 
   under the terms of the GNU General Public License as published by the Free 
@@ -22,6 +22,7 @@
   
   Contact Information:
   Linux NICS <linux.nics@intel.com>
+  e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
   Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
 
 *******************************************************************************/
diff --git a/drivers/net/e1000/e1000_ethtool.c b/drivers/net/e1000/e1000_ethtool.c
index ecccca3..e48dc57 100644
--- a/drivers/net/e1000/e1000_ethtool.c
+++ b/drivers/net/e1000/e1000_ethtool.c
@@ -1,7 +1,7 @@
 /*******************************************************************************
 
   
-  Copyright(c) 1999 - 2005 Intel Corporation. All rights reserved.
+  Copyright(c) 1999 - 2006 Intel Corporation. All rights reserved.
   
   This program is free software; you can redistribute it and/or modify it 
   under the terms of the GNU General Public License as published by the Free 
@@ -22,6 +22,7 @@
   
   Contact Information:
   Linux NICS <linux.nics@intel.com>
+  e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
   Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
 
 *******************************************************************************/
diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
index 523c2c9..4c796e5 100644
--- a/drivers/net/e1000/e1000_hw.c
+++ b/drivers/net/e1000/e1000_hw.c
@@ -1,7 +1,7 @@
 /*******************************************************************************
 
   
-  Copyright(c) 1999 - 2005 Intel Corporation. All rights reserved.
+  Copyright(c) 1999 - 2006 Intel Corporation. All rights reserved.
   
   This program is free software; you can redistribute it and/or modify it 
   under the terms of the GNU General Public License as published by the Free 
@@ -22,6 +22,7 @@
   
   Contact Information:
   Linux NICS <linux.nics@intel.com>
+  e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
   Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
 
 *******************************************************************************/
diff --git a/drivers/net/e1000/e1000_hw.h b/drivers/net/e1000/e1000_hw.h
index 150e45e..03d07eb 100644
--- a/drivers/net/e1000/e1000_hw.h
+++ b/drivers/net/e1000/e1000_hw.h
@@ -1,7 +1,7 @@
 /*******************************************************************************
 
   
-  Copyright(c) 1999 - 2005 Intel Corporation. All rights reserved.
+  Copyright(c) 1999 - 2006 Intel Corporation. All rights reserved.
   
   This program is free software; you can redistribute it and/or modify it 
   under the terms of the GNU General Public License as published by the Free 
@@ -22,6 +22,7 @@
   
   Contact Information:
   Linux NICS <linux.nics@intel.com>
+  e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
   Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
 
 *******************************************************************************/
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 3c37c40..82d443b 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -1,7 +1,7 @@
 /*******************************************************************************
 
   
-  Copyright(c) 1999 - 2005 Intel Corporation. All rights reserved.
+  Copyright(c) 1999 - 2006 Intel Corporation. All rights reserved.
   
   This program is free software; you can redistribute it and/or modify it 
   under the terms of the GNU General Public License as published by the Free 
@@ -22,6 +22,7 @@
   
   Contact Information:
   Linux NICS <linux.nics@intel.com>
+  e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
   Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
 
 *******************************************************************************/
@@ -74,9 +75,9 @@ static char e1000_driver_string[] = "Int
 #else
 #define DRIVERNAPI "-NAPI"
 #endif
-#define DRV_VERSION "7.0.33-k2"DRIVERNAPI
+#define DRV_VERSION "7.0.38-k2"DRIVERNAPI
 char e1000_driver_version[] = DRV_VERSION;
-static char e1000_copyright[] = "Copyright (c) 1999-2005 Intel Corporation.";
+static char e1000_copyright[] = "Copyright (c) 1999-2006 Intel Corporation.";
 
 /* e1000_pci_tbl - PCI Device ID Table
  *
diff --git a/drivers/net/e1000/e1000_osdep.h b/drivers/net/e1000/e1000_osdep.h
index 9790db9..048d052 100644
--- a/drivers/net/e1000/e1000_osdep.h
+++ b/drivers/net/e1000/e1000_osdep.h
@@ -1,7 +1,7 @@
 /*******************************************************************************
 
   
-  Copyright(c) 1999 - 2005 Intel Corporation. All rights reserved.
+  Copyright(c) 1999 - 2006 Intel Corporation. All rights reserved.
   
   This program is free software; you can redistribute it and/or modify it 
   under the terms of the GNU General Public License as published by the Free 
@@ -22,6 +22,7 @@
   
   Contact Information:
   Linux NICS <linux.nics@intel.com>
+  e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
   Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
 
 *******************************************************************************/
diff --git a/drivers/net/e1000/e1000_param.c b/drivers/net/e1000/e1000_param.c
index e0a4d37..e55f896 100644
--- a/drivers/net/e1000/e1000_param.c
+++ b/drivers/net/e1000/e1000_param.c
@@ -1,7 +1,7 @@
 /*******************************************************************************
 
   
-  Copyright(c) 1999 - 2005 Intel Corporation. All rights reserved.
+  Copyright(c) 1999 - 2006 Intel Corporation. All rights reserved.
   
   This program is free software; you can redistribute it and/or modify it 
   under the terms of the GNU General Public License as published by the Free 
@@ -22,6 +22,7 @@
   
   Contact Information:
   Linux NICS <linux.nics@intel.com>
+  e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
   Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
 
 *******************************************************************************/



--
Auke Kok <auke-jan.h.kok@intel.com>
Intel Pro Ethernet Driver Group
LAN Access Division / Digital Enterprise Group 

[PATCH 10/10] {e100{,0},ixgb}: Add Auke Kok as new patch maintainer for e{100,1000} and ixgb

[Kok, Auke]

This adds Auke Kok to the list of maintainers for the Intel NICs.


Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: John Ronciak <john.ronciak@intel.com>
---

 MAINTAINERS |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index d00dea5..6424267 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1395,6 +1395,8 @@ P:	Jesse Brandeburg
 M:	jesse.brandeburg@intel.com
 P:	Jeff Kirsher
 M:	jeffrey.t.kirsher@intel.com
+P:	Auke Kok
+M:	auke-jan.h.kok@intel.com
 W:	http://sourceforge.net/projects/e1000/
 S:	Supported
 
@@ -1407,6 +1409,8 @@ P:	Jesse Brandeburg
 M:	jesse.brandeburg@intel.com
 P:	Jeff Kirsher
 M:	jeffrey.t.kirsher@intel.com
+P:	Auke Kok
+M:	auke-jan.h.kok@intel.com
 W:	http://sourceforge.net/projects/e1000/
 S:	Supported
 
@@ -1419,6 +1423,8 @@ P:	John Ronciak
 M:	john.ronciak@intel.com
 P:	Jesse Brandeburg
 M:	jesse.brandeburg@intel.com
+P:	Auke Kok
+M:	auke-jan.h.kok@intel.com
 W:	http://sourceforge.net/projects/e1000/
 S:	Supported
 



--
Auke Kok <auke-jan.h.kok@intel.com>
Intel Pro Ethernet Driver Group
LAN Access Division / Digital Enterprise Group 

Re: [PATCH 00/10] e1000: Driver fixes and update to 7.0.38-k2

[Jeff Garzik]

Kok, Auke wrote:
> Hi,
> 
> This patch series implements two fixes, a series of cleanups and
> two optimization patches.
> 
> The version of the driver was bumped to 7.0.38-k2. Year changes to 2006.
> 
> As I'm taking over patch duties from Jeff Kirsher, I added myself
> to the MAINTAINERS file.
> 
> These changes are available through git.
> 
> Jeff, please pull from:
> 
> git://63.64.152.142/~ahkok/git/netdev-2.6 e1000-7.0.38-k2
> 
> 
> Cheers,
> 
> Auke Kok
> 
> 
> ---
> 01/10: [FIX] Remove PM warning DPRINTKs breaking 2.4.x kernels
> 02/10: [FIX] Esb2 wol link cycle bug and uninitialized registers
> 03/10: [PATCH] De-inline functions to benefit from compiler smartness
> 04/10: [PATCH] Made an adapter struct variable into a local (txb2b)
> 05/10: [PATCH] Update truesize with the length of the packet for
>                       packet split
> 06/10: [PATCH] Dead variable cleanup
> 07/10: [PATCH] Buffer optimizations for small MTU
> 08/10: [PATCH] implement more efficient tx queue locking
> 09/10: [DOC] Version bump, contact fix, year string change
> 10/10: [MAINTAINERS] {e100{,0},ixgb}: Add Auke Kok as new patch maintainer

These 10 patches look OK, but since the current kernel version is 
2.6.17-rc1, that means we are in "bug fix only" mode right now.

Should I (a) apply these all to netdev2-.6.git#upstream, queueing them 
for 2.6.18, or (b) let you split the submission into two parts, bug 
fixes only and everything else?

	Jeff

Re: [PATCH 00/10] e1000: Driver fixes and update to 7.0.38-k2

[Auke Kok]

Jeff Garzik wrote:
> Kok, Auke wrote:
>> 01/10: [FIX] Remove PM warning DPRINTKs breaking 2.4.x kernels
>> 02/10: [FIX] Esb2 wol link cycle bug and uninitialized registers
>> 03/10: [PATCH] De-inline functions to benefit from compiler smartness
>> 04/10: [PATCH] Made an adapter struct variable into a local (txb2b)
>> 05/10: [PATCH] Update truesize with the length of the packet for
>>                       packet split
>> 06/10: [PATCH] Dead variable cleanup
>> 07/10: [PATCH] Buffer optimizations for small MTU
>> 08/10: [PATCH] implement more efficient tx queue locking
>> 09/10: [DOC] Version bump, contact fix, year string change
>> 10/10: [MAINTAINERS] {e100{,0},ixgb}: Add Auke Kok as new patch 
>> maintainer
> 
> These 10 patches look OK, but since the current kernel version is 
> 2.6.17-rc1, that means we are in "bug fix only" mode right now.
> 
> Should I (a) apply these all to netdev2-.6.git#upstream, queueing them 
> for 2.6.18, or (b) let you split the submission into two parts, bug 
> fixes only and everything else?

jeff,

The two fixes are for low-impact bugs with workarounds, not justifying 
applying them to 2.6.17-rc1 or stable@.

Please apply to netdev-2.6.git#upstream and queue them for 2.6.18. (a).

Cheers,

Auke

Re: [PATCH 05/10] e1000: Update truesize with the length of the packet for packet split

[Jeff Garzik]

Kok, Auke wrote:
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> index 2cc9955..7ba1b16 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -3770,6 +3770,7 @@ e1000_clean_rx_irq_ps(struct e1000_adapt
>  			ps_page->ps_page[j] = NULL;
>  			skb->len += length;
>  			skb->data_len += length;
> +			skb->truesize += length;

NAK, apparently this is adding a very serious bug.

	Jeff

Re: [PATCH 05/10] e1000: Update truesize with the length of the packet for packet split

[Jesse Brandeburg]

On Fri, 14 Apr 2006, Jeff Garzik wrote:

> Kok, Auke wrote:
> > diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> > index 2cc9955..7ba1b16 100644
> > --- a/drivers/net/e1000/e1000_main.c
> > +++ b/drivers/net/e1000/e1000_main.c
> > @@ -3770,6 +3770,7 @@ e1000_clean_rx_irq_ps(struct e1000_adapt
> >  			ps_page->ps_page[j] = NULL;
> >  			skb->len += length;
> >  			skb->data_len += length;
> > +			skb->truesize += length;
> 
> NAK, apparently this is adding a very serious bug.

I'm surprised to hear you say that, here's why.  Elaboration would be 
helpful.

our packet split receive code uses two buffers for any packet the hardware 
was able to split.  The skb when we're receiving say a 1000 byte IPv4 
packet looks like this:

skb -
    |- len = total packet length (1000)
    |- data_len = length in skb_shinfo(skb)->frags[] (1000 - headerlen)
    |- truesize = size of originally allocated skb (128 + NET_IP_ALIGN)

in this case, the stack is using truesize to figure out how much to charge 
the receive socket for our receive packet.

in the old driver we indicate a packet with truesize = 128, which has 1000 
bytes of data in it.  normally (for everyone using only an skb) skb->len, 
and skb->truesize are updated by skb_put();  In this case we don't have a 
kernel support function for adding length to skb->len *and* to 
skb->truesize when chaining on something to ->frags[]

see the code in do_tcp_sendpages for an example of what the stack does in 
setting up a packet for transmit using frags[]

it updates skb->len and skb->truesize, right after calling 
skb_fill_page_desc, just like we do.

interesingly, that is right where sk_forward_alloc is modified (but thats 
another thread)

Jesse

Re: [PATCH 05/10] e1000: Update truesize with the length of the packet for packet split

[David S. Miller]

From: Jesse Brandeburg <jesse.brandeburg@intel.com>
Date: Fri, 14 Apr 2006 15:13:31 -0700 (Pacific Daylight Time)

> in the old driver we indicate a packet with truesize = 128, which has 1000 
> bytes of data in it.  normally (for everyone using only an skb) skb->len, 
> and skb->truesize are updated by skb_put();  In this case we don't have a 
> kernel support function for adding length to skb->len *and* to 
> skb->truesize when chaining on something to ->frags[]

Where does skb_put() update truesize?

Re: [PATCH 05/10] e1000: Update truesize with the length of the packet for packet split

[Jesse Brandeburg]

On Fri, 14 Apr 2006, David S. Miller wrote:

> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Date: Fri, 14 Apr 2006 15:13:31 -0700 (Pacific Daylight Time)
> 
> > in the old driver we indicate a packet with truesize = 128, which has 1000 
> > bytes of data in it.  normally (for everyone using only an skb) skb->len, 
> > and skb->truesize are updated by skb_put();  In this case we don't have a 
> > kernel support function for adding length to skb->len *and* to 
> > skb->truesize when chaining on something to ->frags[]
> 
> Where does skb_put() update truesize?

my mistake, it does not.  but truesize in the "normal" case is already set 
to the full length of the allocated skb (which would hold the whole 
frame).  In the packet split case the received skb looks dramatically 
different, and since we're one of the only drivers using this mechanism.

I could easily be incorrect here, but my interpretation based on the 
stack's use of ->truesize in do_tcp_sendpages is what I based our code 
change on.

Please help me understand how you think it should work when we have a 
device that wants to receive a packet using header in the skb->data, and 
application data in the ->frags[]? 

Jesse

Re: [PATCH 05/10] e1000: Update truesize with the length of the packet for packet split

[David S. Miller]

From: Jesse Brandeburg <jesse.brandeburg@intel.com>
Date: Fri, 14 Apr 2006 15:43:02 -0700 (Pacific Daylight Time)

> Please help me understand how you think it should work when we have a 
> device that wants to receive a packet using header in the skb->data, and 
> application data in the ->frags[]? 

If you're removing the pages from ->frags[] and you already accounted
for them in skb->truesize, then yes I guess it could be correct to
subtract it back out.

Looking at S2IO it's doing something very wrong with it's modification
of skb->truesize.  It's not changing the amount of memory allocated
to an SKB in any way yet it's bumping skb->truesize for some reason.
That will definitely lead to performance problems.

That's why generally when we see an skb->truesize modification, it's
just assumed to be a bug, it's generally not necessary but may be
so in your e1000 situation here.

Another problem to look out for is that once a socket has a reference
to an SKB you must never ever modify skb->truesize because doing so
will break socket memory accounting.  There is exactly one spot in
the TCP stack where we are able to do this by adjusting the socket
memory accounting variables carefully in a locked context at the same
time, but otherwise it's not safe to modify skb->truesize when there
is an attached socket.

Re: [PATCH 05/10] e1000: Update truesize with the length of the packet for packet split

[Jesse Brandeburg]

On Fri, 14 Apr 2006, David S. Miller wrote:

> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Date: Fri, 14 Apr 2006 15:43:02 -0700 (Pacific Daylight Time)
> 
> > Please help me understand how you think it should work when we have a 
> > device that wants to receive a packet using header in the skb->data, and 
> > application data in the ->frags[]? 
> 
> If you're removing the pages from ->frags[] and you already accounted
> for them in skb->truesize, then yes I guess it could be correct to
> subtract it back out.

they are not accounted for in the skb yet.  the way e1000 works is it 
pre-allocates all its 128 byte receive buffers using skb_alloc, and then 
chains in any pages that are used for receive packets.

> Looking at S2IO it's doing something very wrong with it's modification
> of skb->truesize.  It's not changing the amount of memory allocated
> to an SKB in any way yet it's bumping skb->truesize for some reason.
> That will definitely lead to performance problems.

I can only speak to what e1000 is trying to do. we're technically 
increasing the size of the skb after we've allocated it, so

does nr_frags and frags[] need truesize to be correct?

> That's why generally when we see an skb->truesize modification, it's
> just assumed to be a bug, it's generally not necessary but may be
> so in your e1000 situation here.

cool, I would really prefer that we extend the kernel api's to "do the 
right thing" for receives using frags[]

> Another problem to look out for is that once a socket has a reference
> to an SKB you must never ever modify skb->truesize because doing so
> will break socket memory accounting.  There is exactly one spot in
> the TCP stack where we are able to do this by adjusting the socket
> memory accounting variables carefully in a locked context at the same
> time, but otherwise it's not safe to modify skb->truesize when there
> is an attached socket.

Absolutely!  we're indicating the packet to the stack using netif_rx* (and 
we allocated it), so no socket has the reference until after we change 
truesize.

Re: [PATCH 05/10] e1000: Update truesize with the length of the packet for packet split

[David S. Miller]

From: Jesse Brandeburg <jesse.brandeburg@intel.com>
Date: Fri, 14 Apr 2006 16:04:21 -0700 (Pacific Daylight Time)

> On Fri, 14 Apr 2006, David S. Miller wrote:
> 
> > From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> > Date: Fri, 14 Apr 2006 15:43:02 -0700 (Pacific Daylight Time)
> > 
> > > Please help me understand how you think it should work when we have a 
> > > device that wants to receive a packet using header in the skb->data, and 
> > > application data in the ->frags[]? 
> > 
> > If you're removing the pages from ->frags[] and you already accounted
> > for them in skb->truesize, then yes I guess it could be correct to
> > subtract it back out.
> 
> they are not accounted for in the skb yet.  the way e1000 works is it 
> pre-allocates all its 128 byte receive buffers using skb_alloc, and then 
> chains in any pages that are used for receive packets.

Ok so you allocate the base skb->data memory using alloc_skb().  Then
you add on pages and increment skb->truesize to account for the pages.
And you do all of this for receive packets before they are passed up
to the stack.

Right?

If so, then that should be OK.

Jeff, I seem to have misunderstood what e1000 was doing, the patches
changing skb->truesize should be fine.

Re: [PATCH 05/10] e1000: Update truesize with the length of the packet for packet split

[Jesse Brandeburg]

On Fri, 14 Apr 2006, David S. Miller wrote:
> > they are not accounted for in the skb yet.  the way e1000 works is it 
> > pre-allocates all its 128 byte receive buffers using skb_alloc, and then 
> > chains in any pages that are used for receive packets.
> 
> Ok so you allocate the base skb->data memory using alloc_skb().  Then
> you add on pages and increment skb->truesize to account for the pages.
> And you do all of this for receive packets before they are passed up
> to the stack.
> 
> Right?

yes, exactly.

> If so, then that should be OK.
> 
> Jeff, I seem to have misunderstood what e1000 was doing, the patches
> changing skb->truesize should be fine.

Thanks

Re: [PATCH 00/10] e1000: Driver fixes and update to 7.0.38-k2

[Herbert Xu]

Jeff Garzik <jgarzik@pobox.com> wrote:
>
>> 05/10: [PATCH] Update truesize with the length of the packet for
>>                       packet split
> 
> These 10 patches look OK, but since the current kernel version is 
> 2.6.17-rc1, that means we are in "bug fix only" mode right now.
> 
> Should I (a) apply these all to netdev2-.6.git#upstream, queueing them 
> for 2.6.18, or (b) let you split the submission into two parts, bug 
> fixes only and everything else?

It turns out that the truesize patch is pretty important as otherwise
most of TCP receive socket buffer accounting falls apart.  So it should
probably go into 2.6.17 and even 2.6.16-stable.

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

Re: [PATCH 00/10] e1000: Driver fixes and update to 7.0.38-k2

[David S. Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sat, 22 Apr 2006 15:22:54 +1000

> Jeff Garzik <jgarzik@pobox.com> wrote:
> >
> >> 05/10: [PATCH] Update truesize with the length of the packet for
> >>                       packet split
> > 
> > These 10 patches look OK, but since the current kernel version is 
> > 2.6.17-rc1, that means we are in "bug fix only" mode right now.
> > 
> > Should I (a) apply these all to netdev2-.6.git#upstream, queueing them 
> > for 2.6.18, or (b) let you split the submission into two parts, bug 
> > fixes only and everything else?
> 
> It turns out that the truesize patch is pretty important as otherwise
> most of TCP receive socket buffer accounting falls apart.  So it should
> probably go into 2.6.17 and even 2.6.16-stable.

I totally agree.

Jeff please try to get at least this skb->truesize fix queued
to both -stable and current 2.6.x, it's really needed and
important for proper socket memory accounting on receive with
e1000.

Thanks.

Re: [PATCH 00/10] e1000: Driver fixes and update to 7.0.38-k2

[Auke Kok]

David S. Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Sat, 22 Apr 2006 15:22:54 +1000
> 
>> Jeff Garzik <jgarzik@pobox.com> wrote:
>>>> 05/10: [PATCH] Update truesize with the length of the packet for
>>>>                       packet split
>>> These 10 patches look OK, but since the current kernel version is 
>>> 2.6.17-rc1, that means we are in "bug fix only" mode right now.
>>>
>>> Should I (a) apply these all to netdev2-.6.git#upstream, queueing them 
>>> for 2.6.18, or (b) let you split the submission into two parts, bug 
>>> fixes only and everything else?
>> It turns out that the truesize patch is pretty important as otherwise
>> most of TCP receive socket buffer accounting falls apart.  So it should
>> probably go into 2.6.17 and even 2.6.16-stable.
> 
> I totally agree.
> 
> Jeff please try to get at least this skb->truesize fix queued
> to both -stable and current 2.6.x, it's really needed and
> important for proper socket memory accounting on receive with
> e1000.

I agree, Jeff, please go ahead or let me know if you want me to submit this 
separately.

Auke