diff mbox

[BISECTED] Linux 3.2: Networking out of LAN does not work

Message ID 1327489531.2425.19.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet Jan. 25, 2012, 11:05 a.m. UTC
Le mercredi 25 janvier 2012 à 10:40 +0100, Eric Dumazet a écrit :
> Le mercredi 25 janvier 2012 à 10:35 +0100, Rune Magnussen a écrit :
> > Hi
> > 
> > Networking to and from the outside of my LAN does not work with Linux 
> > 3.2.x.
> > Linux 3.1.x works. Using ssh from a PC on the LAN also works.
> > The machine has a VIA C7 CPU and the NIC uses the 8139too-driver.
> > 
> > Another machine with a r8169-NIC and an Atom-CPU (32bit) did not 
> > show any problems.
> > It was used very ligtly though.
> > 
> > Bisection led to this:
> > ---
> > f04565ddf52e401880f8ba51de0dff8ba51c99fd is the first bad commit
> > commit f04565ddf52e401880f8ba51de0dff8ba51c99fd
> > Author: Mihai Maruseac <mihai.maruseac@gmail.com>
> > Date:   Thu Oct 20 20:45:10 2011 +0000
> > 
> >     dev: use name hash for dev_seq_ops
> >  
> >     Instead of using the dev->next chain and trying to resync at each call 
> > to
> >     dev_seq_start, use the name hash, keeping the bucket and the offset in
> >     seq->private field.
> >  
> >     Tests revealed the following results for ifconfig > /dev/null
> >        * 1000 interfaces:
> >           * 0.114s without patch
> >           * 0.089s with patch
> >        * 3000 interfaces:
> >           * 0.489s without patch
> >           * 0.110s with patch
> >        * 5000 interfaces:
> >           * 1.363s without patch
> >           * 0.250s with patch
> >        * 128000 interfaces (other setup):
> >           * ~100s without patch
> >           * ~30s with patch
> >  
> >     Signed-off-by: Mihai Maruseac <mmaruseac@ixiacom.com>
> >     Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> >     Signed-off-by: David S. Miller <davem@davemloft.net>
> > 
> > :040000 040000 2e86e1c9ecaf719d312a8b5863ec9e89d92bc4b3 
> > 8b0d09ca105502dd5389bb6281ef059c8bb38667 M   net
> > ---
> > 
> > Simply reverting the commit on top of Linux 3.2.1 resultet in a build 
> > error.
> > 
> > Used kernel configuration: http://knus.info/kernel/config-3.1.txt
> > 
> > Greetings Rune

OK please test the following patch.

[PATCH] net: use index hash for /proc/net/dev

Commit f04565ddf52e (dev: use name hash for dev_seq_ops) added a
regression for legacy apps expecting to find devices in a particular
order.

Instead of using name hash, we can use dev_index_head hash, based on
device index, to not change device ordering.

Name hash order is not particularly useful, while index ordering is
meaningful on machines where no more than 256 devices are ever created.

"cat /proc/net/dev" and "ip link" will output devices in same order.

Reported-by: Rune Magnussen <rum@bankdata.dk>
Bisected-by: Rune Magnussen <rum@bankdata.dk>
Cc: Mihai Maruseac <mihai.maruseac@gmail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/core/dev.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Miller Jan. 26, 2012, 2:41 a.m. UTC | #1
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 25 Jan 2012 12:05:31 +0100

> OK please test the following patch.
> 
> [PATCH] net: use index hash for /proc/net/dev

Sorry, we never promised to list devices in any particular order,
any such dependency is a bug and must be fixed instead of papered
around by the kernel.

This same issue came up for routes too once we deleted fib_hash
and fib_trie listed them in a different order.

I'm not applying this.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet Jan. 26, 2012, 6:01 a.m. UTC | #2
Le mercredi 25 janvier 2012 à 21:41 -0500, David Miller a écrit :

> Sorry, we never promised to list devices in any particular order,
> any such dependency is a bug and must be fixed instead of papered
> around by the kernel.
> 
> This same issue came up for routes too once we deleted fib_hash
> and fib_trie listed them in a different order.
> 
> I'm not applying this.


I once hit this exact issue with a legacy commercial product using a
licence based on MAC address, and could fix it using a custom
"ifconfig", since last ifconfig versions sort the device list before
output.

I personally dont care, but apparently Rune spent time to bisect the
problem. I presumed it was an issue for him.

Since Rune didnt answer to my mail, I wont argue with you :)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rune Magnussen Jan. 26, 2012, 7:14 a.m. UTC | #3
Eric Dumazet <eric.dumazet@gmail.com> 
Sendt af: netdev-owner@vger.kernel.org
25-01-2012 12:06
58efa841-5316-9749-1633-85629e363385

Til
Rune Magnussen <rum@bankdata.dk>, David Miller <davem@davemloft.net>
cc
netdev@vger.kernel.org, Mihai Maruseac <mihai.maruseac@gmail.com>
Emne
Re: [BISECTED] Linux 3.2: Networking out of LAN does not work







> OK please test the following patch.

> [PATCH] net: use index hash for /proc/net/dev

> Commit f04565ddf52e (dev: use name hash for dev_seq_ops) added a
> regression for legacy apps expecting to find devices in a particular
> order.

> Instead of using name hash, we can use dev_index_head hash, based on
> device index, to not change device ordering.

> Name hash order is not particularly useful, while index ordering is
> meaningful on machines where no more than 256 devices are ever created.

> "cat /proc/net/dev" and "ip link" will output devices in same order.

> Reported-by: Rune Magnussen <rum@bankdata.dk>
> Bisected-by: Rune Magnussen <rum@bankdata.dk>
> Cc: Mihai Maruseac <mihai.maruseac@gmail.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> net/core/dev.c |    4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 115dee1..c58dba3 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4052,9 +4052,9 @@ static inline struct net_device 
*dev_from_same_bucket(struct seq_file *seq)
> 
>                bucket = get_bucket(state->pos);
>                offset = get_offset(state->pos);
> -              h = &net->dev_name_head[bucket];
> +              h = dev_index_hash(net, bucket);
>                count = 0;
> -              hlist_for_each_entry_rcu(dev, p, h, name_hlist) {
> +              hlist_for_each_entry_rcu(dev, p, h, index_hlist) {
>                                if (count++ == offset) {
>                                                state->pos = 
set_bucket_offset(bucket, count);
>                                                return dev;
>

The patch works for me. I applied it on top of v3.2.1 and it fixed my 
problem.
I have not tested it on other computers yet.

Greetings Rune


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rune Magnussen Jan. 26, 2012, 7:55 a.m. UTC | #4
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 25 Jan 2012 12:05:31 +0100

>> OK please test the following patch.
>> 
>> [PATCH] net: use index hash for /proc/net/dev

> Sorry, we never promised to list devices in any particular order,
> any such dependency is a bug and must be fixed instead of papered
> around by the kernel.

OK. I simply asumed it was a kernel issue since it started after an 
upgrade.
Now at lest I have an idea about where to look.

Greetings Rune

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/core/dev.c b/net/core/dev.c
index 115dee1..c58dba3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4052,9 +4052,9 @@  static inline struct net_device *dev_from_same_bucket(struct seq_file *seq)
 
 	bucket = get_bucket(state->pos);
 	offset = get_offset(state->pos);
-	h = &net->dev_name_head[bucket];
+	h = dev_index_hash(net, bucket);
 	count = 0;
-	hlist_for_each_entry_rcu(dev, p, h, name_hlist) {
+	hlist_for_each_entry_rcu(dev, p, h, index_hlist) {
 		if (count++ == offset) {
 			state->pos = set_bucket_offset(bucket, count);
 			return dev;