if the ipv6 dst cache copy from the dst witch generated by ICMPV6 RA packet.
this dst cache will not be checked expire because it has no RTF_EXPIRES flag
So this dst cache always be used until the dst gc run.
add a pointer in struct rt6_info,point to where the dst cache copy from.
in func rt6_check_expired check if rt6->info->rt6i_copy is expired.
Signed-off-by: Gao feng <redacted>
---
include/net/ip6_fib.h | 1 +
net/ipv6/route.c | 16 ++++++++++++++--
2 files changed, 15 insertions(+), 2 deletions(-)
From: Eric Dumazet <hidden> Date: 2012-02-24 06:51:18
Le vendredi 24 février 2012 à 14:20 +0800, Gao feng a écrit :
if the ipv6 dst cache copy from the dst witch generated by ICMPV6 RA packet.
this dst cache will not be checked expire because it has no RTF_EXPIRES flag
So this dst cache always be used until the dst gc run.
add a pointer in struct rt6_info,point to where the dst cache copy from.
in func rt6_check_expired check if rt6->info->rt6i_copy is expired.
Sorry, I really dont understand what you are saying.
Also, adding a pointer to a structure without holding a reference on it
is suspicious.
This bloats up every route and cached entry in the machine, find
another way.
Another way is set dst cache's RTF_EXPIRES flag and expires in ip6_rt_copy
and when receive RA packet,update all the related dst cache's expires.
I don't think this is a good idea.
quoted
+ if ((rt->rt6i_flags & RTF_EXPIRES) &&
+ time_after(jiffies, rt->dst.expires))
Poorly formatted, correct way is:
if ((rt->rt6i_flags & RTF_EXPIRES) &&
time_after(jiffies, rt->dst.expires))
Le vendredi 24 février 2012 à 14:20 +0800, Gao feng a écrit :
quoted
if the ipv6 dst cache copy from the dst witch generated by ICMPV6 RA packet.
this dst cache will not be checked expire because it has no RTF_EXPIRES flag
So this dst cache always be used until the dst gc run.
add a pointer in struct rt6_info,point to where the dst cache copy from.
in func rt6_check_expired check if rt6->info->rt6i_copy is expired.
Sorry, I really dont understand what you are saying.
Just because ipv6 dst cache has no RTF_EXPIRES flag,
so even this dst cache has expired,it will be used until the gc run.
Also, adding a pointer to a structure without holding a reference on it
is suspicious.
OK,I think there is no use to hold a reference.
the dst cache will be deleted before the pointer,
So this pointer will be always usefull.
If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.
Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.
In func rt6_check_expired check if rt6_info.dst.from is expired.
In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.
Signed-off-by: Gao feng <redacted>
---
include/net/dst.h | 11 ++++++++++-
net/ipv6/route.c | 22 +++++++++++++++++++---
2 files changed, 29 insertions(+), 4 deletions(-)
If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.
Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.
In func rt6_check_expired check if rt6_info.dst.from is expired.
In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.
I will send the v3 PATCH.
This version has some changes.
1,hold the ort in ip6_rt_copy and release it in ip6_dst_destroy just as Eric said.
2,add some functions to operate the RTF_EXPIRES flag and expires(from).
3,change the code to use the functions added in 2.
If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.
Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.
In func rt6_check_expired check if rt6_info.dst.from is expired.
In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.
In func ip6_dst_destroy release the ort.
Add some functions to operate the RTF_EXPIRES flag and expires(from)
and change the code to use these new adding functions.
Signed-off-by: Gao feng <redacted>
---
include/net/dst.h | 11 ++++++++++-
include/net/ip6_fib.h | 41 +++++++++++++++++++++++++++++++++++++++++
net/ipv6/addrconf.c | 9 +++------
net/ipv6/ip6_fib.c | 3 +--
net/ipv6/route.c | 47 +++++++++++++++++++++++++++++------------------
5 files changed, 84 insertions(+), 27 deletions(-)
If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.
Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.
In func rt6_check_expired check if rt6_info.dst.from is expired.
In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.
In func ip6_dst_destroy release the ort.
Add some functions to operate the RTF_EXPIRES flag and expires(from)
and change the code to use these new adding functions.
Signed-off-by: Gao feng <redacted>
---
include/net/dst.h | 11 ++++++++++-
include/net/ip6_fib.h | 41 +++++++++++++++++++++++++++++++++++++++++
net/ipv6/addrconf.c | 9 +++------
net/ipv6/ip6_fib.c | 3 +--
net/ipv6/route.c | 49 +++++++++++++++++++++++++++++++------------------
5 files changed, 86 insertions(+), 27 deletions(-)
From: Eric Dumazet <hidden> Date: 2012-02-29 12:14:24
Le mercredi 29 février 2012 à 18:07 +0800, Gao feng a écrit :
quoted hunk
If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.
Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.
In func rt6_check_expired check if rt6_info.dst.from is expired.
In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.
In func ip6_dst_destroy release the ort.
Add some functions to operate the RTF_EXPIRES flag and expires(from)
and change the code to use these new adding functions.
Signed-off-by: Gao feng <redacted>
---
include/net/dst.h | 11 ++++++++++-
include/net/ip6_fib.h | 41 +++++++++++++++++++++++++++++++++++++++++
net/ipv6/addrconf.c | 9 +++------
net/ipv6/ip6_fib.c | 3 +--
net/ipv6/route.c | 49 +++++++++++++++++++++++++++++++------------------
5 files changed, 86 insertions(+), 27 deletions(-)
why rt6_update_expires() takes an "int timeout", promoted to "unsigned
long expires" ? Do you have a 32bit machine by any chance ?
Why is it needed at all, it seems rt6_update_expires() is redundant with
dst_set_expires()
You hold a reference on the "from" dst, which is fine, but some previous
releases are done on dst_release(&rt->dst). So you dont release the
right dst and bad things happen.
I am not really convinced by this patch, too many issues in it.
Please take the time to make sure you submit a nice one on your next
submission. This part of the code is complex and need top quality
patches.
why rt6_update_expires() takes an "int timeout", promoted to "unsigned
long expires" ? Do you have a 32bit machine by any chance ?
Because dst_set_expires takes an "int timeout".
rt6_update_expires provides an interface to change rt->rt6i_flags and rt->dst.expires together.
Just like rt6_clean_expires,rt6_set_expires...
Why is it needed at all, it seems rt6_update_expires() is redundant with
dst_set_expires()
You hold a reference on the "from" dst, which is fine, but some previous
releases are done on dst_release(&rt->dst). So you dont release the
right dst and bad things happen.
I am not really convinced by this patch, too many issues in it.
Please take the time to make sure you submit a nice one on your next
submission. This part of the code is complex and need top quality
patches.
Thanks Eric,I will change this patch carefully,and resend it after test.
If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.
Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.
In func rt6_check_expired check if rt6_info.dst.from is expired.
In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.
In func ip6_dst_destroy release the ort.
Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
and change the code to use these new adding functions.
Signed-off-by: Gao feng <redacted>
---
include/net/dst.h | 7 ++++++-
include/net/ip6_fib.h | 40 ++++++++++++++++++++++++++++++++++++++++
net/ipv6/addrconf.c | 9 +++------
net/ipv6/ip6_fib.c | 3 +--
net/ipv6/route.c | 49 +++++++++++++++++++++++++++++++------------------
5 files changed, 81 insertions(+), 27 deletions(-)
@@ -35,7 +35,12 @@ struct dst_entry {structnet_device*dev;structdst_ops*ops;unsignedlong_metrics;-unsignedlongexpires;++union{+unsignedlongexpires;+/* point to where the dst_entry copied from */+void*from;+};structdst_entry*path;structneighbour__rcu*_neighbour;#ifdef CONFIG_XFRM
+ /* point to where the dst_entry copied from */
+ void *from;
So instead of using a real type here, your just going to cast
this thing to and from "struct dst_entry *" a thousand times?
That's terrible, there is zero value from using a void pointer
here, it just makes the code look ugly.
+ /* point to where the dst_entry copied from */
+ void *from;
So instead of using a real type here, your just going to cast
this thing to and from "struct dst_entry *" a thousand times?
That's terrible, there is zero value from using a void pointer
here, it just makes the code look ugly.
--
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
Hi David
You are right.I will resend this patch changes void * to dst_entry *.
thanks for your patience.
If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.
Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.
In func rt6_check_expired check if rt6_info.dst.from is expired.
In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.
In func ip6_dst_destroy release the ort.
Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
and change the code to use these new adding functions.
Signed-off-by: Gao feng <redacted>
---
include/net/dst.h | 7 ++++++-
include/net/ip6_fib.h | 42 ++++++++++++++++++++++++++++++++++++++++++
net/ipv6/addrconf.c | 9 +++------
net/ipv6/ip6_fib.c | 3 +--
net/ipv6/route.c | 49 +++++++++++++++++++++++++++++++------------------
5 files changed, 83 insertions(+), 27 deletions(-)
@@ -35,7 +35,12 @@ struct dst_entry {structnet_device*dev;structdst_ops*ops;unsignedlong_metrics;-unsignedlongexpires;++union{+unsignedlongexpires;+/* point to where the dst_entry copied from */+structdst_entry*from;+};structdst_entry*path;structneighbour__rcu*_neighbour;#ifdef CONFIG_XFRM
From: RongQing Li <hidden> Date: 2012-03-06 07:01:43
2012/3/5 Gao feng [off-list ref]:
quoted hunk
If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.
Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.
In func rt6_check_expired check if rt6_info.dst.from is expired.
In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.
In func ip6_dst_destroy release the ort.
Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
and change the code to use these new adding functions.
Signed-off-by: Gao feng <redacted>
---
include/net/dst.h | 7 ++++++-
include/net/ip6_fib.h | 42 ++++++++++++++++++++++++++++++++++++++++++
net/ipv6/addrconf.c | 9 +++------
net/ipv6/ip6_fib.c | 3 +--
net/ipv6/route.c | 49 +++++++++++++++++++++++++++++++------------------
5 files changed, 83 insertions(+), 27 deletions(-)
* which is 10 mins. After 10 mins the decreased pmtu is expired
* and detecting PMTU increase will be automatically happened.
*/
- dst_set_expires(&nrt->dst, net->ipv6.sysctl.ip6_rt_mtu_expires);
- nrt->rt6i_flags |= RTF_DYNAMIC|RTF_EXPIRES;
+ rt6_update_expires(nrt, net->ipv6.sysctl.ip6_rt_mtu_expires);
+ nrt->rt6i_flags |= RTF_DYNAMIC;
ip6_ins_rt(nrt);
}
if (rt->rt6i_idev)
in6_dev_hold(rt->rt6i_idev);
rt->dst.lastuse = jiffies;
- rt->dst.expires = 0;
rt->rt6i_gateway = ort->rt6i_gateway;
- rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
+ rt->rt6i_flags = ort->rt6i_flags;
+ rt6_set_from(rt, ort);
+
rt->rt6i_metric = 0;
#ifdef CONFIG_IPV6_SUBTREES
--
1.7.5.4
--
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
If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.
Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.
In func rt6_check_expired check if rt6_info.dst.from is expired.
In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.
In func ip6_dst_destroy release the ort.
Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
and change the code to use these new adding functions.
Signed-off-by: Gao feng <redacted>
I see some unprotected access to dst.expires in the router discovery
code in net/ipv6/ndisc.c, doesn't that need to be updated?
There are probably some more similar cases elsewhere in the ipv6 code
too.
If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.
Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.
In func rt6_check_expired check if rt6_info.dst.from is expired.
In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.
In func ip6_dst_destroy release the ort.
Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
and change the code to use these new adding functions.
Signed-off-by: Gao feng <redacted>
I see some unprotected access to dst.expires in the router discovery
code in net/ipv6/ndisc.c, doesn't that need to be updated?
There are probably some more similar cases elsewhere in the ipv6 code
too.
Hi David
I only search the RTF_EXPIRES flag and forget the expires.I will do this.
BUT what confuse me is that, in func ip6_rt_copy should we do rt6_set_from in any case
or only when the ort has flag RTF_ADDRCONF and RTF_DEFAULT?
thanks.
BUT what confuse me is that, in func ip6_rt_copy should we do
rt6_set_from in any case or only when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT?
Your guess is as good as mine, unfortunately. A lot of code in
this area is hard to decipher.
For example, I've spent the past several months trying to figure out
which kinds of ipv6 routes have explicit neighbour entries attached at
route insert time, which do not, etc.
If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.
Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.
rt6_check_expired check if rt6_info.dst.from is expired.
ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.
ip6_dst_destroy release the ort.
Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
and change the code to use these new adding functions.
Changes from v5:
modify ip6_route_add and ndisc_router_discovery to use new adding functions.
Only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.
Signed-off-by: Gao feng <redacted>
---
include/net/dst.h | 6 +++-
include/net/ip6_fib.h | 42 +++++++++++++++++++++++++++++
net/ipv6/addrconf.c | 9 ++----
net/ipv6/ip6_fib.c | 9 +++---
net/ipv6/ndisc.c | 3 +-
net/ipv6/route.c | 71 ++++++++++++++++++++++++++++++------------------
6 files changed, 99 insertions(+), 41 deletions(-)
@@ -36,7 +36,11 @@ struct dst_entry {structnet_device*dev;structdst_ops*ops;unsignedlong_metrics;-unsignedlongexpires;+union{+unsignedlongexpires;+/* point to where the dst_entry copied from */+structdst_entry*from;+};structdst_entry*path;structneighbour__rcu*_neighbour;#ifdef CONFIG_XFRM
If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.
Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.
rt6_check_expired check if rt6_info.dst.from is expired.
ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.
ip6_dst_destroy release the ort.
Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
and change the code to use these new adding functions.
Changes from v5:
modify ip6_route_add and ndisc_router_discovery to use new adding functions.
Only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.
Signed-off-by: Gao feng <redacted>
Commit 1716a961 (ipv6: fix problem with expired dst cache) broke PMTU
discovery. rt6_update_expires() calls dst_set_expires(), which only updates
dst->expires if it has not been set previously (expires == 0) or if the new
expires is earlier than the current dst->expires.
rt6_update_expires() needs to zero rt->dst.expires, otherwise it will contain
ivalid data left over from rt->dst.from and will confuse dst_set_expires().
Signed-off-by: Jiri Bohac <redacted>
Functionally, this change is a NOP.
Semantically, rt6_clean_expires() wants to do rt->dst.from = NULL instead of
rt->dst.expires = 0. It is clearing the RTF_EXPIRES flag, so the union is going
to be treated as a pointer (dst.from) not a long (dst.expires).
Signed-off-by: Jiri Bohac <redacted>
Commit 1716a961 (ipv6: fix problem with expired dst cache) broke PMTU
discovery. rt6_update_expires() calls dst_set_expires(), which only updates
dst->expires if it has not been set previously (expires == 0) or if the new
expires is earlier than the current dst->expires.
rt6_update_expires() needs to zero rt->dst.expires, otherwise it will contain
ivalid data left over from rt->dst.from and will confuse dst_set_expires().
Signed-off-by: Jiri Bohac <redacted>
Commit 1716a961 (ipv6: fix problem with expired dst cache) broke PMTU
discovery. rt6_update_expires() calls dst_set_expires(), which only updates
dst->expires if it has not been set previously (expires == 0) or if the new
expires is earlier than the current dst->expires.
rt6_update_expires() needs to zero rt->dst.expires, otherwise it will contain
ivalid data left over from rt->dst.from and will confuse dst_set_expires().
Signed-off-by: Jiri Bohac <redacted>
Functionally, this change is a NOP.
Semantically, rt6_clean_expires() wants to do rt->dst.from = NULL instead of
rt->dst.expires = 0. It is clearing the RTF_EXPIRES flag, so the union is going
to be treated as a pointer (dst.from) not a long (dst.expires).
Signed-off-by: Jiri Bohac <redacted>