[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <44299e1f-1e82-83e3-06c6-79a214790689@cumulusnetworks.com>
Date: Wed, 28 Feb 2018 00:38:26 +0200
From: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To: Yuval Mintz <yuvalm@...lanox.com>, netdev@...r.kernel.org
Cc: mlxsw@...lanox.com, kuznet@....inr.ac.ru, yoshfuji@...ux-ipv6.org
Subject: Re: [PATCH net-next 07/11] ipmr, ip6mr: Unite logic for searching in
MFC cache
On 27/02/18 20:58, Yuval Mintz wrote:
> ipmr and ip6mr utilize the exact same methods for searching the
> hashed resolved connections, difference being only in the construction
> of the hash comparison key.
>
> In order to unite the flow, introduce an mr_table operation set that
> would contain the protocol specific information required for common
> flows, in this case - the hash parameters and a comparison key
> representing a (*,*) route.
>
> Signed-off-by: Yuval Mintz <yuvalm@...lanox.com>
> ---
> include/linux/mroute_base.h | 52 +++++++++++++++++++++++++++++--
> net/ipv4/ipmr.c | 71 ++++++++++---------------------------------
> net/ipv4/ipmr_base.c | 54 +++++++++++++++++++++++++++++++--
> net/ipv6/ip6mr.c | 74 +++++++++++----------------------------------
> 4 files changed, 134 insertions(+), 117 deletions(-)
>
> diff --git a/include/linux/mroute_base.h b/include/linux/mroute_base.h
> index 2769e2f..18a1d75 100644
> --- a/include/linux/mroute_base.h
> +++ b/include/linux/mroute_base.h
> @@ -89,10 +89,23 @@ struct mr_mfc {
> struct rcu_head rcu;
> };
>
> +struct mr_table;
> +
> +/**
> + * struct mr_table_ops - callbacks and info for protocol-specific ops
> + * rht_params: parameters for accessing the MFC hash
> + * cmparg_any: a hash key to be used for matching on (*,*) routes
kernel doc style uses @variable
> + */
> +struct mr_table_ops {
> + const struct rhashtable_params *rht_params;
> + void *cmparg_any;
> +};
> +
> /**
> * struct mr_table - a multicast routing table
> * @list: entry within a list of multicast routing tables
> * @net: net where this table belongs
> + * @op: protocol specific operations
nit: s/op/ops/
> * @id: identifier of the table
> * @mroute_sk: socket associated with the table
> * @ipmr_expire_timer: timer for handling unresolved routes
> @@ -109,6 +122,7 @@ struct mr_mfc {
> struct mr_table {
> struct list_head list;
> possible_net_t net;
> + struct mr_table_ops ops;
> u32 id;
> struct sock __rcu *mroute_sk;
> struct timer_list ipmr_expire_timer;
> @@ -133,10 +147,19 @@ void vif_device_init(struct vif_device *v,
>
> struct mr_table *
> mr_table_alloc(struct net *net, u32 id,
> - const struct rhashtable_params *rht_params,
> + struct mr_table_ops *ops,
> void (*expire_func)(struct timer_list *t),
> void (*table_set)(struct mr_table *mrt,
> struct net *net));
> +
> +/* These actually return 'struct mr_mfc *', but to avoid need for explicit
> + * castings they simply return void.
> + */
> +void *mr_mfc_find_parent(struct mr_table *mrt,
> + void *hasharg, int parent);
> +void *mr_mfc_find_any_parent(struct mr_table *mrt, int vifi);
> +void *mr_mfc_find_any(struct mr_table *mrt, int vifi, void *hasharg);
> +
> #else
> static inline void vif_device_init(struct vif_device *v,
> struct net_device *dev,
> @@ -147,14 +170,37 @@ static inline void vif_device_init(struct vif_device *v,
> {
> }
>
> -static inline struct mr_table *
> +static inline void *
> mr_table_alloc(struct net *net, u32 id,
> - const struct rhashtable_params *rht_params,
> + struct mr_table_ops *ops,
> void (*expire_func)(struct timer_list *t),
> void (*table_set)(struct mr_table *mrt,
> struct net *net))
> {
> return NULL;
> }
> +
> +static inline void *mr_mfc_find_parent(struct mr_table *mrt,
> + void *hasharg, int parent)
> +{
> + return NULL;
> +}
> +
> +static inline void *mr_mfc_find_any_parent(struct mr_table *mrt,
> + int vifi)
> +{
> + return NULL;
> +}
> +
> +static inline struct mr_mfc *mr_mfc_find_any(struct mr_table *mrt,
> + int vifi, void *hasharg)
> +{
> + return NULL;
> +}
> #endif
> +
> +static inline void *mr_mfc_find(struct mr_table *mrt, void *hasharg)
> +{
> + return mr_mfc_find_parent(mrt, hasharg, -1);
> +}
> #endif
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index 238a579..d0bf021 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -359,6 +359,16 @@ static void ipmr_new_table_set(struct mr_table *mrt,
> #endif
> }
>
> +static struct mfc_cache_cmp_arg ipmr_mr_table_ops_cmparg_any = {
> + .mfc_mcastgrp = htonl(INADDR_ANY),
> + .mfc_origin = htonl(INADDR_ANY),
> +};
> +
> +static struct mr_table_ops ipmr_mr_table_ops = {
> + .rht_params = &ipmr_rht_params,
> + .cmparg_any = &ipmr_mr_table_ops_cmparg_any,
> +};
> +
> static struct mr_table *ipmr_new_table(struct net *net, u32 id)
> {
> struct mr_table *mrt;
> @@ -371,7 +381,7 @@ static struct mr_table *ipmr_new_table(struct net *net, u32 id)
> if (mrt)
> return mrt;
>
> - return mr_table_alloc(net, id, &ipmr_rht_params,
> + return mr_table_alloc(net, id, &ipmr_mr_table_ops,
> ipmr_expire_process, ipmr_new_table_set);
> }
>
> @@ -972,33 +982,8 @@ static struct mfc_cache *ipmr_cache_find(struct mr_table *mrt,
> .mfc_mcastgrp = mcastgrp,
> .mfc_origin = origin
> };
> - struct rhlist_head *tmp, *list;
> - struct mr_mfc *c;
> -
> - list = rhltable_lookup(&mrt->mfc_hash, &arg, ipmr_rht_params);
> - rhl_for_each_entry_rcu(c, tmp, list, mnode)
> - return (struct mfc_cache *)c;
> -
> - return NULL;
> -}
> -
> -/* Look for a (*,*,oif) entry */
> -static struct mfc_cache *ipmr_cache_find_any_parent(struct mr_table *mrt,
> - int vifi)
> -{
> - struct mfc_cache_cmp_arg arg = {
> - .mfc_mcastgrp = htonl(INADDR_ANY),
> - .mfc_origin = htonl(INADDR_ANY)
> - };
> - struct rhlist_head *tmp, *list;
> - struct mr_mfc *c;
> -
> - list = rhltable_lookup(&mrt->mfc_hash, &arg, ipmr_rht_params);
> - rhl_for_each_entry_rcu(c, tmp, list, mnode)
> - if (c->mfc_un.res.ttls[vifi] < 255)
> - return (struct mfc_cache *)c;
>
> - return NULL;
> + return mr_mfc_find(mrt, &arg);
> }
>
> /* Look for a (*,G) entry */
> @@ -1009,27 +994,10 @@ static struct mfc_cache *ipmr_cache_find_any(struct mr_table *mrt,
> .mfc_mcastgrp = mcastgrp,
> .mfc_origin = htonl(INADDR_ANY)
> };
> - struct rhlist_head *tmp, *list;
> - struct mr_mfc *c;
>
> if (mcastgrp == htonl(INADDR_ANY))
> - goto skip;
> -
> - list = rhltable_lookup(&mrt->mfc_hash, &arg, ipmr_rht_params);
> - rhl_for_each_entry_rcu(c, tmp, list, mnode) {
> - struct mfc_cache *proxy;
> -
> - if (c->mfc_un.res.ttls[vifi] < 255)
> - return (struct mfc_cache *)c;
> -
> - /* It's ok if the vifi is part of the static tree */
> - proxy = ipmr_cache_find_any_parent(mrt, c->mfc_parent);
> - if (proxy && proxy->_c.mfc_un.res.ttls[vifi] < 255)
> - return (struct mfc_cache *)c;
> - }
> -
> -skip:
> - return ipmr_cache_find_any_parent(mrt, vifi);
> + return mr_mfc_find_any_parent(mrt, vifi);
> + return mr_mfc_find_any(mrt, vifi, &arg);
> }
>
> /* Look for a (S,G,iif) entry if parent != -1 */
> @@ -1041,15 +1009,8 @@ static struct mfc_cache *ipmr_cache_find_parent(struct mr_table *mrt,
> .mfc_mcastgrp = mcastgrp,
> .mfc_origin = origin,
> };
> - struct rhlist_head *tmp, *list;
> - struct mr_mfc *c;
>
> - list = rhltable_lookup(&mrt->mfc_hash, &arg, ipmr_rht_params);
> - rhl_for_each_entry_rcu(c, tmp, list, mnode)
> - if (parent == -1 || parent == c->mfc_parent)
> - return (struct mfc_cache *)c;
> -
> - return NULL;
> + return mr_mfc_find_parent(mrt, &arg, parent);
> }
>
> /* Allocate a multicast cache entry */
> @@ -2010,7 +1971,7 @@ static void ip_mr_forward(struct net *net, struct mr_table *mrt,
> /* For an (*,G) entry, we only check that the incomming
> * interface is part of the static tree.
> */
> - cache_proxy = ipmr_cache_find_any_parent(mrt, vif);
> + cache_proxy = mr_mfc_find_any_parent(mrt, vif);
> if (cache_proxy &&
> cache_proxy->_c.mfc_un.res.ttls[true_vifi] < 255)
> goto forward;
> diff --git a/net/ipv4/ipmr_base.c b/net/ipv4/ipmr_base.c
> index 3e21a58..172f92a 100644
> --- a/net/ipv4/ipmr_base.c
> +++ b/net/ipv4/ipmr_base.c
> @@ -29,7 +29,7 @@ EXPORT_SYMBOL(vif_device_init);
>
> struct mr_table *
> mr_table_alloc(struct net *net, u32 id,
> - const struct rhashtable_params *rht_params,
> + struct mr_table_ops *ops,
> void (*expire_func)(struct timer_list *t),
> void (*table_set)(struct mr_table *mrt,
> struct net *net))
> @@ -42,7 +42,8 @@ mr_table_alloc(struct net *net, u32 id,
> mrt->id = id;
> write_pnet(&mrt->net, net);
>
> - rhltable_init(&mrt->mfc_hash, rht_params);
> + mrt->ops = *ops;
> + rhltable_init(&mrt->mfc_hash, mrt->ops.rht_params);
> INIT_LIST_HEAD(&mrt->mfc_cache_list);
> INIT_LIST_HEAD(&mrt->mfc_unres_queue);
>
> @@ -53,3 +54,52 @@ mr_table_alloc(struct net *net, u32 id,
> return mrt;
> }
> EXPORT_SYMBOL(mr_table_alloc);
> +
> +void *mr_mfc_find_parent(struct mr_table *mrt, void *hasharg, int parent)
> +{
> + struct rhlist_head *tmp, *list;
> + struct mr_mfc *c;
> +
> + list = rhltable_lookup(&mrt->mfc_hash, hasharg, *mrt->ops.rht_params);
> + rhl_for_each_entry_rcu(c, tmp, list, mnode)
> + if (parent == -1 || parent == c->mfc_parent)
> + return c;
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL(mr_mfc_find_parent);
> +
> +void *mr_mfc_find_any_parent(struct mr_table *mrt, int vifi)
> +{
> + struct rhlist_head *tmp, *list;
> + struct mr_mfc *c;
> +
> + list = rhltable_lookup(&mrt->mfc_hash, mrt->ops.cmparg_any,
> + *mrt->ops.rht_params);
> + rhl_for_each_entry_rcu(c, tmp, list, mnode)
> + if (c->mfc_un.res.ttls[vifi] < 255)
> + return c;
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL(mr_mfc_find_any_parent);
> +
> +void *mr_mfc_find_any(struct mr_table *mrt, int vifi, void *hasharg)
> +{
> + struct rhlist_head *tmp, *list;
> + struct mr_mfc *c, *proxy;
> +
> + list = rhltable_lookup(&mrt->mfc_hash, hasharg, *mrt->ops.rht_params);
> + rhl_for_each_entry_rcu(c, tmp, list, mnode) {
> + if (c->mfc_un.res.ttls[vifi] < 255)
> + return c;
> +
> + /* It's ok if the vifi is part of the static tree */
> + proxy = mr_mfc_find_any_parent(mrt, c->mfc_parent);
> + if (proxy && proxy->mfc_un.res.ttls[vifi] < 255)
> + return c;
> + }
> +
> + return mr_mfc_find_any_parent(mrt, vifi);
> +}
> +EXPORT_SYMBOL(mr_mfc_find_any);
> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
> index 4db4339..93a8c02 100644
> --- a/net/ipv6/ip6mr.c
> +++ b/net/ipv6/ip6mr.c
> @@ -302,6 +302,16 @@ static void ip6mr_new_table_set(struct mr_table *mrt,
> #endif
> }
>
> +static struct mfc6_cache_cmp_arg ip6mr_mr_table_ops_cmparg_any = {
> + .mf6c_origin = IN6ADDR_ANY_INIT,
> + .mf6c_mcastgrp = IN6ADDR_ANY_INIT,
> +};
> +
> +static struct mr_table_ops ip6mr_mr_table_ops = {
> + .rht_params = &ip6mr_rht_params,
> + .cmparg_any = &ip6mr_mr_table_ops_cmparg_any,
> +};
> +
> static struct mr_table *ip6mr_new_table(struct net *net, u32 id)
> {
> struct mr_table *mrt;
> @@ -310,7 +320,7 @@ static struct mr_table *ip6mr_new_table(struct net *net, u32 id)
> if (mrt)
> return mrt;
>
> - return mr_table_alloc(net, id, &ip6mr_rht_params,
> + return mr_table_alloc(net, id, &ip6mr_mr_table_ops,
> ipmr_expire_process, ip6mr_new_table_set);
> }
>
> @@ -988,33 +998,8 @@ static struct mfc6_cache *ip6mr_cache_find(struct mr_table *mrt,
> .mf6c_origin = *origin,
> .mf6c_mcastgrp = *mcastgrp,
> };
> - struct rhlist_head *tmp, *list;
> - struct mr_mfc *c;
> -
> - list = rhltable_lookup(&mrt->mfc_hash, &arg, ip6mr_rht_params);
> - rhl_for_each_entry_rcu(c, tmp, list, mnode)
> - return (struct mfc6_cache *)c;
>
> - return NULL;
> -}
> -
> -/* Look for a (*,*,oif) entry */
> -static struct mfc6_cache *ip6mr_cache_find_any_parent(struct mr_table *mrt,
> - mifi_t mifi)
> -{
> - struct mfc6_cache_cmp_arg arg = {
> - .mf6c_origin = in6addr_any,
> - .mf6c_mcastgrp = in6addr_any,
> - };
> - struct rhlist_head *tmp, *list;
> - struct mr_mfc *c;
> -
> - list = rhltable_lookup(&mrt->mfc_hash, &arg, ip6mr_rht_params);
> - rhl_for_each_entry_rcu(c, tmp, list, mnode)
> - if (c->mfc_un.res.ttls[mifi] < 255)
> - return (struct mfc6_cache *)c;
> -
> - return NULL;
> + return mr_mfc_find(mrt, &arg);
> }
>
> /* Look for a (*,G) entry */
> @@ -1026,26 +1011,10 @@ static struct mfc6_cache *ip6mr_cache_find_any(struct mr_table *mrt,
> .mf6c_origin = in6addr_any,
> .mf6c_mcastgrp = *mcastgrp,
> };
> - struct rhlist_head *tmp, *list;
> - struct mr_mfc *c;
> - struct mfc6_cache *proxy;
>
> if (ipv6_addr_any(mcastgrp))
> - goto skip;
> -
> - list = rhltable_lookup(&mrt->mfc_hash, &arg, ip6mr_rht_params);
> - rhl_for_each_entry_rcu(c, tmp, list, mnode) {
> - if (c->mfc_un.res.ttls[mifi] < 255)
> - return (struct mfc6_cache *)c;
> -
> - /* It's ok if the mifi is part of the static tree */
> - proxy = ip6mr_cache_find_any_parent(mrt, c->mfc_parent);
> - if (proxy && proxy->_c.mfc_un.res.ttls[mifi] < 255)
> - return (struct mfc6_cache *)c;
> - }
> -
> -skip:
> - return ip6mr_cache_find_any_parent(mrt, mifi);
> + return mr_mfc_find_any_parent(mrt, mifi);
> + return mr_mfc_find_any(mrt, mifi, &arg);
> }
>
> /* Look for a (S,G,iif) entry if parent != -1 */
> @@ -1059,20 +1028,11 @@ ip6mr_cache_find_parent(struct mr_table *mrt,
> .mf6c_origin = *origin,
> .mf6c_mcastgrp = *mcastgrp,
> };
> - struct rhlist_head *tmp, *list;
> - struct mr_mfc *c;
> -
> - list = rhltable_lookup(&mrt->mfc_hash, &arg, ip6mr_rht_params);
> - rhl_for_each_entry_rcu(c, tmp, list, mnode)
> - if (parent == -1 || parent == c->mfc_parent)
> - return (struct mfc6_cache *)c;
>
> - return NULL;
> + return mr_mfc_find_parent(mrt, &arg, parent);
> }
>
> -/*
> - * Allocate a multicast cache entry
> - */
> +/* Allocate a multicast cache entry */
> static struct mfc6_cache *ip6mr_cache_alloc(void)
> {
> struct mfc6_cache *c = kmem_cache_zalloc(mrt_cachep, GFP_KERNEL);
> @@ -2112,7 +2072,7 @@ static void ip6_mr_forward(struct net *net, struct mr_table *mrt,
> * interface is part of the static tree.
> */
> rcu_read_lock();
> - cache_proxy = ip6mr_cache_find_any_parent(mrt, vif);
> + cache_proxy = mr_mfc_find_any_parent(mrt, vif);
> if (cache_proxy &&
> cache_proxy->_c.mfc_un.res.ttls[true_vifi] < 255) {
> rcu_read_unlock();
>
Powered by blists - more mailing lists