net: sched: use reference counting for tcf blocks on rules update
authorVlad Buslov <vladbu@mellanox.com>
Mon, 24 Sep 2018 16:22:58 +0000 (19:22 +0300)
committerDavid S. Miller <davem@davemloft.net>
Wed, 26 Sep 2018 03:17:36 +0000 (20:17 -0700)
In order to remove dependency on rtnl lock on rules update path, always
take reference to block while using it on rules update path. Change
tcf_block_get() error handling to properly release block with reference
counting, instead of just destroying it, in order to accommodate potential
concurrent users.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/sched/cls_api.c

index 49e3c10532ad5f0720383bfe0126c575499b1b5d..3de47e99b788f81e914efebe9ef89e7f32c1f4bc 100644 (file)
@@ -622,7 +622,7 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
        int err = 0;
 
        if (ifindex == TCM_IFINDEX_MAGIC_BLOCK) {
-               block = tcf_block_lookup(net, block_index);
+               block = tcf_block_refcnt_get(net, block_index);
                if (!block) {
                        NL_SET_ERR_MSG(extack, "Block of given index was not found");
                        return ERR_PTR(-EINVAL);
@@ -702,6 +702,14 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
                        err = -EOPNOTSUPP;
                        goto errout_qdisc;
                }
+
+               /* Always take reference to block in order to support execution
+                * of rules update path of cls API without rtnl lock. Caller
+                * must release block when it is finished using it. 'if' block
+                * of this conditional obtain reference to block by calling
+                * tcf_block_refcnt_get().
+                */
+               refcount_inc(&block->refcnt);
        }
 
        return block;
@@ -716,6 +724,9 @@ errout_qdisc:
 
 static void tcf_block_release(struct Qdisc *q, struct tcf_block *block)
 {
+       if (!IS_ERR_OR_NULL(block))
+               tcf_block_refcnt_put(block);
+
        if (q)
                qdisc_put(q);
 }
@@ -785,21 +796,16 @@ int tcf_block_get_ext(struct tcf_block **p_block, struct Qdisc *q,
 {
        struct net *net = qdisc_net(q);
        struct tcf_block *block = NULL;
-       bool created = false;
        int err;
 
-       if (ei->block_index) {
+       if (ei->block_index)
                /* block_index not 0 means the shared block is requested */
-               block = tcf_block_lookup(net, ei->block_index);
-               if (block)
-                       refcount_inc(&block->refcnt);
-       }
+               block = tcf_block_refcnt_get(net, ei->block_index);
 
        if (!block) {
                block = tcf_block_create(net, q, ei->block_index, extack);
                if (IS_ERR(block))
                        return PTR_ERR(block);
-               created = true;
                if (tcf_block_shared(block)) {
                        err = tcf_block_insert(block, net, extack);
                        if (err)
@@ -829,14 +835,8 @@ err_block_offload_bind:
 err_chain0_head_change_cb_add:
        tcf_block_owner_del(block, q, ei->binder_type);
 err_block_owner_add:
-       if (created) {
-               if (tcf_block_shared(block))
-                       tcf_block_remove(block, net);
 err_block_insert:
-               kfree_rcu(block, rcu);
-       } else {
-               refcount_dec(&block->refcnt);
-       }
+       tcf_block_refcnt_put(block);
        return err;
 }
 EXPORT_SYMBOL(tcf_block_get_ext);
@@ -1730,7 +1730,7 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
                return err;
 
        if (tcm->tcm_ifindex == TCM_IFINDEX_MAGIC_BLOCK) {
-               block = tcf_block_lookup(net, tcm->tcm_block_index);
+               block = tcf_block_refcnt_get(net, tcm->tcm_block_index);
                if (!block)
                        goto out;
                /* If we work with block index, q is NULL and parent value
@@ -1789,6 +1789,8 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
                }
        }
 
+       if (tcm->tcm_ifindex == TCM_IFINDEX_MAGIC_BLOCK)
+               tcf_block_refcnt_put(block);
        cb->args[0] = index;
 
 out:
@@ -2055,7 +2057,7 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
                return err;
 
        if (tcm->tcm_ifindex == TCM_IFINDEX_MAGIC_BLOCK) {
-               block = tcf_block_lookup(net, tcm->tcm_block_index);
+               block = tcf_block_refcnt_get(net, tcm->tcm_block_index);
                if (!block)
                        goto out;
                /* If we work with block index, q is NULL and parent value
@@ -2122,6 +2124,8 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
                index++;
        }
 
+       if (tcm->tcm_ifindex == TCM_IFINDEX_MAGIC_BLOCK)
+               tcf_block_refcnt_put(block);
        cb->args[0] = index;
 
 out: