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.
>
> 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}.
> +{
> + 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.
>
> __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.
> }
> 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;
