* [PATCH net 1/2] ipv4: fix value of ->nlmsg_flags reported in RTM_NEWROUTE events
2016-09-07 15:18 [PATCH net 0/2] ip: fix creation flags reported in RTM_NEWROUTE events Guillaume Nault
@ 2016-09-07 15:20 ` Guillaume Nault
2016-09-07 15:21 ` [PATCH net 2/2] ipv6: report NLM_F_CREATE and NLM_F_EXCL flags " Guillaume Nault
2016-09-09 23:51 ` [PATCH net 0/2] ip: fix creation flags reported " David Miller
2 siblings, 0 replies; 5+ messages in thread
From: Guillaume Nault @ 2016-09-07 15:20 UTC (permalink / raw)
To: netdev; +Cc: Roopa Prabhu, Milan Kocian, Michal Kubeček, Nicolas Dichtel
fib_table_insert() inconsistently fills the nlmsg_flags field in its
notification messages.
Since commit b8f558313506 ("[RTNETLINK]: Fix sending netlink message
when replace route."), the netlink message has its nlmsg_flags set to
NLM_F_REPLACE if the route replaced a preexisting one.
Then commit a2bb6d7d6f42 ("ipv4: include NLM_F_APPEND flag in append
route notifications") started setting nlmsg_flags to NLM_F_APPEND if
the route matched a preexisting one but was appended.
In other cases (exclusive creation or prepend), nlmsg_flags is 0.
This patch sets ->nlmsg_flags in all situations, preserving the
semantic of the NLM_F_* bits:
* NLM_F_CREATE: a new fib entry has been created for this route.
* NLM_F_EXCL: no other fib entry existed for this route.
* NLM_F_REPLACE: this route has overwritten a preexisting fib entry.
* NLM_F_APPEND: the new fib entry was added after other entries for
the same route.
As a result, the possible flag combination can now be reported
(iproute2's terminology into parentheses):
* NLM_F_CREATE | NLM_F_EXCL: route didn't exist, exclusive creation
("add").
* NLM_F_CREATE | NLM_F_APPEND: route did already exist, new route
added after preexisting ones ("append").
* NLM_F_CREATE: route did already exist, new route added before
preexisting ones ("prepend").
* NLM_F_REPLACE: route did already exist, new route replaced the
first preexisting one ("change").
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
net/ipv4/fib_trie.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index e2ffc2a..241f27b 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1081,7 +1081,7 @@ int fib_table_insert(struct fib_table *tb, struct fib_config *cfg)
struct trie *t = (struct trie *)tb->tb_data;
struct fib_alias *fa, *new_fa;
struct key_vector *l, *tp;
- unsigned int nlflags = 0;
+ u16 nlflags = NLM_F_EXCL;
struct fib_info *fi;
u8 plen = cfg->fc_dst_len;
u8 slen = KEYLENGTH - plen;
@@ -1126,6 +1126,8 @@ int fib_table_insert(struct fib_table *tb, struct fib_config *cfg)
if (cfg->fc_nlflags & NLM_F_EXCL)
goto out;
+ nlflags &= ~NLM_F_EXCL;
+
/* We have 2 goals:
* 1. Find exact match for type, scope, fib_info to avoid
* duplicate routes
@@ -1151,6 +1153,7 @@ int fib_table_insert(struct fib_table *tb, struct fib_config *cfg)
struct fib_info *fi_drop;
u8 state;
+ nlflags |= NLM_F_REPLACE;
fa = fa_first;
if (fa_match) {
if (fa == fa_match)
@@ -1191,7 +1194,7 @@ int fib_table_insert(struct fib_table *tb, struct fib_config *cfg)
if (state & FA_S_ACCESSED)
rt_cache_flush(cfg->fc_nlinfo.nl_net);
rtmsg_fib(RTM_NEWROUTE, htonl(key), new_fa, plen,
- tb->tb_id, &cfg->fc_nlinfo, NLM_F_REPLACE);
+ tb->tb_id, &cfg->fc_nlinfo, nlflags);
goto succeeded;
}
@@ -1203,7 +1206,7 @@ int fib_table_insert(struct fib_table *tb, struct fib_config *cfg)
goto out;
if (cfg->fc_nlflags & NLM_F_APPEND)
- nlflags = NLM_F_APPEND;
+ nlflags |= NLM_F_APPEND;
else
fa = fa_first;
}
@@ -1211,6 +1214,7 @@ int fib_table_insert(struct fib_table *tb, struct fib_config *cfg)
if (!(cfg->fc_nlflags & NLM_F_CREATE))
goto out;
+ nlflags |= NLM_F_CREATE;
err = -ENOBUFS;
new_fa = kmem_cache_alloc(fn_alias_kmem, GFP_KERNEL);
if (!new_fa)
--
2.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH net 2/2] ipv6: report NLM_F_CREATE and NLM_F_EXCL flags in RTM_NEWROUTE events
2016-09-07 15:18 [PATCH net 0/2] ip: fix creation flags reported in RTM_NEWROUTE events Guillaume Nault
2016-09-07 15:20 ` [PATCH net 1/2] ipv4: fix value of ->nlmsg_flags " Guillaume Nault
@ 2016-09-07 15:21 ` Guillaume Nault
2016-09-09 23:51 ` [PATCH net 0/2] ip: fix creation flags reported " David Miller
2 siblings, 0 replies; 5+ messages in thread
From: Guillaume Nault @ 2016-09-07 15:21 UTC (permalink / raw)
To: netdev; +Cc: Roopa Prabhu, Milan Kocian, Michal Kubeček, Nicolas Dichtel
Since commit 37a1d3611c12 ("ipv6: include NLM_F_REPLACE in route
replace notifications"), RTM_NEWROUTE notifications have their
NLM_F_REPLACE flag set if the new route replaced a preexisting one.
However, other flags aren't set.
This patch reports the missing NLM_F_CREATE and NLM_F_EXCL flag bits.
NLM_F_APPEND is not reported, because in ipv6 a NLM_F_CREATE request
is interpreted as an append request (contrary to ipv4, "prepend" is not
supported, so if NLM_F_EXCL is not set then NLM_F_APPEND is implicit).
As a result, the possible flag combination can now be reported
(iproute2's terminology into parentheses):
* NLM_F_CREATE | NLM_F_EXCL: route didn't exist, exclusive creation
("add").
* NLM_F_CREATE: route did already exist, new route added after
preexisting ones ("append").
* NLM_F_REPLACE: route did already exist, new route replaced the
first preexisting one ("change").
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
net/ipv6/ip6_fib.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 771be1f..ef54852 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -743,6 +743,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
(info->nlh->nlmsg_flags & NLM_F_CREATE));
int found = 0;
bool rt_can_ecmp = rt6_qualify_for_ecmp(rt);
+ u16 nlflags = NLM_F_EXCL;
int err;
ins = &fn->leaf;
@@ -759,6 +760,8 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
if (info->nlh &&
(info->nlh->nlmsg_flags & NLM_F_EXCL))
return -EEXIST;
+
+ nlflags &= ~NLM_F_EXCL;
if (replace) {
if (rt_can_ecmp == rt6_qualify_for_ecmp(iter)) {
found++;
@@ -856,6 +859,7 @@ next_iter:
pr_warn("NLM_F_CREATE should be set when creating new route\n");
add:
+ nlflags |= NLM_F_CREATE;
err = fib6_commit_metrics(&rt->dst, mxc);
if (err)
return err;
@@ -864,7 +868,7 @@ add:
*ins = rt;
rt->rt6i_node = fn;
atomic_inc(&rt->rt6i_ref);
- inet6_rt_notify(RTM_NEWROUTE, rt, info, 0);
+ inet6_rt_notify(RTM_NEWROUTE, rt, info, nlflags);
info->nl_net->ipv6.rt6_stats->fib_rt_entries++;
if (!(fn->fn_flags & RTN_RTINFO)) {
--
2.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net 0/2] ip: fix creation flags reported in RTM_NEWROUTE events
2016-09-07 15:18 [PATCH net 0/2] ip: fix creation flags reported in RTM_NEWROUTE events Guillaume Nault
2016-09-07 15:20 ` [PATCH net 1/2] ipv4: fix value of ->nlmsg_flags " Guillaume Nault
2016-09-07 15:21 ` [PATCH net 2/2] ipv6: report NLM_F_CREATE and NLM_F_EXCL flags " Guillaume Nault
@ 2016-09-09 23:51 ` David Miller
2016-09-12 10:29 ` Guillaume Nault
2 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2016-09-09 23:51 UTC (permalink / raw)
To: g.nault; +Cc: netdev, roopa, milon, mkubecek, nicolas.dichtel
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Wed, 7 Sep 2016 17:18:50 +0200
> Netlink messages sent to user-space upon RTM_NEWROUTE events have their
> nlmsg_flags field inconsistently set. While the NLM_F_REPLACE and
> NLM_F_APPEND bits are correctly handled, NLM_F_CREATE and NLM_F_EXCL
> are always 0.
>
> This series sets the NLM_F_CREATE and NLM_F_EXCL bits when applicable,
> for IPv4 and IPv6.
>
> Since IPv6 ignores the NLM_F_APPEND flags in requests, this flag isn't
> reported in RTM_NEWROUTE IPv6 events. This keeps IPv6 internal
> consistency (same flag semantic for user requests and kernel events) at
> the cost of bringing different flag interpretation for IPv4 and IPv6.
I'm applying this series to net-next so that it has time to cook and
expose anything in userland that might break due to these changes.
I briefly considered applying this to net but I think that is
premature at least for the time being.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net 0/2] ip: fix creation flags reported in RTM_NEWROUTE events
2016-09-09 23:51 ` [PATCH net 0/2] ip: fix creation flags reported " David Miller
@ 2016-09-12 10:29 ` Guillaume Nault
0 siblings, 0 replies; 5+ messages in thread
From: Guillaume Nault @ 2016-09-12 10:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev, roopa, milon, mkubecek, nicolas.dichtel
On Fri, Sep 09, 2016 at 04:51:26PM -0700, David Miller wrote:
> From: Guillaume Nault <g.nault@alphalink.fr>
> Date: Wed, 7 Sep 2016 17:18:50 +0200
>
> > Netlink messages sent to user-space upon RTM_NEWROUTE events have their
> > nlmsg_flags field inconsistently set. While the NLM_F_REPLACE and
> > NLM_F_APPEND bits are correctly handled, NLM_F_CREATE and NLM_F_EXCL
> > are always 0.
> >
> > This series sets the NLM_F_CREATE and NLM_F_EXCL bits when applicable,
> > for IPv4 and IPv6.
> >
> > Since IPv6 ignores the NLM_F_APPEND flags in requests, this flag isn't
> > reported in RTM_NEWROUTE IPv6 events. This keeps IPv6 internal
> > consistency (same flag semantic for user requests and kernel events) at
> > the cost of bringing different flag interpretation for IPv4 and IPv6.
>
> I'm applying this series to net-next so that it has time to cook and
> expose anything in userland that might break due to these changes.
>
> I briefly considered applying this to net but I think that is
> premature at least for the time being.
>
Makes sense, and this could be considered a feature enhancement after
all.
Thanks David.
^ permalink raw reply [flat|nested] 5+ messages in thread