[net-next,v4,07/17] net: sched: protect filter_chain list with filter_chain_lock mutex

Message ID	20190211085548.7190-8-vladbu@mellanox.com
State	Accepted
Delegated to:	David Miller
Headers	show Return-Path: <netdev-owner@vger.kernel.org> From: Vlad Buslov <vladbu@mellanox.com> To: netdev@vger.kernel.org Cc: jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net, ast@kernel.org, daniel@iogearbox.net, Vlad Buslov <vladbu@mellanox.com> Subject: [PATCH net-next v4 07/17] net: sched: protect filter_chain list with filter_chain_lock mutex Date: Mon, 11 Feb 2019 10:55:38 +0200 Message-Id: <20190211085548.7190-8-vladbu@mellanox.com> In-Reply-To: <20190211085548.7190-1-vladbu@mellanox.com> References: <20190211085548.7190-1-vladbu@mellanox.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk
Series	Refactor classifier API to work with chain/classifiers without rtnl lock \| expand [net-next,v4,00/17] Refactor classifier API to work with chain/classifiers without rtnl lock [net-next,v4,01/17] net: sched: protect block state with mutex [net-next,v4,02/17] net: sched: protect chain->explicitly_created with block->lock [net-next,v4,03/17] net: sched: refactor tc_ctl_chain() to use block->lock [net-next,v4,04/17] net: sched: protect block->chain0 with block->lock [net-next,v4,05/17] net: sched: traverse chains in block with tcf_get_next_chain() [net-next,v4,06/17] net: sched: protect chain template accesses with block lock [net-next,v4,07/17] net: sched: protect filter_chain list with filter_chain_lock mutex [net-next,v4,08/17] net: sched: introduce reference counting for tcf_proto [net-next,v4,09/17] net: sched: traverse classifiers in chain with tcf_get_next_proto() [net-next,v4,10/17] net: sched: refactor tp insert/delete for concurrent execution [net-next,v4,11/17] net: sched: prevent insertion of new classifiers during chain flush [net-next,v4,12/17] net: sched: track rtnl lock status when validating extensions [net-next,v4,13/17] net: sched: extend proto ops with 'put' callback [net-next,v4,14/17] net: sched: extend proto ops to support unlocked classifiers [net-next,v4,15/17] net: sched: add flags to Qdisc class ops struct [net-next,v4,16/17] net: sched: refactor tcf_block_find() into standalone functions [net-next,v4,17/17] net: sched: unlock rules update API

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index 31b8ea66a47d..85993d7efee6 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -341,6 +341,8 @@ struct qdisc_skb_cb { typedef void tcf_chain_head_change_t(struct tcf_proto *tp_head, void *priv); struct tcf_chain { + /* Protects filter_chain. */ + struct mutex filter_chain_lock; struct tcf_proto __rcu *filter_chain; struct list_head list; struct tcf_block *block; @@ -374,6 +376,21 @@ struct tcf_block { struct rcu_head rcu; }; +#ifdef CONFIG_PROVE_LOCKING +static inline bool lockdep_tcf_chain_is_locked(struct tcf_chain *chain) +{ + return lockdep_is_held(&chain->filter_chain_lock); +} +#else +static inline bool lockdep_tcf_chain_is_locked(struct tcf_block *chain) +{ + return true; +} +#endif /* #ifdef CONFIG_PROVE_LOCKING */ + +#define tcf_chain_dereference(p, chain) \ + rcu_dereference_protected(p, lockdep_tcf_chain_is_locked(chain)) + static inline void tcf_block_offload_inc(struct tcf_block *block, u32 *flags) { if (*flags & TCA_CLS_FLAGS_IN_HW) diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 0dcce8b0c7b4..3fce30ae9a9b 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -221,6 +221,7 @@ static struct tcf_chain *tcf_chain_create(struct tcf_block *block, if (!chain) return NULL; list_add_tail(&chain->list, &block->chain_list); + mutex_init(&chain->filter_chain_lock); chain->block = block; chain->index = chain_index; chain->refcnt = 1; @@ -280,6 +281,7 @@ static void tcf_chain_destroy(struct tcf_chain *chain, bool free_block) { struct tcf_block *block = chain->block; + mutex_destroy(&chain->filter_chain_lock); kfree(chain); if (free_block) tcf_block_destroy(block); @@ -443,9 +445,13 @@ static void tcf_chain_put_explicitly_created(struct tcf_chain *chain) static void tcf_chain_flush(struct tcf_chain *chain) { - struct tcf_proto *tp = rtnl_dereference(chain->filter_chain); + struct tcf_proto *tp; + mutex_lock(&chain->filter_chain_lock); + tp = tcf_chain_dereference(chain->filter_chain, chain); tcf_chain0_head_change(chain, NULL); + mutex_unlock(&chain->filter_chain_lock); + while (tp) { RCU_INIT_POINTER(chain->filter_chain, tp->next); tcf_proto_destroy(tp, NULL); @@ -785,11 +791,29 @@ tcf_chain0_head_change_cb_add(struct tcf_block *block, mutex_lock(&block->lock); chain0 = block->chain0.chain; - if (chain0 && chain0->filter_chain) - tcf_chain_head_change_item(item, chain0->filter_chain); - list_add(&item->list, &block->chain0.filter_chain_list); + if (chain0) + tcf_chain_hold(chain0); + else + list_add(&item->list, &block->chain0.filter_chain_list); mutex_unlock(&block->lock); + if (chain0) { + struct tcf_proto *tp_head; + + mutex_lock(&chain0->filter_chain_lock); + + tp_head = tcf_chain_dereference(chain0->filter_chain, chain0); + if (tp_head) + tcf_chain_head_change_item(item, tp_head); + + mutex_lock(&block->lock); + list_add(&item->list, &block->chain0.filter_chain_list); + mutex_unlock(&block->lock); + + mutex_unlock(&chain0->filter_chain_lock); + tcf_chain_put(chain0); + } + return 0; } @@ -1464,9 +1488,10 @@ struct tcf_chain_info { struct tcf_proto __rcu *next; }; -static struct tcf_proto *tcf_chain_tp_prev(struct tcf_chain_info *chain_info) +static struct tcf_proto *tcf_chain_tp_prev(struct tcf_chain *chain, + struct tcf_chain_info *chain_info) { - return rtnl_dereference(*chain_info->pprev); + return tcf_chain_dereference(*chain_info->pprev, chain); } static void tcf_chain_tp_insert(struct tcf_chain *chain, @@ -1475,7 +1500,7 @@ static void tcf_chain_tp_insert(struct tcf_chain *chain, { if (*chain_info->pprev == chain->filter_chain) tcf_chain0_head_change(chain, tp); - RCU_INIT_POINTER(tp->next, tcf_chain_tp_prev(chain_info)); + RCU_INIT_POINTER(tp->next, tcf_chain_tp_prev(chain, chain_info)); rcu_assign_pointer(*chain_info->pprev, tp); tcf_chain_hold(chain); } @@ -1484,7 +1509,7 @@ static void tcf_chain_tp_remove(struct tcf_chain *chain, struct tcf_chain_info *chain_info, struct tcf_proto *tp) { - struct tcf_proto *next = rtnl_dereference(chain_info->next); + struct tcf_proto *next = tcf_chain_dereference(chain_info->next, chain); if (tp == chain->filter_chain) tcf_chain0_head_change(chain, next); @@ -1502,7 +1527,8 @@ static struct tcf_proto *tcf_chain_tp_find(struct tcf_chain *chain, /* Check the chain for existence of proto-tcf with this priority */ for (pprev = &chain->filter_chain; - (tp = rtnl_dereference(*pprev)); pprev = &tp->next) { + (tp = tcf_chain_dereference(*pprev, chain)); + pprev = &tp->next) { if (tp->prio >= prio) { if (tp->prio == prio) { if (prio_allocate || @@ -1710,12 +1736,13 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n, goto errout; } + mutex_lock(&chain->filter_chain_lock); tp = tcf_chain_tp_find(chain, &chain_info, protocol, prio, prio_allocate); if (IS_ERR(tp)) { NL_SET_ERR_MSG(extack, "Filter with specified priority/protocol not found"); err = PTR_ERR(tp); - goto errout; + goto errout_locked; } if (tp == NULL) { @@ -1724,29 +1751,37 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n, if (tca[TCA_KIND] == NULL || !protocol) { NL_SET_ERR_MSG(extack, "Filter kind and protocol must be specified"); err = -EINVAL; - goto errout; + goto errout_locked; } if (!(n->nlmsg_flags & NLM_F_CREATE)) { NL_SET_ERR_MSG(extack, "Need both RTM_NEWTFILTER and NLM_F_CREATE to create a new filter"); err = -ENOENT; - goto errout; + goto errout_locked; } if (prio_allocate) - prio = tcf_auto_prio(tcf_chain_tp_prev(&chain_info)); + prio = tcf_auto_prio(tcf_chain_tp_prev(chain, + &chain_info)); + mutex_unlock(&chain->filter_chain_lock); tp = tcf_proto_create(nla_data(tca[TCA_KIND]), protocol, prio, chain, extack); if (IS_ERR(tp)) { err = PTR_ERR(tp); goto errout; } + + mutex_lock(&chain->filter_chain_lock); + tcf_chain_tp_insert(chain, &chain_info, tp); + mutex_unlock(&chain->filter_chain_lock); tp_created = 1; } else if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], tp->ops->kind)) { NL_SET_ERR_MSG(extack, "Specified filter kind does not match existing one"); err = -EINVAL; - goto errout; + goto errout_locked; + } else { + mutex_unlock(&chain->filter_chain_lock); } fh = tp->ops->get(tp, t->tcm_handle); @@ -1772,15 +1807,11 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n, err = tp->ops->change(net, skb, tp, cl, t->tcm_handle, tca, &fh, n->nlmsg_flags & NLM_F_CREATE ? TCA_ACT_NOREPLACE : TCA_ACT_REPLACE, extack); - if (err == 0) { - if (tp_created) - tcf_chain_tp_insert(chain, &chain_info, tp); + if (err == 0) tfilter_notify(net, skb, n, tp, block, q, parent, fh, RTM_NEWTFILTER, false); - } else { - if (tp_created) - tcf_proto_destroy(tp, NULL); - } + else if (tp_created) + tcf_proto_destroy(tp, NULL); errout: if (chain) @@ -1790,6 +1821,10 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n, /* Replay the request. */ goto replay; return err; + +errout_locked: + mutex_unlock(&chain->filter_chain_lock); + goto errout; } static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n, @@ -1865,31 +1900,34 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n, goto errout; } + mutex_lock(&chain->filter_chain_lock); tp = tcf_chain_tp_find(chain, &chain_info, protocol, prio, false); if (!tp || IS_ERR(tp)) { NL_SET_ERR_MSG(extack, "Filter with specified priority/protocol not found"); err = tp ? PTR_ERR(tp) : -ENOENT; - goto errout; + goto errout_locked; } else if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], tp->ops->kind)) { NL_SET_ERR_MSG(extack, "Specified filter kind does not match existing one"); err = -EINVAL; + goto errout_locked; + } else if (t->tcm_handle == 0) { + tcf_chain_tp_remove(chain, &chain_info, tp); + mutex_unlock(&chain->filter_chain_lock); + + tfilter_notify(net, skb, n, tp, block, q, parent, fh, + RTM_DELTFILTER, false); + tcf_proto_destroy(tp, extack); + err = 0; goto errout; } + mutex_unlock(&chain->filter_chain_lock); fh = tp->ops->get(tp, t->tcm_handle); if (!fh) { - if (t->tcm_handle == 0) { - tcf_chain_tp_remove(chain, &chain_info, tp); - tfilter_notify(net, skb, n, tp, block, q, parent, fh, - RTM_DELTFILTER, false); - tcf_proto_destroy(tp, extack); - err = 0; - } else { - NL_SET_ERR_MSG(extack, "Specified filter handle not found"); - err = -ENOENT; - } + NL_SET_ERR_MSG(extack, "Specified filter handle not found"); + err = -ENOENT; } else { bool last; @@ -1899,7 +1937,10 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n, if (err) goto errout; if (last) { + mutex_lock(&chain->filter_chain_lock); tcf_chain_tp_remove(chain, &chain_info, tp); + mutex_unlock(&chain->filter_chain_lock); + tcf_proto_destroy(tp, extack); } } @@ -1909,6 +1950,10 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n, tcf_chain_put(chain); tcf_block_release(q, block); return err; + +errout_locked: + mutex_unlock(&chain->filter_chain_lock); + goto errout; } static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n, @@ -1966,8 +2011,10 @@ static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n, goto errout; } + mutex_lock(&chain->filter_chain_lock); tp = tcf_chain_tp_find(chain, &chain_info, protocol, prio, false); + mutex_unlock(&chain->filter_chain_lock); if (!tp || IS_ERR(tp)) { NL_SET_ERR_MSG(extack, "Filter with specified priority/protocol not found"); err = tp ? PTR_ERR(tp) : -ENOENT; diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c index 66ba2ce2320f..e24568f9246c 100644 --- a/net/sched/sch_generic.c +++ b/net/sched/sch_generic.c @@ -1366,7 +1366,11 @@ static void mini_qdisc_rcu_func(struct rcu_head *head) void mini_qdisc_pair_swap(struct mini_Qdisc_pair *miniqp, struct tcf_proto *tp_head) { - struct mini_Qdisc *miniq_old = rtnl_dereference(*miniqp->p_miniq); + /* Protected with chain0->filter_chain_lock. + * Can't access chain directly because tp_head can be NULL. + */ + struct mini_Qdisc *miniq_old = + rcu_dereference_protected(*miniqp->p_miniq, 1); struct mini_Qdisc *miniq; if (!tp_head) {

[net-next,v4,07/17] net: sched: protect filter_chain list with filter_chain_lock mutex

Commit Message

Comments

Patch