Misuse of LRO, how widespread

Previous thread: [PATCH] Convert netpoll blocking api in bonding driver to be a counter by nhorman on Monday, December 6, 2010 - 12:05 pm. (1 message)

Next thread: Behaviour of ETHTOOL_GLINK for an interface that's down by Ben Hutchings on Monday, December 6, 2010 - 2:59 pm. (5 messages)
From: Stephen Hemminger
Date: Monday, December 6, 2010 - 1:18 pm

I inspected all drivers in net-next to see which drivers are using
LRO and which ones are broken. Most concerning is that Chelsio
and Solarflare drivers ignore ETH_FLAG_LRO.

The ones that are using LRO but allow disabling it:
  qlcnic, netxen, mv643, s2io, myi10ge, bnx2x, ixgbe, vmxnet3

One driver seems confused about LRO vs GRO:
  mlx4 - comments about LRO and depends on LRO but driver is using GRO

Drivers with not using ethtool interface to disable LRO:
  pasemi_mac, sfc, ehea, cxgb3, cxgb4

--

From: Ben Hutchings
Date: Monday, December 6, 2010 - 1:30 pm

sfc is also in this category.  (And it's not confused, it was using

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--

From: Stephen Hemminger
Date: Monday, December 6, 2010 - 3:33 pm

How about this?

Subject: sfc: convert references to LRO to GRO

This driver now uses Generic Receive Offload, not the older LRO.
Change references to LRO in names and comments.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/drivers/net/sfc/rx.c	2010-12-06 14:28:25.320929804 -0800
+++ b/drivers/net/sfc/rx.c	2010-12-06 14:29:54.424930087 -0800
@@ -37,7 +37,7 @@
  * This driver supports two methods for allocating and using RX buffers:
  * each RX buffer may be backed by an skb or by an order-n page.
  *
- * When LRO is in use then the second method has a lower overhead,
+ * When GRO is in use then the second method has a lower overhead,
  * since we don't have to allocate then free skbs on reassembled frames.
  *
  * Values:
@@ -50,25 +50,25 @@
  *
  *   - Since pushing and popping descriptors are separated by the rx_queue
  *     size, so the watermarks should be ~rxd_size.
- *   - The performance win by using page-based allocation for LRO is less
- *     than the performance hit of using page-based allocation of non-LRO,
+ *   - The performance win by using page-based allocation for GRO is less
+ *     than the performance hit of using page-based allocation of non-GRO,
  *     so the watermarks should reflect this.
  *
  * Per channel we maintain a single variable, updated by each channel:
  *
- *   rx_alloc_level += (lro_performed ? RX_ALLOC_FACTOR_LRO :
+ *   rx_alloc_level += (gro_performed ? RX_ALLOC_FACTOR_GRO :
  *                      RX_ALLOC_FACTOR_SKB)
  * Per NAPI poll interval, we constrain rx_alloc_level to 0..MAX (which
  * limits the hysteresis), and update the allocation strategy:
  *
- *   rx_alloc_method = (rx_alloc_level > RX_ALLOC_LEVEL_LRO ?
+ *   rx_alloc_method = (rx_alloc_level > RX_ALLOC_LEVEL_GRO ?
  *                      RX_ALLOC_METHOD_PAGE : RX_ALLOC_METHOD_SKB)
  */
 static int rx_alloc_method = RX_ALLOC_METHOD_AUTO;
 
-#define RX_ALLOC_LEVEL_LRO 0x2000
+#define RX_ALLOC_LEVEL_GRO 0x2000
 #define ...
From: Ben Hutchings
Date: Monday, December 6, 2010 - 3:35 pm

Acked-by: Ben Hutchings <bhutchings@solarflare.com>

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--

From: David Miller
Date: Friday, December 10, 2010 - 4:03 pm

From: Ben Hutchings <bhutchings@solarflare.com>

Applied to net-next-2.6, thanks!
--

From: Dimitris Michailidis
Date: Monday, December 6, 2010 - 2:15 pm

cxgb4 uses GRO, not LRO.
--

From: Stephen Hemminger
Date: Monday, December 6, 2010 - 3:27 pm

On Mon, 06 Dec 2010 13:15:42 -0800

Ok. but cxgb3 still uses LRO (or it least calls it lro).

-- 
--

From: Dimitris Michailidis
Date: Monday, December 6, 2010 - 4:05 pm

cxgb3 was the driver Herbert implemented GRO on I think, and he converted it 
to GRO.  It possibly has leftover LRO references as it was using LRO before.
--

From: Ben Hutchings
Date: Monday, December 6, 2010 - 4:22 pm

There's a fair amount of code setting LRO flags in various structures,
so either the driver still enables LRO in hardware/firmware or this is
dead code.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--

From: Dimitris Michailidis
Date: Monday, December 6, 2010 - 4:36 pm

From a quick look it appears to have a per queue lro flag.  HW/FW are not 
aware of LRO, whatever flags that driver has are for SW.  I'd say it's 

--

Previous thread: [PATCH] Convert netpoll blocking api in bonding driver to be a counter by nhorman on Monday, December 6, 2010 - 12:05 pm. (1 message)

Next thread: Behaviour of ETHTOOL_GLINK for an interface that's down by Ben Hutchings on Monday, December 6, 2010 - 2:59 pm. (5 messages)