* [PATCH] ipv6 route: Aggregate table getting code @ 2015-10-18 0:51 Masashi Honma 2015-10-19 6:09 ` David Miller 0 siblings, 1 reply; 5+ messages in thread From: Masashi Honma @ 2015-10-18 0:51 UTC (permalink / raw) To: netdev; +Cc: Masashi Honma These lines could be aggregated to one line because fib6_new_table() calls fib6_get_table() inside on both cases CONFIG_IPV6_MULTIPLE_TABLES is enabled or not. Signed-off-by: Masashi Honma <masashi.honma@gmail.com> --- net/ipv6/route.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/net/ipv6/route.c b/net/ipv6/route.c index cb32ce2..1ff4130 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -1778,16 +1778,7 @@ int ip6_route_info_create(struct fib6_config *cfg, struct rt6_info **rt_ret) cfg->fc_metric = IP6_RT_PRIO_USER; err = -ENOBUFS; - if (cfg->fc_nlinfo.nlh && - !(cfg->fc_nlinfo.nlh->nlmsg_flags & NLM_F_CREATE)) { - table = fib6_get_table(net, cfg->fc_table); - if (!table) { - pr_warn("NLM_F_CREATE should be specified when creating new route\n"); - table = fib6_new_table(net, cfg->fc_table); - } - } else { - table = fib6_new_table(net, cfg->fc_table); - } + table = fib6_new_table(net, cfg->fc_table); if (!table) goto out; -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ipv6 route: Aggregate table getting code 2015-10-18 0:51 [PATCH] ipv6 route: Aggregate table getting code Masashi Honma @ 2015-10-19 6:09 ` David Miller 2015-10-25 2:42 ` Masashi Honma 0 siblings, 1 reply; 5+ messages in thread From: David Miller @ 2015-10-19 6:09 UTC (permalink / raw) To: masashi.honma; +Cc: netdev From: Masashi Honma <masashi.honma@gmail.com> Date: Sun, 18 Oct 2015 09:51:39 +0900 > These lines could be aggregated to one line because fib6_new_table() calls > fib6_get_table() inside on both cases CONFIG_IPV6_MULTIPLE_TABLES is enabled or > not. > > Signed-off-by: Masashi Honma <masashi.honma@gmail.com> This is not correct. The whole point of the test is so that the kernel log message warning for failing to provide NLM_F_CREATE can be printed in precisely the correct conditions. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ipv6 route: Aggregate table getting code 2015-10-19 6:09 ` David Miller @ 2015-10-25 2:42 ` Masashi Honma 2015-10-25 2:44 ` [PATCH] ipv6 route: Use flag instead of calling fib6_get_table() twice Masashi Honma 0 siblings, 1 reply; 5+ messages in thread From: Masashi Honma @ 2015-10-25 2:42 UTC (permalink / raw) To: David Miller; +Cc: netdev On 2015/10/19 15:09, David Miller wrote: > This is not correct. > > The whole point of the test is so that the kernel log message > warning for failing to provide NLM_F_CREATE can be printed > in precisely the correct conditions. Thanks. Now I understand importance of the warning. Though fib6_get_table() is called twice to show the warning. So I made another patch to reduce calling the function. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] ipv6 route: Use flag instead of calling fib6_get_table() twice 2015-10-25 2:42 ` Masashi Honma @ 2015-10-25 2:44 ` Masashi Honma 2015-10-27 5:05 ` David Miller 0 siblings, 1 reply; 5+ messages in thread From: Masashi Honma @ 2015-10-25 2:44 UTC (permalink / raw) To: davem; +Cc: netdev, Masashi Honma The fib6_get_table() is called twice to show the warning. This patch reduces calling the function. Signed-off-by: Masashi Honma <masashi.honma@gmail.com> --- include/net/ip6_fib.h | 2 +- net/ipv6/fib6_rules.c | 3 ++- net/ipv6/ip6_fib.c | 10 +++++++--- net/ipv6/route.c | 13 ++++--------- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h index aaf9700..d6c5dff 100644 --- a/include/net/ip6_fib.h +++ b/include/net/ip6_fib.h @@ -256,7 +256,7 @@ typedef struct rt6_info *(*pol_lookup_t)(struct net *, */ struct fib6_table *fib6_get_table(struct net *net, u32 id); -struct fib6_table *fib6_new_table(struct net *net, u32 id); +struct fib6_table *fib6_new_table(struct net *net, u32 id, int *exist); struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6, int flags, pol_lookup_t lookup); diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c index 9f777ec..7512941 100644 --- a/net/ipv6/fib6_rules.c +++ b/net/ipv6/fib6_rules.c @@ -187,12 +187,13 @@ static int fib6_rule_configure(struct fib_rule *rule, struct sk_buff *skb, int err = -EINVAL; struct net *net = sock_net(skb->sk); struct fib6_rule *rule6 = (struct fib6_rule *) rule; + int exist; if (rule->action == FR_ACT_TO_TBL) { if (rule->table == RT6_TABLE_UNSPEC) goto errout; - if (fib6_new_table(net, rule->table) == NULL) { + if (fib6_new_table(net, rule->table, &exist) == NULL) { err = -ENOBUFS; goto errout; } diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c index 7d2e002..abf65ef 100644 --- a/net/ipv6/ip6_fib.c +++ b/net/ipv6/ip6_fib.c @@ -226,16 +226,19 @@ static struct fib6_table *fib6_alloc_table(struct net *net, u32 id) return table; } -struct fib6_table *fib6_new_table(struct net *net, u32 id) +struct fib6_table *fib6_new_table(struct net *net, u32 id, int *exist) { struct fib6_table *tb; if (id == 0) id = RT6_TABLE_MAIN; tb = fib6_get_table(net, id); - if (tb) + if (tb) { + *exist = 1; return tb; + } + *exist = 0; tb = fib6_alloc_table(net, id); if (tb) fib6_link_table(net, tb); @@ -272,8 +275,9 @@ static void __net_init fib6_tables_init(struct net *net) } #else -struct fib6_table *fib6_new_table(struct net *net, u32 id) +struct fib6_table *fib6_new_table(struct net *net, u32 id, int *exist) { + *exist = 1; return fib6_get_table(net, id); } diff --git a/net/ipv6/route.c b/net/ipv6/route.c index cb32ce2..7c4e0c2 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -1757,6 +1757,7 @@ int ip6_route_info_create(struct fib6_config *cfg, struct rt6_info **rt_ret) struct inet6_dev *idev = NULL; struct fib6_table *table; int addr_type; + int exist; if (cfg->fc_dst_len > 128 || cfg->fc_src_len > 128) return -EINVAL; @@ -1778,16 +1779,10 @@ int ip6_route_info_create(struct fib6_config *cfg, struct rt6_info **rt_ret) cfg->fc_metric = IP6_RT_PRIO_USER; err = -ENOBUFS; + table = fib6_new_table(net, cfg->fc_table, &exist); if (cfg->fc_nlinfo.nlh && - !(cfg->fc_nlinfo.nlh->nlmsg_flags & NLM_F_CREATE)) { - table = fib6_get_table(net, cfg->fc_table); - if (!table) { - pr_warn("NLM_F_CREATE should be specified when creating new route\n"); - table = fib6_new_table(net, cfg->fc_table); - } - } else { - table = fib6_new_table(net, cfg->fc_table); - } + !(cfg->fc_nlinfo.nlh->nlmsg_flags & NLM_F_CREATE) && !exist) + pr_warn("NLM_F_CREATE should be specified when creating new route\n"); if (!table) goto out; -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ipv6 route: Use flag instead of calling fib6_get_table() twice 2015-10-25 2:44 ` [PATCH] ipv6 route: Use flag instead of calling fib6_get_table() twice Masashi Honma @ 2015-10-27 5:05 ` David Miller 0 siblings, 0 replies; 5+ messages in thread From: David Miller @ 2015-10-27 5:05 UTC (permalink / raw) To: masashi.honma; +Cc: netdev From: Masashi Honma <masashi.honma@gmail.com> Date: Sun, 25 Oct 2015 11:44:27 +0900 > The fib6_get_table() is called twice to show the warning. > This patch reduces calling the function. > > Signed-off-by: Masashi Honma <masashi.honma@gmail.com> I think the added cost of passing a reference to an on-stack variable exceeds the value of whatever you think you are improving here. The code as-is is fine as far as I'm concerned. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-10-27 4:48 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-18 0:51 [PATCH] ipv6 route: Aggregate table getting code Masashi Honma 2015-10-19 6:09 ` David Miller 2015-10-25 2:42 ` Masashi Honma 2015-10-25 2:44 ` [PATCH] ipv6 route: Use flag instead of calling fib6_get_table() twice Masashi Honma 2015-10-27 5:05 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).