From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f68.google.com ([74.125.82.68]:38340 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751974AbeB0Wia (ORCPT ); Tue, 27 Feb 2018 17:38:30 -0500 Received: by mail-wm0-f68.google.com with SMTP id z9so1514462wmb.3 for ; Tue, 27 Feb 2018 14:38:29 -0800 (PST) Subject: Re: [PATCH net-next 07/11] ipmr, ip6mr: Unite logic for searching in MFC cache To: Yuval Mintz , netdev@vger.kernel.org Cc: mlxsw@mellanox.com, kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org References: <1519757908-32863-1-git-send-email-yuvalm@mellanox.com> <1519757908-32863-8-git-send-email-yuvalm@mellanox.com> From: Nikolay Aleksandrov Message-ID: <44299e1f-1e82-83e3-06c6-79a214790689@cumulusnetworks.com> Date: Wed, 28 Feb 2018 00:38:26 +0200 MIME-Version: 1.0 In-Reply-To: <1519757908-32863-8-git-send-email-yuvalm@mellanox.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: 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 > --- > 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(); >