net: sched: implement reference counted action release

Implement helper delete function that uses new action ops 'delete', instead
of destroying action directly. This is required so act API could delete
actions by index, without holding any references to action that is being
deleted.

Implement function __tcf_action_put() that releases reference to action and
frees it, if necessary. Refactor action deletion code to use new put
function and not to rely on rtnl lock. Remove rtnl lock assertions that are
no longer needed.

Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 0f31f09946ab0df45a443a413867a30cc5ac36c4..a023873db7137a708222869c33084267dbe1b694 100644 (file)
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -90,21 +90,39 @@ static void free_tcf(struct tc_action *p)
        kfree(p);
 }
 
-static void tcf_idr_remove(struct tcf_idrinfo *idrinfo, struct tc_action *p)
+static void tcf_action_cleanup(struct tc_action *p)
 {
-       spin_lock(&idrinfo->lock);
-       idr_remove(&idrinfo->action_idr, p->tcfa_index);
-       spin_unlock(&idrinfo->lock);
+       if (p->ops->cleanup)
+               p->ops->cleanup(p);
+
        gen_kill_estimator(&p->tcfa_rate_est);
        free_tcf(p);
 }
 
+static int __tcf_action_put(struct tc_action *p, bool bind)
+{
+       struct tcf_idrinfo *idrinfo = p->idrinfo;
+
+       if (refcount_dec_and_lock(&p->tcfa_refcnt, &idrinfo->lock)) {
+               if (bind)
+                       atomic_dec(&p->tcfa_bindcnt);
+               idr_remove(&idrinfo->action_idr, p->tcfa_index);
+               spin_unlock(&idrinfo->lock);
+
+               tcf_action_cleanup(p);
+               return 1;
+       }
+
+       if (bind)
+               atomic_dec(&p->tcfa_bindcnt);
+
+       return 0;
+}
+
 int __tcf_idr_release(struct tc_action *p, bool bind, bool strict)
 {
        int ret = 0;
 
-       ASSERT_RTNL();
-
        /* Release with strict==1 and bind==0 is only called through act API
         * interface (classifiers always bind). Only case when action with
         * positive reference count and zero bind count can exist is when it was
@@ -118,18 +136,11 @@ int __tcf_idr_release(struct tc_action *p, bool bind, bool strict)
         * are acceptable.
         */
        if (p) {
-               if (bind)
-                       atomic_dec(&p->tcfa_bindcnt);
-               else if (strict && atomic_read(&p->tcfa_bindcnt) > 0)
+               if (!bind && strict && atomic_read(&p->tcfa_bindcnt) > 0)
                        return -EPERM;
 
-               if (atomic_read(&p->tcfa_bindcnt) <= 0 &&
-                   refcount_dec_and_test(&p->tcfa_refcnt)) {
-                       if (p->ops->cleanup)
-                               p->ops->cleanup(p);
-                       tcf_idr_remove(p->idrinfo, p);
+               if (__tcf_action_put(p, bind))
                        ret = ACT_P_DELETED;
-               }
        }
 
        return ret;
@@ -340,11 +351,7 @@ int tcf_idr_delete_index(struct tc_action_net *tn, u32 index)
                                                p->tcfa_index));
                        spin_unlock(&idrinfo->lock);
 
-                       if (p->ops->cleanup)
-                               p->ops->cleanup(p);
-
-                       gen_kill_estimator(&p->tcfa_rate_est);
-                       free_tcf(p);
+                       tcf_action_cleanup(p);
                        module_put(owner);
                        return 0;
                }
@@ -615,6 +622,11 @@ int tcf_action_destroy(struct list_head *actions, int bind)
        return ret;
 }
 
+static int tcf_action_put(struct tc_action *p)
+{
+       return __tcf_action_put(p, false);
+}
+
 int
 tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
 {
@@ -1092,6 +1104,35 @@ err_out:
        return err;
 }
 
+static int tcf_action_delete(struct net *net, struct list_head *actions,
+                            struct netlink_ext_ack *extack)
+{
+       struct tc_action *a, *tmp;
+       u32 act_index;
+       int ret;
+
+       list_for_each_entry_safe(a, tmp, actions, list) {
+               const struct tc_action_ops *ops = a->ops;
+
+               /* Actions can be deleted concurrently so we must save their
+                * type and id to search again after reference is released.
+                */
+               act_index = a->tcfa_index;
+
+               list_del(&a->list);
+               if (tcf_action_put(a)) {
+                       /* last reference, action was deleted concurrently */
+                       module_put(ops->owner);
+               } else  {
+                       /* now do the delete */
+                       ret = ops->delete(net, act_index);
+                       if (ret < 0)
+                               return ret;
+               }
+       }
+       return 0;
+}
+
 static int
 tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
               u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
@@ -1112,7 +1153,7 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
        }
 
        /* now do the delete */
-       ret = tcf_action_destroy(actions, 0);
+       ret = tcf_action_delete(net, actions, extack);
        if (ret < 0) {
                NL_SET_ERR_MSG(extack, "Failed to delete TC action");
                kfree_skb(skb);
@@ -1164,7 +1205,6 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
        if (event == RTM_GETACTION)
                ret = tcf_get_notify(net, portid, n, &actions, event, extack);
        else { /* delete */
-               cleanup_a(&actions, 1); /* lookup took reference */
                ret = tcf_del_notify(net, n, &actions, portid, attr_size, extack);
                if (ret)
                        goto err;

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index ebc2b9dd783f86e97784182450ff1700373d404a..9041f0e43e9a69323b03e3bb412904b98c7afc2f 100644 (file)
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1611,7 +1611,6 @@ void tcf_exts_destroy(struct tcf_exts *exts)
 #ifdef CONFIG_NET_CLS_ACT
        LIST_HEAD(actions);
 
-       ASSERT_RTNL();
        tcf_exts_to_list(exts, &actions);
        tcf_action_destroy(&actions, TCA_ACT_UNBIND);
        kfree(exts->actions);