| Message ID | 5279EF31.4040705@cn.fujitsu.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, Nov 06, 2013 at 03:26:41PM +0800, Duan Jiong wrote: > > As the rfc 4191 said, the Router Preference and Lifetime values in a > ::/0 Route Information Option should override the preference and lifetime > values in the Router Advertisement header. But when the kernel deals with > a ::/0 Route Information Option, the rt6_get_route_info() always return > NULL, that means that overriding will not happen, because those default > routers were added without flag RTF_ROUTEINFO in rt6_add_dflt_router(). > > In order to match those default routers, we can replace RTF_ROUTEINFO > with RTF_ADDRCONF in rt6_get_route_info(). > > Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com> Hmm, that looks like a bug. Nice catch! Couldn't we just replace the rt6_get_route_info in rt6_route_rcv with a call to rt6_get_dflt_router? Seems easier, already handles the ::/0 case and also does preserve the check for the RTF_ROUTEINFO flag in rt6_add_route_info. Greetings, Hannes -- 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
于 2013年11月07日 09:51, Hannes Frederic Sowa 写道: > On Wed, Nov 06, 2013 at 03:26:41PM +0800, Duan Jiong wrote: >> >> As the rfc 4191 said, the Router Preference and Lifetime values in a >> ::/0 Route Information Option should override the preference and lifetime >> values in the Router Advertisement header. But when the kernel deals with >> a ::/0 Route Information Option, the rt6_get_route_info() always return >> NULL, that means that overriding will not happen, because those default >> routers were added without flag RTF_ROUTEINFO in rt6_add_dflt_router(). >> >> In order to match those default routers, we can replace RTF_ROUTEINFO >> with RTF_ADDRCONF in rt6_get_route_info(). >> >> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com> > > Hmm, that looks like a bug. Nice catch! > > Couldn't we just replace the rt6_get_route_info in rt6_route_rcv with a call > to rt6_get_dflt_router? Seems easier, already handles the ::/0 case and also > does preserve the check for the RTF_ROUTEINFO flag in rt6_add_route_info. Yeah, your idea is better. I will modify my patch. Thanks, Duan > > Greetings, > > Hannes > > -- > 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 > -- 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
On Thu, Nov 07, 2013 at 10:01:27AM +0800, Duan Jiong wrote: > 于 2013年11月07日 09:51, Hannes Frederic Sowa 写道: > > On Wed, Nov 06, 2013 at 03:26:41PM +0800, Duan Jiong wrote: > >> > >> As the rfc 4191 said, the Router Preference and Lifetime values in a > >> ::/0 Route Information Option should override the preference and lifetime > >> values in the Router Advertisement header. But when the kernel deals with > >> a ::/0 Route Information Option, the rt6_get_route_info() always return > >> NULL, that means that overriding will not happen, because those default > >> routers were added without flag RTF_ROUTEINFO in rt6_add_dflt_router(). > >> > >> In order to match those default routers, we can replace RTF_ROUTEINFO > >> with RTF_ADDRCONF in rt6_get_route_info(). > >> > >> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com> > > > > Hmm, that looks like a bug. Nice catch! > > > > Couldn't we just replace the rt6_get_route_info in rt6_route_rcv with a call > > to rt6_get_dflt_router? Seems easier, already handles the ::/0 case and also > > does preserve the check for the RTF_ROUTEINFO flag in rt6_add_route_info. > > Yeah, your idea is better. I will modify my patch. Stop, stop, stop, sorry that suggestion is plain wrong and stupid. But I still would like to see the check for RTF_ROUTEINFO happening in rt6_add_route_info. We should also rename the function if you only query for RTF_ADDRCONF and not RTF_ROUTEINFO. Sorry, Hannes -- 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
于 2013年11月07日 10:42, Hannes Frederic Sowa 写道: > On Thu, Nov 07, 2013 at 10:01:27AM +0800, Duan Jiong wrote: >> 于 2013年11月07日 09:51, Hannes Frederic Sowa 写道: >>> On Wed, Nov 06, 2013 at 03:26:41PM +0800, Duan Jiong wrote: >>>> >>>> As the rfc 4191 said, the Router Preference and Lifetime values in a >>>> ::/0 Route Information Option should override the preference and lifetime >>>> values in the Router Advertisement header. But when the kernel deals with >>>> a ::/0 Route Information Option, the rt6_get_route_info() always return >>>> NULL, that means that overriding will not happen, because those default >>>> routers were added without flag RTF_ROUTEINFO in rt6_add_dflt_router(). >>>> >>>> In order to match those default routers, we can replace RTF_ROUTEINFO >>>> with RTF_ADDRCONF in rt6_get_route_info(). >>>> >>>> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com> >>> >>> Hmm, that looks like a bug. Nice catch! >>> >>> Couldn't we just replace the rt6_get_route_info in rt6_route_rcv with a call >>> to rt6_get_dflt_router? Seems easier, already handles the ::/0 case and also >>> does preserve the check for the RTF_ROUTEINFO flag in rt6_add_route_info. >> >> Yeah, your idea is better. I will modify my patch. > > Stop, stop, stop, sorry that suggestion is plain wrong and stupid. > In my opinion, i think that is well. According to your suggestion, we can call the rt6_get_dflt_router or rt6_get_route_info in different situation. if (rinfo->prefix_len == 0) rt = rt6_get_dflt_router(gwaddr, dev); else rt = rt6_get_route_info(net, prefix, rinfo->prefix_len, gwaddr, dev->ifindex); How do you think of the change above? Thanks, Duan > But I still would like to see the check for RTF_ROUTEINFO happening > in rt6_add_route_info. We should also rename the function if you only query > for RTF_ADDRCONF and not RTF_ROUTEINFO. > > Sorry, > > Hannes > > -- 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
On Thu, Nov 07, 2013 at 12:19:05PM +0800, Duan Jiong wrote: > 于 2013年11月07日 10:42, Hannes Frederic Sowa 写道: > > On Thu, Nov 07, 2013 at 10:01:27AM +0800, Duan Jiong wrote: > >> 于 2013年11月07日 09:51, Hannes Frederic Sowa 写道: > >>> On Wed, Nov 06, 2013 at 03:26:41PM +0800, Duan Jiong wrote: > >>>> > >>>> As the rfc 4191 said, the Router Preference and Lifetime values in a > >>>> ::/0 Route Information Option should override the preference and lifetime > >>>> values in the Router Advertisement header. But when the kernel deals with > >>>> a ::/0 Route Information Option, the rt6_get_route_info() always return > >>>> NULL, that means that overriding will not happen, because those default > >>>> routers were added without flag RTF_ROUTEINFO in rt6_add_dflt_router(). > >>>> > >>>> In order to match those default routers, we can replace RTF_ROUTEINFO > >>>> with RTF_ADDRCONF in rt6_get_route_info(). > >>>> > >>>> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com> > >>> > >>> Hmm, that looks like a bug. Nice catch! > >>> > >>> Couldn't we just replace the rt6_get_route_info in rt6_route_rcv with a call > >>> to rt6_get_dflt_router? Seems easier, already handles the ::/0 case and also > >>> does preserve the check for the RTF_ROUTEINFO flag in rt6_add_route_info. > >> > >> Yeah, your idea is better. I will modify my patch. > > > > Stop, stop, stop, sorry that suggestion is plain wrong and stupid. > > > > In my opinion, i think that is well. According to your suggestion, we can > call the rt6_get_dflt_router or rt6_get_route_info in different situation. > > if (rinfo->prefix_len == 0) > rt = rt6_get_dflt_router(gwaddr, dev); > else > rt = rt6_get_route_info(net, prefix, rinfo->prefix_len, gwaddr, > dev->ifindex); That's perfect. Greetings, Hannes -- 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 --git a/net/ipv6/route.c b/net/ipv6/route.c index 04e17b3..549aa3b 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -1940,7 +1940,8 @@ static struct rt6_info *rt6_get_route_info(struct net *net, for (rt = fn->leaf; rt; rt = rt->dst.rt6_next) { if (rt->dst.dev->ifindex != ifindex) continue; - if ((rt->rt6i_flags & (RTF_ROUTEINFO|RTF_GATEWAY)) != (RTF_ROUTEINFO|RTF_GATEWAY)) + if ((rt->rt6i_flags & (RTF_ADDRCONF|RTF_GATEWAY)) != + (RTF_ADDRCONF|RTF_GATEWAY)) continue; if (!ipv6_addr_equal(&rt->rt6i_gateway, gwaddr)) continue;
As the rfc 4191 said, the Router Preference and Lifetime values in a ::/0 Route Information Option should override the preference and lifetime values in the Router Advertisement header. But when the kernel deals with a ::/0 Route Information Option, the rt6_get_route_info() always return NULL, that means that overriding will not happen, because those default routers were added without flag RTF_ROUTEINFO in rt6_add_dflt_router(). In order to match those default routers, we can replace RTF_ROUTEINFO with RTF_ADDRCONF in rt6_get_route_info(). Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com> --- net/ipv6/route.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)