net: sched: use Qdisc rcu API instead of relying on rtnl lock
authorVlad Buslov <vladbu@mellanox.com>
Mon, 24 Sep 2018 16:22:53 +0000 (19:22 +0300)
committerDavid S. Miller <davem@davemloft.net>
Wed, 26 Sep 2018 03:17:36 +0000 (20:17 -0700)
As a preparation from removing rtnl lock dependency from rules update path,
use Qdisc rcu and reference counting capabilities instead of relying on
rtnl lock while working with Qdiscs. Create new tcf_block_release()
function, and use it to free resources taken by tcf_block_find().
Currently, this function only releases Qdisc and it is extended in next
patches in this series.

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 0a75cb2e5e7ba2d83b063956c0ce91af25679cb1..c33636e7b43127c108f0fefdbe9032d3bbc7fe5c 100644 (file)
@@ -537,6 +537,7 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
                                        struct netlink_ext_ack *extack)
 {
        struct tcf_block *block;
+       int err = 0;
 
        if (ifindex == TCM_IFINDEX_MAGIC_BLOCK) {
                block = tcf_block_lookup(net, block_index);
@@ -548,55 +549,93 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
                const struct Qdisc_class_ops *cops;
                struct net_device *dev;
 
+               rcu_read_lock();
+
                /* Find link */
-               dev = __dev_get_by_index(net, ifindex);
-               if (!dev)
+               dev = dev_get_by_index_rcu(net, ifindex);
+               if (!dev) {
+                       rcu_read_unlock();
                        return ERR_PTR(-ENODEV);
+               }
 
                /* Find qdisc */
                if (!*parent) {
                        *q = dev->qdisc;
                        *parent = (*q)->handle;
                } else {
-                       *q = qdisc_lookup(dev, TC_H_MAJ(*parent));
+                       *q = qdisc_lookup_rcu(dev, TC_H_MAJ(*parent));
                        if (!*q) {
                                NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists");
-                               return ERR_PTR(-EINVAL);
+                               err = -EINVAL;
+                               goto errout_rcu;
                        }
                }
 
+               *q = qdisc_refcount_inc_nz(*q);
+               if (!*q) {
+                       NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists");
+                       err = -EINVAL;
+                       goto errout_rcu;
+               }
+
                /* Is it classful? */
                cops = (*q)->ops->cl_ops;
                if (!cops) {
                        NL_SET_ERR_MSG(extack, "Qdisc not classful");
-                       return ERR_PTR(-EINVAL);
+                       err = -EINVAL;
+                       goto errout_rcu;
                }
 
                if (!cops->tcf_block) {
                        NL_SET_ERR_MSG(extack, "Class doesn't support blocks");
-                       return ERR_PTR(-EOPNOTSUPP);
+                       err = -EOPNOTSUPP;
+                       goto errout_rcu;
                }
 
+               /* At this point we know that qdisc is not noop_qdisc,
+                * which means that qdisc holds a reference to net_device
+                * and we hold a reference to qdisc, so it is safe to release
+                * rcu read lock.
+                */
+               rcu_read_unlock();
+
                /* Do we search for filter, attached to class? */
                if (TC_H_MIN(*parent)) {
                        *cl = cops->find(*q, *parent);
                        if (*cl == 0) {
                                NL_SET_ERR_MSG(extack, "Specified class doesn't exist");
-                               return ERR_PTR(-ENOENT);
+                               err = -ENOENT;
+                               goto errout_qdisc;
                        }
                }
 
                /* And the last stroke */
                block = cops->tcf_block(*q, *cl, extack);
-               if (!block)
-                       return ERR_PTR(-EINVAL);
+               if (!block) {
+                       err = -EINVAL;
+                       goto errout_qdisc;
+               }
                if (tcf_block_shared(block)) {
                        NL_SET_ERR_MSG(extack, "This filter block is shared. Please use the block index to manipulate the filters");
-                       return ERR_PTR(-EOPNOTSUPP);
+                       err = -EOPNOTSUPP;
+                       goto errout_qdisc;
                }
        }
 
        return block;
+
+errout_rcu:
+       rcu_read_unlock();
+errout_qdisc:
+       if (*q)
+               qdisc_put(*q);
+       return ERR_PTR(err);
+}
+
+static void tcf_block_release(struct Qdisc *q, struct tcf_block *block)
+{
+       if (q)
+               qdisc_put(q);
 }
 
 struct tcf_block_owner_item {
@@ -1332,6 +1371,7 @@ replay:
 errout:
        if (chain)
                tcf_chain_put(chain);
+       tcf_block_release(q, block);
        if (err == -EAGAIN)
                /* Replay the request. */
                goto replay;
@@ -1453,6 +1493,7 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 errout:
        if (chain)
                tcf_chain_put(chain);
+       tcf_block_release(q, block);
        return err;
 }
 
@@ -1538,6 +1579,7 @@ static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 errout:
        if (chain)
                tcf_chain_put(chain);
+       tcf_block_release(q, block);
        return err;
 }
 
@@ -1854,7 +1896,8 @@ replay:
        chain_index = tca[TCA_CHAIN] ? nla_get_u32(tca[TCA_CHAIN]) : 0;
        if (chain_index > TC_ACT_EXT_VAL_MASK) {
                NL_SET_ERR_MSG(extack, "Specified chain index exceeds upper limit");
-               return -EINVAL;
+               err = -EINVAL;
+               goto errout_block;
        }
        chain = tcf_chain_lookup(block, chain_index);
        if (n->nlmsg_type == RTM_NEWCHAIN) {
@@ -1866,23 +1909,27 @@ replay:
                                tcf_chain_hold(chain);
                        } else {
                                NL_SET_ERR_MSG(extack, "Filter chain already exists");
-                               return -EEXIST;
+                               err = -EEXIST;
+                               goto errout_block;
                        }
                } else {
                        if (!(n->nlmsg_flags & NLM_F_CREATE)) {
                                NL_SET_ERR_MSG(extack, "Need both RTM_NEWCHAIN and NLM_F_CREATE to create a new chain");
-                               return -ENOENT;
+                               err = -ENOENT;
+                               goto errout_block;
                        }
                        chain = tcf_chain_create(block, chain_index);
                        if (!chain) {
                                NL_SET_ERR_MSG(extack, "Failed to create filter chain");
-                               return -ENOMEM;
+                               err = -ENOMEM;
+                               goto errout_block;
                        }
                }
        } else {
                if (!chain || tcf_chain_held_by_acts_only(chain)) {
                        NL_SET_ERR_MSG(extack, "Cannot find specified filter chain");
-                       return -EINVAL;
+                       err = -EINVAL;
+                       goto errout_block;
                }
                tcf_chain_hold(chain);
        }
@@ -1926,6 +1973,8 @@ replay:
 
 errout:
        tcf_chain_put(chain);
+errout_block:
+       tcf_block_release(q, block);
        if (err == -EAGAIN)
                /* Replay the request. */
                goto replay;