diff mbox

[Bugme-new,Bug,29252] New: IPv6 doesn't work in a kvm guest.

Message ID 20110309.200437.226773227.davem@davemloft.net
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

David Miller March 10, 2011, 4:04 a.m. UTC
From: David Miller <davem@davemloft.net>
Date: Wed, 09 Mar 2011 19:20:12 -0800 (PST)

> From: David Miller <davem@davemloft.net>
> Date: Wed, 09 Mar 2011 15:58:18 -0800 (PST)
> 
>> Ok, the following should address both bugs, #29252 and #30462, please
>> give it some testing.
>> 
>> --------------------
>> ipv6: Don't create clones of nonexthop routes forever.
> 
> Nevermind, this patch has problems, I'm still debugging and trying to
> come up with a proper fix.
> 
> Thanks in advance for your patience.

Ok, I'm more confident in this version of the fix.  It passes all of
my tests, and I've added instrumentation to make sure various cases
are performing the operations the way I expect them to.

--------------------
ipv6: Don't create clones of host routes.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Addresses https://bugzilla.kernel.org/show_bug.cgi?id=29252
Addresses https://bugzilla.kernel.org/show_bug.cgi?id=30462

In commit d80bc0fd262ef840ed4e82593ad6416fa1ba3fc4 ("ipv6: Always
clone offlink routes.") we forced the kernel to always clone offlink
routes.

The reason we do that is to make sure we never bind an inetpeer to a
prefixed route.

The logic turned on here has existed in the tree for many years,
but was always off due to a protecting CPP define.  So perhaps
it's no surprise that there is a logic bug here.

The problem is that we canot clone a route that is already a
host route (ie. has DST_HOST set).  Because if we do, an identical
entry already exists in the routing tree and therefore the
ip6_rt_ins() call is going to fail.

This sets off a series of failures and high cpu usage, because when
ip6_rt_ins() fails we loop retrying this operation a few times in
order to handle a race between two threads trying to clone and insert
the same host route at the same time.

Fix this by simply using the route as-is when DST_HOST is set.

Reported-by: slash@ac.auone-net.jp
Reported-by: Ernst Sjöstrand <ernstp@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/ipv6/route.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

Comments

Ernst Sjöstrand March 11, 2011, 7:44 p.m. UTC | #1
Hi,

this patch solved the issue I reported. Applied the patch on -rc8.

Regards
//Ernst

On Thu, Mar 10, 2011 at 05:04, David Miller <davem@davemloft.net> wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 09 Mar 2011 19:20:12 -0800 (PST)
>
>> From: David Miller <davem@davemloft.net>
>> Date: Wed, 09 Mar 2011 15:58:18 -0800 (PST)
>>
>>> Ok, the following should address both bugs, #29252 and #30462, please
>>> give it some testing.
>>>
>>> --------------------
>>> ipv6: Don't create clones of nonexthop routes forever.
>>
>> Nevermind, this patch has problems, I'm still debugging and trying to
>> come up with a proper fix.
>>
>> Thanks in advance for your patience.
>
> Ok, I'm more confident in this version of the fix.  It passes all of
> my tests, and I've added instrumentation to make sure various cases
> are performing the operations the way I expect them to.
>
> --------------------
> ipv6: Don't create clones of host routes.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Addresses https://bugzilla.kernel.org/show_bug.cgi?id=29252
> Addresses https://bugzilla.kernel.org/show_bug.cgi?id=30462
>
> In commit d80bc0fd262ef840ed4e82593ad6416fa1ba3fc4 ("ipv6: Always
> clone offlink routes.") we forced the kernel to always clone offlink
> routes.
>
> The reason we do that is to make sure we never bind an inetpeer to a
> prefixed route.
>
> The logic turned on here has existed in the tree for many years,
> but was always off due to a protecting CPP define.  So perhaps
> it's no surprise that there is a logic bug here.
>
> The problem is that we canot clone a route that is already a
> host route (ie. has DST_HOST set).  Because if we do, an identical
> entry already exists in the routing tree and therefore the
> ip6_rt_ins() call is going to fail.
>
> This sets off a series of failures and high cpu usage, because when
> ip6_rt_ins() fails we loop retrying this operation a few times in
> order to handle a race between two threads trying to clone and insert
> the same host route at the same time.
>
> Fix this by simply using the route as-is when DST_HOST is set.
>
> Reported-by: slash@ac.auone-net.jp
> Reported-by: Ernst Sjöstrand <ernstp@gmail.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>  net/ipv6/route.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 904312e..e7db701 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -739,8 +739,10 @@ restart:
>
>        if (!rt->rt6i_nexthop && !(rt->rt6i_flags & RTF_NONEXTHOP))
>                nrt = rt6_alloc_cow(rt, &fl->fl6_dst, &fl->fl6_src);
> -       else
> +       else if (!(rt->dst.flags & DST_HOST))
>                nrt = rt6_alloc_clone(rt, &fl->fl6_dst);
> +       else
> +               goto out2;
>
>        dst_release(&rt->dst);
>        rt = nrt ? : net->ipv6.ip6_null_entry;
> --
> 1.7.4.1
>
>
--
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
David Miller March 11, 2011, 8:15 p.m. UTC | #2
From: Ernst Sjöstrand <ernstp@gmail.com>
Date: Fri, 11 Mar 2011 20:44:39 +0100

> this patch solved the issue I reported. Applied the patch on -rc8.

Thank you for testing.
--
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/ipv6/route.c b/net/ipv6/route.c
index 904312e..e7db701 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -739,8 +739,10 @@  restart:
 
 	if (!rt->rt6i_nexthop && !(rt->rt6i_flags & RTF_NONEXTHOP))
 		nrt = rt6_alloc_cow(rt, &fl->fl6_dst, &fl->fl6_src);
-	else
+	else if (!(rt->dst.flags & DST_HOST))
 		nrt = rt6_alloc_clone(rt, &fl->fl6_dst);
+	else
+		goto out2;
 
 	dst_release(&rt->dst);
 	rt = nrt ? : net->ipv6.ip6_null_entry;