Netdev List
 help / color / mirror / Atom feed
From: Vlad Buslov <vladbu@mellanox.com>
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, jhs@mojatatu.com, xiyou.wangcong@gmail.com,
	jiri@resnulli.us, pablo@netfilter.org, kadlec@blackhole.kfki.hu,
	fw@strlen.de, ast@kernel.org, daniel@iogearbox.net,
	edumazet@google.com, keescook@chromium.org,
	marcelo.leitner@gmail.com, Vlad Buslov <vladbu@mellanox.com>
Subject: [PATCH net-next v2 04/15] net: sched: act_ife: remove dependency on rtnl lock
Date: Fri, 10 Aug 2018 20:51:44 +0300	[thread overview]
Message-ID: <1533923515-5664-5-git-send-email-vladbu@mellanox.com> (raw)
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>

Use tcf spinlock and rcu to protect params pointer from concurrent
modification during dump and init. Use rcu swap operation to reassign
params pointer under protection of tcf lock. (old params value is not used
by init, so there is no need of standalone rcu dereference step)

Ife action has meta-actions that are compiled as standalone modules. Rtnl
mutex must be released while loading a kernel module. In order to support
execution without rtnl mutex, propagate 'rtnl_held' argument to meta action
loading functions. When requesting meta action module, conditionally
release rtnl lock depending on 'rtnl_held' argument.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 net/sched/act_ife.c | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index df4060e32d43..5d200495e467 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -268,7 +268,8 @@ static const char *ife_meta_id2name(u32 metaid)
  * under ife->tcf_lock for existing action
 */
 static int load_metaops_and_vet(struct tcf_ife_info *ife, u32 metaid,
-				void *val, int len, bool exists)
+				void *val, int len, bool exists,
+				bool rtnl_held)
 {
 	struct tcf_meta_ops *ops = find_ife_oplist(metaid);
 	int ret = 0;
@@ -278,9 +279,11 @@ static int load_metaops_and_vet(struct tcf_ife_info *ife, u32 metaid,
 #ifdef CONFIG_MODULES
 		if (exists)
 			spin_unlock_bh(&ife->tcf_lock);
-		rtnl_unlock();
+		if (rtnl_held)
+			rtnl_unlock();
 		request_module("ife-meta-%s", ife_meta_id2name(metaid));
-		rtnl_lock();
+		if (rtnl_held)
+			rtnl_lock();
 		if (exists)
 			spin_lock_bh(&ife->tcf_lock);
 		ops = find_ife_oplist(metaid);
@@ -421,7 +424,7 @@ static void tcf_ife_cleanup(struct tc_action *a)
 
 /* under ife->tcf_lock for existing action */
 static int populate_metalist(struct tcf_ife_info *ife, struct nlattr **tb,
-			     bool exists)
+			     bool exists, bool rtnl_held)
 {
 	int len = 0;
 	int rc = 0;
@@ -433,7 +436,8 @@ static int populate_metalist(struct tcf_ife_info *ife, struct nlattr **tb,
 			val = nla_data(tb[i]);
 			len = nla_len(tb[i]);
 
-			rc = load_metaops_and_vet(ife, i, val, len, exists);
+			rc = load_metaops_and_vet(ife, i, val, len, exists,
+						  rtnl_held);
 			if (rc != 0)
 				return rc;
 
@@ -454,7 +458,7 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
 	struct tc_action_net *tn = net_generic(net, ife_net_id);
 	struct nlattr *tb[TCA_IFE_MAX + 1];
 	struct nlattr *tb2[IFE_META_MAX + 1];
-	struct tcf_ife_params *p, *p_old;
+	struct tcf_ife_params *p;
 	struct tcf_ife_info *ife;
 	u16 ife_type = ETH_P_IFE;
 	struct tc_ife *parm;
@@ -558,7 +562,7 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
 			return err;
 		}
 
-		err = populate_metalist(ife, tb2, exists);
+		err = populate_metalist(ife, tb2, exists, rtnl_held);
 		if (err)
 			goto metadata_parse_err;
 
@@ -581,13 +585,13 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
 	}
 
 	ife->tcf_action = parm->action;
+	/* protected by tcf_lock when modifying existing action */
+	rcu_swap_protected(ife->params, p, 1);
+
 	if (exists)
 		spin_unlock_bh(&ife->tcf_lock);
-
-	p_old = rtnl_dereference(ife->params);
-	rcu_assign_pointer(ife->params, p);
-	if (p_old)
-		kfree_rcu(p_old, rcu);
+	if (p)
+		kfree_rcu(p, rcu);
 
 	if (ret == ACT_P_CREATED)
 		tcf_idr_insert(tn, *a);
@@ -600,16 +604,20 @@ static int tcf_ife_dump(struct sk_buff *skb, struct tc_action *a, int bind,
 {
 	unsigned char *b = skb_tail_pointer(skb);
 	struct tcf_ife_info *ife = to_ife(a);
-	struct tcf_ife_params *p = rtnl_dereference(ife->params);
+	struct tcf_ife_params *p;
 	struct tc_ife opt = {
 		.index = ife->tcf_index,
 		.refcnt = refcount_read(&ife->tcf_refcnt) - ref,
 		.bindcnt = atomic_read(&ife->tcf_bindcnt) - bind,
-		.action = ife->tcf_action,
-		.flags = p->flags,
 	};
 	struct tcf_t t;
 
+	spin_lock_bh(&ife->tcf_lock);
+	opt.action = ife->tcf_action;
+	p = rcu_dereference_protected(ife->params,
+				      lockdep_is_held(&ife->tcf_lock));
+	opt.flags = p->flags;
+
 	if (nla_put(skb, TCA_IFE_PARMS, sizeof(opt), &opt))
 		goto nla_put_failure;
 
@@ -635,9 +643,11 @@ static int tcf_ife_dump(struct sk_buff *skb, struct tc_action *a, int bind,
 		pr_info("Failed to dump metalist\n");
 	}
 
+	spin_unlock_bh(&ife->tcf_lock);
 	return skb->len;
 
 nla_put_failure:
+	spin_unlock_bh(&ife->tcf_lock);
 	nlmsg_trim(skb, b);
 	return -1;
 }
-- 
2.7.5

  parent reply	other threads:[~2018-08-10 20:23 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-10 17:51 [PATCH net-next v2 00/15] Remove rtnl lock dependency from all action implementations Vlad Buslov
2018-08-10 17:51 ` [PATCH net-next v2 01/15] net: sched: act_bpf: remove dependency on rtnl lock Vlad Buslov
2018-08-10 17:51 ` [PATCH net-next v2 02/15] net: sched: act_csum: " Vlad Buslov
2018-08-10 17:51 ` [PATCH net-next v2 03/15] net: sched: act_gact: " Vlad Buslov
2018-08-10 17:51 ` Vlad Buslov [this message]
2018-08-10 17:51 ` [PATCH net-next v2 05/15] net: sched: act_ipt: " Vlad Buslov
2018-08-10 17:51 ` [PATCH net-next v2 06/15] net: sched: act_pedit: " Vlad Buslov
2018-08-10 17:51 ` [PATCH net-next v2 07/15] net: sched: act_sample: " Vlad Buslov
2018-08-10 17:51 ` [PATCH net-next v2 08/15] net: sched: act_simple: " Vlad Buslov
2018-08-10 17:51 ` [PATCH net-next v2 09/15] net: sched: act_skbmod: " Vlad Buslov
2018-08-10 17:51 ` [PATCH net-next v2 10/15] net: sched: act_tunnel_key: " Vlad Buslov
2018-08-10 17:51 ` [PATCH net-next v2 11/15] net: sched: act_vlan: " Vlad Buslov
2018-08-10 17:51 ` [PATCH net-next v2 12/15] net: sched: extend action ops with put_dev callback Vlad Buslov
2018-08-10 17:51 ` [PATCH net-next v2 13/15] net: sched: act_mirred: remove dependency on rtnl lock Vlad Buslov
2018-08-10 17:51 ` [PATCH net-next v2 14/15] net: core: protect rate estimator statistics pointer with lock Vlad Buslov
2018-08-10 17:51 ` [PATCH net-next v2 15/15] net: sched: act_police: remove dependency on rtnl lock Vlad Buslov
2018-08-11 19:38 ` [PATCH net-next v2 00/15] Remove rtnl lock dependency from all action implementations David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1533923515-5664-5-git-send-email-vladbu@mellanox.com \
    --to=vladbu@mellanox.com \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fw@strlen.de \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kadlec@blackhole.kfki.hu \
    --cc=keescook@chromium.org \
    --cc=marcelo.leitner@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox