[PATCH net-next v2] net: sched: change tcf_del_walker() to take idrinfo->lock

[Vlad Buslov <vladbu@mellanox.com>]

Action API was changed to work with actions and action_idr in concurrency
safe manner, however tcf_del_walker() still uses actions without taking a
reference or idrinfo->lock first, and deletes them directly, disregarding
possible concurrent delete.

Add tc_action_wq workqueue to action API. Implement
tcf_idr_release_unsafe() that assumes external synchronization by caller
and delays blocking action cleanup part to tc_action_wq workqueue. Extend
tcf_action_cleanup() with 'async' argument to indicate that function should
free action asynchronously.

Change tcf_del_walker() to take idrinfo->lock while iterating over actions
and use new tcf_idr_release_unsafe() to release them while holding the
lock.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 include/net/act_api.h |  1 +
 net/sched/act_api.c   | 73 ++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index c6f195b3c706..4c5117bc4afb 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -38,6 +38,7 @@ struct tc_action {
 	struct gnet_stats_queue __percpu *cpu_qstats;
 	struct tc_cookie	__rcu *act_cookie;
 	struct tcf_chain	*goto_chain;
+	struct work_struct	work;
 };
 #define tcf_index	common.tcfa_index
 #define tcf_refcnt	common.tcfa_refcnt
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 6f118d62c731..4ad9062c34b3 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -90,13 +90,38 @@ static void free_tcf(struct tc_action *p)
 	kfree(p);
 }
 
-static void tcf_action_cleanup(struct tc_action *p)
+static void tcf_action_free(struct tc_action *p)
+{
+	gen_kill_estimator(&p->tcfa_rate_est);
+	free_tcf(p);
+}
+
+static void tcf_action_free_work(struct work_struct *work)
+{
+	struct tc_action *p = container_of(work,
+					   struct tc_action,
+					   work);
+
+	tcf_action_free(p);
+}
+
+static struct workqueue_struct *tc_action_wq;
+
+static bool tcf_action_queue_work(struct work_struct *work, work_func_t func)
+{
+	INIT_WORK(work, func);
+	return queue_work(tc_action_wq, work);
+}
+
+static void tcf_action_cleanup(struct tc_action *p, bool async)
 {
 	if (p->ops->cleanup)
 		p->ops->cleanup(p);
 
-	gen_kill_estimator(&p->tcfa_rate_est);
-	free_tcf(p);
+	if (async)
+		tcf_action_queue_work(&p->work, tcf_action_free_work);
+	else
+		tcf_action_free(p);
 }
 
 static int __tcf_action_put(struct tc_action *p, bool bind)
@@ -109,7 +134,7 @@ static int __tcf_action_put(struct tc_action *p, bool bind)
 		idr_remove(&idrinfo->action_idr, p->tcfa_index);
 		spin_unlock(&idrinfo->lock);
 
-		tcf_action_cleanup(p);
+		tcf_action_cleanup(p, false);
 		return 1;
 	}
 
@@ -147,6 +172,24 @@ int __tcf_idr_release(struct tc_action *p, bool bind, bool strict)
 }
 EXPORT_SYMBOL(__tcf_idr_release);
 
+/* Release idr without obtaining idrinfo->lock. Caller must prevent any
+ * concurrent modifications of idrinfo->action_idr!
+ */
+
+static int tcf_idr_release_unsafe(struct tc_action *p)
+{
+	if (atomic_read(&p->tcfa_bindcnt) > 0)
+		return -EPERM;
+
+	if (refcount_dec_and_test(&p->tcfa_refcnt)) {
+		idr_remove(&p->idrinfo->action_idr, p->tcfa_index);
+		tcf_action_cleanup(p, true);
+		return ACT_P_DELETED;
+	}
+
+	return 0;
+}
+
 static size_t tcf_action_shared_attrs_size(const struct tc_action *act)
 {
 	struct tc_cookie *act_cookie;
@@ -262,20 +305,25 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
 	if (nla_put_string(skb, TCA_KIND, ops->kind))
 		goto nla_put_failure;
 
+	spin_lock(&idrinfo->lock);
 	idr_for_each_entry_ul(idr, p, id) {
-		ret = __tcf_idr_release(p, false, true);
+		ret = tcf_idr_release_unsafe(p);
 		if (ret == ACT_P_DELETED) {
 			module_put(ops->owner);
 			n_i++;
 		} else if (ret < 0) {
-			goto nla_put_failure;
+			goto nla_put_failure_locked;
 		}
 	}
+	spin_unlock(&idrinfo->lock);
+
 	if (nla_put_u32(skb, TCA_FCNT, n_i))
 		goto nla_put_failure;
 	nla_nest_end(skb, nest);
 
 	return n_i;
+nla_put_failure_locked:
+	spin_unlock(&idrinfo->lock);
 nla_put_failure:
 	nla_nest_cancel(skb, nest);
 	return ret;
@@ -341,7 +389,7 @@ static int tcf_idr_delete_index(struct tcf_idrinfo *idrinfo, u32 index)
 						p->tcfa_index));
 			spin_unlock(&idrinfo->lock);
 
-			tcf_action_cleanup(p);
+			tcf_action_cleanup(p, false);
 			module_put(owner);
 			return 0;
 		}
@@ -1713,16 +1761,23 @@ static int __init tc_action_init(void)
 {
 	int err;
 
+	tc_action_wq = alloc_ordered_workqueue("tc_action_workqueue", 0);
+	if (!tc_action_wq)
+		return -ENOMEM;
+
 	err = register_pernet_subsys(&tcf_action_net_ops);
 	if (err)
-		return err;
+		goto err_register_pernet_subsys;
 
 	rtnl_register(PF_UNSPEC, RTM_NEWACTION, tc_ctl_action, NULL, 0);
 	rtnl_register(PF_UNSPEC, RTM_DELACTION, tc_ctl_action, NULL, 0);
 	rtnl_register(PF_UNSPEC, RTM_GETACTION, tc_ctl_action, tc_dump_action,
 		      0);
+	return err;
 
-	return 0;
+err_register_pernet_subsys:
+	destroy_workqueue(tc_action_wq);
+	return err;
 }
 
 subsys_initcall(tc_action_init);
-- 
2.7.5