Netdev List
 help / color / mirror / Atom feed
From: Jakub Kicinski <jakub.kicinski@netronome.com>
To: netdev@vger.kernel.org, alexei.starovoitov@gmail.com,
	daniel@iogearbox.net
Cc: ktkhai@virtuozzo.com, oss-drivers@netronome.com,
	Jakub Kicinski <jakub.kicinski@netronome.com>
Subject: [PATCH bpf-next v2 1/8] bpf: offload: don't require rtnl for dev list manipulation
Date: Thu, 21 Dec 2017 13:01:13 -0800	[thread overview]
Message-ID: <20171221210120.30166-2-jakub.kicinski@netronome.com> (raw)
In-Reply-To: <20171221210120.30166-1-jakub.kicinski@netronome.com>

We don't need the RTNL lock for all operations on offload state.
We only need to hold it around ndo calls.  The device offload
initialization doesn't require it.  The soon-to-come querying
of the offload info will only need it partially.  We will also
be able to remove the waitqueue in following patches.

Use struct rw_semaphore because map offload will require sleeping
with the semaphore held for read.

Suggested-by: Kirill Tkhai <ktkhai@virtuozzo.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
v2:
 - use dev_get_by_index_rcu() instead of implicit lock dependencies;
 - use DECLARE_RWSEM() instead of init_rwsem() (Kirill).
---
 kernel/bpf/offload.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index 8455b89d1bbf..f049073a37e6 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -20,8 +20,12 @@
 #include <linux/netdevice.h>
 #include <linux/printk.h>
 #include <linux/rtnetlink.h>
+#include <linux/rwsem.h>
 
-/* protected by RTNL */
+/* Protects bpf_prog_offload_devs and offload members of all progs.
+ * RTNL lock cannot be taken when holding this lock.
+ */
+static DECLARE_RWSEM(bpf_devs_lock);
 static LIST_HEAD(bpf_prog_offload_devs);
 
 int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr)
@@ -43,19 +47,30 @@ int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr)
 	offload->prog = prog;
 	init_waitqueue_head(&offload->verifier_done);
 
-	rtnl_lock();
-	offload->netdev = __dev_get_by_index(net, attr->prog_ifindex);
+	rcu_read_lock();
+	offload->netdev = dev_get_by_index_rcu(net, attr->prog_ifindex);
 	if (!offload->netdev) {
-		rtnl_unlock();
-		kfree(offload);
-		return -EINVAL;
+		rcu_read_unlock();
+		goto err_free;
 	}
+	dev_hold(offload->netdev);
+	rcu_read_unlock();
 
+	down_write(&bpf_devs_lock);
+	if (offload->netdev->reg_state != NETREG_REGISTERED)
+		goto err_unlock;
 	prog->aux->offload = offload;
 	list_add_tail(&offload->offloads, &bpf_prog_offload_devs);
-	rtnl_unlock();
+	dev_put(offload->netdev);
+	up_write(&bpf_devs_lock);
 
 	return 0;
+err_unlock:
+	up_write(&bpf_devs_lock);
+	dev_put(offload->netdev);
+err_free:
+	kfree(offload);
+	return -EINVAL;
 }
 
 static int __bpf_offload_ndo(struct bpf_prog *prog, enum bpf_netdev_command cmd,
@@ -126,7 +141,9 @@ void bpf_prog_offload_destroy(struct bpf_prog *prog)
 	wake_up(&offload->verifier_done);
 
 	rtnl_lock();
+	down_write(&bpf_devs_lock);
 	__bpf_prog_offload_destroy(prog);
+	up_write(&bpf_devs_lock);
 	rtnl_unlock();
 
 	kfree(offload);
@@ -181,11 +198,13 @@ static int bpf_offload_notification(struct notifier_block *notifier,
 		if (netdev->reg_state != NETREG_UNREGISTERING)
 			break;
 
+		down_write(&bpf_devs_lock);
 		list_for_each_entry_safe(offload, tmp, &bpf_prog_offload_devs,
 					 offloads) {
 			if (offload->netdev == netdev)
 				__bpf_prog_offload_destroy(offload->prog);
 		}
+		up_write(&bpf_devs_lock);
 		break;
 	default:
 		break;
-- 
2.15.1

  reply	other threads:[~2017-12-21 21:01 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-21 21:01 [PATCH bpf-next v2 0/8] bpf: offload: report device back to user space (take 2) Jakub Kicinski
2017-12-21 21:01 ` Jakub Kicinski [this message]
2017-12-26 16:02   ` [PATCH bpf-next v2 1/8] bpf: offload: don't require rtnl for dev list manipulation Kirill Tkhai
2017-12-21 21:01 ` [PATCH bpf-next v2 2/8] bpf: offload: don't use prog->aux->offload as boolean Jakub Kicinski
2017-12-21 21:01 ` [PATCH bpf-next v2 3/8] bpf: offload: allow netdev to disappear while verifier is running Jakub Kicinski
2017-12-21 21:01 ` [PATCH bpf-next v2 4/8] bpf: offload: free prog->aux->offload when device disappears Jakub Kicinski
2017-12-21 21:01 ` [PATCH bpf-next v2 5/8] bpf: offload: free program id " Jakub Kicinski
2017-12-21 21:01 ` [PATCH bpf-next v2 6/8] bpf: offload: report device information for offloaded programs Jakub Kicinski
2017-12-26 22:42   ` Alexei Starovoitov
2017-12-27 17:26   ` Daniel Borkmann
2017-12-27 23:18     ` Jakub Kicinski
2017-12-21 21:01 ` [PATCH bpf-next v2 7/8] tools: bpftool: " Jakub Kicinski
2017-12-21 21:01 ` [PATCH bpf-next v2 8/8] selftests/bpf: test device info reporting for bound progs Jakub Kicinski

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=20171221210120.30166-2-jakub.kicinski@netronome.com \
    --to=jakub.kicinski@netronome.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=ktkhai@virtuozzo.com \
    --cc=netdev@vger.kernel.org \
    --cc=oss-drivers@netronome.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