* [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
@ 2017-07-20 14:51 Hangbin Liu
2017-07-20 15:06 ` Hangbin Liu
` (4 more replies)
0 siblings, 5 replies; 46+ messages in thread
From: Hangbin Liu @ 2017-07-20 14:51 UTC (permalink / raw)
To: netdev; +Cc: Roopa Prabhu, WANG Cong, 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 error -13
After:
+ ip netns exec server ip -6 route get 2002::1
RTNETLINK answers: Network is unreachable
Since we will check the ip6_null_entry later. There is not sense
to check the dst.error after get rt.
Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/ipv6/route.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4d30c96..8fc52de 100644
--- a/net/ipv6/route.c
+++ 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);
- if (rt->dst.error) {
- err = rt->dst.error;
- ip6_rt_put(rt);
- goto errout;
- }
-
if (rt == net->ipv6.ip6_null_entry) {
err = rt->dst.error;
ip6_rt_put(rt);
--
2.5.5
^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-20 14:51 [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry Hangbin Liu
@ 2017-07-20 15:06 ` Hangbin Liu
2017-07-20 15:23 ` Hangbin Liu
2017-07-21 3:47 ` [PATCHv2 net] ipv6: should not return rt->dst.error if it is prohibit or blk hole entry Hangbin Liu
` (3 subsequent siblings)
4 siblings, 1 reply; 46+ messages in thread
From: Hangbin Liu @ 2017-07-20 15:06 UTC (permalink / raw)
To: network dev; +Cc: Roopa Prabhu, WANG Cong, Hangbin Liu
Hi Roopa, Cong,
2017-07-20 22:51 GMT+08:00 Hangbin Liu <liuhangbin@gmail.com>:
> 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 error -13
>
> After:
> + ip netns exec server ip -6 route get 2002::1
> RTNETLINK answers: Network is unreachable
>
> Since we will check the ip6_null_entry later. There is not sense
> to check the dst.error after get rt.
>
> Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> net/ipv6/route.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 4d30c96..8fc52de 100644
> --- a/net/ipv6/route.c
> +++ 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);
> - if (rt->dst.error) {
> - err = rt->dst.error;
> - ip6_rt_put(rt);
> - goto errout;
> - }
hmm... or instead of remove this check, should we check all the entry? Like
if ((rt->dst.error && rt != net->ipv6.ip6_null_entry && rt !=
net->ipv6.ip6_blk_hole_entry) ||
rt == net->ipv6.ip6_null_entry )
What do you think?
Thanks
Hangbin
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-20 15:06 ` Hangbin Liu
@ 2017-07-20 15:23 ` Hangbin Liu
2017-07-21 15:53 ` David Ahern
2017-07-21 18:42 ` Cong Wang
0 siblings, 2 replies; 46+ messages in thread
From: Hangbin Liu @ 2017-07-20 15:23 UTC (permalink / raw)
To: network dev; +Cc: Roopa Prabhu, WANG Cong, Hangbin Liu
2017-07-20 23:06 GMT+08:00 Hangbin Liu <liuhangbin@gmail.com>:
>> +++ 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);
>> - if (rt->dst.error) {
>> - err = rt->dst.error;
>> - ip6_rt_put(rt);
>> - goto errout;
>> - }
>
> hmm... or instead of remove this check, should we check all the entry? Like
> if ((rt->dst.error && rt != net->ipv6.ip6_null_entry && rt !=
^^ mistake here
> net->ipv6.ip6_blk_hole_entry) ||
> rt == net->ipv6.ip6_null_entry )
Sorry, there should be no need to check ip6_null_entry since the
error is already
-ENETUNREACH. So how about
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 = ip6_route_lookup(net, &fl6, 0);
rt = container_of(dst, struct rt6_info, dst);
- if (rt->dst.error) {
- err = rt->dst.error;
- ip6_rt_put(rt);
- goto errout;
- }
-
- if (rt == net->ipv6.ip6_null_entry) {
+ if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
+ rt != net->ipv6.ip6_blk_hole_entry) {
err = rt->dst.error;
ip6_rt_put(rt);
goto errout;
Thanks
Hangbin
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCHv2 net] ipv6: should not return rt->dst.error if it is prohibit or blk hole entry.
2017-07-20 14:51 [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry Hangbin Liu
2017-07-20 15:06 ` Hangbin Liu
@ 2017-07-21 3:47 ` Hangbin Liu
2017-07-21 15:29 ` kbuild test robot
2017-07-21 16:34 ` kbuild test robot
2017-07-23 4:55 ` [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry Roopa Prabhu
` (2 subsequent siblings)
4 siblings, 2 replies; 46+ messages in thread
From: Hangbin Liu @ 2017-07-21 3:47 UTC (permalink / raw)
To: netdev; +Cc: Roopa Prabhu, WANG Cong, 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 error -13
After:
+ ip netns exec server ip -6 route get 2002::1
RTNETLINK answers: Permission denied
Fix this by add prohibit and blk hole check. Since ip6_null_entry's
error is already -ENETUNREACH. Merge the ip6_null_entry check and error
check together.
Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/ipv6/route.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
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 = ip6_route_lookup(net, &fl6, 0);
rt = container_of(dst, struct rt6_info, dst);
- if (rt->dst.error) {
- err = rt->dst.error;
- ip6_rt_put(rt);
- goto errout;
- }
-
- if (rt == net->ipv6.ip6_null_entry) {
+ if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
+ rt != net->ipv6.ip6_blk_hole_entry) {
err = rt->dst.error;
ip6_rt_put(rt);
goto errout;
--
2.5.5
^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCHv2 net] ipv6: should not return rt->dst.error if it is prohibit or blk hole entry.
2017-07-21 3:47 ` [PATCHv2 net] ipv6: should not return rt->dst.error if it is prohibit or blk hole entry Hangbin Liu
@ 2017-07-21 15:29 ` kbuild test robot
2017-07-21 16:34 ` kbuild test robot
1 sibling, 0 replies; 46+ messages in thread
From: kbuild test robot @ 2017-07-21 15:29 UTC (permalink / raw)
To: Hangbin Liu; +Cc: kbuild-all, netdev, Roopa Prabhu, WANG Cong, Hangbin Liu
[-- Attachment #1: Type: text/plain, Size: 4517 bytes --]
Hi Hangbin,
[auto build test ERROR on net/master]
url: https://github.com/0day-ci/linux/commits/Hangbin-Liu/ipv6-should-not-return-rt-dst-error-if-it-is-prohibit-or-blk-hole-entry/20170721-204554
config: m68k-sun3_defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k
All errors (new ones prefixed by >>):
net//ipv6/route.c: In function 'inet6_rtm_getroute':
>> net//ipv6/route.c:3640:38: error: 'struct netns_ipv6' has no member named 'ip6_prohibit_entry'
if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
^
>> net//ipv6/route.c:3641:21: error: 'struct netns_ipv6' has no member named 'ip6_blk_hole_entry'
rt != net->ipv6.ip6_blk_hole_entry) {
^
vim +3640 net//ipv6/route.c
3558
3559 static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
3560 struct netlink_ext_ack *extack)
3561 {
3562 struct net *net = sock_net(in_skb->sk);
3563 struct nlattr *tb[RTA_MAX+1];
3564 int err, iif = 0, oif = 0;
3565 struct dst_entry *dst;
3566 struct rt6_info *rt;
3567 struct sk_buff *skb;
3568 struct rtmsg *rtm;
3569 struct flowi6 fl6;
3570 bool fibmatch;
3571
3572 err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_ipv6_policy,
3573 extack);
3574 if (err < 0)
3575 goto errout;
3576
3577 err = -EINVAL;
3578 memset(&fl6, 0, sizeof(fl6));
3579 rtm = nlmsg_data(nlh);
3580 fl6.flowlabel = ip6_make_flowinfo(rtm->rtm_tos, 0);
3581 fibmatch = !!(rtm->rtm_flags & RTM_F_FIB_MATCH);
3582
3583 if (tb[RTA_SRC]) {
3584 if (nla_len(tb[RTA_SRC]) < sizeof(struct in6_addr))
3585 goto errout;
3586
3587 fl6.saddr = *(struct in6_addr *)nla_data(tb[RTA_SRC]);
3588 }
3589
3590 if (tb[RTA_DST]) {
3591 if (nla_len(tb[RTA_DST]) < sizeof(struct in6_addr))
3592 goto errout;
3593
3594 fl6.daddr = *(struct in6_addr *)nla_data(tb[RTA_DST]);
3595 }
3596
3597 if (tb[RTA_IIF])
3598 iif = nla_get_u32(tb[RTA_IIF]);
3599
3600 if (tb[RTA_OIF])
3601 oif = nla_get_u32(tb[RTA_OIF]);
3602
3603 if (tb[RTA_MARK])
3604 fl6.flowi6_mark = nla_get_u32(tb[RTA_MARK]);
3605
3606 if (tb[RTA_UID])
3607 fl6.flowi6_uid = make_kuid(current_user_ns(),
3608 nla_get_u32(tb[RTA_UID]));
3609 else
3610 fl6.flowi6_uid = iif ? INVALID_UID : current_uid();
3611
3612 if (iif) {
3613 struct net_device *dev;
3614 int flags = 0;
3615
3616 dev = __dev_get_by_index(net, iif);
3617 if (!dev) {
3618 err = -ENODEV;
3619 goto errout;
3620 }
3621
3622 fl6.flowi6_iif = iif;
3623
3624 if (!ipv6_addr_any(&fl6.saddr))
3625 flags |= RT6_LOOKUP_F_HAS_SADDR;
3626
3627 if (!fibmatch)
3628 dst = ip6_route_input_lookup(net, dev, &fl6, flags);
3629 } else {
3630 fl6.flowi6_oif = oif;
3631
3632 if (!fibmatch)
3633 dst = ip6_route_output(net, NULL, &fl6);
3634 }
3635
3636 if (fibmatch)
3637 dst = ip6_route_lookup(net, &fl6, 0);
3638
3639 rt = container_of(dst, struct rt6_info, dst);
> 3640 if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
> 3641 rt != net->ipv6.ip6_blk_hole_entry) {
3642 err = rt->dst.error;
3643 ip6_rt_put(rt);
3644 goto errout;
3645 }
3646
3647 skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
3648 if (!skb) {
3649 ip6_rt_put(rt);
3650 err = -ENOBUFS;
3651 goto errout;
3652 }
3653
3654 skb_dst_set(skb, &rt->dst);
3655 if (fibmatch)
3656 err = rt6_fill_node(net, skb, rt, NULL, NULL, iif,
3657 RTM_NEWROUTE, NETLINK_CB(in_skb).portid,
3658 nlh->nlmsg_seq, 0);
3659 else
3660 err = rt6_fill_node(net, skb, rt, &fl6.daddr, &fl6.saddr, iif,
3661 RTM_NEWROUTE, NETLINK_CB(in_skb).portid,
3662 nlh->nlmsg_seq, 0);
3663 if (err < 0) {
3664 kfree_skb(skb);
3665 goto errout;
3666 }
3667
3668 err = rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
3669 errout:
3670 return err;
3671 }
3672
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 12059 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-20 15:23 ` Hangbin Liu
@ 2017-07-21 15:53 ` David Ahern
2017-07-21 18:42 ` Cong Wang
1 sibling, 0 replies; 46+ messages in thread
From: David Ahern @ 2017-07-21 15:53 UTC (permalink / raw)
To: Hangbin Liu, network dev; +Cc: Roopa Prabhu, WANG Cong
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 = ip6_route_lookup(net, &fl6, 0);
>
> rt = container_of(dst, struct rt6_info, dst);
> - if (rt->dst.error) {
> - err = rt->dst.error;
> - ip6_rt_put(rt);
> - goto errout;
> - }
The above 5 lines introduced the change in behavior, so removing them
should be sufficient.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCHv2 net] ipv6: should not return rt->dst.error if it is prohibit or blk hole entry.
2017-07-21 3:47 ` [PATCHv2 net] ipv6: should not return rt->dst.error if it is prohibit or blk hole entry Hangbin Liu
2017-07-21 15:29 ` kbuild test robot
@ 2017-07-21 16:34 ` kbuild test robot
1 sibling, 0 replies; 46+ messages in thread
From: kbuild test robot @ 2017-07-21 16:34 UTC (permalink / raw)
To: Hangbin Liu; +Cc: kbuild-all, netdev, Roopa Prabhu, WANG Cong, Hangbin Liu
[-- Attachment #1: Type: text/plain, Size: 12502 bytes --]
Hi Hangbin,
[auto build test WARNING on net/master]
url: https://github.com/0day-ci/linux/commits/Hangbin-Liu/ipv6-should-not-return-rt-dst-error-if-it-is-prohibit-or-blk-hole-entry/20170721-204554
config: x86_64-randconfig-x003-07211556 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/uapi/linux/capability.h:16,
from include/linux/capability.h:15,
from net/ipv6/route.c:29:
net/ipv6/route.c: In function 'inet6_rtm_getroute':
net/ipv6/route.c:3640:38: error: 'struct netns_ipv6' has no member named 'ip6_prohibit_entry'; did you mean 'ip6_null_entry'?
if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
^
include/linux/compiler.h:156:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> net/ipv6/route.c:3640:2: note: in expansion of macro 'if'
if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
^~
net/ipv6/route.c:3641:21: error: 'struct netns_ipv6' has no member named 'ip6_blk_hole_entry'; did you mean 'ip6_null_entry'?
rt != net->ipv6.ip6_blk_hole_entry) {
^
include/linux/compiler.h:156:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> net/ipv6/route.c:3640:2: note: in expansion of macro 'if'
if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
^~
net/ipv6/route.c:3640:38: error: 'struct netns_ipv6' has no member named 'ip6_prohibit_entry'; did you mean 'ip6_null_entry'?
if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
^
include/linux/compiler.h:156:42: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> net/ipv6/route.c:3640:2: note: in expansion of macro 'if'
if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
^~
net/ipv6/route.c:3641:21: error: 'struct netns_ipv6' has no member named 'ip6_blk_hole_entry'; did you mean 'ip6_null_entry'?
rt != net->ipv6.ip6_blk_hole_entry) {
^
include/linux/compiler.h:156:42: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> net/ipv6/route.c:3640:2: note: in expansion of macro 'if'
if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
^~
net/ipv6/route.c:3640:38: error: 'struct netns_ipv6' has no member named 'ip6_prohibit_entry'; did you mean 'ip6_null_entry'?
if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
^
include/linux/compiler.h:167:16: note: in definition of macro '__trace_if'
______r = !!(cond); \
^~~~
>> net/ipv6/route.c:3640:2: note: in expansion of macro 'if'
if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
^~
net/ipv6/route.c:3641:21: error: 'struct netns_ipv6' has no member named 'ip6_blk_hole_entry'; did you mean 'ip6_null_entry'?
rt != net->ipv6.ip6_blk_hole_entry) {
^
include/linux/compiler.h:167:16: note: in definition of macro '__trace_if'
______r = !!(cond); \
^~~~
>> net/ipv6/route.c:3640:2: note: in expansion of macro 'if'
if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
^~
net/ipv6/route.c: At top level:
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strcpy' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:390:2: note: in expansion of macro 'if'
if (p_size == (size_t)-1 && q_size == (size_t)-1)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'kmemdup' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:380:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'kmemdup' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:378:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr_inv' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:369:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr_inv' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:367:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:358:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:356:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:348:2: note: in expansion of macro 'if'
if (p_size < size || q_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:345:3: note: in expansion of macro 'if'
if (q_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:343:3: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:342:2: note: in expansion of macro 'if'
vim +/if +3640 net/ipv6/route.c
3558
3559 static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
3560 struct netlink_ext_ack *extack)
3561 {
3562 struct net *net = sock_net(in_skb->sk);
3563 struct nlattr *tb[RTA_MAX+1];
3564 int err, iif = 0, oif = 0;
3565 struct dst_entry *dst;
3566 struct rt6_info *rt;
3567 struct sk_buff *skb;
3568 struct rtmsg *rtm;
3569 struct flowi6 fl6;
3570 bool fibmatch;
3571
3572 err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_ipv6_policy,
3573 extack);
3574 if (err < 0)
3575 goto errout;
3576
3577 err = -EINVAL;
3578 memset(&fl6, 0, sizeof(fl6));
3579 rtm = nlmsg_data(nlh);
3580 fl6.flowlabel = ip6_make_flowinfo(rtm->rtm_tos, 0);
3581 fibmatch = !!(rtm->rtm_flags & RTM_F_FIB_MATCH);
3582
3583 if (tb[RTA_SRC]) {
3584 if (nla_len(tb[RTA_SRC]) < sizeof(struct in6_addr))
3585 goto errout;
3586
3587 fl6.saddr = *(struct in6_addr *)nla_data(tb[RTA_SRC]);
3588 }
3589
3590 if (tb[RTA_DST]) {
3591 if (nla_len(tb[RTA_DST]) < sizeof(struct in6_addr))
3592 goto errout;
3593
3594 fl6.daddr = *(struct in6_addr *)nla_data(tb[RTA_DST]);
3595 }
3596
3597 if (tb[RTA_IIF])
3598 iif = nla_get_u32(tb[RTA_IIF]);
3599
3600 if (tb[RTA_OIF])
3601 oif = nla_get_u32(tb[RTA_OIF]);
3602
3603 if (tb[RTA_MARK])
3604 fl6.flowi6_mark = nla_get_u32(tb[RTA_MARK]);
3605
3606 if (tb[RTA_UID])
3607 fl6.flowi6_uid = make_kuid(current_user_ns(),
3608 nla_get_u32(tb[RTA_UID]));
3609 else
3610 fl6.flowi6_uid = iif ? INVALID_UID : current_uid();
3611
3612 if (iif) {
3613 struct net_device *dev;
3614 int flags = 0;
3615
3616 dev = __dev_get_by_index(net, iif);
3617 if (!dev) {
3618 err = -ENODEV;
3619 goto errout;
3620 }
3621
3622 fl6.flowi6_iif = iif;
3623
3624 if (!ipv6_addr_any(&fl6.saddr))
3625 flags |= RT6_LOOKUP_F_HAS_SADDR;
3626
3627 if (!fibmatch)
3628 dst = ip6_route_input_lookup(net, dev, &fl6, flags);
3629 } else {
3630 fl6.flowi6_oif = oif;
3631
3632 if (!fibmatch)
3633 dst = ip6_route_output(net, NULL, &fl6);
3634 }
3635
3636 if (fibmatch)
3637 dst = ip6_route_lookup(net, &fl6, 0);
3638
3639 rt = container_of(dst, struct rt6_info, dst);
> 3640 if (rt->dst.error && rt != net->ipv6.ip6_prohibit_entry &&
3641 rt != net->ipv6.ip6_blk_hole_entry) {
3642 err = rt->dst.error;
3643 ip6_rt_put(rt);
3644 goto errout;
3645 }
3646
3647 skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
3648 if (!skb) {
3649 ip6_rt_put(rt);
3650 err = -ENOBUFS;
3651 goto errout;
3652 }
3653
3654 skb_dst_set(skb, &rt->dst);
3655 if (fibmatch)
3656 err = rt6_fill_node(net, skb, rt, NULL, NULL, iif,
3657 RTM_NEWROUTE, NETLINK_CB(in_skb).portid,
3658 nlh->nlmsg_seq, 0);
3659 else
3660 err = rt6_fill_node(net, skb, rt, &fl6.daddr, &fl6.saddr, iif,
3661 RTM_NEWROUTE, NETLINK_CB(in_skb).portid,
3662 nlh->nlmsg_seq, 0);
3663 if (err < 0) {
3664 kfree_skb(skb);
3665 goto errout;
3666 }
3667
3668 err = rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
3669 errout:
3670 return err;
3671 }
3672
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31411 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-20 15:23 ` Hangbin Liu
2017-07-21 15:53 ` David Ahern
@ 2017-07-21 18:42 ` Cong Wang
2017-07-21 21:53 ` Roopa Prabhu
2017-07-24 3:09 ` Hangbin Liu
1 sibling, 2 replies; 46+ messages in thread
From: Cong Wang @ 2017-07-21 18:42 UTC (permalink / raw)
To: Hangbin Liu; +Cc: network dev, Roopa Prabhu
On Thu, Jul 20, 2017 at 8:23 AM, Hangbin Liu <liuhangbin@gmail.com> wrote:
> 2017-07-20 23:06 GMT+08:00 Hangbin Liu <liuhangbin@gmail.com>:
>>> +++ 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);
>>> - if (rt->dst.error) {
>>> - err = rt->dst.error;
>>> - ip6_rt_put(rt);
>>> - goto errout;
>>> - }
>>
>> hmm... or instead of remove this check, should we check all the entry? Like
>> if ((rt->dst.error && rt != net->ipv6.ip6_null_entry && rt !=
> ^^ mistake here
>> net->ipv6.ip6_blk_hole_entry) ||
>> rt == net->ipv6.ip6_null_entry )
>
> Sorry, there should be no need to check ip6_null_entry since the
> error is already
> -ENETUNREACH. So how about
Hmm? All of these 3 entries have error set, right??
So we should only check dst.error...
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-21 18:42 ` Cong Wang
@ 2017-07-21 21:53 ` Roopa Prabhu
2017-07-23 4:54 ` Roopa Prabhu
2017-07-24 3:09 ` Hangbin Liu
1 sibling, 1 reply; 46+ messages in thread
From: Roopa Prabhu @ 2017-07-21 21:53 UTC (permalink / raw)
To: Cong Wang; +Cc: Hangbin Liu, network dev
On Fri, Jul 21, 2017 at 11:42 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Thu, Jul 20, 2017 at 8:23 AM, Hangbin Liu <liuhangbin@gmail.com> wrote:
>> 2017-07-20 23:06 GMT+08:00 Hangbin Liu <liuhangbin@gmail.com>:
>>>> +++ 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);
>>>> - if (rt->dst.error) {
>>>> - err = rt->dst.error;
>>>> - ip6_rt_put(rt);
>>>> - goto errout;
>>>> - }
>>>
>>> hmm... or instead of remove this check, should we check all the entry? Like
>>> if ((rt->dst.error && rt != net->ipv6.ip6_null_entry && rt !=
>> ^^ mistake here
>>> net->ipv6.ip6_blk_hole_entry) ||
>>> rt == net->ipv6.ip6_null_entry )
>>
>> Sorry, there should be no need to check ip6_null_entry since the
>> error is already
>> -ENETUNREACH. So how about
>
> Hmm? All of these 3 entries have error set, right??
> So we should only check dst.error...
removing the check seems ok to me. We can also make the check
conditional to fibmatch code only
to eliminate any change in behavior introduced by fibmatch.
ie if (fibmatch && rt->dst.error).
Hangbin, can you also pls check that fibmatch works ok for such routes
with your patch applied ?.
ip netns exec client ip -6 route get fibmatch 2003::1 (with latest iproute2)
thank you.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-21 21:53 ` Roopa Prabhu
@ 2017-07-23 4:54 ` Roopa Prabhu
0 siblings, 0 replies; 46+ messages in thread
From: Roopa Prabhu @ 2017-07-23 4:54 UTC (permalink / raw)
To: Cong Wang; +Cc: Hangbin Liu, network dev
On Fri, Jul 21, 2017 at 2:53 PM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> On Fri, Jul 21, 2017 at 11:42 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Thu, Jul 20, 2017 at 8:23 AM, Hangbin Liu <liuhangbin@gmail.com> wrote:
>>> 2017-07-20 23:06 GMT+08:00 Hangbin Liu <liuhangbin@gmail.com>:
>>>>> +++ 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);
>>>>> - if (rt->dst.error) {
>>>>> - err = rt->dst.error;
>>>>> - ip6_rt_put(rt);
>>>>> - goto errout;
>>>>> - }
>>>>
>>>> hmm... or instead of remove this check, should we check all the entry? Like
>>>> if ((rt->dst.error && rt != net->ipv6.ip6_null_entry && rt !=
>>> ^^ mistake here
>>>> net->ipv6.ip6_blk_hole_entry) ||
>>>> rt == net->ipv6.ip6_null_entry )
>>>
>>> Sorry, there should be no need to check ip6_null_entry since the
>>> error is already
>>> -ENETUNREACH. So how about
>>
>> Hmm? All of these 3 entries have error set, right??
>> So we should only check dst.error...
>
> removing the check seems ok to me. We can also make the check
> conditional to fibmatch code only
> to eliminate any change in behavior introduced by fibmatch.
>
> ie if (fibmatch && rt->dst.error).
>
> Hangbin, can you also pls check that fibmatch works ok for such routes
> with your patch applied ?.
>
> ip netns exec client ip -6 route get fibmatch 2003::1 (with latest iproute2)
>
> thank you.
I tried this with your patch and this works for the fibmatch case as well.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-20 14:51 [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry Hangbin Liu
2017-07-20 15:06 ` Hangbin Liu
2017-07-21 3:47 ` [PATCHv2 net] ipv6: should not return rt->dst.error if it is prohibit or blk hole entry Hangbin Liu
@ 2017-07-23 4:55 ` Roopa Prabhu
2017-07-24 2:28 ` Hangbin Liu
2017-07-26 9:20 ` [PATCHv3 net] ipv6: no need to return rt->dst.error if it is prohibit entry Hangbin Liu
2017-07-27 16:25 ` [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info Hangbin Liu
4 siblings, 1 reply; 46+ messages in thread
From: Roopa Prabhu @ 2017-07-23 4:55 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, WANG Cong
On Thu, Jul 20, 2017 at 7:51 AM, Hangbin Liu <liuhangbin@gmail.com> 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 dev lo table unspec proto kernel src 2001::1 metric
> 4294967295 error -13
>
> After:
> + ip netns exec server ip -6 route get 2002::1
> RTNETLINK answers: Network is unreachable
>
> Since we will check the ip6_null_entry later. There is not sense
> to check the dst.error after get rt.
>
> Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
thanks.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-23 4:55 ` [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry Roopa Prabhu
@ 2017-07-24 2:28 ` Hangbin Liu
0 siblings, 0 replies; 46+ messages in thread
From: Hangbin Liu @ 2017-07-24 2:28 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: netdev, WANG Cong
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 <liuhangbin@gmail.com> 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 dev lo table unspec proto kernel src 2001::1 metric
> > 4294967295 error -13
> >
> > After:
> > + ip netns exec server ip -6 route get 2002::1
> > RTNETLINK answers: Network is unreachable
Sorry, I have a copy/paste error here, the return error should be
+ ip netns exec server ip -6 route get 2002::1
RTNETLINK answers: Permission denied
I fixed it in v2 patch, but forgot to add
#ifdef CONFIG_IPV6_MULTIPLE_TABLES before net->ipv6.ip6_prohibit_entry.
Since you acked this patch. I will post a v3 patch with fixed comment.
> >
> > Since we will check the ip6_null_entry later. There is not sense
> > to check the dst.error after get rt.
> >
> > Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > ---
>
> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Thanks
Hangbin
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-21 18:42 ` Cong Wang
2017-07-21 21:53 ` Roopa Prabhu
@ 2017-07-24 3:09 ` Hangbin Liu
2017-07-24 19:57 ` Cong Wang
1 sibling, 1 reply; 46+ messages in thread
From: Hangbin Liu @ 2017-07-24 3:09 UTC (permalink / raw)
To: Cong Wang; +Cc: network dev, Roopa Prabhu
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 <liuhangbin@gmail.com> wrote:
> > 2017-07-20 23:06 GMT+08:00 Hangbin Liu <liuhangbin@gmail.com>:
> >>> +++ 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);
> >>> - if (rt->dst.error) {
> >>> - err = rt->dst.error;
> >>> - ip6_rt_put(rt);
> >>> - goto errout;
> >>> - }
> >>
> >> hmm... or instead of remove this check, should we check all the entry? Like
> >> if ((rt->dst.error && rt != net->ipv6.ip6_null_entry && rt !=
> > ^^ mistake here
> >> net->ipv6.ip6_blk_hole_entry) ||
> >> rt == net->ipv6.ip6_null_entry )
> >
> > Sorry, there should be no need to check ip6_null_entry since the
> > error is already
> > -ENETUNREACH. So how about
>
> Hmm? All of these 3 entries have error set, right??
> So we should only check dst.error...
A question, since you have fixed the ip6_null_entry->rt6i_idev issue via
ipv6: initialize route null entry in addrconf_init()
ipv6: reorder ip6_route_dev_notifier after ipv6_dev_notf
ipv6: only call ip6_route_dev_notify() once for NETDEV_UNREGISTER
ipv6: avoid unregistering inet6_dev for loopback
Do we still need this net->ipv6.ip6_null_entry check? How about remove all
the checks?
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4d30c96..b5e3fe0 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3637,17 +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);
- if (rt->dst.error) {
- err = rt->dst.error;
- ip6_rt_put(rt);
- goto errout;
- }
-
- if (rt == net->ipv6.ip6_null_entry) {
- err = rt->dst.error;
- ip6_rt_put(rt);
- goto errout;
- }
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!skb) {
Before this patch:
+ ip netns exec client ip -6 route get 2003::1
RTNETLINK answers: Network is unreachable
After this patch:
+ ip netns exec client ip -6 route get 2003::1
unreachable 2003::1 dev lo table unspec proto kernel src 2001::1 metric 4294967295 error -101
Thanks
Hangbin
^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-24 3:09 ` Hangbin Liu
@ 2017-07-24 19:57 ` Cong Wang
2017-07-25 0:08 ` Hangbin Liu
0 siblings, 1 reply; 46+ messages in thread
From: Cong Wang @ 2017-07-24 19:57 UTC (permalink / raw)
To: Hangbin Liu; +Cc: network dev, Roopa Prabhu
On Sun, Jul 23, 2017 at 8:09 PM, Hangbin Liu <liuhangbin@gmail.com> 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.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-24 19:57 ` Cong Wang
@ 2017-07-25 0:08 ` Hangbin Liu
2017-07-25 3:28 ` David Ahern
2017-07-25 17:49 ` Cong Wang
0 siblings, 2 replies; 46+ messages in thread
From: Hangbin Liu @ 2017-07-25 0:08 UTC (permalink / raw)
To: Cong Wang; +Cc: network dev, Roopa Prabhu
On Mon, Jul 24, 2017 at 12:57:43PM -0700, Cong Wang wrote:
> On Sun, Jul 23, 2017 at 8:09 PM, Hangbin Liu <liuhangbin@gmail.com> 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.
Yes, I saw it. That why I send v2 patch to check both rt->dst.error and
ip6_null_entry.
The question is the other two caller are rpfilter_lookup_reverse6() and
nft_fib6_eval(). From the code it looks these two caller only care about
device match.
if (rt->rt6i_idev->dev == dev || (flags & XT_RPFILTER_LOOSE))
ret = true;
And the device would be lo if it is ip6_null_entry. So they just discard it.
I'm not familiar with netfilter, Please correct me if I make any mistake.
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.
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().
What do you think?
Thanks
Hangbin
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-25 0:08 ` Hangbin Liu
@ 2017-07-25 3:28 ` David Ahern
2017-07-25 7:32 ` Hangbin Liu
2017-07-25 17:49 ` Cong Wang
1 sibling, 1 reply; 46+ messages in thread
From: David Ahern @ 2017-07-25 3:28 UTC (permalink / raw)
To: Hangbin Liu, Cong Wang; +Cc: network dev, Roopa Prabhu
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.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-25 3:28 ` David Ahern
@ 2017-07-25 7:32 ` Hangbin Liu
2017-07-26 17:18 ` David Ahern
0 siblings, 1 reply; 46+ messages in thread
From: Hangbin Liu @ 2017-07-25 7:32 UTC (permalink / raw)
To: David Ahern; +Cc: Cong Wang, network dev, Roopa Prabhu
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 blame net/ipv6/route.c
>
> find the commits and review.
Hi David,
Sorry, would you like to give me more hints?
I saw your commit 1f17e2f2c8a8 ("net: ipv6: ignore null_entry on route
dumps"). But I think this issue has been fixed by
2f460933f58e ("ipv6: initialize route null entry in addrconf_init()") and
242d3a49a2a1 ("ipv6: reorder ip6_route_dev_notifier after ipv6_dev_notf")
I use a reproducer which add unreachable route in netns with lo down. And I
could not trigger Panic in the fixed kernel. That's why I think we could
remove ip6_null_entry check in rt6_dump_route().
Please correct me if I make any mistake.
Thanks
Hangbin
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-25 0:08 ` Hangbin Liu
2017-07-25 3:28 ` David Ahern
@ 2017-07-25 17:49 ` Cong Wang
2017-07-26 9:18 ` Hangbin Liu
1 sibling, 1 reply; 46+ messages in thread
From: Cong Wang @ 2017-07-25 17:49 UTC (permalink / raw)
To: Hangbin Liu; +Cc: network dev, Roopa Prabhu
On Mon, Jul 24, 2017 at 5:08 PM, Hangbin Liu <liuhangbin@gmail.com> 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, and rt->dst.error check is still needed because we
could return error on other normal entries too, IOW, your v2 is correct
if dumping prohibit/blackhole is expected.
I thought we don't dump them. I am not sure about the behavior either.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-25 17:49 ` Cong Wang
@ 2017-07-26 9:18 ` Hangbin Liu
0 siblings, 0 replies; 46+ messages in thread
From: Hangbin Liu @ 2017-07-26 9:18 UTC (permalink / raw)
To: Cong Wang; +Cc: network dev, Roopa Prabhu, David Ahern
On Tue, Jul 25, 2017 at 10:49:05AM -0700, Cong Wang wrote:
> On Mon, Jul 24, 2017 at 5:08 PM, Hangbin Liu <liuhangbin@gmail.com> 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, and rt->dst.error check is still needed because we
I could not reproduce the NULL rt6i_idev issue after you route init fix, So
I think it's also safe to dump null_entry now. There is a v3 patch. After
the patch, we could dump unreachable route info like:
+ ip netns exec client ip -6 route get 2003::1
unreachable 2003::1 dev lo table unspec proto kernel metric 4294967295 error -101
> could return error on other normal entries too, IOW, your v2 is correct
> if dumping prohibit/blackhole is expected.
Would you hlep review the v3 patch? I prepare not touch the check in
function rt6_dump_route() now.
Thanks
Hangbin
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCHv3 net] ipv6: no need to return rt->dst.error if it is prohibit entry
2017-07-20 14:51 [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry Hangbin Liu
` (2 preceding siblings ...)
2017-07-23 4:55 ` [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry Roopa Prabhu
@ 2017-07-26 9:20 ` Hangbin Liu
2017-07-26 17:09 ` David Ahern
2017-07-27 16:25 ` [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info Hangbin Liu
4 siblings, 1 reply; 46+ messages in thread
From: Hangbin Liu @ 2017-07-26 9:20 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Roopa Prabhu, David Ahern, 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 error -13
After:
+ ip netns exec server ip -6 route get 2002::1
RTNETLINK answers: Permission denied
Fix this by add prohibit and blk hole check.
At the same time, after commit
2f460933f58e ("ipv6: initialize route null entry in addrconf_init()") and
242d3a49a2a1 ("ipv6: reorder ip6_route_dev_notifier after ipv6_dev_notf")
We will init rt6i_idev correctly. So we could dump ip6_null_entry
(unreachable route entry) safely now.
Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/ipv6/route.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4d30c96..b05da74 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3637,13 +3637,12 @@ 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);
- if (rt->dst.error) {
- err = rt->dst.error;
- ip6_rt_put(rt);
- goto errout;
- }
-
- if (rt == net->ipv6.ip6_null_entry) {
+ if (rt->dst.error &&
+#ifdef CONFIG_IPV6_MULTIPLE_TABLES
+ rt != net->ipv6.ip6_prohibit_entry &&
+ rt != net->ipv6.ip6_blk_hole_entry &&
+#endif
+ rt != net->ipv6.ip6_null_entry) {
err = rt->dst.error;
ip6_rt_put(rt);
goto errout;
--
2.5.5
^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCHv3 net] ipv6: no need to return rt->dst.error if it is prohibit entry
2017-07-26 9:20 ` [PATCHv3 net] ipv6: no need to return rt->dst.error if it is prohibit entry Hangbin Liu
@ 2017-07-26 17:09 ` David Ahern
2017-07-26 18:48 ` David Ahern
2017-07-27 13:48 ` Hangbin Liu
0 siblings, 2 replies; 46+ messages in thread
From: David Ahern @ 2017-07-26 17:09 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: Cong Wang, Roopa Prabhu
On 7/26/17 3:20 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:
Do you mean "Before commit 18c3a61c4264?"
> + ip netns exec client ip -6 route get 2003::1
> prohibit 2003::1 dev lo table unspec proto kernel src 2001::1 metric
> 4294967295 error -13
>
> After:
And "After commit 18c3a61c4264?"
> + ip netns exec server ip -6 route get 2002::1
> RTNETLINK answers: Permission denied
>
> Fix this by add prohibit and blk hole check.
>
> At the same time, after commit
> 2f460933f58e ("ipv6: initialize route null entry in addrconf_init()") and
> 242d3a49a2a1 ("ipv6: reorder ip6_route_dev_notifier after ipv6_dev_notf")
> We will init rt6i_idev correctly. So we could dump ip6_null_entry
> (unreachable route entry) safely now.
>
> Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> net/ipv6/route.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 4d30c96..b05da74 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -3637,13 +3637,12 @@ 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);
> - if (rt->dst.error) {
> - err = rt->dst.error;
> - ip6_rt_put(rt);
> - goto errout;
> - }
> -
> - if (rt == net->ipv6.ip6_null_entry) {
> + if (rt->dst.error &&
> +#ifdef CONFIG_IPV6_MULTIPLE_TABLES
> + rt != net->ipv6.ip6_prohibit_entry &&
> + rt != net->ipv6.ip6_blk_hole_entry &&
> +#endif
> + rt != net->ipv6.ip6_null_entry) {
> err = rt->dst.error;
> ip6_rt_put(rt);
> goto errout;
>
This is what I see with your patch:
# ip -6 ro ls vrf red
2001:db8:1::/120 dev eth1 proto kernel metric 256 pref medium
prohibit 5000::/120 dev lo metric 1024 error -13 pref medium
fe80::/64 dev eth1 proto kernel metric 256 pref medium
ff00::/8 dev eth1 metric 256 pref medium
unreachable default dev lo metric 8192 error -113 pref medium
ie., I added a prohibit route for 5000:/120
and then running:
# ip -6 ro get vrf red 5000::1
RTNETLINK answers: Permission denied
Which is the behavior without your patch.
Now if I delete just the first bit:
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4d30c96a819d..8fc52de40175 100644
--- a/net/ipv6/route.c
+++ 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);
- if (rt->dst.error) {
- err = rt->dst.error;
- ip6_rt_put(rt);
- goto errout;
- }
-
if (rt == net->ipv6.ip6_null_entry) {
err = rt->dst.error;
ip6_rt_put(rt);
Then I get:
# ip -6 ro get vrf red 5000::1
prohibit 5000::1 from :: dev lo table red src 2001:db8::2 metric 1024
error -13 pref medium
which seems to be your objective.
I don't understand why you are focused on the built-in null and prohibit
route entries. When I add a default unreachable or prohibit route those
are different rt6_info entries. Take a look at ip6_route_info_create.
^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-25 7:32 ` Hangbin Liu
@ 2017-07-26 17:18 ` David Ahern
2017-07-26 18:27 ` Roopa Prabhu
0 siblings, 1 reply; 46+ messages in thread
From: David Ahern @ 2017-07-26 17:18 UTC (permalink / raw)
To: Hangbin Liu; +Cc: Cong Wang, network dev, Roopa Prabhu
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_entry
>>> check in rt6_dump_route().
>>
>> git blame net/ipv6/route.c
>>
>> find the commits and review.
>
> Hi David,
>
> Sorry, would you like to give me more hints?
>
> I saw your commit 1f17e2f2c8a8 ("net: ipv6: ignore null_entry on route
> dumps"). But I think this issue has been fixed by
>
> 2f460933f58e ("ipv6: initialize route null entry in addrconf_init()") and
> 242d3a49a2a1 ("ipv6: reorder ip6_route_dev_notifier after ipv6_dev_notf")
>
> I use a reproducer which add unreachable route in netns with lo down. And I
> could not trigger Panic in the fixed kernel. That's why I think we could
> remove ip6_null_entry check in rt6_dump_route().
Understood. Cong's patch to fix the intialization order (lo device
before route init) makes sure the idev is not null. That said, the
null_entry route is internal ipv6 logic and is not a route entry to be
returned to userspace.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-26 17:18 ` David Ahern
@ 2017-07-26 18:27 ` Roopa Prabhu
2017-07-26 18:49 ` David Ahern
0 siblings, 1 reply; 46+ messages in thread
From: Roopa Prabhu @ 2017-07-26 18:27 UTC (permalink / raw)
To: David Ahern; +Cc: Hangbin Liu, Cong Wang, network dev
On Wed, Jul 26, 2017 at 10:18 AM, David Ahern <dsahern@gmail.com> 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 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.
>>
>> Hi David,
>>
>> Sorry, would you like to give me more hints?
>>
>> I saw your commit 1f17e2f2c8a8 ("net: ipv6: ignore null_entry on route
>> dumps"). But I think this issue has been fixed by
>>
>> 2f460933f58e ("ipv6: initialize route null entry in addrconf_init()") and
>> 242d3a49a2a1 ("ipv6: reorder ip6_route_dev_notifier after ipv6_dev_notf")
>>
>> I use a reproducer which add unreachable route in netns with lo down. And I
>> could not trigger Panic in the fixed kernel. That's why I think we could
>> remove ip6_null_entry check in rt6_dump_route().
>
> Understood. Cong's patch to fix the intialization order (lo device
> before route init) makes sure the idev is not null. That said, the
> null_entry route is internal ipv6 logic and is not a route entry to be
> returned to userspace.
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 != net->ipv6.ip6_blk_hole_entry &&
+#endif
+ )) {
err = rt->dst.error;
ip6_rt_put(rt);
goto errout;
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCHv3 net] ipv6: no need to return rt->dst.error if it is prohibit entry
2017-07-26 17:09 ` David Ahern
@ 2017-07-26 18:48 ` David Ahern
2017-07-27 13:48 ` Hangbin Liu
1 sibling, 0 replies; 46+ messages in thread
From: David Ahern @ 2017-07-26 18:48 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: Cong Wang, Roopa Prabhu
On 7/26/17 11:09 AM, David Ahern wrote:
> I don't understand why you are focused on the built-in null and prohibit
> route entries.
I see. You are using fib rules for the prohibit entry; I am using an
explicit route entry.
If I run 'ip ro get fibmatch' for the latter I want to see that route
entry since it is a route in the FIB:
# ip -6 ro get fibmatch vrf red 5000::1
prohibit 5000::/120 dev lo table red metric 1024 error -13 pref medium
So there are multiple cases to verify.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-26 18:27 ` Roopa Prabhu
@ 2017-07-26 18:49 ` David Ahern
2017-07-26 18:55 ` Roopa Prabhu
2017-07-28 4:56 ` Cong Wang
0 siblings, 2 replies; 46+ messages in thread
From: David Ahern @ 2017-07-26 18:49 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: Hangbin Liu, Cong Wang, network dev
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 != net->ipv6.ip6_blk_hole_entry &&
> +#endif
> + )) {
> err = rt->dst.error;
> ip6_rt_put(rt);
> goto errout;
>
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.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-26 18:49 ` David Ahern
@ 2017-07-26 18:55 ` Roopa Prabhu
2017-07-26 19:00 ` David Ahern
2017-07-28 4:56 ` Cong Wang
1 sibling, 1 reply; 46+ messages in thread
From: Roopa Prabhu @ 2017-07-26 18:55 UTC (permalink / raw)
To: David Ahern; +Cc: Hangbin Liu, Cong Wang, network dev
On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> 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 != net->ipv6.ip6_prohibit_entry &&
>> + rt != net->ipv6.ip6_blk_hole_entry &&
>> +#endif
>> + )) {
>> err = rt->dst.error;
>> ip6_rt_put(rt);
>> goto errout;
>>
>
> 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
it fall through to the route fill code ?
ah...but i guess you are saying that they will have rt6_info's of
their own and will not match. got it. ack.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-26 18:55 ` Roopa Prabhu
@ 2017-07-26 19:00 ` David Ahern
2017-07-26 19:38 ` Roopa Prabhu
2017-07-27 16:08 ` Hangbin Liu
0 siblings, 2 replies; 46+ messages in thread
From: David Ahern @ 2017-07-26 19:00 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: Hangbin Liu, Cong Wang, network dev
On 7/26/17 12:55 PM, Roopa Prabhu wrote:
> On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> 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 != net->ipv6.ip6_prohibit_entry &&
>>> + rt != net->ipv6.ip6_blk_hole_entry &&
>>> +#endif
>>> + )) {
>>> err = rt->dst.error;
>>> ip6_rt_put(rt);
>>> goto errout;
>>>
>>
>> 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
> it fall through to the route fill code ?
>
> ah...but i guess you are saying that they will have rt6_info's of
> their own and will not match. got it. ack.
>
This:
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4d30c96a819d..24de81c804c2 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3637,11 +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);
- if (rt->dst.error) {
- err = rt->dst.error;
- ip6_rt_put(rt);
- goto errout;
- }
if (rt == net->ipv6.ip6_null_entry) {
err = rt->dst.error;
Puts back the original behavior. In that case, only rt == null_entry
drops to the error path which is correct. All other rt values will drop
to rt6_fill_node and return rt data.
^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-26 19:00 ` David Ahern
@ 2017-07-26 19:38 ` Roopa Prabhu
2017-07-27 16:08 ` Hangbin Liu
1 sibling, 0 replies; 46+ messages in thread
From: Roopa Prabhu @ 2017-07-26 19:38 UTC (permalink / raw)
To: David Ahern; +Cc: Hangbin Liu, Cong Wang, network dev
On Wed, Jul 26, 2017 at 12:00 PM, David Ahern <dsahern@gmail.com> wrote:
> On 7/26/17 12:55 PM, Roopa Prabhu wrote:
>> On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> 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 != net->ipv6.ip6_prohibit_entry &&
>>>> + rt != net->ipv6.ip6_blk_hole_entry &&
>>>> +#endif
>>>> + )) {
>>>> err = rt->dst.error;
>>>> ip6_rt_put(rt);
>>>> goto errout;
>>>>
>>>
>>> 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
>> it fall through to the route fill code ?
>>
>> ah...but i guess you are saying that they will have rt6_info's of
>> their own and will not match. got it. ack.
>>
>
> This:
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 4d30c96a819d..24de81c804c2 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -3637,11 +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);
> - if (rt->dst.error) {
> - err = rt->dst.error;
> - ip6_rt_put(rt);
> - goto errout;
> - }
>
> if (rt == net->ipv6.ip6_null_entry) {
> err = rt->dst.error;
>
> Puts back the original behavior. In that case, only rt == null_entry
> drops to the error path which is correct. All other rt values will drop
> to rt6_fill_node and return rt data.
yes, i thought so too and hence acked v1. But, following congs
comment, realized that it may mask some real errors for fibmatch ?
I just tested a case of unreachable route with just the above patch
you posted, and I do get the error correctly.
so, I guess you are saying all real errors for fibmatch will have "rt
== net->ipv6.ip6_null_entry" and we should be ok.
sounds good to me.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCHv3 net] ipv6: no need to return rt->dst.error if it is prohibit entry
2017-07-26 17:09 ` David Ahern
2017-07-26 18:48 ` David Ahern
@ 2017-07-27 13:48 ` Hangbin Liu
1 sibling, 0 replies; 46+ messages in thread
From: Hangbin Liu @ 2017-07-27 13:48 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Cong Wang, Roopa Prabhu
On Wed, Jul 26, 2017 at 11:09:39AM -0600, David Ahern wrote:
> On 7/26/17 3:20 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:
>
> Do you mean "Before commit 18c3a61c4264?"
>
> > + ip netns exec client ip -6 route get 2003::1
> > prohibit 2003::1 dev lo table unspec proto kernel src 2001::1 metric
> > 4294967295 error -13
> >
> > After:
>
> And "After commit 18c3a61c4264?"
Yes. Sorry I didn't make it clear.
>
> > + ip netns exec server ip -6 route get 2002::1
> > RTNETLINK answers: Permission denied
> >
> > Fix this by add prohibit and blk hole check.
> >
> > At the same time, after commit
> > 2f460933f58e ("ipv6: initialize route null entry in addrconf_init()") and
> > 242d3a49a2a1 ("ipv6: reorder ip6_route_dev_notifier after ipv6_dev_notf")
> > We will init rt6i_idev correctly. So we could dump ip6_null_entry
> > (unreachable route entry) safely now.
> >
> > Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>
> This is what I see with your patch:
>
> # ip -6 ro ls vrf red
> 2001:db8:1::/120 dev eth1 proto kernel metric 256 pref medium
> prohibit 5000::/120 dev lo metric 1024 error -13 pref medium
> fe80::/64 dev eth1 proto kernel metric 256 pref medium
> ff00::/8 dev eth1 metric 256 pref medium
> unreachable default dev lo metric 8192 error -113 pref medium
>
> ie., I added a prohibit route for 5000:/120
>
> and then running:
> # ip -6 ro get vrf red 5000::1
> RTNETLINK answers: Permission denied
>
> Which is the behavior without your patch.
>
> Now if I delete just the first bit:
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 4d30c96a819d..8fc52de40175 100644
> --- a/net/ipv6/route.c
> +++ 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);
> - if (rt->dst.error) {
> - err = rt->dst.error;
> - ip6_rt_put(rt);
> - goto errout;
> - }
> -
> if (rt == net->ipv6.ip6_null_entry) {
> err = rt->dst.error;
> ip6_rt_put(rt);
>
> Then I get:
>
> # ip -6 ro get vrf red 5000::1
> prohibit 5000::1 from :: dev lo table red src 2001:db8::2 metric 1024
> error -13 pref medium
>
> which seems to be your objective.
Yes
>
>> I don't understand why you are focused on the built-in null and prohibit
>> route entries.
>
>I see. You are using fib rules for the prohibit entry; I am using an
>explicit route entry.
Yes, This is my fault. I should put my steps in the commit.
# ip -6 rule add to 2003::1/64 table 100 prohibit
# ip -6 route get 2003::1
RTNETLINK answers: Permission denied
>
>If I run 'ip ro get fibmatch' for the latter I want to see that route
>entry since it is a route in the FIB:
>
># ip -6 ro get fibmatch vrf red 5000::1
>prohibit 5000::/120 dev lo table red metric 1024 error -13 pref medium
>
>So there are multiple cases to verify.
Yeah... there are more stuff I need to learn.
Thanks
Hangbin
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-26 19:00 ` David Ahern
2017-07-26 19:38 ` Roopa Prabhu
@ 2017-07-27 16:08 ` Hangbin Liu
1 sibling, 0 replies; 46+ messages in thread
From: Hangbin Liu @ 2017-07-27 16:08 UTC (permalink / raw)
To: David Ahern; +Cc: Roopa Prabhu, Cong Wang, network dev
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
> > it fall through to the route fill code ?
> >
> > ah...but i guess you are saying that they will have rt6_info's of
> > their own and will not match. got it. ack.
> >
>
> This:
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 4d30c96a819d..24de81c804c2 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -3637,11 +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);
> - if (rt->dst.error) {
> - err = rt->dst.error;
> - ip6_rt_put(rt);
> - goto errout;
> - }
>
> if (rt == net->ipv6.ip6_null_entry) {
> err = rt->dst.error;
>
> Puts back the original behavior. In that case, only rt == null_entry
> drops to the error path which is correct. All other rt values will drop
> to rt6_fill_node and return rt data.
Thanks for your explains. Now I know where I made the mistake. I mis-looked
FR_ACT_UNREACHABLE to RTN_UNREACHABLE and thought we return rt =
net->ipv6.ip6_null_entry in fib6_rule_action().
With you help I know we just set rt->dst.error to -EACCES for prohibit
entry in ip6_route_info_create. So remove the rt->dst.error check is enought.
Thanks
Hangbin
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info
2017-07-20 14:51 [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry Hangbin Liu
` (3 preceding siblings ...)
2017-07-26 9:20 ` [PATCHv3 net] ipv6: no need to return rt->dst.error if it is prohibit entry Hangbin Liu
@ 2017-07-27 16:25 ` Hangbin Liu
2017-07-27 18:03 ` David Ahern
` (2 more replies)
4 siblings, 3 replies; 46+ messages in thread
From: Hangbin Liu @ 2017-07-27 16:25 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Roopa Prabhu, David Ahern, 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 instead of dump route info.
Fix it by remove the rt->dst.error check.
Before fix:
\# ip -6 route add prohibit 2003::/64 dev eth1
\# ip -6 route get fibmatch 2003::1
RTNETLINK answers: Permission denied
\# ip -6 route add unreachable 2004::/64 dev eth1
\# ip -6 route get fibmatch 2004::1
RTNETLINK answers: No route to host
After fix:
\# ip -6 route add prohibit 2003::/64 dev eth1
\# ip -6 route get fibmatch 2003::1
prohibit 2003::/64 dev lo metric 1024 error -13 pref medium
\# ip -6 route add unreachable 2004::/64 dev eth1
\# ip -6 route get fibmatch 2004::1
unreachable 2004::/64 dev lo metric 1024 error -113 pref medium
Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/ipv6/route.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4d30c96..8fc52de 100644
--- a/net/ipv6/route.c
+++ 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);
- if (rt->dst.error) {
- err = rt->dst.error;
- ip6_rt_put(rt);
- goto errout;
- }
-
if (rt == net->ipv6.ip6_null_entry) {
err = rt->dst.error;
ip6_rt_put(rt);
--
2.5.5
^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info
2017-07-27 16:25 ` [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info Hangbin Liu
@ 2017-07-27 18:03 ` David Ahern
2017-07-28 17:23 ` David Ahern
2017-07-27 19:52 ` Roopa Prabhu
2017-07-31 23:22 ` David Miller
2 siblings, 1 reply; 46+ messages in thread
From: David Ahern @ 2017-07-27 18:03 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: Cong Wang, Roopa Prabhu
On 7/27/17 10:25 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 instead of dump route info.
>
> Fix it by remove the rt->dst.error check.
>
> Before fix:
> \# ip -6 route add prohibit 2003::/64 dev eth1
> \# ip -6 route get fibmatch 2003::1
> RTNETLINK answers: Permission denied
> \# ip -6 route add unreachable 2004::/64 dev eth1
> \# ip -6 route get fibmatch 2004::1
> RTNETLINK answers: No route to host
>
> After fix:
> \# ip -6 route add prohibit 2003::/64 dev eth1
> \# ip -6 route get fibmatch 2003::1
> prohibit 2003::/64 dev lo metric 1024 error -13 pref medium
> \# ip -6 route add unreachable 2004::/64 dev eth1
> \# ip -6 route get fibmatch 2004::1
> unreachable 2004::/64 dev lo metric 1024 error -113 pref medium
>
> Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> net/ipv6/route.c | 6 ------
> 1 file changed, 6 deletions(-)
>
Acked-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info
2017-07-27 16:25 ` [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info Hangbin Liu
2017-07-27 18:03 ` David Ahern
@ 2017-07-27 19:52 ` Roopa Prabhu
2017-07-31 23:22 ` David Miller
2 siblings, 0 replies; 46+ messages in thread
From: Roopa Prabhu @ 2017-07-27 19:52 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Cong Wang, David Ahern
On Thu, Jul 27, 2017 at 9:25 AM, Hangbin Liu <liuhangbin@gmail.com> 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 instead of dump route info.
>
> Fix it by remove the rt->dst.error check.
>
> Before fix:
> \# ip -6 route add prohibit 2003::/64 dev eth1
> \# ip -6 route get fibmatch 2003::1
> RTNETLINK answers: Permission denied
> \# ip -6 route add unreachable 2004::/64 dev eth1
> \# ip -6 route get fibmatch 2004::1
> RTNETLINK answers: No route to host
>
> After fix:
> \# ip -6 route add prohibit 2003::/64 dev eth1
> \# ip -6 route get fibmatch 2003::1
> prohibit 2003::/64 dev lo metric 1024 error -13 pref medium
> \# ip -6 route add unreachable 2004::/64 dev eth1
> \# ip -6 route get fibmatch 2004::1
> unreachable 2004::/64 dev lo metric 1024 error -113 pref medium
>
> Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-26 18:49 ` David Ahern
2017-07-26 18:55 ` Roopa Prabhu
@ 2017-07-28 4:56 ` Cong Wang
2017-07-28 11:04 ` Hangbin Liu
2017-07-28 15:10 ` David Ahern
1 sibling, 2 replies; 46+ messages in thread
From: Cong Wang @ 2017-07-28 4:56 UTC (permalink / raw)
To: David Ahern; +Cc: Roopa Prabhu, Hangbin Liu, network dev
On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> 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 != net->ipv6.ip6_prohibit_entry &&
>> + rt != net->ipv6.ip6_blk_hole_entry &&
>> +#endif
>> + )) {
>> err = rt->dst.error;
>> ip6_rt_put(rt);
>> goto errout;
>>
>
> 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.
But net->ipv6.ip6_prohibit_entry is not the prohibit route you can
add in user-space, it is only used by rule actions. So do you really
want to dump it?? My gut feeling is no, but I am definitely not sure.
When you add a prohibit route, a new rt is allocated dynamically,
net->ipv6.ip6_prohibit_entry is relatively static, internal and is the
only one per netns. (Same for net->ipv6.ip6_blk_hole_entry)
I think Hangbin's example doesn't have ip rules, so this case
is not shown up.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-28 4:56 ` Cong Wang
@ 2017-07-28 11:04 ` Hangbin Liu
2017-07-28 15:10 ` David Ahern
1 sibling, 0 replies; 46+ messages in thread
From: Hangbin Liu @ 2017-07-28 11:04 UTC (permalink / raw)
To: Cong Wang; +Cc: David Ahern, Roopa Prabhu, network dev
On Thu, Jul 27, 2017 at 09:56:08PM -0700, Cong Wang wrote:
> On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> 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 != net->ipv6.ip6_prohibit_entry &&
> >> + rt != net->ipv6.ip6_blk_hole_entry &&
> >> +#endif
> >> + )) {
> >> err = rt->dst.error;
> >> ip6_rt_put(rt);
> >> goto errout;
> >>
> >
> > 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.
>
> But net->ipv6.ip6_prohibit_entry is not the prohibit route you can
> add in user-space, it is only used by rule actions. So do you really
> want to dump it?? My gut feeling is no, but I am definitely not sure.
>
> When you add a prohibit route, a new rt is allocated dynamically,
> net->ipv6.ip6_prohibit_entry is relatively static, internal and is the
> only one per netns. (Same for net->ipv6.ip6_blk_hole_entry)
>
> I think Hangbin's example doesn't have ip rules, so this case
> is not shown up.
I mixed the rule entry and route entry these days. And with your help I can
separate them now.
When first time I find the rt->dst.error return directly issue, I was testing
ip rule actually.
e.g.
+ ip netns exec client ip -6 rule add to 2003::1/64 table 100 unreachable
+ ip netns exec server ip -6 rule add to 2001::1/64 table 100 prohibit
+ ip netns exec client ip -6 route get 2003::1
RTNETLINK answers: Network is unreachable
+ ip netns exec client ip -6 route get 2001::1
RTNETLINK answers: Permission denied
After check I thought we returned net->ipv6.ip6_null_entry /
net->ipv6.ip6_blk_hole_entry in function fib6_rule_action().
That's the reason I want to delete both rt->dst.error and
net->ipv6.ip6_null_entry check in patch v2 and v3.
Then with David's comments I realise we also need to take care about ip route
entrys.
my last mail's comment:
> Thanks for your explains. Now I know where I made the mistake. I mis-looked
> FR_ACT_UNREACHABLE to RTN_UNREACHABLE and thought we return rt =
> net->ipv6.ip6_null_entry in fib6_rule_action().
But then I fall in to the code logic and get lost... And thought
FR_ACT_UNREACHABLE and RTN_UNREACHABLE are not same. Today I re-check the
code and realise RTN_UNREACHABLE is defined in user space and FR_ACT_UNREACHABLE
is in kernel. Actually they are the same.
So after PATCH v4, we fixed the route side. And part of ip rule(prohibit and
blk hole). I will think over of this.
Thanks
Hangbin
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-28 4:56 ` Cong Wang
2017-07-28 11:04 ` Hangbin Liu
@ 2017-07-28 15:10 ` David Ahern
2017-07-28 17:13 ` Roopa Prabhu
1 sibling, 1 reply; 46+ messages in thread
From: David Ahern @ 2017-07-28 15:10 UTC (permalink / raw)
To: Cong Wang, Roopa Prabhu; +Cc: Hangbin Liu, network dev
On 7/27/17 10:56 PM, Cong Wang wrote:
> On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> 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 != net->ipv6.ip6_prohibit_entry &&
>>> + rt != net->ipv6.ip6_blk_hole_entry &&
>>> +#endif
>>> + )) {
>>> err = rt->dst.error;
>>> ip6_rt_put(rt);
>>> goto errout;
>>>
>>
>> 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.
>
> But net->ipv6.ip6_prohibit_entry is not the prohibit route you can
> add in user-space, it is only used by rule actions. So do you really
> want to dump it?? My gut feeling is no, but I am definitely not sure.
>
> When you add a prohibit route, a new rt is allocated dynamically,
> net->ipv6.ip6_prohibit_entry is relatively static, internal and is the
> only one per netns. (Same for net->ipv6.ip6_blk_hole_entry)
>
> I think Hangbin's example doesn't have ip rules, so this case
> is not shown up.
>
Understood. The v4 patch returns getroute to the original behavior. The
original behavior returned a route entry not just an error code. The
following is at 5dafc87f40d7 which the commit before roopa's patch set:
# ip -6 ru add to 6000::/120 prohibit
# ip -6 ro get 6000::1
prohibit 6000::1 from :: dev lo proto kernel src 2001:db8::3 metric
4294967295 error -13 pref medium
# ip -6 ro add vrf red prohibit 5000::1/120
# ip -6 ro get vrf red 5000::1
prohibit 5000::1 from :: dev lo table red src 2001:db8::3 metric 1024
error -13 pref medium
Generically, the only time you get just an error response is when the
lookup fails to find a match and returns the null_entry which has
dst.error = -ENETUNREACH.
Now to your point about the new fibmatch option I have gone back and
forth but in the end I think returning the route associated with the FIB
rule is better than just failing with an error code.
Roopa?
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-28 15:10 ` David Ahern
@ 2017-07-28 17:13 ` Roopa Prabhu
2017-07-28 17:39 ` David Ahern
0 siblings, 1 reply; 46+ messages in thread
From: Roopa Prabhu @ 2017-07-28 17:13 UTC (permalink / raw)
To: David Ahern; +Cc: Cong Wang, Hangbin Liu, network dev
On Fri, Jul 28, 2017 at 8:10 AM, David Ahern <dsahern@gmail.com> wrote:
> On 7/27/17 10:56 PM, Cong Wang wrote:
>> On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> 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 != net->ipv6.ip6_prohibit_entry &&
>>>> + rt != net->ipv6.ip6_blk_hole_entry &&
>>>> +#endif
>>>> + )) {
>>>> err = rt->dst.error;
>>>> ip6_rt_put(rt);
>>>> goto errout;
>>>>
>>>
>>> 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.
>>
>> But net->ipv6.ip6_prohibit_entry is not the prohibit route you can
>> add in user-space, it is only used by rule actions. So do you really
>> want to dump it?? My gut feeling is no, but I am definitely not sure.
>>
>> When you add a prohibit route, a new rt is allocated dynamically,
>> net->ipv6.ip6_prohibit_entry is relatively static, internal and is the
>> only one per netns. (Same for net->ipv6.ip6_blk_hole_entry)
>>
>> I think Hangbin's example doesn't have ip rules, so this case
>> is not shown up.
>>
>
> Understood. The v4 patch returns getroute to the original behavior. The
> original behavior returned a route entry not just an error code. The
> following is at 5dafc87f40d7 which the commit before roopa's patch set:
>
> # ip -6 ru add to 6000::/120 prohibit
> # ip -6 ro get 6000::1
> prohibit 6000::1 from :: dev lo proto kernel src 2001:db8::3 metric
> 4294967295 error -13 pref medium
>
> # ip -6 ro add vrf red prohibit 5000::1/120
> # ip -6 ro get vrf red 5000::1
> prohibit 5000::1 from :: dev lo table red src 2001:db8::3 metric 1024
> error -13 pref medium
>
> Generically, the only time you get just an error response is when the
> lookup fails to find a match and returns the null_entry which has
> dst.error = -ENETUNREACH.
>
>
> Now to your point about the new fibmatch option I have gone back and
> forth but in the end I think returning the route associated with the FIB
> rule is better than just failing with an error code.
>
> Roopa?
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,
it does not return an error directly on an existing prohibit route
entry in the fib.
This is probably doable by checking for appropriate
net->ipv6.ip6_prohibit_entry entries.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info
2017-07-27 18:03 ` David Ahern
@ 2017-07-28 17:23 ` David Ahern
0 siblings, 0 replies; 46+ messages in thread
From: David Ahern @ 2017-07-28 17:23 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: Cong Wang, Roopa Prabhu, David Miller
On 7/27/17 12:03 PM, David Ahern wrote:
> On 7/27/17 10:25 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 instead of dump route info.
>>
>> Fix it by remove the rt->dst.error check.
>>
>> Before fix:
>> \# ip -6 route add prohibit 2003::/64 dev eth1
>> \# ip -6 route get fibmatch 2003::1
>> RTNETLINK answers: Permission denied
>> \# ip -6 route add unreachable 2004::/64 dev eth1
>> \# ip -6 route get fibmatch 2004::1
>> RTNETLINK answers: No route to host
>>
>> After fix:
>> \# ip -6 route add prohibit 2003::/64 dev eth1
>> \# ip -6 route get fibmatch 2003::1
>> prohibit 2003::/64 dev lo metric 1024 error -13 pref medium
>> \# ip -6 route add unreachable 2004::/64 dev eth1
>> \# ip -6 route get fibmatch 2004::1
>> unreachable 2004::/64 dev lo metric 1024 error -113 pref medium
>>
>> Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
>> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>> ---
>> net/ipv6/route.c | 6 ------
>> 1 file changed, 6 deletions(-)
>>
>
> Acked-by: David Ahern <dsahern@gmail.com>
>
Dave: please hold off on applying this patch.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-28 17:13 ` Roopa Prabhu
@ 2017-07-28 17:39 ` David Ahern
2017-07-28 19:52 ` Roopa Prabhu
2017-07-31 18:37 ` Cong Wang
0 siblings, 2 replies; 46+ messages in thread
From: David Ahern @ 2017-07-28 17:39 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: Cong Wang, Hangbin Liu, network dev
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,
> it does not return an error directly on an existing prohibit route
> entry in the fib.
> This is probably doable by checking for appropriate
> net->ipv6.ip6_prohibit_entry entries.
>
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 following cases:
# ip -4 rule add to 172.16.60.0/24 prohibit
# ip -4 route add prohibit 172.16.50.0/24
# ip -6 rule add to 6000::/120 prohibit
# ip -6 route add prohibit 5000::/120
Behavior before Roopa's patch set:
Rule match:
# ip ro get 172.16.60.1
RTNETLINK answers: Permission denied
# ip -6 ro get 6000::1
prohibit 6000::1 from :: dev lo proto kernel src 2001:db8::3 metric
4294967295 error -13 pref medium
Route match:
# ip ro get 172.16.50.1
RTNETLINK answers: Permission denied
# ip -6 ro get 5000::1
prohibit 5000::1 from :: dev lo table red src 2001:db8::3 metric
1024 error -13 pref medium
Behavior after Roopa's patch set:
Rule match:
# ip ro get 172.16.60.1
RTNETLINK answers: Permission denied
# ip -6 ro get 6000::1
RTNETLINK answers: Permission denied
Route match:
# ip ro get 172.16.50.1
RTNETLINK answers: Permission denied
# ip -6 ro get 5000::1
RTNETLINK answers: Permission denied
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.
If that is not acceptable, then let's wrap the change in 'if (fibmatch)'
so that when fibmatch is requested we have consistency between IPv4 and
IPv6 when it is set.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-28 17:39 ` David Ahern
@ 2017-07-28 19:52 ` Roopa Prabhu
2017-07-29 14:41 ` David Ahern
2017-07-31 18:37 ` Cong Wang
1 sibling, 1 reply; 46+ messages in thread
From: Roopa Prabhu @ 2017-07-28 19:52 UTC (permalink / raw)
To: David Ahern; +Cc: Cong Wang, Hangbin Liu, network dev
On Fri, Jul 28, 2017 at 10:39 AM, David Ahern <dsahern@gmail.com> 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 it. But, i do want to make sure that for the fibmatch case,
>> it does not return an error directly on an existing prohibit route
>> entry in the fib.
>> This is probably doable by checking for appropriate
>> net->ipv6.ip6_prohibit_entry entries.
>>
>
> 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 following cases:
>
> # ip -4 rule add to 172.16.60.0/24 prohibit
> # ip -4 route add prohibit 172.16.50.0/24
> # ip -6 rule add to 6000::/120 prohibit
> # ip -6 route add prohibit 5000::/120
>
>
> Behavior before Roopa's patch set:
> Rule match:
> # ip ro get 172.16.60.1
> RTNETLINK answers: Permission denied
>
> # ip -6 ro get 6000::1
> prohibit 6000::1 from :: dev lo proto kernel src 2001:db8::3 metric
> 4294967295 error -13 pref medium
>
> Route match:
> # ip ro get 172.16.50.1
> RTNETLINK answers: Permission denied
>
> # ip -6 ro get 5000::1
> prohibit 5000::1 from :: dev lo table red src 2001:db8::3 metric
> 1024 error -13 pref medium
>
>
> Behavior after Roopa's patch set:
> Rule match:
> # ip ro get 172.16.60.1
> RTNETLINK answers: Permission denied
>
> # ip -6 ro get 6000::1
> RTNETLINK answers: Permission denied
>
> Route match:
> # ip ro get 172.16.50.1
> RTNETLINK answers: Permission denied
>
> # ip -6 ro get 5000::1
> RTNETLINK answers: Permission denied
>
>
> 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.
>
> If that is not acceptable, then let's wrap the change in 'if (fibmatch)'
> so that when fibmatch is requested we have consistency between IPv4 and
> IPv6 when it is set.
David, Thanks for listing all the cases and options.
for the route match fibmatch case, if a prohibit route entry exists
(added by user), I was hoping fibmatch can return that entry...
# ip -6 ro get fibmatch 5000::1
prohibit 5000::1 from :: dev lo
because the semantics of fibmatch is to return the matching route
entry if exists.
I am assuming that is possible with appropriate checks around the
dst.error check for fibmatch. what do you say ?
I need to verify if this can work for ipv4 the same way.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-28 19:52 ` Roopa Prabhu
@ 2017-07-29 14:41 ` David Ahern
0 siblings, 0 replies; 46+ messages in thread
From: David Ahern @ 2017-07-29 14:41 UTC (permalink / raw)
To: Roopa Prabhu, David Miller; +Cc: Cong Wang, Hangbin Liu, network dev
On 7/28/17 1:52 PM, Roopa Prabhu wrote:
> On Fri, Jul 28, 2017 at 10:39 AM, David Ahern <dsahern@gmail.com> 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 following cases:
>>
>> # ip -4 rule add to 172.16.60.0/24 prohibit
>> # ip -4 route add prohibit 172.16.50.0/24
>> # ip -6 rule add to 6000::/120 prohibit
>> # ip -6 route add prohibit 5000::/120
>>
>>
>> Behavior before Roopa's patch set:
>> Rule match:
>> # ip ro get 172.16.60.1
>> RTNETLINK answers: Permission denied
>>
>> # ip -6 ro get 6000::1
>> prohibit 6000::1 from :: dev lo proto kernel src 2001:db8::3 metric
>> 4294967295 error -13 pref medium
>>
>> Route match:
>> # ip ro get 172.16.50.1
>> RTNETLINK answers: Permission denied
>>
>> # ip -6 ro get 5000::1
>> prohibit 5000::1 from :: dev lo table red src 2001:db8::3 metric
>> 1024 error -13 pref medium
>>
>>
>> Behavior after Roopa's patch set:
>> Rule match:
>> # ip ro get 172.16.60.1
>> RTNETLINK answers: Permission denied
>>
>> # ip -6 ro get 6000::1
>> RTNETLINK answers: Permission denied
>>
>> Route match:
>> # ip ro get 172.16.50.1
>> RTNETLINK answers: Permission denied
>>
>> # ip -6 ro get 5000::1
>> RTNETLINK answers: Permission denied
>>
>>
>> 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.
>>
>> If that is not acceptable, then let's wrap the change in 'if (fibmatch)'
>> so that when fibmatch is requested we have consistency between IPv4 and
>> IPv6 when it is set.
>
>
> David, Thanks for listing all the cases and options.
>
> for the route match fibmatch case, if a prohibit route entry exists
> (added by user), I was hoping fibmatch can return that entry...
>
> # ip -6 ro get fibmatch 5000::1
> prohibit 5000::1 from :: dev lo
>
> because the semantics of fibmatch is to return the matching route
> entry if exists.
> I am assuming that is possible with appropriate checks around the
> dst.error check for fibmatch. what do you say ?
> I need to verify if this can work for ipv4 the same way.
>
Routes such as prohibit, blackhole and unreachable cause
fib_table_lookup to return an error (see the fib_props reference in
fib_table_lookup) and the IPv4 code does not generate an rtable for
them. IMO it does not make sense to complicate the general IPv4 lookup
code to create an rtable for these 'special' routes just for the
fibmatch getroute case. Furthermore, I think consistency between IPv4
and IPv6 is important from a programmability perspective which is what
we have now.
DaveM: as I outlined above, Roopa's fibmatch patches caused a change in
user behavior in IPv6 getroute for prohibit, blackhole and unreachable
route entries. Opinions on whether we should limit that new behavior to
just the fibmatch lookup in which case a patch is needed or take the new
behavior and consistency in which case nothing is needed?
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-28 17:39 ` David Ahern
2017-07-28 19:52 ` Roopa Prabhu
@ 2017-07-31 18:37 ` Cong Wang
2017-07-31 18:40 ` David Ahern
1 sibling, 1 reply; 46+ messages in thread
From: Cong Wang @ 2017-07-31 18:37 UTC (permalink / raw)
To: David Ahern; +Cc: Roopa Prabhu, Hangbin Liu, network dev
On Fri, Jul 28, 2017 at 10:39 AM, David Ahern <dsahern@gmail.com> 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 it. But, i do want to make sure that for the fibmatch case,
>> it does not return an error directly on an existing prohibit route
>> entry in the fib.
>> This is probably doable by checking for appropriate
>> net->ipv6.ip6_prohibit_entry entries.
>>
>
> 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 following cases:
>
> # ip -4 rule add to 172.16.60.0/24 prohibit
> # ip -4 route add prohibit 172.16.50.0/24
> # ip -6 rule add to 6000::/120 prohibit
> # ip -6 route add prohibit 5000::/120
>
>
> Behavior before Roopa's patch set:
> Rule match:
> # ip ro get 172.16.60.1
> RTNETLINK answers: Permission denied
>
> # ip -6 ro get 6000::1
> prohibit 6000::1 from :: dev lo proto kernel src 2001:db8::3 metric
> 4294967295 error -13 pref medium
>
> Route match:
> # ip ro get 172.16.50.1
> RTNETLINK answers: Permission denied
>
> # ip -6 ro get 5000::1
> prohibit 5000::1 from :: dev lo table red src 2001:db8::3 metric
> 1024 error -13 pref medium
>
>
> Behavior after Roopa's patch set:
> Rule match:
> # ip ro get 172.16.60.1
> RTNETLINK answers: Permission denied
>
> # ip -6 ro get 6000::1
> RTNETLINK answers: Permission denied
>
> Route match:
> # ip ro get 172.16.50.1
> RTNETLINK answers: Permission denied
>
> # ip -6 ro get 5000::1
> RTNETLINK answers: Permission denied
>
There must be a reason why we allocate prohibit entries
dynamically for IPv6 despite we already have a (relatively)
static one.
>From this point of view, we need to dump them, that is,
restore the behavior before Roopa's patch.
>
> 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 don't see why this one is so special
that we have to make it consistent.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
2017-07-31 18:37 ` Cong Wang
@ 2017-07-31 18:40 ` David Ahern
0 siblings, 0 replies; 46+ messages in thread
From: David Ahern @ 2017-07-31 18:40 UTC (permalink / raw)
To: Cong Wang; +Cc: Roopa Prabhu, Hangbin Liu, network dev
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 don't see why this one is so special
> that we have to make it consistent.
yes, and I have sent a number of patches closing the gap.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info
2017-07-27 16:25 ` [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info Hangbin Liu
2017-07-27 18:03 ` David Ahern
2017-07-27 19:52 ` Roopa Prabhu
@ 2017-07-31 23:22 ` David Miller
2017-07-31 23:34 ` David Ahern
2 siblings, 1 reply; 46+ messages in thread
From: David Miller @ 2017-07-31 23:22 UTC (permalink / raw)
To: liuhangbin; +Cc: netdev, xiyou.wangcong, roopa, dsahern
From: Hangbin Liu <liuhangbin@gmail.com>
Date: Fri, 28 Jul 2017 00:25:36 +0800
> After commit 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib
> result when requested"). When we get a prohibit ertry, we will return
> -EACCES directly instead of dump route info.
>
> Fix it by remove the rt->dst.error check.
...
> Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
David A., where are we on this?
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info
2017-07-31 23:22 ` David Miller
@ 2017-07-31 23:34 ` David Ahern
2017-07-31 23:39 ` David Miller
0 siblings, 1 reply; 46+ messages in thread
From: David Ahern @ 2017-07-31 23:34 UTC (permalink / raw)
To: David Miller, liuhangbin; +Cc: netdev, xiyou.wangcong, roopa
On 7/31/17 5:22 PM, David Miller wrote:
> From: Hangbin Liu <liuhangbin@gmail.com>
> Date: Fri, 28 Jul 2017 00:25:36 +0800
>
>> After commit 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib
>> result when requested"). When we get a prohibit ertry, we will return
>> -EACCES directly instead of dump route info.
>>
>> Fix it by remove the rt->dst.error check.
> ...
>> Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
>> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>
> David A., where are we on this?
>
Dizzy from running in circles.
Question I posed to you Saturday morning, 8:41 MDT [1]:
"... Roopa's fibmatch patches caused a change in user behavior in IPv6
getroute for prohibit, blackhole and unreachable route entries. Opinions
on whether we should limit that new behavior to just the fibmatch lookup
in which case a patch is needed or take the new behavior and consistency
in which case nothing is needed?"
Personally, after all the discussion I think the behavior as it is right
now is best.
[1] https://www.spinics.net/lists/netdev/msg446571.html
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info
2017-07-31 23:34 ` David Ahern
@ 2017-07-31 23:39 ` David Miller
0 siblings, 0 replies; 46+ messages in thread
From: David Miller @ 2017-07-31 23:39 UTC (permalink / raw)
To: dsahern; +Cc: liuhangbin, netdev, xiyou.wangcong, roopa
From: David Ahern <dsahern@gmail.com>
Date: Mon, 31 Jul 2017 17:34:09 -0600
> On 7/31/17 5:22 PM, David Miller wrote:
>> From: Hangbin Liu <liuhangbin@gmail.com>
>> Date: Fri, 28 Jul 2017 00:25:36 +0800
>>
>>> After commit 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib
>>> result when requested"). When we get a prohibit ertry, we will return
>>> -EACCES directly instead of dump route info.
>>>
>>> Fix it by remove the rt->dst.error check.
>> ...
>>> Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
>>> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>>
>> David A., where are we on this?
>>
>
> Dizzy from running in circles.
:-)
> Question I posed to you Saturday morning, 8:41 MDT [1]:
>
> "... Roopa's fibmatch patches caused a change in user behavior in IPv6
> getroute for prohibit, blackhole and unreachable route entries. Opinions
> on whether we should limit that new behavior to just the fibmatch lookup
> in which case a patch is needed or take the new behavior and consistency
> in which case nothing is needed?"
>
> Personally, after all the discussion I think the behavior as it is right
> now is best.
>
> [1] https://www.spinics.net/lists/netdev/msg446571.html
I agree with you that we should keep the behavior as is.
^ permalink raw reply [flat|nested] 46+ messages in thread
end of thread, other threads:[~2017-07-31 23:39 UTC | newest]
Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-20 14:51 [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry Hangbin Liu
2017-07-20 15:06 ` Hangbin Liu
2017-07-20 15:23 ` Hangbin Liu
2017-07-21 15:53 ` David Ahern
2017-07-21 18:42 ` Cong Wang
2017-07-21 21:53 ` Roopa Prabhu
2017-07-23 4:54 ` Roopa Prabhu
2017-07-24 3:09 ` Hangbin Liu
2017-07-24 19:57 ` Cong Wang
2017-07-25 0:08 ` Hangbin Liu
2017-07-25 3:28 ` David Ahern
2017-07-25 7:32 ` Hangbin Liu
2017-07-26 17:18 ` David Ahern
2017-07-26 18:27 ` Roopa Prabhu
2017-07-26 18:49 ` David Ahern
2017-07-26 18:55 ` Roopa Prabhu
2017-07-26 19:00 ` David Ahern
2017-07-26 19:38 ` Roopa Prabhu
2017-07-27 16:08 ` Hangbin Liu
2017-07-28 4:56 ` Cong Wang
2017-07-28 11:04 ` Hangbin Liu
2017-07-28 15:10 ` David Ahern
2017-07-28 17:13 ` Roopa Prabhu
2017-07-28 17:39 ` David Ahern
2017-07-28 19:52 ` Roopa Prabhu
2017-07-29 14:41 ` David Ahern
2017-07-31 18:37 ` Cong Wang
2017-07-31 18:40 ` David Ahern
2017-07-25 17:49 ` Cong Wang
2017-07-26 9:18 ` Hangbin Liu
2017-07-21 3:47 ` [PATCHv2 net] ipv6: should not return rt->dst.error if it is prohibit or blk hole entry Hangbin Liu
2017-07-21 15:29 ` kbuild test robot
2017-07-21 16:34 ` kbuild test robot
2017-07-23 4:55 ` [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry Roopa Prabhu
2017-07-24 2:28 ` Hangbin Liu
2017-07-26 9:20 ` [PATCHv3 net] ipv6: no need to return rt->dst.error if it is prohibit entry Hangbin Liu
2017-07-26 17:09 ` David Ahern
2017-07-26 18:48 ` David Ahern
2017-07-27 13:48 ` Hangbin Liu
2017-07-27 16:25 ` [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info Hangbin Liu
2017-07-27 18:03 ` David Ahern
2017-07-28 17:23 ` David Ahern
2017-07-27 19:52 ` Roopa Prabhu
2017-07-31 23:22 ` David Miller
2017-07-31 23:34 ` David Ahern
2017-07-31 23:39 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.