[PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-20 Thread Hangbin Liu
After commit 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib result when requested"). When we get a prohibit ertry, we will return -EACCES directly. Before: + ip netns exec client ip -6 route get 2003::1 prohibit 2003::1 dev lo table unspec proto kernel src 2001::1 metric 4294967295 err

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-20 Thread Hangbin Liu
Hi Roopa, Cong, 2017-07-20 22:51 GMT+08:00 Hangbin Liu : > After commit 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib > result when requested"). When we get a prohibit ertry, we will return > -EACCES directly. > > Before: > + ip netns exec client ip -6 route get 2003::1 > prohibit 200

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-20 Thread Hangbin Liu
2017-07-20 23:06 GMT+08:00 Hangbin Liu : >> +++ b/net/ipv6/route.c >> @@ -3637,12 +3637,6 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, >> struct nlmsghdr *nlh, >> dst = ip6_route_lookup(net, &fl6, 0); >> >> rt = container_of(dst, struct rt6_info, dst); >> -

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-21 Thread David Ahern
On 7/20/17 9:23 AM, Hangbin Liu wrote: > diff --git a/net/ipv6/route.c b/net/ipv6/route.c > index 4d30c96..c290aa4 100644 > --- a/net/ipv6/route.c > +++ b/net/ipv6/route.c > @@ -3637,13 +3637,8 @@ static int inet6_rtm_getroute(struct sk_buff > *in_skb, struct nlmsghdr *nlh, > dst =

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-21 Thread Cong Wang
On Thu, Jul 20, 2017 at 8:23 AM, Hangbin Liu wrote: > 2017-07-20 23:06 GMT+08:00 Hangbin Liu : >>> +++ b/net/ipv6/route.c >>> @@ -3637,12 +3637,6 @@ static int inet6_rtm_getroute(struct sk_buff >>> *in_skb, struct nlmsghdr *nlh, >>> dst = ip6_route_lookup(net, &fl6, 0); >>> >>>

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-21 Thread Roopa Prabhu
On Fri, Jul 21, 2017 at 11:42 AM, Cong Wang wrote: > On Thu, Jul 20, 2017 at 8:23 AM, Hangbin Liu wrote: >> 2017-07-20 23:06 GMT+08:00 Hangbin Liu : +++ b/net/ipv6/route.c @@ -3637,12 +3637,6 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-22 Thread Roopa Prabhu
On Fri, Jul 21, 2017 at 2:53 PM, Roopa Prabhu wrote: > On Fri, Jul 21, 2017 at 11:42 AM, Cong Wang wrote: >> On Thu, Jul 20, 2017 at 8:23 AM, Hangbin Liu wrote: >>> 2017-07-20 23:06 GMT+08:00 Hangbin Liu : > +++ b/net/ipv6/route.c > @@ -3637,12 +3637,6 @@ static int inet6_rtm_getroute(st

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-22 Thread Roopa Prabhu
On Thu, Jul 20, 2017 at 7:51 AM, Hangbin Liu wrote: > After commit 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib > result when requested"). When we get a prohibit ertry, we will return > -EACCES directly. > > Before: > + ip netns exec client ip -6 route get 2003::1 > prohibit 2003::1

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-23 Thread Hangbin Liu
Hi Roopa, On Sat, Jul 22, 2017 at 09:55:51PM -0700, Roopa Prabhu wrote: > On Thu, Jul 20, 2017 at 7:51 AM, Hangbin Liu wrote: > > After commit 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib > > result when requested"). When we get a prohibit ertry, we will return > > -EACCES directly.

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-23 Thread Hangbin Liu
Hi Cong, On Fri, Jul 21, 2017 at 11:42:46AM -0700, Cong Wang wrote: > On Thu, Jul 20, 2017 at 8:23 AM, Hangbin Liu wrote: > > 2017-07-20 23:06 GMT+08:00 Hangbin Liu : > >>> +++ b/net/ipv6/route.c > >>> @@ -3637,12 +3637,6 @@ static int inet6_rtm_getroute(struct sk_buff > >>> *in_skb, struct nlmsg

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-24 Thread Cong Wang
On Sun, Jul 23, 2017 at 8:09 PM, Hangbin Liu wrote: > Do we still need this net->ipv6.ip6_null_entry check? How about remove all > the checks? I believe you only need to check for rt->dst.error, no need to check against NULL or ip6_null_entry. Take a look at other ip6_route_lookup() callers.

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-24 Thread Hangbin Liu
On Mon, Jul 24, 2017 at 12:57:43PM -0700, Cong Wang wrote: > On Sun, Jul 23, 2017 at 8:09 PM, Hangbin Liu wrote: > > Do we still need this net->ipv6.ip6_null_entry check? How about remove all > > the checks? > > I believe you only need to check for rt->dst.error, no need to check against > NULL o

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-24 Thread David Ahern
On 7/24/17 6:08 PM, Hangbin Liu wrote: > That's why I think we should remove both rt->dst.error and ip6_null_entry > check in inet6_rtm_getroute(). And even further, remove the ip6_null_entry > check in rt6_dump_route(). git blame net/ipv6/route.c find the commits and review.

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-25 Thread Hangbin Liu
On Mon, Jul 24, 2017 at 09:28:07PM -0600, David Ahern wrote: > On 7/24/17 6:08 PM, Hangbin Liu wrote: > > That's why I think we should remove both rt->dst.error and ip6_null_entry > > check in inet6_rtm_getroute(). And even further, remove the ip6_null_entry > > check in rt6_dump_route(). > > git

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-25 Thread Cong Wang
On Mon, Jul 24, 2017 at 5:08 PM, Hangbin Liu wrote: > But what we want in inet6_rtm_getroute() and rt6_dump_route() is to > get/dump the route info. So we should get the info even it's unreachable or > prohibit. If you want to dump prohibit/blackhole entry, then you have to check for null_entry,

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-26 Thread Hangbin Liu
On Tue, Jul 25, 2017 at 10:49:05AM -0700, Cong Wang wrote: > On Mon, Jul 24, 2017 at 5:08 PM, Hangbin Liu wrote: > > But what we want in inet6_rtm_getroute() and rt6_dump_route() is to > > get/dump the route info. So we should get the info even it's unreachable or > > prohibit. > > If you want to

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-26 Thread David Ahern
On 7/25/17 1:32 AM, Hangbin Liu wrote: > On Mon, Jul 24, 2017 at 09:28:07PM -0600, David Ahern wrote: >> On 7/24/17 6:08 PM, Hangbin Liu wrote: >>> That's why I think we should remove both rt->dst.error and ip6_null_entry >>> check in inet6_rtm_getroute(). And even further, remove the ip6_null_entr

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-26 Thread Roopa Prabhu
On Wed, Jul 26, 2017 at 10:18 AM, David Ahern wrote: > On 7/25/17 1:32 AM, Hangbin Liu wrote: >> On Mon, Jul 24, 2017 at 09:28:07PM -0600, David Ahern wrote: >>> On 7/24/17 6:08 PM, Hangbin Liu wrote: That's why I think we should remove both rt->dst.error and ip6_null_entry check in inet

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-26 Thread David Ahern
On 7/26/17 12:27 PM, Roopa Prabhu wrote: > agreed...so looks like the check in v3 should be > > > + if ( rt == net->ipv6.ip6_null_entry || > +(rt->dst.error && > + #ifdef CONFIG_IPV6_MULTIPLE_TABLES > + rt != net->ipv6.ip6_prohibit_entry && > + rt != ne

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-26 Thread Roopa Prabhu
On Wed, Jul 26, 2017 at 11:49 AM, David Ahern wrote: > On 7/26/17 12:27 PM, Roopa Prabhu wrote: >> agreed...so looks like the check in v3 should be >> >> >> + if ( rt == net->ipv6.ip6_null_entry || >> +(rt->dst.error && >> + #ifdef CONFIG_IPV6_MULTIPLE_TABLES >> + rt

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-26 Thread David Ahern
On 7/26/17 12:55 PM, Roopa Prabhu wrote: > On Wed, Jul 26, 2017 at 11:49 AM, David Ahern wrote: >> On 7/26/17 12:27 PM, Roopa Prabhu wrote: >>> agreed...so looks like the check in v3 should be >>> >>> >>> + if ( rt == net->ipv6.ip6_null_entry || >>> +(rt->dst.error && >>> + #ifde

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-26 Thread Roopa Prabhu
On Wed, Jul 26, 2017 at 12:00 PM, David Ahern wrote: > On 7/26/17 12:55 PM, Roopa Prabhu wrote: >> On Wed, Jul 26, 2017 at 11:49 AM, David Ahern wrote: >>> On 7/26/17 12:27 PM, Roopa Prabhu wrote: agreed...so looks like the check in v3 should be + if ( rt == net->ipv6.ip

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-27 Thread Hangbin Liu
Hi David, On Wed, Jul 26, 2017 at 01:00:26PM -0600, David Ahern wrote: > >> I don't think so. If I add a prohibit route and use the fibmatch > >> attribute, I want to see the route from the FIB that was matched. > > > > > > yes, exactly. wouldn't 'rt != net->ipv6.ip6_prohibit_entry' above let >

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-27 Thread Cong Wang
On Wed, Jul 26, 2017 at 11:49 AM, David Ahern wrote: > On 7/26/17 12:27 PM, Roopa Prabhu wrote: >> agreed...so looks like the check in v3 should be >> >> >> + if ( rt == net->ipv6.ip6_null_entry || >> +(rt->dst.error && >> + #ifdef CONFIG_IPV6_MULTIPLE_TABLES >> + rt

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-28 Thread Hangbin Liu
On Thu, Jul 27, 2017 at 09:56:08PM -0700, Cong Wang wrote: > On Wed, Jul 26, 2017 at 11:49 AM, David Ahern wrote: > > On 7/26/17 12:27 PM, Roopa Prabhu wrote: > >> agreed...so looks like the check in v3 should be > >> > >> > >> + if ( rt == net->ipv6.ip6_null_entry || > >> +(rt->

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-28 Thread David Ahern
On 7/27/17 10:56 PM, Cong Wang wrote: > On Wed, Jul 26, 2017 at 11:49 AM, David Ahern wrote: >> On 7/26/17 12:27 PM, Roopa Prabhu wrote: >>> agreed...so looks like the check in v3 should be >>> >>> >>> + if ( rt == net->ipv6.ip6_null_entry || >>> +(rt->dst.error && >>> + #ifdef C

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-28 Thread Roopa Prabhu
On Fri, Jul 28, 2017 at 8:10 AM, David Ahern wrote: > On 7/27/17 10:56 PM, Cong Wang wrote: >> On Wed, Jul 26, 2017 at 11:49 AM, David Ahern wrote: >>> On 7/26/17 12:27 PM, Roopa Prabhu wrote: agreed...so looks like the check in v3 should be + if ( rt == net->ipv6.ip6_nu

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-28 Thread David Ahern
On 7/28/17 11:13 AM, Roopa Prabhu wrote: > for fibmatch, my original intent was to return with an error code. > This is similar > to the ipv4 behavior. One option is to keep the check in there and put > the 'fibmatch' > condition around it. But, i do want to make sure that for the fibmatch case, >

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-28 Thread Roopa Prabhu
On Fri, Jul 28, 2017 at 10:39 AM, David Ahern wrote: > On 7/28/17 11:13 AM, Roopa Prabhu wrote: >> for fibmatch, my original intent was to return with an error code. >> This is similar >> to the ipv4 behavior. One option is to keep the check in there and put >> the 'fibmatch' >> condition around i

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-29 Thread David Ahern
On 7/28/17 1:52 PM, Roopa Prabhu wrote: > On Fri, Jul 28, 2017 at 10:39 AM, David Ahern wrote: >> IPv4 does not have the notion of null_entry or prohibit route entries >> which makes IPv4 and IPv6 inconsistent - something we really need to be >> avoiding from a user experience. >> >> We have the f

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-31 Thread Cong Wang
On Fri, Jul 28, 2017 at 10:39 AM, David Ahern wrote: > On 7/28/17 11:13 AM, Roopa Prabhu wrote: >> for fibmatch, my original intent was to return with an error code. >> This is similar >> to the ipv4 behavior. One option is to keep the check in there and put >> the 'fibmatch' >> condition around i

Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

2017-07-31 Thread David Ahern
On 7/31/17 12:37 PM, Cong Wang wrote: > >> So Roopa's fibmatch patches brings consistency between IPv4 and IPv6 at >> the cost of breaking backwards compatibility for IPv6 when the prohibit >> or blackhole routes are hit. >> > There are already many differences between IPv4 and > IPv6 behaviors, I