bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 bpf-next 0/5] BPF link force-detach support
@ 2020-07-31 18:28 Andrii Nakryiko
  2020-07-31 18:28 ` [PATCH v2 bpf-next 1/5] bpf: add support for forced LINK_DETACH command Andrii Nakryiko
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Andrii Nakryiko @ 2020-07-31 18:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

This patch set adds new BPF link operation, LINK_DETACH, allowing processes
with BPF link FD to force-detach it from respective BPF hook, similarly how
BPF link is auto-detached when such BPF hook (e.g., cgroup, net_device, netns,
etc) is removed. This facility allows admin to forcefully undo BPF link
attachment, while process that created BPF link in the first place is left
intact.

Once force-detached, BPF link stays valid in the kernel as long as there is at
least one FD open against it. It goes into defunct state, just like
auto-detached BPF link.

bpftool also got `link detach` command to allow triggering this in
non-programmatic fashion.

v1->v2:
- improve error reporting in `bpftool link detach` (Song).

Andrii Nakryiko (5):
  bpf: add support for forced LINK_DETACH command
  libbpf: add bpf_link detach APIs
  selftests/bpf: add link detach tests for cgroup, netns, and xdp
    bpf_links
  tools/bpftool: add `link detach` subcommand
  tools/bpftool: add documentation and bash-completion for `link detach`

 include/linux/bpf.h                           |  1 +
 include/uapi/linux/bpf.h                      |  5 ++
 kernel/bpf/cgroup.c                           | 15 +++++-
 kernel/bpf/net_namespace.c                    |  8 +++
 kernel/bpf/syscall.c                          | 26 ++++++++++
 net/core/dev.c                                | 11 +++-
 .../bpftool/Documentation/bpftool-link.rst    |  8 +++
 tools/bpf/bpftool/bash-completion/bpftool     |  4 +-
 tools/bpf/bpftool/link.c                      | 37 +++++++++++++-
 tools/include/uapi/linux/bpf.h                |  5 ++
 tools/lib/bpf/bpf.c                           | 10 ++++
 tools/lib/bpf/bpf.h                           |  2 +
 tools/lib/bpf/libbpf.c                        |  5 ++
 tools/lib/bpf/libbpf.h                        |  1 +
 tools/lib/bpf/libbpf.map                      |  2 +
 .../selftests/bpf/prog_tests/cgroup_link.c    | 20 +++++++-
 .../selftests/bpf/prog_tests/sk_lookup.c      | 51 +++++++++----------
 .../selftests/bpf/prog_tests/xdp_link.c       | 14 +++++
 tools/testing/selftests/bpf/testing_helpers.c | 14 +++++
 tools/testing/selftests/bpf/testing_helpers.h |  3 ++
 20 files changed, 208 insertions(+), 34 deletions(-)

-- 
2.24.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 bpf-next 1/5] bpf: add support for forced LINK_DETACH command
  2020-07-31 18:28 [PATCH v2 bpf-next 0/5] BPF link force-detach support Andrii Nakryiko
@ 2020-07-31 18:28 ` Andrii Nakryiko
  2020-07-31 23:19   ` John Fastabend
  2020-07-31 18:28 ` [PATCH v2 bpf-next 2/5] libbpf: add bpf_link detach APIs Andrii Nakryiko
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2020-07-31 18:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Song Liu

Add LINK_DETACH command to force-detach bpf_link without destroying it. It has
the same behavior as auto-detaching of bpf_link due to cgroup dying for
bpf_cgroup_link or net_device being destroyed for bpf_xdp_link. In such case,
bpf_link is still a valid kernel object, but is defuncts and doesn't hold BPF
program attached to corresponding BPF hook. This functionality allows users
with enough access rights to manually force-detach attached bpf_link without
killing respective owner process.

This patch implements LINK_DETACH for cgroup, xdp, and netns links, mostly
re-using existing link release handling code.

Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 include/linux/bpf.h        |  1 +
 include/uapi/linux/bpf.h   |  5 +++++
 kernel/bpf/cgroup.c        | 15 ++++++++++++++-
 kernel/bpf/net_namespace.c |  8 ++++++++
 kernel/bpf/syscall.c       | 26 ++++++++++++++++++++++++++
 net/core/dev.c             | 11 ++++++++++-
 6 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 40c5e206ecf2..cef4ef0d2b4e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -793,6 +793,7 @@ struct bpf_link {
 struct bpf_link_ops {
 	void (*release)(struct bpf_link *link);
 	void (*dealloc)(struct bpf_link *link);
+	int (*detach)(struct bpf_link *link);
 	int (*update_prog)(struct bpf_link *link, struct bpf_prog *new_prog,
 			   struct bpf_prog *old_prog);
 	void (*show_fdinfo)(const struct bpf_link *link, struct seq_file *seq);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index eb5e0c38eb2c..b134e679e9db 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -117,6 +117,7 @@ enum bpf_cmd {
 	BPF_LINK_GET_NEXT_ID,
 	BPF_ENABLE_STATS,
 	BPF_ITER_CREATE,
+	BPF_LINK_DETACH,
 };
 
 enum bpf_map_type {
@@ -634,6 +635,10 @@ union bpf_attr {
 		__u32		old_prog_fd;
 	} link_update;
 
+	struct {
+		__u32		link_fd;
+	} link_detach;
+
 	struct { /* struct used by BPF_ENABLE_STATS command */
 		__u32		type;
 	} enable_stats;
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 957cce1d5168..83ff127ef7ae 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -814,6 +814,7 @@ static void bpf_cgroup_link_release(struct bpf_link *link)
 {
 	struct bpf_cgroup_link *cg_link =
 		container_of(link, struct bpf_cgroup_link, link);
+	struct cgroup *cg;
 
 	/* link might have been auto-detached by dying cgroup already,
 	 * in that case our work is done here
@@ -832,8 +833,12 @@ static void bpf_cgroup_link_release(struct bpf_link *link)
 	WARN_ON(__cgroup_bpf_detach(cg_link->cgroup, NULL, cg_link,
 				    cg_link->type));
 
+	cg = cg_link->cgroup;
+	cg_link->cgroup = NULL;
+
 	mutex_unlock(&cgroup_mutex);
-	cgroup_put(cg_link->cgroup);
+
+	cgroup_put(cg);
 }
 
 static void bpf_cgroup_link_dealloc(struct bpf_link *link)
@@ -844,6 +849,13 @@ static void bpf_cgroup_link_dealloc(struct bpf_link *link)
 	kfree(cg_link);
 }
 
+static int bpf_cgroup_link_detach(struct bpf_link *link)
+{
+	bpf_cgroup_link_release(link);
+
+	return 0;
+}
+
 static void bpf_cgroup_link_show_fdinfo(const struct bpf_link *link,
 					struct seq_file *seq)
 {
@@ -883,6 +895,7 @@ static int bpf_cgroup_link_fill_link_info(const struct bpf_link *link,
 static const struct bpf_link_ops bpf_cgroup_link_lops = {
 	.release = bpf_cgroup_link_release,
 	.dealloc = bpf_cgroup_link_dealloc,
+	.detach = bpf_cgroup_link_detach,
 	.update_prog = cgroup_bpf_replace,
 	.show_fdinfo = bpf_cgroup_link_show_fdinfo,
 	.fill_link_info = bpf_cgroup_link_fill_link_info,
diff --git a/kernel/bpf/net_namespace.c b/kernel/bpf/net_namespace.c
index 71405edd667c..542f275bf252 100644
--- a/kernel/bpf/net_namespace.c
+++ b/kernel/bpf/net_namespace.c
@@ -142,9 +142,16 @@ static void bpf_netns_link_release(struct bpf_link *link)
 	bpf_prog_array_free(old_array);
 
 out_unlock:
+	net_link->net = NULL;
 	mutex_unlock(&netns_bpf_mutex);
 }
 
+static int bpf_netns_link_detach(struct bpf_link *link)
+{
+	bpf_netns_link_release(link);
+	return 0;
+}
+
 static void bpf_netns_link_dealloc(struct bpf_link *link)
 {
 	struct bpf_netns_link *net_link =
@@ -228,6 +235,7 @@ static void bpf_netns_link_show_fdinfo(const struct bpf_link *link,
 static const struct bpf_link_ops bpf_netns_link_ops = {
 	.release = bpf_netns_link_release,
 	.dealloc = bpf_netns_link_dealloc,
+	.detach = bpf_netns_link_detach,
 	.update_prog = bpf_netns_link_update_prog,
 	.fill_link_info = bpf_netns_link_fill_info,
 	.show_fdinfo = bpf_netns_link_show_fdinfo,
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index cd3d599e9e90..2f343ce15747 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3991,6 +3991,29 @@ static int link_update(union bpf_attr *attr)
 	return ret;
 }
 
+#define BPF_LINK_DETACH_LAST_FIELD link_detach.link_fd
+
+static int link_detach(union bpf_attr *attr)
+{
+	struct bpf_link *link;
+	int ret;
+
+	if (CHECK_ATTR(BPF_LINK_DETACH))
+		return -EINVAL;
+
+	link = bpf_link_get_from_fd(attr->link_detach.link_fd);
+	if (IS_ERR(link))
+		return PTR_ERR(link);
+
+	if (link->ops->detach)
+		ret = link->ops->detach(link);
+	else
+		ret = -EOPNOTSUPP;
+
+	bpf_link_put(link);
+	return ret;
+}
+
 static int bpf_link_inc_not_zero(struct bpf_link *link)
 {
 	return atomic64_fetch_add_unless(&link->refcnt, 1, 0) ? 0 : -ENOENT;
@@ -4240,6 +4263,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
 	case BPF_ITER_CREATE:
 		err = bpf_iter_create(&attr);
 		break;
+	case BPF_LINK_DETACH:
+		err = link_detach(&attr);
+		break;
 	default:
 		err = -EINVAL;
 		break;
diff --git a/net/core/dev.c b/net/core/dev.c
index a2a57988880a..c8b911b10187 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8979,12 +8979,20 @@ static void bpf_xdp_link_release(struct bpf_link *link)
 	/* if racing with net_device's tear down, xdp_link->dev might be
 	 * already NULL, in which case link was already auto-detached
 	 */
-	if (xdp_link->dev)
+	if (xdp_link->dev) {
 		WARN_ON(dev_xdp_detach_link(xdp_link->dev, NULL, xdp_link));
+		xdp_link->dev = NULL;
+	}
 
 	rtnl_unlock();
 }
 
+static int bpf_xdp_link_detach(struct bpf_link *link)
+{
+	bpf_xdp_link_release(link);
+	return 0;
+}
+
 static void bpf_xdp_link_dealloc(struct bpf_link *link)
 {
 	struct bpf_xdp_link *xdp_link = container_of(link, struct bpf_xdp_link, link);
@@ -9066,6 +9074,7 @@ static int bpf_xdp_link_update(struct bpf_link *link, struct bpf_prog *new_prog,
 static const struct bpf_link_ops bpf_xdp_link_lops = {
 	.release = bpf_xdp_link_release,
 	.dealloc = bpf_xdp_link_dealloc,
+	.detach = bpf_xdp_link_detach,
 	.show_fdinfo = bpf_xdp_link_show_fdinfo,
 	.fill_link_info = bpf_xdp_link_fill_link_info,
 	.update_prog = bpf_xdp_link_update,
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 bpf-next 2/5] libbpf: add bpf_link detach APIs
  2020-07-31 18:28 [PATCH v2 bpf-next 0/5] BPF link force-detach support Andrii Nakryiko
  2020-07-31 18:28 ` [PATCH v2 bpf-next 1/5] bpf: add support for forced LINK_DETACH command Andrii Nakryiko
@ 2020-07-31 18:28 ` Andrii Nakryiko
  2020-07-31 23:22   ` John Fastabend
  2020-07-31 18:28 ` [PATCH v2 bpf-next 3/5] selftests/bpf: add link detach tests for cgroup, netns, and xdp bpf_links Andrii Nakryiko
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2020-07-31 18:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Song Liu

Add low-level bpf_link_detach() API. Also add higher-level bpf_link__detach()
one.

Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/include/uapi/linux/bpf.h |  5 +++++
 tools/lib/bpf/bpf.c            | 10 ++++++++++
 tools/lib/bpf/bpf.h            |  2 ++
 tools/lib/bpf/libbpf.c         |  5 +++++
 tools/lib/bpf/libbpf.h         |  1 +
 tools/lib/bpf/libbpf.map       |  2 ++
 6 files changed, 25 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index eb5e0c38eb2c..b134e679e9db 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -117,6 +117,7 @@ enum bpf_cmd {
 	BPF_LINK_GET_NEXT_ID,
 	BPF_ENABLE_STATS,
 	BPF_ITER_CREATE,
+	BPF_LINK_DETACH,
 };
 
 enum bpf_map_type {
@@ -634,6 +635,10 @@ union bpf_attr {
 		__u32		old_prog_fd;
 	} link_update;
 
+	struct {
+		__u32		link_fd;
+	} link_detach;
+
 	struct { /* struct used by BPF_ENABLE_STATS command */
 		__u32		type;
 	} enable_stats;
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index e1bdf214f75f..eab14c97c15d 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -603,6 +603,16 @@ int bpf_link_create(int prog_fd, int target_fd,
 	return sys_bpf(BPF_LINK_CREATE, &attr, sizeof(attr));
 }
 
+int bpf_link_detach(int link_fd)
+{
+	union bpf_attr attr;
+
+	memset(&attr, 0, sizeof(attr));
+	attr.link_detach.link_fd = link_fd;
+
+	return sys_bpf(BPF_LINK_DETACH, &attr, sizeof(attr));
+}
+
 int bpf_link_update(int link_fd, int new_prog_fd,
 		    const struct bpf_link_update_opts *opts)
 {
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 6d367e01d05e..28855fd5b5f4 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -178,6 +178,8 @@ LIBBPF_API int bpf_link_create(int prog_fd, int target_fd,
 			       enum bpf_attach_type attach_type,
 			       const struct bpf_link_create_opts *opts);
 
+LIBBPF_API int bpf_link_detach(int link_fd);
+
 struct bpf_link_update_opts {
 	size_t sz; /* size of this struct for forward/backward compatibility */
 	__u32 flags;	   /* extra flags */
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b9f11f854985..7be04e45d29c 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -7748,6 +7748,11 @@ struct bpf_link *bpf_link__open(const char *path)
 	return link;
 }
 
+int bpf_link__detach(struct bpf_link *link)
+{
+	return bpf_link_detach(link->fd) ? -errno : 0;
+}
+
 int bpf_link__pin(struct bpf_link *link, const char *path)
 {
 	int err;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 9924385462ab..3ed1399bfbbc 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -229,6 +229,7 @@ LIBBPF_API int bpf_link__unpin(struct bpf_link *link);
 LIBBPF_API int bpf_link__update_program(struct bpf_link *link,
 					struct bpf_program *prog);
 LIBBPF_API void bpf_link__disconnect(struct bpf_link *link);
+LIBBPF_API int bpf_link__detach(struct bpf_link *link);
 LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
 
 LIBBPF_API struct bpf_link *
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index ca49a6a7e5b2..099863411f7d 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -273,6 +273,8 @@ LIBBPF_0.0.9 {
 
 LIBBPF_0.1.0 {
 	global:
+		bpf_link__detach;
+		bpf_link_detach;
 		bpf_map__ifindex;
 		bpf_map__key_size;
 		bpf_map__map_flags;
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 bpf-next 3/5] selftests/bpf: add link detach tests for cgroup, netns, and xdp bpf_links
  2020-07-31 18:28 [PATCH v2 bpf-next 0/5] BPF link force-detach support Andrii Nakryiko
  2020-07-31 18:28 ` [PATCH v2 bpf-next 1/5] bpf: add support for forced LINK_DETACH command Andrii Nakryiko
  2020-07-31 18:28 ` [PATCH v2 bpf-next 2/5] libbpf: add bpf_link detach APIs Andrii Nakryiko
@ 2020-07-31 18:28 ` Andrii Nakryiko
  2020-07-31 23:29   ` John Fastabend
  2020-07-31 18:28 ` [PATCH v2 bpf-next 4/5] tools/bpftool: add `link detach` subcommand Andrii Nakryiko
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2020-07-31 18:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Song Liu

Add bpf_link__detach() testing to selftests for cgroup, netns, and xdp
bpf_links.

Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../selftests/bpf/prog_tests/cgroup_link.c    | 20 +++++++-
 .../selftests/bpf/prog_tests/sk_lookup.c      | 51 +++++++++----------
 .../selftests/bpf/prog_tests/xdp_link.c       | 14 +++++
 tools/testing/selftests/bpf/testing_helpers.c | 14 +++++
 tools/testing/selftests/bpf/testing_helpers.h |  3 ++
 5 files changed, 73 insertions(+), 29 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
index 6e04f8d1d15b..4d9b514b3fd9 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
@@ -2,6 +2,7 @@
 
 #include <test_progs.h>
 #include "cgroup_helpers.h"
+#include "testing_helpers.h"
 #include "test_cgroup_link.skel.h"
 
 static __u32 duration = 0;
@@ -37,7 +38,8 @@ void test_cgroup_link(void)
 	int last_cg = ARRAY_SIZE(cgs) - 1, cg_nr = ARRAY_SIZE(cgs);
 	DECLARE_LIBBPF_OPTS(bpf_link_update_opts, link_upd_opts);
 	struct bpf_link *links[ARRAY_SIZE(cgs)] = {}, *tmp_link;
-	__u32 prog_ids[ARRAY_SIZE(cgs)], prog_cnt = 0, attach_flags;
+	__u32 prog_ids[ARRAY_SIZE(cgs)], prog_cnt = 0, attach_flags, prog_id;
+	struct bpf_link_info info;
 	int i = 0, err, prog_fd;
 	bool detach_legacy = false;
 
@@ -219,6 +221,22 @@ void test_cgroup_link(void)
 	/* BPF programs should still get called */
 	ping_and_check(0, cg_nr);
 
+	prog_id = link_info_prog_id(links[0], &info);
+	CHECK(prog_id == 0, "link_info", "failed\n");
+	CHECK(info.cgroup.cgroup_id == 0, "cgroup_id", "unexpected %llu\n", info.cgroup.cgroup_id);
+
+	err = bpf_link__detach(links[0]);
+	if (CHECK(err, "link_detach", "failed %d\n", err))
+		goto cleanup;
+
+	/* cgroup_id should be zero in link_info */
+	prog_id = link_info_prog_id(links[0], &info);
+	CHECK(prog_id == 0, "link_info", "failed\n");
+	CHECK(info.cgroup.cgroup_id != 0, "cgroup_id", "unexpected %llu\n", info.cgroup.cgroup_id);
+
+	/* First BPF program shouldn't be called anymore */
+	ping_and_check(0, cg_nr - 1);
+
 	/* leave cgroup and remove them, don't detach programs */
 	cleanup_cgroup_environment();
 
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
index 379da6f10ee9..c571584c00f5 100644
--- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
+++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
@@ -34,6 +34,7 @@
 #include "bpf_util.h"
 #include "cgroup_helpers.h"
 #include "network_helpers.h"
+#include "testing_helpers.h"
 #include "test_sk_lookup.skel.h"
 
 /* External (address, port) pairs the client sends packets to. */
@@ -469,34 +470,10 @@ static int update_lookup_map(struct bpf_map *map, int index, int sock_fd)
 	return 0;
 }
 
-static __u32 link_info_prog_id(struct bpf_link *link)
-{
-	struct bpf_link_info info = {};
-	__u32 info_len = sizeof(info);
-	int link_fd, err;
-
-	link_fd = bpf_link__fd(link);
-	if (CHECK(link_fd < 0, "bpf_link__fd", "failed\n")) {
-		errno = -link_fd;
-		log_err("bpf_link__fd failed");
-		return 0;
-	}
-
-	err = bpf_obj_get_info_by_fd(link_fd, &info, &info_len);
-	if (CHECK(err, "bpf_obj_get_info_by_fd", "failed\n")) {
-		log_err("bpf_obj_get_info_by_fd");
-		return 0;
-	}
-	if (CHECK(info_len != sizeof(info), "bpf_obj_get_info_by_fd",
-		  "unexpected info len %u\n", info_len))
-		return 0;
-
-	return info.prog_id;
-}
-
 static void query_lookup_prog(struct test_sk_lookup *skel)
 {
 	struct bpf_link *link[3] = {};
+	struct bpf_link_info info;
 	__u32 attach_flags = 0;
 	__u32 prog_ids[3] = {};
 	__u32 prog_cnt = 3;
@@ -534,18 +511,36 @@ static void query_lookup_prog(struct test_sk_lookup *skel)
 	if (CHECK(prog_cnt != 3, "bpf_prog_query",
 		  "wrong program count on query: %u", prog_cnt))
 		goto detach;
-	prog_id = link_info_prog_id(link[0]);
+	prog_id = link_info_prog_id(link[0], &info);
 	CHECK(prog_ids[0] != prog_id, "bpf_prog_query",
 	      "invalid program #0 id on query: %u != %u\n",
 	      prog_ids[0], prog_id);
-	prog_id = link_info_prog_id(link[1]);
+	CHECK(info.netns.netns_ino == 0, "netns_ino",
+	      "unexpected netns_ino: %u\n", info.netns.netns_ino);
+	prog_id = link_info_prog_id(link[1], &info);
 	CHECK(prog_ids[1] != prog_id, "bpf_prog_query",
 	      "invalid program #1 id on query: %u != %u\n",
 	      prog_ids[1], prog_id);
-	prog_id = link_info_prog_id(link[2]);
+	CHECK(info.netns.netns_ino == 0, "netns_ino",
+	      "unexpected netns_ino: %u\n", info.netns.netns_ino);
+	prog_id = link_info_prog_id(link[2], &info);
 	CHECK(prog_ids[2] != prog_id, "bpf_prog_query",
 	      "invalid program #2 id on query: %u != %u\n",
 	      prog_ids[2], prog_id);
+	CHECK(info.netns.netns_ino == 0, "netns_ino",
+	      "unexpected netns_ino: %u\n", info.netns.netns_ino);
+
+	err = bpf_link__detach(link[0]);
+	if (CHECK(err, "link_detach", "failed %d\n", err))
+		goto detach;
+
+	/* prog id is still there, but netns_ino is zeroed out */
+	prog_id = link_info_prog_id(link[0], &info);
+	CHECK(prog_ids[0] != prog_id, "bpf_prog_query",
+	      "invalid program #0 id on query: %u != %u\n",
+	      prog_ids[0], prog_id);
+	CHECK(info.netns.netns_ino != 0, "netns_ino",
+	      "unexpected netns_ino: %u\n", info.netns.netns_ino);
 
 detach:
 	if (link[2])
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_link.c b/tools/testing/selftests/bpf/prog_tests/xdp_link.c
index 52cba6795d40..6f814999b395 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_link.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_link.c
@@ -131,6 +131,20 @@ void test_xdp_link(void)
 	CHECK(link_info.xdp.ifindex != IFINDEX_LO, "link_ifindex",
 	      "got %u != exp %u\n", link_info.xdp.ifindex, IFINDEX_LO);
 
+	err = bpf_link__detach(link);
+	if (CHECK(err, "link_detach", "failed %d\n", err))
+		goto cleanup;
+
+	memset(&link_info, 0, sizeof(link_info));
+	err = bpf_obj_get_info_by_fd(bpf_link__fd(link), &link_info, &link_info_len);
+	if (CHECK(err, "link_info", "failed: %d\n", err))
+		goto cleanup;
+	CHECK(link_info.prog_id != id1, "link_prog_id",
+	      "got %u != exp %u\n", link_info.prog_id, id1);
+	/* ifindex should be zeroed out */
+	CHECK(link_info.xdp.ifindex != 0, "link_ifindex",
+	      "got %u != exp %u\n", link_info.xdp.ifindex, 0);
+
 cleanup:
 	test_xdp_link__destroy(skel1);
 	test_xdp_link__destroy(skel2);
diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
index 0af6337a8962..800d503e5cb4 100644
--- a/tools/testing/selftests/bpf/testing_helpers.c
+++ b/tools/testing/selftests/bpf/testing_helpers.c
@@ -64,3 +64,17 @@ int parse_num_list(const char *s, bool **num_set, int *num_set_len)
 
 	return 0;
 }
+
+__u32 link_info_prog_id(const struct bpf_link *link, struct bpf_link_info *info)
+{
+	__u32 info_len = sizeof(*info);
+	int err;
+
+	memset(info, 0, sizeof(*info));
+	err = bpf_obj_get_info_by_fd(bpf_link__fd(link), info, &info_len);
+	if (err) {
+		printf("failed to get link info: %d\n", -errno);
+		return 0;
+	}
+	return info->prog_id;
+}
diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h
index 923b51762759..d4f8e749611b 100644
--- a/tools/testing/selftests/bpf/testing_helpers.h
+++ b/tools/testing/selftests/bpf/testing_helpers.h
@@ -1,5 +1,8 @@
 /* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
 /* Copyright (C) 2020 Facebook, Inc. */
 #include <stdbool.h>
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
 
 int parse_num_list(const char *s, bool **set, int *set_len);
+__u32 link_info_prog_id(const struct bpf_link *link, struct bpf_link_info *info);
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 bpf-next 4/5] tools/bpftool: add `link detach` subcommand
  2020-07-31 18:28 [PATCH v2 bpf-next 0/5] BPF link force-detach support Andrii Nakryiko
                   ` (2 preceding siblings ...)
  2020-07-31 18:28 ` [PATCH v2 bpf-next 3/5] selftests/bpf: add link detach tests for cgroup, netns, and xdp bpf_links Andrii Nakryiko
@ 2020-07-31 18:28 ` Andrii Nakryiko
  2020-07-31 23:32   ` John Fastabend
  2020-07-31 18:28 ` [PATCH v2 bpf-next 5/5] tools/bpftool: add documentation and bash-completion for `link detach` Andrii Nakryiko
  2020-08-02  3:48 ` [PATCH v2 bpf-next 0/5] BPF link force-detach support Alexei Starovoitov
  5 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2020-07-31 18:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Song Liu

Add ability to force-detach BPF link. Also add missing error message, if
specified link ID is wrong.

Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/bpf/bpftool/link.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 326b8fdf0243..1b793759170e 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -22,6 +22,8 @@ static const char * const link_type_name[] = {
 
 static int link_parse_fd(int *argc, char ***argv)
 {
+	int fd;
+
 	if (is_prefix(**argv, "id")) {
 		unsigned int id;
 		char *endptr;
@@ -35,7 +37,10 @@ static int link_parse_fd(int *argc, char ***argv)
 		}
 		NEXT_ARGP();
 
-		return bpf_link_get_fd_by_id(id);
+		fd = bpf_link_get_fd_by_id(id);
+		if (fd < 0)
+			p_err("failed to get link with ID %d: %s", id, strerror(errno));
+		return fd;
 	} else if (is_prefix(**argv, "pinned")) {
 		char *path;
 
@@ -316,6 +321,34 @@ static int do_pin(int argc, char **argv)
 	return err;
 }
 
+static int do_detach(int argc, char **argv)
+{
+	int err, fd;
+
+	if (argc != 2) {
+		p_err("link specifier is invalid or missing\n");
+		return 1;
+	}
+
+	fd = link_parse_fd(&argc, &argv);
+	if (fd < 0)
+		return 1;
+
+	err = bpf_link_detach(fd);
+	if (err)
+		err = -errno;
+	close(fd);
+	if (err) {
+		p_err("failed link detach: %s", strerror(-err));
+		return 1;
+	}
+
+	if (json_output)
+		jsonw_null(json_wtr);
+
+	return 0;
+}
+
 static int do_help(int argc, char **argv)
 {
 	if (json_output) {
@@ -326,6 +359,7 @@ static int do_help(int argc, char **argv)
 	fprintf(stderr,
 		"Usage: %1$s %2$s { show | list }   [LINK]\n"
 		"       %1$s %2$s pin        LINK  FILE\n"
+		"       %1$s %2$s detach     LINK\n"
 		"       %1$s %2$s help\n"
 		"\n"
 		"       " HELP_SPEC_LINK "\n"
@@ -341,6 +375,7 @@ static const struct cmd cmds[] = {
 	{ "list",	do_show },
 	{ "help",	do_help },
 	{ "pin",	do_pin },
+	{ "detach",	do_detach },
 	{ 0 }
 };
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 bpf-next 5/5] tools/bpftool: add documentation and bash-completion for `link detach`
  2020-07-31 18:28 [PATCH v2 bpf-next 0/5] BPF link force-detach support Andrii Nakryiko
                   ` (3 preceding siblings ...)
  2020-07-31 18:28 ` [PATCH v2 bpf-next 4/5] tools/bpftool: add `link detach` subcommand Andrii Nakryiko
@ 2020-07-31 18:28 ` Andrii Nakryiko
  2020-07-31 23:35   ` John Fastabend
  2020-08-02  3:48 ` [PATCH v2 bpf-next 0/5] BPF link force-detach support Alexei Starovoitov
  5 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2020-07-31 18:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Song Liu

Add info on link detach sub-command to man page. Add detach to bash-completion
as well.

Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/bpf/bpftool/Documentation/bpftool-link.rst | 8 ++++++++
 tools/bpf/bpftool/bash-completion/bpftool        | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-link.rst b/tools/bpf/bpftool/Documentation/bpftool-link.rst
index 38b0949a185b..4a52e7a93339 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-link.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-link.rst
@@ -21,6 +21,7 @@ LINK COMMANDS
 
 |	**bpftool** **link { show | list }** [*LINK*]
 |	**bpftool** **link pin** *LINK* *FILE*
+|	**bpftool** **link detach *LINK*
 |	**bpftool** **link help**
 |
 |	*LINK* := { **id** *LINK_ID* | **pinned** *FILE* }
@@ -49,6 +50,13 @@ DESCRIPTION
 		  contain a dot character ('.'), which is reserved for future
 		  extensions of *bpffs*.
 
+	**bpftool link detach** *LINK*
+		  Force-detach link *LINK*. BPF link and its underlying BPF
+		  program will stay valid, but they will be detached from the
+		  respective BPF hook and BPF link will transition into
+		  a defunct state until last open file descriptor for that
+		  link is closed.
+
 	**bpftool link help**
 		  Print short help message.
 
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 257fa310ea2b..f53ed2f1a4aa 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -1122,7 +1122,7 @@ _bpftool()
             ;;
         link)
             case $command in
-                show|list|pin)
+                show|list|pin|detach)
                     case $prev in
                         id)
                             _bpftool_get_link_ids
@@ -1139,7 +1139,7 @@ _bpftool()
                     COMPREPLY=( $( compgen -W "$LINK_TYPE" -- "$cur" ) )
                     return 0
                     ;;
-                pin)
+                pin|detach)
                     if [[ $prev == "$command" ]]; then
                         COMPREPLY=( $( compgen -W "$LINK_TYPE" -- "$cur" ) )
                     else
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* RE: [PATCH v2 bpf-next 1/5] bpf: add support for forced LINK_DETACH command
  2020-07-31 18:28 ` [PATCH v2 bpf-next 1/5] bpf: add support for forced LINK_DETACH command Andrii Nakryiko
@ 2020-07-31 23:19   ` John Fastabend
  0 siblings, 0 replies; 12+ messages in thread
From: John Fastabend @ 2020-07-31 23:19 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Song Liu

Andrii Nakryiko wrote:
> Add LINK_DETACH command to force-detach bpf_link without destroying it. It has
> the same behavior as auto-detaching of bpf_link due to cgroup dying for
> bpf_cgroup_link or net_device being destroyed for bpf_xdp_link. In such case,
> bpf_link is still a valid kernel object, but is defuncts and doesn't hold BPF
> program attached to corresponding BPF hook. This functionality allows users
> with enough access rights to manually force-detach attached bpf_link without
> killing respective owner process.
> 
> This patch implements LINK_DETACH for cgroup, xdp, and netns links, mostly
> re-using existing link release handling code.
> 
> Acked-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---

Looks necessary otherwise we have no way, as far as I read it, to delete
an XDP program and go back the no prog state after link_{create|update}.

Acked-by: John Fastabend <john.fastabend@gmail.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH v2 bpf-next 2/5] libbpf: add bpf_link detach APIs
  2020-07-31 18:28 ` [PATCH v2 bpf-next 2/5] libbpf: add bpf_link detach APIs Andrii Nakryiko
@ 2020-07-31 23:22   ` John Fastabend
  0 siblings, 0 replies; 12+ messages in thread
From: John Fastabend @ 2020-07-31 23:22 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Song Liu

Andrii Nakryiko wrote:
> Add low-level bpf_link_detach() API. Also add higher-level bpf_link__detach()
> one.
> 
> Acked-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---

I like it nice and straightforward.

Acked-by: John Fastabend <john.fastabend@gmail.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH v2 bpf-next 3/5] selftests/bpf: add link detach tests for cgroup, netns, and xdp bpf_links
  2020-07-31 18:28 ` [PATCH v2 bpf-next 3/5] selftests/bpf: add link detach tests for cgroup, netns, and xdp bpf_links Andrii Nakryiko
@ 2020-07-31 23:29   ` John Fastabend
  0 siblings, 0 replies; 12+ messages in thread
From: John Fastabend @ 2020-07-31 23:29 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Song Liu

Andrii Nakryiko wrote:
> Add bpf_link__detach() testing to selftests for cgroup, netns, and xdp
> bpf_links.
> 
> Acked-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---

Acked-by: John Fastabend <john.fastabend@gmail.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH v2 bpf-next 4/5] tools/bpftool: add `link detach` subcommand
  2020-07-31 18:28 ` [PATCH v2 bpf-next 4/5] tools/bpftool: add `link detach` subcommand Andrii Nakryiko
@ 2020-07-31 23:32   ` John Fastabend
  0 siblings, 0 replies; 12+ messages in thread
From: John Fastabend @ 2020-07-31 23:32 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Song Liu

Andrii Nakryiko wrote:
> Add ability to force-detach BPF link. Also add missing error message, if
> specified link ID is wrong.
> 
> Acked-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---

Acked-by: John Fastabend <john.fastabend@gmail.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH v2 bpf-next 5/5] tools/bpftool: add documentation and bash-completion for `link detach`
  2020-07-31 18:28 ` [PATCH v2 bpf-next 5/5] tools/bpftool: add documentation and bash-completion for `link detach` Andrii Nakryiko
@ 2020-07-31 23:35   ` John Fastabend
  0 siblings, 0 replies; 12+ messages in thread
From: John Fastabend @ 2020-07-31 23:35 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Song Liu

Andrii Nakryiko wrote:
> Add info on link detach sub-command to man page. Add detach to bash-completion
> as well.
> 
> Acked-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---

Acked-by: John Fastabend <john.fastabend@gmail.com.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 bpf-next 0/5] BPF link force-detach support
  2020-07-31 18:28 [PATCH v2 bpf-next 0/5] BPF link force-detach support Andrii Nakryiko
                   ` (4 preceding siblings ...)
  2020-07-31 18:28 ` [PATCH v2 bpf-next 5/5] tools/bpftool: add documentation and bash-completion for `link detach` Andrii Nakryiko
@ 2020-08-02  3:48 ` Alexei Starovoitov
  5 siblings, 0 replies; 12+ messages in thread
From: Alexei Starovoitov @ 2020-08-02  3:48 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, Network Development, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Kernel Team

On Fri, Jul 31, 2020 at 11:29 AM Andrii Nakryiko <andriin@fb.com> wrote:
>
> This patch set adds new BPF link operation, LINK_DETACH, allowing processes
> with BPF link FD to force-detach it from respective BPF hook, similarly how
> BPF link is auto-detached when such BPF hook (e.g., cgroup, net_device, netns,
> etc) is removed. This facility allows admin to forcefully undo BPF link
> attachment, while process that created BPF link in the first place is left
> intact.
>
> Once force-detached, BPF link stays valid in the kernel as long as there is at
> least one FD open against it. It goes into defunct state, just like
> auto-detached BPF link.
>
> bpftool also got `link detach` command to allow triggering this in
> non-programmatic fashion.
>
> v1->v2:
> - improve error reporting in `bpftool link detach` (Song).

Applied. Thanks

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-08-02  3:48 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-31 18:28 [PATCH v2 bpf-next 0/5] BPF link force-detach support Andrii Nakryiko
2020-07-31 18:28 ` [PATCH v2 bpf-next 1/5] bpf: add support for forced LINK_DETACH command Andrii Nakryiko
2020-07-31 23:19   ` John Fastabend
2020-07-31 18:28 ` [PATCH v2 bpf-next 2/5] libbpf: add bpf_link detach APIs Andrii Nakryiko
2020-07-31 23:22   ` John Fastabend
2020-07-31 18:28 ` [PATCH v2 bpf-next 3/5] selftests/bpf: add link detach tests for cgroup, netns, and xdp bpf_links Andrii Nakryiko
2020-07-31 23:29   ` John Fastabend
2020-07-31 18:28 ` [PATCH v2 bpf-next 4/5] tools/bpftool: add `link detach` subcommand Andrii Nakryiko
2020-07-31 23:32   ` John Fastabend
2020-07-31 18:28 ` [PATCH v2 bpf-next 5/5] tools/bpftool: add documentation and bash-completion for `link detach` Andrii Nakryiko
2020-07-31 23:35   ` John Fastabend
2020-08-02  3:48 ` [PATCH v2 bpf-next 0/5] BPF link force-detach support Alexei Starovoitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).