Commit ce9ac056 authored by David Ahern's avatar David Ahern Committed by David S. Miller

nexthop: Fix fdb labeling for groups

fdb nexthops are marked with a flag. For standalone nexthops, a flag was
added to the nh_info struct. For groups that flag was added to struct
nexthop when it should have been added to the group information. Fix
by removing the flag from the nexthop struct and adding a flag to nh_group
that mirrors nh_info and is really only a caching of the individual types.
Add a helper, nexthop_is_fdb, for use by the vxlan code and fixup the
internal code to use the flag from either nh_info or nh_group.

v2
- propagate fdb_nh in remove_nh_grp_entry

Fixes: 38428d68 ("nexthop: support for fdb ecmp nexthops")
Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
Signed-off-by: default avatarDavid Ahern <dsahern@kernel.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 89dc6853
...@@ -876,7 +876,7 @@ static int vxlan_fdb_nh_update(struct vxlan_dev *vxlan, struct vxlan_fdb *fdb, ...@@ -876,7 +876,7 @@ static int vxlan_fdb_nh_update(struct vxlan_dev *vxlan, struct vxlan_fdb *fdb,
nh = NULL; nh = NULL;
goto err_inval; goto err_inval;
} }
if (!nh->is_fdb_nh) { if (!nexthop_is_fdb(nh)) {
NL_SET_ERR_MSG(extack, "Nexthop is not a fdb nexthop"); NL_SET_ERR_MSG(extack, "Nexthop is not a fdb nexthop");
goto err_inval; goto err_inval;
} }
......
...@@ -76,6 +76,7 @@ struct nh_group { ...@@ -76,6 +76,7 @@ struct nh_group {
struct nh_group *spare; /* spare group for removals */ struct nh_group *spare; /* spare group for removals */
u16 num_nh; u16 num_nh;
bool mpath; bool mpath;
bool fdb_nh;
bool has_v4; bool has_v4;
struct nh_grp_entry nh_entries[]; struct nh_grp_entry nh_entries[];
}; };
...@@ -93,7 +94,6 @@ struct nexthop { ...@@ -93,7 +94,6 @@ struct nexthop {
u8 protocol; /* app managing this nh */ u8 protocol; /* app managing this nh */
u8 nh_flags; u8 nh_flags;
bool is_group; bool is_group;
bool is_fdb_nh;
refcount_t refcnt; refcount_t refcnt;
struct rcu_head rcu; struct rcu_head rcu;
...@@ -136,6 +136,21 @@ static inline bool nexthop_cmp(const struct nexthop *nh1, ...@@ -136,6 +136,21 @@ static inline bool nexthop_cmp(const struct nexthop *nh1,
return nh1 == nh2; return nh1 == nh2;
} }
static inline bool nexthop_is_fdb(const struct nexthop *nh)
{
if (nh->is_group) {
const struct nh_group *nh_grp;
nh_grp = rcu_dereference_rtnl(nh->nh_grp);
return nh_grp->fdb_nh;
} else {
const struct nh_info *nhi;
nhi = rcu_dereference_rtnl(nh->nh_info);
return nhi->fdb_nh;
}
}
static inline bool nexthop_is_multipath(const struct nexthop *nh) static inline bool nexthop_is_multipath(const struct nexthop *nh)
{ {
if (nh->is_group) { if (nh->is_group) {
......
...@@ -247,12 +247,11 @@ static int nh_fill_node(struct sk_buff *skb, struct nexthop *nh, ...@@ -247,12 +247,11 @@ static int nh_fill_node(struct sk_buff *skb, struct nexthop *nh,
if (nla_put_u32(skb, NHA_ID, nh->id)) if (nla_put_u32(skb, NHA_ID, nh->id))
goto nla_put_failure; goto nla_put_failure;
if (nh->is_fdb_nh && nla_put_flag(skb, NHA_FDB))
goto nla_put_failure;
if (nh->is_group) { if (nh->is_group) {
struct nh_group *nhg = rtnl_dereference(nh->nh_grp); struct nh_group *nhg = rtnl_dereference(nh->nh_grp);
if (nhg->fdb_nh && nla_put_flag(skb, NHA_FDB))
goto nla_put_failure;
if (nla_put_nh_group(skb, nhg)) if (nla_put_nh_group(skb, nhg))
goto nla_put_failure; goto nla_put_failure;
goto out; goto out;
...@@ -264,7 +263,10 @@ static int nh_fill_node(struct sk_buff *skb, struct nexthop *nh, ...@@ -264,7 +263,10 @@ static int nh_fill_node(struct sk_buff *skb, struct nexthop *nh,
if (nla_put_flag(skb, NHA_BLACKHOLE)) if (nla_put_flag(skb, NHA_BLACKHOLE))
goto nla_put_failure; goto nla_put_failure;
goto out; goto out;
} else if (!nh->is_fdb_nh) { } else if (nhi->fdb_nh) {
if (nla_put_flag(skb, NHA_FDB))
goto nla_put_failure;
} else {
const struct net_device *dev; const struct net_device *dev;
dev = nhi->fib_nhc.nhc_dev; dev = nhi->fib_nhc.nhc_dev;
...@@ -385,7 +387,7 @@ static void nexthop_notify(int event, struct nexthop *nh, struct nl_info *info) ...@@ -385,7 +387,7 @@ static void nexthop_notify(int event, struct nexthop *nh, struct nl_info *info)
} }
static bool valid_group_nh(struct nexthop *nh, unsigned int npaths, static bool valid_group_nh(struct nexthop *nh, unsigned int npaths,
struct netlink_ext_ack *extack) bool *is_fdb, struct netlink_ext_ack *extack)
{ {
if (nh->is_group) { if (nh->is_group) {
struct nh_group *nhg = rtnl_dereference(nh->nh_grp); struct nh_group *nhg = rtnl_dereference(nh->nh_grp);
...@@ -398,6 +400,7 @@ static bool valid_group_nh(struct nexthop *nh, unsigned int npaths, ...@@ -398,6 +400,7 @@ static bool valid_group_nh(struct nexthop *nh, unsigned int npaths,
"Multipath group can not be a nexthop within a group"); "Multipath group can not be a nexthop within a group");
return false; return false;
} }
*is_fdb = nhg->fdb_nh;
} else { } else {
struct nh_info *nhi = rtnl_dereference(nh->nh_info); struct nh_info *nhi = rtnl_dereference(nh->nh_info);
...@@ -406,6 +409,7 @@ static bool valid_group_nh(struct nexthop *nh, unsigned int npaths, ...@@ -406,6 +409,7 @@ static bool valid_group_nh(struct nexthop *nh, unsigned int npaths,
"Blackhole nexthop can not be used in a group with more than 1 path"); "Blackhole nexthop can not be used in a group with more than 1 path");
return false; return false;
} }
*is_fdb = nhi->fdb_nh;
} }
return true; return true;
...@@ -416,12 +420,13 @@ static int nh_check_attr_fdb_group(struct nexthop *nh, u8 *nh_family, ...@@ -416,12 +420,13 @@ static int nh_check_attr_fdb_group(struct nexthop *nh, u8 *nh_family,
{ {
struct nh_info *nhi; struct nh_info *nhi;
if (!nh->is_fdb_nh) { nhi = rtnl_dereference(nh->nh_info);
if (!nhi->fdb_nh) {
NL_SET_ERR_MSG(extack, "FDB nexthop group can only have fdb nexthops"); NL_SET_ERR_MSG(extack, "FDB nexthop group can only have fdb nexthops");
return -EINVAL; return -EINVAL;
} }
nhi = rtnl_dereference(nh->nh_info);
if (*nh_family == AF_UNSPEC) { if (*nh_family == AF_UNSPEC) {
*nh_family = nhi->family; *nh_family = nhi->family;
} else if (*nh_family != nhi->family) { } else if (*nh_family != nhi->family) {
...@@ -473,19 +478,20 @@ static int nh_check_attr_group(struct net *net, struct nlattr *tb[], ...@@ -473,19 +478,20 @@ static int nh_check_attr_group(struct net *net, struct nlattr *tb[],
nhg = nla_data(tb[NHA_GROUP]); nhg = nla_data(tb[NHA_GROUP]);
for (i = 0; i < len; ++i) { for (i = 0; i < len; ++i) {
struct nexthop *nh; struct nexthop *nh;
bool is_fdb_nh;
nh = nexthop_find_by_id(net, nhg[i].id); nh = nexthop_find_by_id(net, nhg[i].id);
if (!nh) { if (!nh) {
NL_SET_ERR_MSG(extack, "Invalid nexthop id"); NL_SET_ERR_MSG(extack, "Invalid nexthop id");
return -EINVAL; return -EINVAL;
} }
if (!valid_group_nh(nh, len, extack)) if (!valid_group_nh(nh, len, &is_fdb_nh, extack))
return -EINVAL; return -EINVAL;
if (nhg_fdb && nh_check_attr_fdb_group(nh, &nh_family, extack)) if (nhg_fdb && nh_check_attr_fdb_group(nh, &nh_family, extack))
return -EINVAL; return -EINVAL;
if (!nhg_fdb && nh->is_fdb_nh) { if (!nhg_fdb && is_fdb_nh) {
NL_SET_ERR_MSG(extack, "Non FDB nexthop group cannot have fdb nexthops"); NL_SET_ERR_MSG(extack, "Non FDB nexthop group cannot have fdb nexthops");
return -EINVAL; return -EINVAL;
} }
...@@ -553,13 +559,13 @@ struct nexthop *nexthop_select_path(struct nexthop *nh, int hash) ...@@ -553,13 +559,13 @@ struct nexthop *nexthop_select_path(struct nexthop *nh, int hash)
if (hash > atomic_read(&nhge->upper_bound)) if (hash > atomic_read(&nhge->upper_bound))
continue; continue;
if (nhge->nh->is_fdb_nh) nhi = rcu_dereference(nhge->nh->nh_info);
if (nhi->fdb_nh)
return nhge->nh; return nhge->nh;
/* nexthops always check if it is good and does /* nexthops always check if it is good and does
* not rely on a sysctl for this behavior * not rely on a sysctl for this behavior
*/ */
nhi = rcu_dereference(nhge->nh->nh_info);
switch (nhi->family) { switch (nhi->family) {
case AF_INET: case AF_INET:
if (ipv4_good_nh(&nhi->fib_nh)) if (ipv4_good_nh(&nhi->fib_nh))
...@@ -624,11 +630,7 @@ int fib6_check_nexthop(struct nexthop *nh, struct fib6_config *cfg, ...@@ -624,11 +630,7 @@ int fib6_check_nexthop(struct nexthop *nh, struct fib6_config *cfg,
struct netlink_ext_ack *extack) struct netlink_ext_ack *extack)
{ {
struct nh_info *nhi; struct nh_info *nhi;
bool is_fdb_nh;
if (nh->is_fdb_nh) {
NL_SET_ERR_MSG(extack, "Route cannot point to a fdb nexthop");
return -EINVAL;
}
/* fib6_src is unique to a fib6_info and limits the ability to cache /* fib6_src is unique to a fib6_info and limits the ability to cache
* routes in fib6_nh within a nexthop that is potentially shared * routes in fib6_nh within a nexthop that is potentially shared
...@@ -645,10 +647,17 @@ int fib6_check_nexthop(struct nexthop *nh, struct fib6_config *cfg, ...@@ -645,10 +647,17 @@ int fib6_check_nexthop(struct nexthop *nh, struct fib6_config *cfg,
nhg = rtnl_dereference(nh->nh_grp); nhg = rtnl_dereference(nh->nh_grp);
if (nhg->has_v4) if (nhg->has_v4)
goto no_v4_nh; goto no_v4_nh;
is_fdb_nh = nhg->fdb_nh;
} else { } else {
nhi = rtnl_dereference(nh->nh_info); nhi = rtnl_dereference(nh->nh_info);
if (nhi->family == AF_INET) if (nhi->family == AF_INET)
goto no_v4_nh; goto no_v4_nh;
is_fdb_nh = nhi->fdb_nh;
}
if (is_fdb_nh) {
NL_SET_ERR_MSG(extack, "Route cannot point to a fdb nexthop");
return -EINVAL;
} }
return 0; return 0;
...@@ -677,12 +686,9 @@ static int fib6_check_nh_list(struct nexthop *old, struct nexthop *new, ...@@ -677,12 +686,9 @@ static int fib6_check_nh_list(struct nexthop *old, struct nexthop *new,
return fib6_check_nexthop(new, NULL, extack); return fib6_check_nexthop(new, NULL, extack);
} }
static int nexthop_check_scope(struct nexthop *nh, u8 scope, static int nexthop_check_scope(struct nh_info *nhi, u8 scope,
struct netlink_ext_ack *extack) struct netlink_ext_ack *extack)
{ {
struct nh_info *nhi;
nhi = rtnl_dereference(nh->nh_info);
if (scope == RT_SCOPE_HOST && nhi->fib_nhc.nhc_gw_family) { if (scope == RT_SCOPE_HOST && nhi->fib_nhc.nhc_gw_family) {
NL_SET_ERR_MSG(extack, NL_SET_ERR_MSG(extack,
"Route with host scope can not have a gateway"); "Route with host scope can not have a gateway");
...@@ -704,29 +710,38 @@ static int nexthop_check_scope(struct nexthop *nh, u8 scope, ...@@ -704,29 +710,38 @@ static int nexthop_check_scope(struct nexthop *nh, u8 scope,
int fib_check_nexthop(struct nexthop *nh, u8 scope, int fib_check_nexthop(struct nexthop *nh, u8 scope,
struct netlink_ext_ack *extack) struct netlink_ext_ack *extack)
{ {
struct nh_info *nhi;
int err = 0; int err = 0;
if (nh->is_fdb_nh) {
NL_SET_ERR_MSG(extack, "Route cannot point to a fdb nexthop");
err = -EINVAL;
goto out;
}
if (nh->is_group) { if (nh->is_group) {
struct nh_group *nhg; struct nh_group *nhg;
nhg = rtnl_dereference(nh->nh_grp);
if (nhg->fdb_nh) {
NL_SET_ERR_MSG(extack, "Route cannot point to a fdb nexthop");
err = -EINVAL;
goto out;
}
if (scope == RT_SCOPE_HOST) { if (scope == RT_SCOPE_HOST) {
NL_SET_ERR_MSG(extack, "Route with host scope can not have multiple nexthops"); NL_SET_ERR_MSG(extack, "Route with host scope can not have multiple nexthops");
err = -EINVAL; err = -EINVAL;
goto out; goto out;
} }
nhg = rtnl_dereference(nh->nh_grp);
/* all nexthops in a group have the same scope */ /* all nexthops in a group have the same scope */
err = nexthop_check_scope(nhg->nh_entries[0].nh, scope, extack); nhi = rtnl_dereference(nhg->nh_entries[0].nh->nh_info);
err = nexthop_check_scope(nhi, scope, extack);
} else { } else {
err = nexthop_check_scope(nh, scope, extack); nhi = rtnl_dereference(nh->nh_info);
if (nhi->fdb_nh) {
NL_SET_ERR_MSG(extack, "Route cannot point to a fdb nexthop");
err = -EINVAL;
goto out;
}
err = nexthop_check_scope(nhi, scope, extack);
} }
out: out:
return err; return err;
} }
...@@ -787,6 +802,7 @@ static void remove_nh_grp_entry(struct net *net, struct nh_grp_entry *nhge, ...@@ -787,6 +802,7 @@ static void remove_nh_grp_entry(struct net *net, struct nh_grp_entry *nhge,
newg->has_v4 = nhg->has_v4; newg->has_v4 = nhg->has_v4;
newg->mpath = nhg->mpath; newg->mpath = nhg->mpath;
newg->fdb_nh = nhg->fdb_nh;
newg->num_nh = nhg->num_nh; newg->num_nh = nhg->num_nh;
/* copy old entries to new except the one getting removed */ /* copy old entries to new except the one getting removed */
...@@ -1216,7 +1232,7 @@ static struct nexthop *nexthop_create_group(struct net *net, ...@@ -1216,7 +1232,7 @@ static struct nexthop *nexthop_create_group(struct net *net,
} }
if (cfg->nh_fdb) if (cfg->nh_fdb)
nh->is_fdb_nh = 1; nhg->fdb_nh = 1;
rcu_assign_pointer(nh->nh_grp, nhg); rcu_assign_pointer(nh->nh_grp, nhg);
...@@ -1255,7 +1271,7 @@ static int nh_create_ipv4(struct net *net, struct nexthop *nh, ...@@ -1255,7 +1271,7 @@ static int nh_create_ipv4(struct net *net, struct nexthop *nh,
goto out; goto out;
} }
if (nh->is_fdb_nh) if (nhi->fdb_nh)
goto out; goto out;
/* sets nh_dev if successful */ /* sets nh_dev if successful */
...@@ -1326,7 +1342,7 @@ static struct nexthop *nexthop_create(struct net *net, struct nh_config *cfg, ...@@ -1326,7 +1342,7 @@ static struct nexthop *nexthop_create(struct net *net, struct nh_config *cfg,
nhi->fib_nhc.nhc_scope = RT_SCOPE_LINK; nhi->fib_nhc.nhc_scope = RT_SCOPE_LINK;
if (cfg->nh_fdb) if (cfg->nh_fdb)
nh->is_fdb_nh = 1; nhi->fdb_nh = 1;
if (cfg->nh_blackhole) { if (cfg->nh_blackhole) {
nhi->reject_nh = 1; nhi->reject_nh = 1;
...@@ -1349,7 +1365,7 @@ static struct nexthop *nexthop_create(struct net *net, struct nh_config *cfg, ...@@ -1349,7 +1365,7 @@ static struct nexthop *nexthop_create(struct net *net, struct nh_config *cfg,
} }
/* add the entry to the device based hash */ /* add the entry to the device based hash */
if (!nh->is_fdb_nh) if (!nhi->fdb_nh)
nexthop_devhash_add(net, nhi); nexthop_devhash_add(net, nhi);
rcu_assign_pointer(nh->nh_info, nhi); rcu_assign_pointer(nh->nh_info, nhi);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment