* [PATCH net 1/2] ipv6: reset dst.expires value when clearing expire flag
@ 2013-10-24 5:48 Hannes Frederic Sowa
2013-10-24 6:36 ` Eric Dumazet
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Hannes Frederic Sowa @ 2013-10-24 5:48 UTC (permalink / raw)
To: netdev; +Cc: sgunderson, valentyn, yoshfuji
On receiving a packet too big icmp error we update the expire value by
calling rt6_update_expires. This function uses dst_set_expires which is
implemented that it can only reduce the expiration value of the dst entry.
If we insert new routing non-expiry information into the ipv6 fib where
we already have a matching rt6_info we only clear the RTF_EXPIRES flag
in rt6i_flags and leave the dst.expires value as is.
When new mtu information arrives for that cached dst_entry we again
call dst_set_expires. This time it won't update the dst.expire value
because we left the dst.expire value intact from the last update. So
dst_set_expires won't touch dst.expires.
Fix this by resetting dst.expires when clearing the RTF_EXPIRE flag.
dst_set_expires checks for a zero expiration and updates the
dst.expires.
In the past this (not updating dst.expires) was necessary because
dst.expire was placed in a union with the dst_entry *from reference. So
an update on the value would have caused page faults. This split happend
in ecd9883724b78cc72ed92c98bcb1a46c764fff21 ("ipv6: fix race condition
regarding dst->expires and dst->from").
Reported-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
Reported-by: Valentijn Sessink <valentyn@blub.net>
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
I would propose this patch for -stable.
include/net/ip6_fib.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 48ec25a..5e661a9 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -165,6 +165,7 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
static inline void rt6_clean_expires(struct rt6_info *rt)
{
rt->rt6i_flags &= ~RTF_EXPIRES;
+ rt->dst.expires = 0;
}
static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net 1/2] ipv6: reset dst.expires value when clearing expire flag
2013-10-24 5:48 [PATCH net 1/2] ipv6: reset dst.expires value when clearing expire flag Hannes Frederic Sowa
@ 2013-10-24 6:36 ` Eric Dumazet
2013-10-24 7:52 ` Hannes Frederic Sowa
2013-10-24 7:47 ` Valentijn Sessink
2013-10-24 8:14 ` [PATCH net v2 " Hannes Frederic Sowa
2 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2013-10-24 6:36 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev, sgunderson, valentyn, yoshfuji
On Thu, 2013-10-24 at 07:48 +0200, Hannes Frederic Sowa wrote:
> On receiving a packet too big icmp error we update the expire value by
> calling rt6_update_expires. This function uses dst_set_expires which is
> implemented that it can only reduce the expiration value of the dst entry.
>
> If we insert new routing non-expiry information into the ipv6 fib where
> we already have a matching rt6_info we only clear the RTF_EXPIRES flag
> in rt6i_flags and leave the dst.expires value as is.
>
> When new mtu information arrives for that cached dst_entry we again
> call dst_set_expires. This time it won't update the dst.expire value
> because we left the dst.expire value intact from the last update. So
> dst_set_expires won't touch dst.expires.
>
> Fix this by resetting dst.expires when clearing the RTF_EXPIRE flag.
> dst_set_expires checks for a zero expiration and updates the
> dst.expires.
>
> In the past this (not updating dst.expires) was necessary because
> dst.expire was placed in a union with the dst_entry *from reference. So
> an update on the value would have caused page faults. This split happend
> in ecd9883724b78cc72ed92c98bcb1a46c764fff21 ("ipv6: fix race condition
> regarding dst->expires and dst->from").
>
Well, this patch removed the :
rt->dst.from = NULL;
from rt6_clean_expires(), right ?
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net 1/2] ipv6: reset dst.expires value when clearing expire flag
2013-10-24 6:36 ` Eric Dumazet
@ 2013-10-24 7:52 ` Hannes Frederic Sowa
0 siblings, 0 replies; 6+ messages in thread
From: Hannes Frederic Sowa @ 2013-10-24 7:52 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, sgunderson, valentyn, yoshfuji
On Wed, Oct 23, 2013 at 11:36:53PM -0700, Eric Dumazet wrote:
> On Thu, 2013-10-24 at 07:48 +0200, Hannes Frederic Sowa wrote:
> > On receiving a packet too big icmp error we update the expire value by
> > calling rt6_update_expires. This function uses dst_set_expires which is
> > implemented that it can only reduce the expiration value of the dst entry.
> >
> > If we insert new routing non-expiry information into the ipv6 fib where
> > we already have a matching rt6_info we only clear the RTF_EXPIRES flag
> > in rt6i_flags and leave the dst.expires value as is.
> >
> > When new mtu information arrives for that cached dst_entry we again
> > call dst_set_expires. This time it won't update the dst.expire value
> > because we left the dst.expire value intact from the last update. So
> > dst_set_expires won't touch dst.expires.
> >
> > Fix this by resetting dst.expires when clearing the RTF_EXPIRE flag.
> > dst_set_expires checks for a zero expiration and updates the
> > dst.expires.
> >
> > In the past this (not updating dst.expires) was necessary because
> > dst.expire was placed in a union with the dst_entry *from reference. So
> > an update on the value would have caused page faults. This split happend
> > in ecd9883724b78cc72ed92c98bcb1a46c764fff21 ("ipv6: fix race condition
> > regarding dst->expires and dst->from").
> >
>
> Well, this patch removed the :
>
> rt->dst.from = NULL;
>
> from rt6_clean_expires(), right ?
Exactly.
Greetings,
Hannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] ipv6: reset dst.expires value when clearing expire flag
2013-10-24 5:48 [PATCH net 1/2] ipv6: reset dst.expires value when clearing expire flag Hannes Frederic Sowa
2013-10-24 6:36 ` Eric Dumazet
@ 2013-10-24 7:47 ` Valentijn Sessink
2013-10-24 8:14 ` [PATCH net v2 " Hannes Frederic Sowa
2 siblings, 0 replies; 6+ messages in thread
From: Valentijn Sessink @ 2013-10-24 7:47 UTC (permalink / raw)
To: netdev
On 24-10-13 07:48, Hannes Frederic Sowa wrote:
> On receiving a packet too big icmp error we update the expire value by
> calling rt6_update_expires. This function uses dst_set_expires which is
> implemented that it can only reduce the expiration value of the dst entry.
>
> If we insert new routing non-expiry information into the ipv6 fib where
> we already have a matching rt6_info we only clear the RTF_EXPIRES flag
> in rt6i_flags and leave the dst.expires value as is.
>
> When new mtu information arrives for that cached dst_entry we again
> call dst_set_expires. This time it won't update the dst.expire value
> because we left the dst.expire value intact from the last update. So
> dst_set_expires won't touch dst.expires.
>
> Fix this by resetting dst.expires when clearing the RTF_EXPIRE flag.
> dst_set_expires checks for a zero expiration and updates the
> dst.expires.
>
> In the past this (not updating dst.expires) was necessary because
> dst.expire was placed in a union with the dst_entry *from reference. So
> an update on the value would have caused page faults. This split happend
> in ecd9883724b78cc72ed92c98bcb1a46c764fff21 ("ipv6: fix race condition
> regarding dst->expires and dst->from").
>
> Reported-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
> Reported-by: Valentijn Sessink <valentyn@blub.net>
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
> I would propose this patch for -stable.
>
> include/net/ip6_fib.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> index 48ec25a..5e661a9 100644
> --- a/include/net/ip6_fib.h
> +++ b/include/net/ip6_fib.h
> @@ -165,6 +165,7 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
> static inline void rt6_clean_expires(struct rt6_info *rt)
> {
> rt->rt6i_flags &= ~RTF_EXPIRES;
> + rt->dst.expires = 0;
> }
>
> static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
>
Tested-by: Valentijn Sessink <valentyn@blub.net>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net v2 1/2] ipv6: reset dst.expires value when clearing expire flag
2013-10-24 5:48 [PATCH net 1/2] ipv6: reset dst.expires value when clearing expire flag Hannes Frederic Sowa
2013-10-24 6:36 ` Eric Dumazet
2013-10-24 7:47 ` Valentijn Sessink
@ 2013-10-24 8:14 ` Hannes Frederic Sowa
2013-10-25 23:27 ` David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Hannes Frederic Sowa @ 2013-10-24 8:14 UTC (permalink / raw)
To: netdev, sgunderson, valentyn, yoshfuji, edumazet
On receiving a packet too big icmp error we update the expire value by
calling rt6_update_expires. This function uses dst_set_expires which is
implemented that it can only reduce the expiration value of the dst entry.
If we insert new routing non-expiry information into the ipv6 fib where
we already have a matching rt6_info we only clear the RTF_EXPIRES flag
in rt6i_flags and leave the dst.expires value as is.
When new mtu information arrives for that cached dst_entry we again
call dst_set_expires. This time it won't update the dst.expire value
because we left the dst.expire value intact from the last update. So
dst_set_expires won't touch dst.expires.
Fix this by resetting dst.expires when clearing the RTF_EXPIRE flag.
dst_set_expires checks for a zero expiration and updates the
dst.expires.
In the past this (not updating dst.expires) was necessary because
dst.expire was placed in a union with the dst_entry *from reference
and rt6_clean_expires did assign NULL to it. This split happend in
ecd9883724b78cc72ed92c98bcb1a46c764fff21 ("ipv6: fix race condition
regarding dst->expires and dst->from").
Reported-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
Reported-by: Valentijn Sessink <valentyn@blub.net>
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Acked-by: Eric Dumazet <edumazet@google.com>
Tested-by: Valentijn Sessink <valentyn@blub.net>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
v2: I wrote the commit message this morning and forgot about the fact that
that the mentioned commit did actually reset the expire value before. So
just improve the commit message to clear things up. Thanks, Eric!
include/net/ip6_fib.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 48ec25a..5e661a9 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -165,6 +165,7 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
static inline void rt6_clean_expires(struct rt6_info *rt)
{
rt->rt6i_flags &= ~RTF_EXPIRES;
+ rt->dst.expires = 0;
}
static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net v2 1/2] ipv6: reset dst.expires value when clearing expire flag
2013-10-24 8:14 ` [PATCH net v2 " Hannes Frederic Sowa
@ 2013-10-25 23:27 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-10-25 23:27 UTC (permalink / raw)
To: hannes; +Cc: netdev, sgunderson, valentyn, yoshfuji, edumazet
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Thu, 24 Oct 2013 10:14:27 +0200
> On receiving a packet too big icmp error we update the expire value by
> calling rt6_update_expires. This function uses dst_set_expires which is
> implemented that it can only reduce the expiration value of the dst entry.
>
> If we insert new routing non-expiry information into the ipv6 fib where
> we already have a matching rt6_info we only clear the RTF_EXPIRES flag
> in rt6i_flags and leave the dst.expires value as is.
>
> When new mtu information arrives for that cached dst_entry we again
> call dst_set_expires. This time it won't update the dst.expire value
> because we left the dst.expire value intact from the last update. So
> dst_set_expires won't touch dst.expires.
>
> Fix this by resetting dst.expires when clearing the RTF_EXPIRE flag.
> dst_set_expires checks for a zero expiration and updates the
> dst.expires.
>
> In the past this (not updating dst.expires) was necessary because
> dst.expire was placed in a union with the dst_entry *from reference
> and rt6_clean_expires did assign NULL to it. This split happend in
> ecd9883724b78cc72ed92c98bcb1a46c764fff21 ("ipv6: fix race condition
> regarding dst->expires and dst->from").
>
> Reported-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
> Reported-by: Valentijn Sessink <valentyn@blub.net>
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Acked-by: Eric Dumazet <edumazet@google.com>
> Tested-by: Valentijn Sessink <valentyn@blub.net>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-25 23:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-24 5:48 [PATCH net 1/2] ipv6: reset dst.expires value when clearing expire flag Hannes Frederic Sowa
2013-10-24 6:36 ` Eric Dumazet
2013-10-24 7:52 ` Hannes Frederic Sowa
2013-10-24 7:47 ` Valentijn Sessink
2013-10-24 8:14 ` [PATCH net v2 " Hannes Frederic Sowa
2013-10-25 23:27 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).