* [PATCH net-next] ipv6: Convert gateway validation to use fib6_info
@ 2019-06-20 19:05 David Ahern
2019-06-20 23:43 ` Wei Wang
0 siblings, 1 reply; 4+ messages in thread
From: David Ahern @ 2019-06-20 19:05 UTC (permalink / raw)
To: davem; +Cc: netdev, kafai, weiwan, David Ahern
From: David Ahern <dsahern@gmail.com>
Gateway validation does not need a dst_entry, it only needs the fib
entry to validate the gateway resolution and egress device. So,
convert ip6_nh_lookup_table from ip6_pol_route to fib6_table_lookup
and ip6_route_check_nh to use fib6_lookup over rt6_lookup.
ip6_pol_route is a call to fib6_table_lookup and if successful a call
to fib6_select_path. From there the exception cache is searched for an
entry or a dst_entry is created to return to the caller. The exception
entry is not relevant for gateway validation, so what matters are the
calls to fib6_table_lookup and then fib6_select_path.
Similarly, rt6_lookup can be replaced with a call to fib6_lookup with
RT6_LOOKUP_F_IFACE set in flags (saddr is not set for gateway validation,
so RT6_LOOKUP_F_HAS_SADDR is not relevant). From there ip6_pol_route_lookup
lookup function is flipped to fib6_table_lookup for the per-table search.
Again, the exception cache search is not relevant, only the lookup with
path selection.
Adjust the users, ip6_route_check_nh_onlink and ip6_route_check_nh to
handle a fib6_info vs a rt6_info when performing validation checks.
Existing selftests fib-onlink-tests.sh and fib_tests.sh used to verify
the changes.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv6/route.c | 119 ++++++++++++++++++++++++++-----------------------------
1 file changed, 57 insertions(+), 62 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index c4d285fe0adc..4937084610b5 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3131,10 +3131,9 @@ static int ip6_dst_gc(struct dst_ops *ops)
return entries > rt_max_size;
}
-static struct rt6_info *ip6_nh_lookup_table(struct net *net,
- struct fib6_config *cfg,
- const struct in6_addr *gw_addr,
- u32 tbid, int flags)
+static int ip6_nh_lookup_table(struct net *net, struct fib6_config *cfg,
+ const struct in6_addr *gw_addr, u32 tbid,
+ int flags, struct fib6_result *res)
{
struct flowi6 fl6 = {
.flowi6_oif = cfg->fc_ifindex,
@@ -3142,25 +3141,23 @@ static struct rt6_info *ip6_nh_lookup_table(struct net *net,
.saddr = cfg->fc_prefsrc,
};
struct fib6_table *table;
- struct rt6_info *rt;
+ int err;
table = fib6_get_table(net, tbid);
if (!table)
- return NULL;
+ return -EINVAL;
if (!ipv6_addr_any(&cfg->fc_prefsrc))
flags |= RT6_LOOKUP_F_HAS_SADDR;
flags |= RT6_LOOKUP_F_IGNORE_LINKSTATE;
- rt = ip6_pol_route(net, table, cfg->fc_ifindex, &fl6, NULL, flags);
- /* if table lookup failed, fall back to full lookup */
- if (rt == net->ipv6.ip6_null_entry) {
- ip6_rt_put(rt);
- rt = NULL;
- }
+ err = fib6_table_lookup(net, table, cfg->fc_ifindex, &fl6, res, flags);
+ if (!err && res->f6i != net->ipv6.fib6_null_entry)
+ fib6_select_path(net, res, &fl6, cfg->fc_ifindex,
+ cfg->fc_ifindex != 0, NULL, flags);
- return rt;
+ return err;
}
static int ip6_route_check_nh_onlink(struct net *net,
@@ -3168,29 +3165,19 @@ static int ip6_route_check_nh_onlink(struct net *net,
const struct net_device *dev,
struct netlink_ext_ack *extack)
{
- u32 tbid = l3mdev_fib_table(dev) ? : RT_TABLE_MAIN;
+ u32 tbid = l3mdev_fib_table_rcu(dev) ? : RT_TABLE_MAIN;
const struct in6_addr *gw_addr = &cfg->fc_gateway;
- u32 flags = RTF_LOCAL | RTF_ANYCAST | RTF_REJECT;
- struct fib6_info *from;
- struct rt6_info *grt;
+ struct fib6_result res = {};
int err;
- err = 0;
- grt = ip6_nh_lookup_table(net, cfg, gw_addr, tbid, 0);
- if (grt) {
- rcu_read_lock();
- from = rcu_dereference(grt->from);
- if (!grt->dst.error &&
- /* ignore match if it is the default route */
- from && !ipv6_addr_any(&from->fib6_dst.addr) &&
- (grt->rt6i_flags & flags || dev != grt->dst.dev)) {
- NL_SET_ERR_MSG(extack,
- "Nexthop has invalid gateway or device mismatch");
- err = -EINVAL;
- }
- rcu_read_unlock();
-
- ip6_rt_put(grt);
+ err = ip6_nh_lookup_table(net, cfg, gw_addr, tbid, 0, &res);
+ if (!err && !(res.fib6_flags & RTF_REJECT) &&
+ /* ignore match if it is the default route */
+ !ipv6_addr_any(&res.f6i->fib6_dst.addr) &&
+ (res.fib6_type != RTN_UNICAST || dev != res.nh->fib_nh_dev)) {
+ NL_SET_ERR_MSG(extack,
+ "Nexthop has invalid gateway or device mismatch");
+ err = -EINVAL;
}
return err;
@@ -3203,47 +3190,51 @@ static int ip6_route_check_nh(struct net *net,
{
const struct in6_addr *gw_addr = &cfg->fc_gateway;
struct net_device *dev = _dev ? *_dev : NULL;
- struct rt6_info *grt = NULL;
+ int flags = RT6_LOOKUP_F_IFACE;
+ struct fib6_result res = {};
int err = -EHOSTUNREACH;
if (cfg->fc_table) {
- int flags = RT6_LOOKUP_F_IFACE;
-
- grt = ip6_nh_lookup_table(net, cfg, gw_addr,
- cfg->fc_table, flags);
- if (grt) {
- if (grt->rt6i_flags & RTF_GATEWAY ||
- (dev && dev != grt->dst.dev)) {
- ip6_rt_put(grt);
- grt = NULL;
- }
- }
+ err = ip6_nh_lookup_table(net, cfg, gw_addr,
+ cfg->fc_table, flags, &res);
+ /* gw_addr can not require a gateway or resolve to a reject
+ * route. If a device is given, it must match the result.
+ */
+ if (err || res.fib6_flags & RTF_REJECT ||
+ res.nh->fib_nh_gw_family ||
+ (dev && dev != res.nh->fib_nh_dev))
+ err = -EHOSTUNREACH;
}
- if (!grt)
- grt = rt6_lookup(net, gw_addr, NULL, cfg->fc_ifindex, NULL, 1);
+ if (err < 0) {
+ struct flowi6 fl6 = {
+ .flowi6_oif = cfg->fc_ifindex,
+ .daddr = *gw_addr,
+ };
- if (!grt)
- goto out;
+ err = fib6_lookup(net, cfg->fc_ifindex, &fl6, &res, flags);
+ if (err || res.fib6_flags & RTF_REJECT ||
+ res.nh->fib_nh_gw_family)
+ err = -EHOSTUNREACH;
+
+ if (err)
+ return err;
+
+ fib6_select_path(net, &res, &fl6, cfg->fc_ifindex,
+ cfg->fc_ifindex != 0, NULL, flags);
+ }
+ err = 0;
if (dev) {
- if (dev != grt->dst.dev) {
- ip6_rt_put(grt);
- goto out;
- }
+ if (dev != res.nh->fib_nh_dev)
+ err = -EHOSTUNREACH;
} else {
- *_dev = dev = grt->dst.dev;
- *idev = grt->rt6i_idev;
+ *_dev = dev = res.nh->fib_nh_dev;
+ *idev = __in6_dev_get(dev);
dev_hold(dev);
- in6_dev_hold(grt->rt6i_idev);
+ in6_dev_hold(*idev);
}
- if (!(grt->rt6i_flags & RTF_GATEWAY))
- err = 0;
-
- ip6_rt_put(grt);
-
-out:
return err;
}
@@ -3284,11 +3275,15 @@ static int ip6_validate_gw(struct net *net, struct fib6_config *cfg,
goto out;
}
+ rcu_read_lock();
+
if (cfg->fc_flags & RTNH_F_ONLINK)
err = ip6_route_check_nh_onlink(net, cfg, dev, extack);
else
err = ip6_route_check_nh(net, cfg, _dev, idev);
+ rcu_read_unlock();
+
if (err)
goto out;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] ipv6: Convert gateway validation to use fib6_info
2019-06-20 19:05 [PATCH net-next] ipv6: Convert gateway validation to use fib6_info David Ahern
@ 2019-06-20 23:43 ` Wei Wang
2019-06-21 0:52 ` David Ahern
2019-06-21 21:04 ` David Ahern
0 siblings, 2 replies; 4+ messages in thread
From: Wei Wang @ 2019-06-20 23:43 UTC (permalink / raw)
To: David Ahern
Cc: David S . Miller, Linux Kernel Network Developers,
Martin KaFai Lau, David Ahern
On Thu, Jun 20, 2019 at 12:05 PM David Ahern <dsahern@kernel.org> wrote:
>
> From: David Ahern <dsahern@gmail.com>
>
> Gateway validation does not need a dst_entry, it only needs the fib
> entry to validate the gateway resolution and egress device. So,
> convert ip6_nh_lookup_table from ip6_pol_route to fib6_table_lookup
> and ip6_route_check_nh to use fib6_lookup over rt6_lookup.
>
> ip6_pol_route is a call to fib6_table_lookup and if successful a call
> to fib6_select_path. From there the exception cache is searched for an
> entry or a dst_entry is created to return to the caller. The exception
> entry is not relevant for gateway validation, so what matters are the
> calls to fib6_table_lookup and then fib6_select_path.
>
> Similarly, rt6_lookup can be replaced with a call to fib6_lookup with
> RT6_LOOKUP_F_IFACE set in flags (saddr is not set for gateway validation,
> so RT6_LOOKUP_F_HAS_SADDR is not relevant). From there ip6_pol_route_lookup
> lookup function is flipped to fib6_table_lookup for the per-table search.
> Again, the exception cache search is not relevant, only the lookup with
> path selection.
>
> Adjust the users, ip6_route_check_nh_onlink and ip6_route_check_nh to
> handle a fib6_info vs a rt6_info when performing validation checks.
>
> Existing selftests fib-onlink-tests.sh and fib_tests.sh used to verify
> the changes.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> net/ipv6/route.c | 119 ++++++++++++++++++++++++++-----------------------------
> 1 file changed, 57 insertions(+), 62 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index c4d285fe0adc..4937084610b5 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -3131,10 +3131,9 @@ static int ip6_dst_gc(struct dst_ops *ops)
> return entries > rt_max_size;
> }
>
> -static struct rt6_info *ip6_nh_lookup_table(struct net *net,
> - struct fib6_config *cfg,
> - const struct in6_addr *gw_addr,
> - u32 tbid, int flags)
> +static int ip6_nh_lookup_table(struct net *net, struct fib6_config *cfg,
> + const struct in6_addr *gw_addr, u32 tbid,
> + int flags, struct fib6_result *res)
> {
> struct flowi6 fl6 = {
> .flowi6_oif = cfg->fc_ifindex,
> @@ -3142,25 +3141,23 @@ static struct rt6_info *ip6_nh_lookup_table(struct net *net,
> .saddr = cfg->fc_prefsrc,
> };
> struct fib6_table *table;
> - struct rt6_info *rt;
> + int err;
>
> table = fib6_get_table(net, tbid);
> if (!table)
> - return NULL;
> + return -EINVAL;
>
> if (!ipv6_addr_any(&cfg->fc_prefsrc))
> flags |= RT6_LOOKUP_F_HAS_SADDR;
>
> flags |= RT6_LOOKUP_F_IGNORE_LINKSTATE;
> - rt = ip6_pol_route(net, table, cfg->fc_ifindex, &fl6, NULL, flags);
>
> - /* if table lookup failed, fall back to full lookup */
> - if (rt == net->ipv6.ip6_null_entry) {
> - ip6_rt_put(rt);
> - rt = NULL;
> - }
> + err = fib6_table_lookup(net, table, cfg->fc_ifindex, &fl6, res, flags);
> + if (!err && res->f6i != net->ipv6.fib6_null_entry)
> + fib6_select_path(net, res, &fl6, cfg->fc_ifindex,
> + cfg->fc_ifindex != 0, NULL, flags);
>
> - return rt;
> + return err;
> }
>
> static int ip6_route_check_nh_onlink(struct net *net,
> @@ -3168,29 +3165,19 @@ static int ip6_route_check_nh_onlink(struct net *net,
> const struct net_device *dev,
> struct netlink_ext_ack *extack)
> {
> - u32 tbid = l3mdev_fib_table(dev) ? : RT_TABLE_MAIN;
> + u32 tbid = l3mdev_fib_table_rcu(dev) ? : RT_TABLE_MAIN;
> const struct in6_addr *gw_addr = &cfg->fc_gateway;
> - u32 flags = RTF_LOCAL | RTF_ANYCAST | RTF_REJECT;
> - struct fib6_info *from;
> - struct rt6_info *grt;
> + struct fib6_result res = {};
> int err;
>
> - err = 0;
> - grt = ip6_nh_lookup_table(net, cfg, gw_addr, tbid, 0);
> - if (grt) {
> - rcu_read_lock();
> - from = rcu_dereference(grt->from);
> - if (!grt->dst.error &&
> - /* ignore match if it is the default route */
> - from && !ipv6_addr_any(&from->fib6_dst.addr) &&
> - (grt->rt6i_flags & flags || dev != grt->dst.dev)) {
> - NL_SET_ERR_MSG(extack,
> - "Nexthop has invalid gateway or device mismatch");
> - err = -EINVAL;
> - }
> - rcu_read_unlock();
> -
> - ip6_rt_put(grt);
> + err = ip6_nh_lookup_table(net, cfg, gw_addr, tbid, 0, &res);
> + if (!err && !(res.fib6_flags & RTF_REJECT) &&
> + /* ignore match if it is the default route */
> + !ipv6_addr_any(&res.f6i->fib6_dst.addr) &&
> + (res.fib6_type != RTN_UNICAST || dev != res.nh->fib_nh_dev)) {
> + NL_SET_ERR_MSG(extack,
> + "Nexthop has invalid gateway or device mismatch");
> + err = -EINVAL;
> }
>
> return err;
> @@ -3203,47 +3190,51 @@ static int ip6_route_check_nh(struct net *net,
> {
> const struct in6_addr *gw_addr = &cfg->fc_gateway;
> struct net_device *dev = _dev ? *_dev : NULL;
> - struct rt6_info *grt = NULL;
> + int flags = RT6_LOOKUP_F_IFACE;
> + struct fib6_result res = {};
> int err = -EHOSTUNREACH;
>
> if (cfg->fc_table) {
> - int flags = RT6_LOOKUP_F_IFACE;
> -
> - grt = ip6_nh_lookup_table(net, cfg, gw_addr,
> - cfg->fc_table, flags);
> - if (grt) {
> - if (grt->rt6i_flags & RTF_GATEWAY ||
> - (dev && dev != grt->dst.dev)) {
> - ip6_rt_put(grt);
> - grt = NULL;
> - }
> - }
> + err = ip6_nh_lookup_table(net, cfg, gw_addr,
> + cfg->fc_table, flags, &res);
> + /* gw_addr can not require a gateway or resolve to a reject
> + * route. If a device is given, it must match the result.
> + */
> + if (err || res.fib6_flags & RTF_REJECT ||
> + res.nh->fib_nh_gw_family ||
> + (dev && dev != res.nh->fib_nh_dev))
> + err = -EHOSTUNREACH;
> }
>
> - if (!grt)
> - grt = rt6_lookup(net, gw_addr, NULL, cfg->fc_ifindex, NULL, 1);
> + if (err < 0) {
> + struct flowi6 fl6 = {
> + .flowi6_oif = cfg->fc_ifindex,
> + .daddr = *gw_addr,
> + };
>
> - if (!grt)
> - goto out;
> + err = fib6_lookup(net, cfg->fc_ifindex, &fl6, &res, flags);
I am not very convinced that fib6_lookup() could be equivalent to
rt6_lookup(). Specifically, rt6_lookup() calls rt6_device_match()
while fib6_lookup() calls rt6_select() to match the oif. From a brief
glance, it does seem to be similar, especially considering that saddr
is NULL. So it probably is OK?
> + if (err || res.fib6_flags & RTF_REJECT ||
> + res.nh->fib_nh_gw_family)
> + err = -EHOSTUNREACH;
> +
> + if (err)
> + return err;
> +
> + fib6_select_path(net, &res, &fl6, cfg->fc_ifindex,
> + cfg->fc_ifindex != 0, NULL, flags);
> + }
>
> + err = 0;
> if (dev) {
> - if (dev != grt->dst.dev) {
> - ip6_rt_put(grt);
> - goto out;
> - }
> + if (dev != res.nh->fib_nh_dev)
> + err = -EHOSTUNREACH;
> } else {
> - *_dev = dev = grt->dst.dev;
> - *idev = grt->rt6i_idev;
> + *_dev = dev = res.nh->fib_nh_dev;
> + *idev = __in6_dev_get(dev);
> dev_hold(dev);
> - in6_dev_hold(grt->rt6i_idev);
> + in6_dev_hold(*idev);
nit: directly do *idev = in6_dev_get(dev) instead of 2 instructions.
> }
>
> - if (!(grt->rt6i_flags & RTF_GATEWAY))
> - err = 0;
> -
> - ip6_rt_put(grt);
> -
> -out:
> return err;
> }
>
> @@ -3284,11 +3275,15 @@ static int ip6_validate_gw(struct net *net, struct fib6_config *cfg,
> goto out;
> }
>
> + rcu_read_lock();
> +
> if (cfg->fc_flags & RTNH_F_ONLINK)
> err = ip6_route_check_nh_onlink(net, cfg, dev, extack);
> else
> err = ip6_route_check_nh(net, cfg, _dev, idev);
>
> + rcu_read_unlock();
> +
> if (err)
> goto out;
> }
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] ipv6: Convert gateway validation to use fib6_info
2019-06-20 23:43 ` Wei Wang
@ 2019-06-21 0:52 ` David Ahern
2019-06-21 21:04 ` David Ahern
1 sibling, 0 replies; 4+ messages in thread
From: David Ahern @ 2019-06-21 0:52 UTC (permalink / raw)
To: Wei Wang, David Ahern
Cc: David S . Miller, Linux Kernel Network Developers, Martin KaFai Lau
On 6/20/19 5:43 PM, Wei Wang wrote:
> I am not very convinced that fib6_lookup() could be equivalent to
> rt6_lookup(). Specifically, rt6_lookup() calls rt6_device_match()
> while fib6_lookup() calls rt6_select() to match the oif. From a brief
> glance, it does seem to be similar, especially considering that saddr
> is NULL. So it probably is OK?
I believe so, but I'll walk the 2 paths again and do the compare based
on the limited flags and inputs for the gateway validation.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] ipv6: Convert gateway validation to use fib6_info
2019-06-20 23:43 ` Wei Wang
2019-06-21 0:52 ` David Ahern
@ 2019-06-21 21:04 ` David Ahern
1 sibling, 0 replies; 4+ messages in thread
From: David Ahern @ 2019-06-21 21:04 UTC (permalink / raw)
To: Wei Wang, David Ahern
Cc: David S . Miller, Linux Kernel Network Developers, Martin KaFai Lau
On 6/20/19 5:43 PM, Wei Wang wrote:
> I am not very convinced that fib6_lookup() could be equivalent to
> rt6_lookup(). Specifically, rt6_lookup() calls rt6_device_match()
> while fib6_lookup() calls rt6_select() to match the oif. From a brief
> glance, it does seem to be similar, especially considering that saddr
> is NULL. So it probably is OK?
When you remove the rt6_check_neigh call since RT6_LOOKUP_F_REACHABLE is
not set that removes the RT6_NUD_FAIL_DO_RR return and round-robin
logic. I am reasonably confident that given the use case - validate the
gateway and optionally given the device - it is the same.
rt6_select is much more complicated than rt6_device_match, so there is a
small possibility that in some corner case gateway validation fails /
succeeds with fib6_table_lookup where it would succeed / fail with
ip6_pol_route_lookup. But, ip6_pol_route and fib6_table_lookup is the
code path actually used for packet Rx and Tx, so it seems to me to be
the more proper one for gateway validation.
I will send a v2 with idev change you mentioned.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-06-21 21:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-20 19:05 [PATCH net-next] ipv6: Convert gateway validation to use fib6_info David Ahern
2019-06-20 23:43 ` Wei Wang
2019-06-21 0:52 ` David Ahern
2019-06-21 21:04 ` David Ahern
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).