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 <vla...@mellanox.com>
Acked-by: Jiri Pirko <j...@mellanox.com>
---
 net/sched/cls_api.c | 79 +++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 64 insertions(+), 15 deletions(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 0a75cb2e5e7b..c33636e7b431 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -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 @@ static int tc_new_tfilter(struct sk_buff *skb, struct 
nlmsghdr *n,
 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 @@ static int tc_ctl_chain(struct sk_buff *skb, struct 
nlmsghdr *n,
        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 @@ static int tc_ctl_chain(struct sk_buff *skb, struct 
nlmsghdr *n,
                                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 @@ static int tc_ctl_chain(struct sk_buff *skb, struct 
nlmsghdr *n,
 
 errout:
        tcf_chain_put(chain);
+errout_block:
+       tcf_block_release(q, block);
        if (err == -EAGAIN)
                /* Replay the request. */
                goto replay;
-- 
2.7.5

Reply via email to