Home » Mailing list archives » linux-kernel » 2008 » January » 8

Re: [PATCH][LRO] Fix lro_mgr->features checks

Previous thread: Rename suspend/hibernation stuff for clarity by Pavel Machek on Monday, January 7, 2008 - 9:52 am. (3 messages)

Next thread: PROBLEM: I/O scheduler problem with an 8 SATA disks raid 5 under heavy load ? by Guillaume Laurès on Monday, January 7, 2008 - 10:58 am. (3 messages)
[view original message]
From: Brice Goglin <Brice.Goglin@...>
To: David S. Miller <davem@...>
Cc: Andrew J. Gallatin <gallatin@...>, Jan-Bernd Themann <themann@...>, Linus Torvalds <torvalds@...>, LKML <linux-kernel@...>, <netdev@...>
Subject: [PATCH][LRO] Fix lro_mgr-&gt;features checks
Date: Monday, January 7, 2008 - 9:33 am

Hi Dave,


The LRO_F_* checks in inet_lro.c are buggy, the following patch

is needed to check lro_mgr->features correctly.


I decided to follow the NETIF_F_* convention and thus removed

test_bit. Some people might like it better if we keep using

test_bit and just set lro_mgr->features to 1<<LRO_F_NAPI instead

of just LRO_F_NAPI. I don't know what's the best.


Without this patch, we noticed several problems with myri10ge,

at least when using non-IP packets. I guess the ehea driver did

not expose any big problem because test_bit(NAPI) returns true

thanks to lro_mgr->features containing LRO_F_EXTRACT_VLAN_ID.


Please apply for 2.6.24.


Brice


[PATCH][LRO] Fix lro_mgr->features checks


lro_mgr->features contains a bitmask of LRO_F_* values which are

defined as power of two, not as bit indexes.

They must be checked with x&LRO_F_FOO, not with test_bit(LRO_F_FOO,&x).


Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>

Acked-by: Andrew Gallatin <gallatin@myri.com>

---

diff --git a/net/ipv4/inet_lro.c b/net/ipv4/inet_lro.c

index 9a96c27..4a4d49f 100644

--- a/net/ipv4/inet_lro.c

+++ b/net/ipv4/inet_lro.c

@@ -310,7 +310,7 @@ static void lro_flush(struct net_lro_mgr *lro_mgr,

 	skb_shinfo(lro_desc->parent)->gso_size = lro_desc->mss;


 	if (lro_desc->vgrp) {

-		if (test_bit(LRO_F_NAPI, &lro_mgr->features))

+		if (lro_mgr->features & LRO_F_NAPI)

 			vlan_hwaccel_receive_skb(lro_desc->parent,

 						 lro_desc->vgrp,

 						 lro_desc->vlan_tag);

@@ -320,7 +320,7 @@ static void lro_flush(struct net_lro_mgr *lro_mgr,

 					lro_desc->vlan_tag);


 	} else {

-		if (test_bit(LRO_F_NAPI, &lro_mgr->features))

+		if (lro_mgr->features & LRO_F_NAPI)

 			netif_receive_skb(lro_desc->parent);

 		else

 			netif_rx(lro_desc->parent);

@@ -352,7 +352,7 @@ static int __lro_proc_skb(struct net_lro_mgr *lro_mgr, struct sk_buff *skb,

 		goto out;


 	if ((skb->protocol == htons(ETH_P_8021Q))...
[ message continues ]
[view original message]
From: David Miller <davem@...>
To: <Brice.Goglin@...>
Cc: <gallatin@...>, <themann@...>, <torvalds@...>, <linux-kernel@...>, <netdev@...>
Subject: Re: [PATCH][LRO] Fix lro_mgr-&gt;features checks
Date: Tuesday, January 8, 2008 - 2:09 am

From: Brice Goglin <Brice.Goglin@inria.fr>


Applied, thanks a lot.

--
Previous thread: Rename suspend/hibernation stuff for clarity by Pavel Machek on Monday, January 7, 2008 - 9:52 am. (3 messages)

Next thread: PROBLEM: I/O scheduler problem with an 8 SATA disks raid 5 under heavy load ? by Guillaume Laurès on Monday, January 7, 2008 - 10:58 am. (3 messages)