On Mon, May 4, 2020 at 8:23 PM David Ahern <dsah...@gmail.com> wrote: > > On 5/4/20 4:28 PM, Roopa Prabhu wrote: > > include/net/nexthop.h | 14 ++++++ > > include/uapi/linux/nexthop.h | 1 + > > net/ipv4/nexthop.c | 101 > > +++++++++++++++++++++++++++++++++---------- > > 3 files changed, 93 insertions(+), 23 deletions(-) > > pretty cool that you can extend this from routes to fdb entries with > such a small change stat.
yep, exactly. everything fell into place so neatly. > > > > > diff --git a/include/net/nexthop.h b/include/net/nexthop.h > > index c440ccc..3ad4e97 100644 > > --- a/include/net/nexthop.h > > +++ b/include/net/nexthop.h > > @@ -26,6 +26,7 @@ struct nh_config { > > u8 nh_family; > > u8 nh_protocol; > > u8 nh_blackhole; > > + u8 nh_fdb; > > u32 nh_flags; > > > > int nh_ifindex; > > @@ -52,6 +53,7 @@ struct nh_info { > > > > u8 family; > > bool reject_nh; > > + bool fdb_nh; > > > > union { > > struct fib_nh_common fib_nhc; > > @@ -80,6 +82,7 @@ struct nexthop { > > struct rb_node rb_node; /* entry on netns rbtree */ > > struct list_head fi_list; /* v4 entries using nh */ > > struct list_head f6i_list; /* v6 entries using nh */ > > + struct list_head fdb_list; /* fdb entries using this nh */ > > struct list_head grp_list; /* nh group entries using this nh > > */ > > struct net *net; > > > > @@ -88,6 +91,7 @@ struct nexthop { > > u8 protocol; /* app managing this nh */ > > u8 nh_flags; > > bool is_group; > > + bool is_fdb_nh; > > > > refcount_t refcnt; > > struct rcu_head rcu; > > @@ -304,4 +308,14 @@ static inline void nexthop_path_fib6_result(struct > > fib6_result *res, int hash) > > int nexthop_for_each_fib6_nh(struct nexthop *nh, > > int (*cb)(struct fib6_nh *nh, void *arg), > > void *arg); > > + > > +static inline struct nh_info *nexthop_path_fdb(struct nexthop *nh, u32 > > hash) > > this is used in the next patch. Any way to not leak the nh_info struct > into vxlan code? Right now nh_info is only used in nexthop.{c,h}. > yes, sure I think I can do that. I will add an api to get the nh family and use nh everywhere. > > +{ > > + struct nh_info *nhi; > > + > > + nh = nexthop_select_path(nh, hash); > > + nhi = rcu_dereference(nh->nh_info); > > + > > + return nhi; > > +} > > #endif > > diff --git a/include/uapi/linux/nexthop.h b/include/uapi/linux/nexthop.h > > index 7b61867..19a234a 100644 > > --- a/include/uapi/linux/nexthop.h > > +++ b/include/uapi/linux/nexthop.h > > @@ -48,6 +48,7 @@ enum { > > */ > > NHA_GROUPS, /* flag; only return nexthop groups in dump */ > > NHA_MASTER, /* u32; only return nexthops with given master dev */ > > + NHA_FDB, /* nexthop belongs to a bridge fdb */ > > please add the 'type' to the comment; I tried to make this uapi file > completely self-documenting. ie., no one should have to consult the code > to know what kind of attribute NHA_FDB is. > yep, will do. not sure how i missed it, > > > > __NHA_MAX, > > }; > > diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c > > index 3957364..98f8d2a 100644 > > --- a/net/ipv4/nexthop.c > > +++ b/net/ipv4/nexthop.c > > @@ -33,6 +33,7 @@ static const struct nla_policy rtm_nh_policy[NHA_MAX + 1] > > = { > > [NHA_ENCAP] = { .type = NLA_NESTED }, > > [NHA_GROUPS] = { .type = NLA_FLAG }, > > [NHA_MASTER] = { .type = NLA_U32 }, > > + [NHA_FDB] = { .type = NLA_FLAG }, > > }; > > > > static unsigned int nh_dev_hashfn(unsigned int val) > > @@ -107,6 +108,7 @@ static struct nexthop *nexthop_alloc(void) > > INIT_LIST_HEAD(&nh->fi_list); > > INIT_LIST_HEAD(&nh->f6i_list); > > INIT_LIST_HEAD(&nh->grp_list); > > + INIT_LIST_HEAD(&nh->fdb_list); > > } > > return nh; > > } > > @@ -227,6 +229,9 @@ static int nh_fill_node(struct sk_buff *skb, struct > > nexthop *nh, > > if (nla_put_u32(skb, NHA_ID, nh->id)) > > goto nla_put_failure; > > > > + if (nh->is_fdb_nh && nla_put_flag(skb, NHA_FDB)) > > + goto nla_put_failure; > > + > > if (nh->is_group) { > > struct nh_group *nhg = rtnl_dereference(nh->nh_grp); > > > > @@ -241,7 +246,7 @@ static int nh_fill_node(struct sk_buff *skb, struct > > nexthop *nh, > > if (nla_put_flag(skb, NHA_BLACKHOLE)) > > goto nla_put_failure; > > goto out; > > - } else { > > + } else if (!nh->is_fdb_nh) { > > const struct net_device *dev; > > > > dev = nhi->fib_nhc.nhc_dev; > > @@ -393,6 +398,7 @@ static int nh_check_attr_group(struct net *net, struct > > nlattr *tb[], > > unsigned int len = nla_len(tb[NHA_GROUP]); > > struct nexthop_grp *nhg; > > unsigned int i, j; > > + u8 nhg_fdb = 0; > > > > if (len & (sizeof(struct nexthop_grp) - 1)) { > > NL_SET_ERR_MSG(extack, > > @@ -421,6 +427,8 @@ static int nh_check_attr_group(struct net *net, struct > > nlattr *tb[], > > } > > } > > > > + if (tb[NHA_FDB]) > > + nhg_fdb = 1; > > nhg = nla_data(tb[NHA_GROUP]); > > for (i = 0; i < len; ++i) { > > struct nexthop *nh; > > @@ -432,11 +440,16 @@ static int nh_check_attr_group(struct net *net, > > struct nlattr *tb[], > > } > > if (!valid_group_nh(nh, len, extack)) > > return -EINVAL; > > + if (nhg_fdb && !nh->is_fdb_nh) { > > + NL_SET_ERR_MSG(extack, "FDB Multipath group can only > > have fdb nexthops"); > > + return -EINVAL; > > + } > > you should check the reverse as well -- non-nhg_fdb can not use an fdb > nh. ie., a group can not be a mix of fdb and route entries. > > Make sure the selftests covers the permutations as well. > yep, will do > > } > > for (i = NHA_GROUP + 1; i < __NHA_MAX; ++i) { > > if (!tb[i]) > > continue; > > - > > + if (tb[NHA_FDB]) > > + continue; > > NL_SET_ERR_MSG(extack, > > "No other attributes can be set in nexthop > > groups"); > > return -EINVAL; > >