All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next v2 0/2] Introduce a new kfunc of bpf_task_under_cgroup
@ 2023-04-21  9:04 Feng zhou
  2023-04-21  9:04 ` [PATCH bpf-next v2 1/2] bpf: Add bpf_task_under_cgroup() kfunc Feng zhou
  2023-04-21  9:04 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add testcase for bpf_task_under_cgroup Feng zhou
  0 siblings, 2 replies; 6+ messages in thread
From: Feng zhou @ 2023-04-21  9:04 UTC (permalink / raw)
  To: martin.lau, ast, daniel, andrii, song, yhs, john.fastabend,
	kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba, pabeni,
	mykolal, shuah
  Cc: bpf, linux-kernel, netdev, linux-kselftest, yangzhenze,
	wangdongdong.6, zhoufeng.zf

From: Feng Zhou <zhoufeng.zf@bytedance.com>

Trace sched related functions, such as enqueue_task_fair, it is necessary to
specify a task instead of the current task which within a given cgroup.

Feng Zhou (2):
  bpf: Add bpf_task_under_cgroup() kfunc
  selftests/bpf: Add testcase for bpf_task_under_cgroup

Changelog:
v1->v2: Addressed comments from Alexei Starovoitov
- Add kfunc instead.
Details in here:
https://lore.kernel.org/all/20230420072657.80324-1-zhoufeng.zf@bytedance.com/

 kernel/bpf/helpers.c                          | 19 ++++++++
 .../bpf/prog_tests/task_under_cgroup.c        | 46 +++++++++++++++++++
 .../selftests/bpf/progs/cgrp_kfunc_common.h   |  1 +
 .../bpf/progs/test_task_under_cgroup.c        | 40 ++++++++++++++++
 4 files changed, 106 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_task_under_cgroup.c

-- 
2.20.1


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

* [PATCH bpf-next v2 1/2] bpf: Add bpf_task_under_cgroup() kfunc
  2023-04-21  9:04 [PATCH bpf-next v2 0/2] Introduce a new kfunc of bpf_task_under_cgroup Feng zhou
@ 2023-04-21  9:04 ` Feng zhou
  2023-04-21 18:20   ` Alexei Starovoitov
  2023-04-21  9:04 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add testcase for bpf_task_under_cgroup Feng zhou
  1 sibling, 1 reply; 6+ messages in thread
From: Feng zhou @ 2023-04-21  9:04 UTC (permalink / raw)
  To: martin.lau, ast, daniel, andrii, song, yhs, john.fastabend,
	kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba, pabeni,
	mykolal, shuah
  Cc: bpf, linux-kernel, netdev, linux-kselftest, yangzhenze,
	wangdongdong.6, zhoufeng.zf

From: Feng Zhou <zhoufeng.zf@bytedance.com>

Add a kfunc that's similar to the bpf_current_task_under_cgroup.
The difference is that it is a designated task.

When hook sched related functions, sometimes it is necessary to
specify a task instead of the current task.

Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
---
 kernel/bpf/helpers.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 00e5fb0682ac..88e3247b5c44 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2142,6 +2142,24 @@ __bpf_kfunc struct cgroup *bpf_cgroup_from_id(u64 cgid)
 		return NULL;
 	return cgrp;
 }
+
+/**
+ * bpf_task_under_cgroup - Check whether the task is a given subset of the
+ * cgroup2 hierarchy. The cgroup2 to test is assigned by cgrp.
+ * @cgrp: assigned cgrp.
+ * @task: assigned task.
+ */
+__bpf_kfunc int bpf_task_under_cgroup(struct cgroup *cgrp,
+				      struct task_struct *task)
+{
+	if (unlikely(!cgrp))
+		return -EAGAIN;
+
+	if (unlikely(!task))
+		return -ENOENT;
+
+	return task_under_cgroup_hierarchy(task, cgrp);
+}
 #endif /* CONFIG_CGROUPS */
 
 /**
@@ -2341,6 +2359,7 @@ BTF_ID_FLAGS(func, bpf_cgroup_acquire, KF_ACQUIRE | KF_RCU | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_cgroup_release, KF_RELEASE)
 BTF_ID_FLAGS(func, bpf_cgroup_ancestor, KF_ACQUIRE | KF_RCU | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_cgroup_from_id, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_task_under_cgroup, KF_RCU)
 #endif
 BTF_ID_FLAGS(func, bpf_task_from_pid, KF_ACQUIRE | KF_RET_NULL)
 BTF_SET8_END(generic_btf_ids)
-- 
2.20.1


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

* [PATCH bpf-next v2 2/2] selftests/bpf: Add testcase for bpf_task_under_cgroup
  2023-04-21  9:04 [PATCH bpf-next v2 0/2] Introduce a new kfunc of bpf_task_under_cgroup Feng zhou
  2023-04-21  9:04 ` [PATCH bpf-next v2 1/2] bpf: Add bpf_task_under_cgroup() kfunc Feng zhou
@ 2023-04-21  9:04 ` Feng zhou
  2023-04-21 18:24   ` Alexei Starovoitov
  1 sibling, 1 reply; 6+ messages in thread
From: Feng zhou @ 2023-04-21  9:04 UTC (permalink / raw)
  To: martin.lau, ast, daniel, andrii, song, yhs, john.fastabend,
	kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba, pabeni,
	mykolal, shuah
  Cc: bpf, linux-kernel, netdev, linux-kselftest, yangzhenze,
	wangdongdong.6, zhoufeng.zf

From: Feng Zhou <zhoufeng.zf@bytedance.com>

test_progs:
Tests new kfunc bpf_task_under_cgroup().

The bpf program saves the pid which call the getuid syscall within a
given cgroup to the remote_pid, which is convenient for the user-mode
program to verify the test correctness.

The user-mode program creates its own mount namespace, and mounts the
cgroupsv2 hierarchy in there, call the getuid syscall, then check if
remote_pid and local_pid are equal.

Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
---
 .../bpf/prog_tests/task_under_cgroup.c        | 46 +++++++++++++++++++
 .../selftests/bpf/progs/cgrp_kfunc_common.h   |  1 +
 .../bpf/progs/test_task_under_cgroup.c        | 40 ++++++++++++++++
 3 files changed, 87 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_task_under_cgroup.c

diff --git a/tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c b/tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
new file mode 100644
index 000000000000..bd3deb469938
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Bytedance */
+
+#include <test_progs.h>
+#include <cgroup_helpers.h>
+#include "test_task_under_cgroup.skel.h"
+
+#define FOO	"/foo"
+
+void test_task_under_cgroup(void)
+{
+	struct test_task_under_cgroup *skel;
+	int ret, foo = -1;
+
+	foo = test__join_cgroup(FOO);
+	if (!ASSERT_OK(foo < 0, "cgroup_join_foo"))
+		return;
+
+	skel = test_task_under_cgroup__open();
+	if (!ASSERT_OK_PTR(skel, "test_task_under_cgroup__open"))
+		goto cleanup;
+
+	skel->rodata->local_pid = getpid();
+	skel->rodata->cgid = get_cgroup_id(FOO);
+
+	ret = test_task_under_cgroup__load(skel);
+	if (!ASSERT_OK(ret, "test_task_under_cgroup__load"))
+		goto cleanup;
+
+	ret = test_task_under_cgroup__attach(skel);
+	if (!ASSERT_OK(ret, "test_task_under_cgroup__attach"))
+		goto cleanup;
+
+	syscall(__NR_getuid);
+
+	test_task_under_cgroup__detach(skel);
+
+	ASSERT_EQ(skel->bss->remote_pid, skel->rodata->local_pid,
+		  "test task_under_cgroup");
+
+cleanup:
+	if (foo)
+		close(foo);
+
+	test_task_under_cgroup__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h b/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h
index 22914a70db54..41b3ea231698 100644
--- a/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h
+++ b/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h
@@ -26,6 +26,7 @@ struct cgroup *bpf_cgroup_ancestor(struct cgroup *cgrp, int level) __ksym;
 struct cgroup *bpf_cgroup_from_id(u64 cgid) __ksym;
 void bpf_rcu_read_lock(void) __ksym;
 void bpf_rcu_read_unlock(void) __ksym;
+int bpf_task_under_cgroup(struct cgroup *cgrp, struct task_struct *task) __ksym;
 
 static inline struct __cgrps_kfunc_map_value *cgrps_kfunc_map_value_lookup(struct cgroup *cgrp)
 {
diff --git a/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c
new file mode 100644
index 000000000000..e2740f9b029d
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Bytedance */
+
+#include <vmlinux.h>
+#include <asm/unistd.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_helpers.h>
+
+#include "cgrp_kfunc_common.h"
+
+const volatile int local_pid;
+const volatile long cgid;
+int remote_pid;
+
+SEC("tp_btf/sys_enter")
+int BPF_PROG(sysenter, struct pt_regs *regs, long id)
+{
+	struct cgroup *cgrp;
+
+	if (id != __NR_getuid)
+		return 0;
+
+	if (local_pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
+
+	cgrp = bpf_cgroup_from_id(cgid);
+	if (!cgrp)
+		return 0;
+
+	if (!bpf_task_under_cgroup(cgrp, bpf_get_current_task_btf()))
+		goto out;
+
+	remote_pid = local_pid;
+
+out:
+	bpf_cgroup_release(cgrp);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.20.1


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

* Re: [PATCH bpf-next v2 1/2] bpf: Add bpf_task_under_cgroup() kfunc
  2023-04-21  9:04 ` [PATCH bpf-next v2 1/2] bpf: Add bpf_task_under_cgroup() kfunc Feng zhou
@ 2023-04-21 18:20   ` Alexei Starovoitov
  0 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2023-04-21 18:20 UTC (permalink / raw)
  To: Feng zhou
  Cc: martin.lau, ast, daniel, andrii, song, yhs, john.fastabend,
	kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba, pabeni,
	mykolal, shuah, bpf, linux-kernel, netdev, linux-kselftest,
	yangzhenze, wangdongdong.6

On Fri, Apr 21, 2023 at 05:04:02PM +0800, Feng zhou wrote:
> From: Feng Zhou <zhoufeng.zf@bytedance.com>
> 
> Add a kfunc that's similar to the bpf_current_task_under_cgroup.
> The difference is that it is a designated task.
> 
> When hook sched related functions, sometimes it is necessary to
> specify a task instead of the current task.
> 
> Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
> ---
>  kernel/bpf/helpers.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 00e5fb0682ac..88e3247b5c44 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2142,6 +2142,24 @@ __bpf_kfunc struct cgroup *bpf_cgroup_from_id(u64 cgid)
>  		return NULL;
>  	return cgrp;
>  }
> +
> +/**
> + * bpf_task_under_cgroup - Check whether the task is a given subset of the
> + * cgroup2 hierarchy. The cgroup2 to test is assigned by cgrp.

That doesn't read right.

> + * @cgrp: assigned cgrp.
> + * @task: assigned task.

This is also wrong.
Please copy paste from task_under_cgroup_hierarchy.

> + */
> +__bpf_kfunc int bpf_task_under_cgroup(struct cgroup *cgrp,
> +				      struct task_struct *task)

return type needs to be 'long'. 'int' might have issues.
Also the args should be task, cgrp to match task_under_cgroup_hierarchy.

> +{
> +	if (unlikely(!cgrp))
> +		return -EAGAIN;
> +
> +	if (unlikely(!task))
> +		return -ENOENT;

Please do
if (unlikely(!cgrp || !task))
        return -EINVAL;

> +
> +	return task_under_cgroup_hierarchy(task, cgrp);
> +}
>  #endif /* CONFIG_CGROUPS */
>  
>  /**
> @@ -2341,6 +2359,7 @@ BTF_ID_FLAGS(func, bpf_cgroup_acquire, KF_ACQUIRE | KF_RCU | KF_RET_NULL)
>  BTF_ID_FLAGS(func, bpf_cgroup_release, KF_RELEASE)
>  BTF_ID_FLAGS(func, bpf_cgroup_ancestor, KF_ACQUIRE | KF_RCU | KF_RET_NULL)
>  BTF_ID_FLAGS(func, bpf_cgroup_from_id, KF_ACQUIRE | KF_RET_NULL)
> +BTF_ID_FLAGS(func, bpf_task_under_cgroup, KF_RCU)
>  #endif
>  BTF_ID_FLAGS(func, bpf_task_from_pid, KF_ACQUIRE | KF_RET_NULL)
>  BTF_SET8_END(generic_btf_ids)
> -- 
> 2.20.1
> 

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

* Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add testcase for bpf_task_under_cgroup
  2023-04-21  9:04 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add testcase for bpf_task_under_cgroup Feng zhou
@ 2023-04-21 18:24   ` Alexei Starovoitov
       [not found]     ` <f860cf89-74b2-9102-d28b-abec7d51f349@bytedance.com>
  0 siblings, 1 reply; 6+ messages in thread
From: Alexei Starovoitov @ 2023-04-21 18:24 UTC (permalink / raw)
  To: Feng zhou
  Cc: martin.lau, ast, daniel, andrii, song, yhs, john.fastabend,
	kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba, pabeni,
	mykolal, shuah, bpf, linux-kernel, netdev, linux-kselftest,
	yangzhenze, wangdongdong.6

On Fri, Apr 21, 2023 at 05:04:03PM +0800, Feng zhou wrote:
> From: Feng Zhou <zhoufeng.zf@bytedance.com>
> 
> test_progs:
> Tests new kfunc bpf_task_under_cgroup().
> 
> The bpf program saves the pid which call the getuid syscall within a
> given cgroup to the remote_pid, which is convenient for the user-mode
> program to verify the test correctness.
> 
> The user-mode program creates its own mount namespace, and mounts the
> cgroupsv2 hierarchy in there, call the getuid syscall, then check if
> remote_pid and local_pid are equal.
> 
> Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
> ---
>  .../bpf/prog_tests/task_under_cgroup.c        | 46 +++++++++++++++++++
>  .../selftests/bpf/progs/cgrp_kfunc_common.h   |  1 +
>  .../bpf/progs/test_task_under_cgroup.c        | 40 ++++++++++++++++
>  3 files changed, 87 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_task_under_cgroup.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c b/tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
> new file mode 100644
> index 000000000000..bd3deb469938
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Bytedance */
> +
> +#include <test_progs.h>
> +#include <cgroup_helpers.h>
> +#include "test_task_under_cgroup.skel.h"
> +
> +#define FOO	"/foo"
> +
> +void test_task_under_cgroup(void)
> +{
> +	struct test_task_under_cgroup *skel;
> +	int ret, foo = -1;
> +
> +	foo = test__join_cgroup(FOO);
> +	if (!ASSERT_OK(foo < 0, "cgroup_join_foo"))
> +		return;
> +
> +	skel = test_task_under_cgroup__open();
> +	if (!ASSERT_OK_PTR(skel, "test_task_under_cgroup__open"))
> +		goto cleanup;
> +
> +	skel->rodata->local_pid = getpid();
> +	skel->rodata->cgid = get_cgroup_id(FOO);
> +
> +	ret = test_task_under_cgroup__load(skel);
> +	if (!ASSERT_OK(ret, "test_task_under_cgroup__load"))
> +		goto cleanup;
> +
> +	ret = test_task_under_cgroup__attach(skel);
> +	if (!ASSERT_OK(ret, "test_task_under_cgroup__attach"))
> +		goto cleanup;
> +
> +	syscall(__NR_getuid);
> +
> +	test_task_under_cgroup__detach(skel);
> +
> +	ASSERT_EQ(skel->bss->remote_pid, skel->rodata->local_pid,
> +		  "test task_under_cgroup");
> +
> +cleanup:
> +	if (foo)
> +		close(foo);

Looks wrong. should be if (foo >= 0) ?

> +
> +	test_task_under_cgroup__destroy(skel);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h b/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h
> index 22914a70db54..41b3ea231698 100644
> --- a/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h
> +++ b/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h
> @@ -26,6 +26,7 @@ struct cgroup *bpf_cgroup_ancestor(struct cgroup *cgrp, int level) __ksym;
>  struct cgroup *bpf_cgroup_from_id(u64 cgid) __ksym;
>  void bpf_rcu_read_lock(void) __ksym;
>  void bpf_rcu_read_unlock(void) __ksym;
> +int bpf_task_under_cgroup(struct cgroup *cgrp, struct task_struct *task) __ksym;
>  
>  static inline struct __cgrps_kfunc_map_value *cgrps_kfunc_map_value_lookup(struct cgroup *cgrp)
>  {
> diff --git a/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c
> new file mode 100644
> index 000000000000..e2740f9b029d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Bytedance */
> +
> +#include <vmlinux.h>
> +#include <asm/unistd.h>
> +#include <bpf/bpf_tracing.h>
> +#include <bpf/bpf_helpers.h>
> +
> +#include "cgrp_kfunc_common.h"
> +
> +const volatile int local_pid;
> +const volatile long cgid;
> +int remote_pid;
> +
> +SEC("tp_btf/sys_enter")

pls narrow down to specific syscall. Like you use in user space part: getuid

Also add this test to denylist.s390. See BPF CI failure.

> +int BPF_PROG(sysenter, struct pt_regs *regs, long id)
> +{
> +	struct cgroup *cgrp;
> +
> +	if (id != __NR_getuid)
> +		return 0;
> +
> +	if (local_pid != (bpf_get_current_pid_tgid() >> 32))
> +		return 0;
> +
> +	cgrp = bpf_cgroup_from_id(cgid);
> +	if (!cgrp)
> +		return 0;
> +
> +	if (!bpf_task_under_cgroup(cgrp, bpf_get_current_task_btf()))
> +		goto out;
> +
> +	remote_pid = local_pid;
> +
> +out:
> +	bpf_cgroup_release(cgrp);
> +	return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> -- 
> 2.20.1
> 

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

* Re: [External] Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add testcase for bpf_task_under_cgroup
       [not found]     ` <f860cf89-74b2-9102-d28b-abec7d51f349@bytedance.com>
@ 2023-04-23 16:22       ` Alexei Starovoitov
  0 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2023-04-23 16:22 UTC (permalink / raw)
  To: Feng Zhou
  Cc: Martin KaFai Lau, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Mykola Lysenko, Shuah Khan, bpf, LKML, Network Development,
	open list:KERNEL SELFTEST FRAMEWORK, yangzhenze, Dongdong Wang

On Sat, Apr 22, 2023 at 8:51 PM Feng Zhou <zhoufeng.zf@bytedance.com> wrote:
>
> 在 2023/4/22 02:24, Alexei Starovoitov 写道:
>
> On Fri, Apr 21, 2023 at 05:04:03PM +0800, Feng zhou wrote:
>
> From: Feng Zhou <zhoufeng.zf@bytedance.com>
>
> test_progs:
> Tests new kfunc bpf_task_under_cgroup().
>
> The bpf program saves the pid which call the getuid syscall within a
> given cgroup to the remote_pid, which is convenient for the user-mode
> program to verify the test correctness.
>
> The user-mode program creates its own mount namespace, and mounts the
> cgroupsv2 hierarchy in there, call the getuid syscall, then check if
> remote_pid and local_pid are equal.
>
> Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
> ---
>  .../bpf/prog_tests/task_under_cgroup.c        | 46 +++++++++++++++++++
>  .../selftests/bpf/progs/cgrp_kfunc_common.h   |  1 +
>  .../bpf/progs/test_task_under_cgroup.c        | 40 ++++++++++++++++
>  3 files changed, 87 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_task_under_cgroup.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c b/tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
> new file mode 100644
> index 000000000000..bd3deb469938
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Bytedance */
> +
> +#include <test_progs.h>
> +#include <cgroup_helpers.h>
> +#include "test_task_under_cgroup.skel.h"
> +
> +#define FOO "/foo"
> +
> +void test_task_under_cgroup(void)
> +{
> + struct test_task_under_cgroup *skel;
> + int ret, foo = -1;
> +
> + foo = test__join_cgroup(FOO);
> + if (!ASSERT_OK(foo < 0, "cgroup_join_foo"))
> + return;
> +
> + skel = test_task_under_cgroup__open();
> + if (!ASSERT_OK_PTR(skel, "test_task_under_cgroup__open"))
> + goto cleanup;
> +
> + skel->rodata->local_pid = getpid();
> + skel->rodata->cgid = get_cgroup_id(FOO);
> +
> + ret = test_task_under_cgroup__load(skel);
> + if (!ASSERT_OK(ret, "test_task_under_cgroup__load"))
> + goto cleanup;
> +
> + ret = test_task_under_cgroup__attach(skel);
> + if (!ASSERT_OK(ret, "test_task_under_cgroup__attach"))
> + goto cleanup;
> +
> + syscall(__NR_getuid);
> +
> + test_task_under_cgroup__detach(skel);
> +
> + ASSERT_EQ(skel->bss->remote_pid, skel->rodata->local_pid,
> +  "test task_under_cgroup");
> +
> +cleanup:
> + if (foo)
> + close(foo);
>
> Looks wrong. should be if (foo >= 0) ?
>
> Yes.
>
> +
> + test_task_under_cgroup__destroy(skel);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h b/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h
> index 22914a70db54..41b3ea231698 100644
> --- a/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h
> +++ b/tools/testing/selftests/bpf/progs/cgrp_kfunc_common.h
> @@ -26,6 +26,7 @@ struct cgroup *bpf_cgroup_ancestor(struct cgroup *cgrp, int level) __ksym;
>  struct cgroup *bpf_cgroup_from_id(u64 cgid) __ksym;
>  void bpf_rcu_read_lock(void) __ksym;
>  void bpf_rcu_read_unlock(void) __ksym;
> +int bpf_task_under_cgroup(struct cgroup *cgrp, struct task_struct *task) __ksym;
>
>  static inline struct __cgrps_kfunc_map_value *cgrps_kfunc_map_value_lookup(struct cgroup *cgrp)
>  {
> diff --git a/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c
> new file mode 100644
> index 000000000000..e2740f9b029d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Bytedance */
> +
> +#include <vmlinux.h>
> +#include <asm/unistd.h>
> +#include <bpf/bpf_tracing.h>
> +#include <bpf/bpf_helpers.h>
> +
> +#include "cgrp_kfunc_common.h"
> +
> +const volatile int local_pid;
> +const volatile long cgid;
> +int remote_pid;
> +
> +SEC("tp_btf/sys_enter")
>
> pls narrow down to specific syscall. Like you use in user space part: getuid
>
> Also add this test to denylist.s390. See BPF CI failure.
>
> bpf_task_under_cgroup is placed in generic_btf_ids, belongs to BPF_PROG_TYPE_TRACING,
>
> if narrow down to specific syscall and uses SEC ("tp/syscalls/sys_enter_getuid"),
>
> bpf prog type is TRACEPOINT, kfunc cannot be used, and reports
>
> "calls kernel function bpf_cgroup_from_id is not allowed".

sure, pls narrow it down. sys_enter is too broad.
In parallel test_progs the bpf prog will be way too many times.

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

end of thread, other threads:[~2023-04-23 16:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-21  9:04 [PATCH bpf-next v2 0/2] Introduce a new kfunc of bpf_task_under_cgroup Feng zhou
2023-04-21  9:04 ` [PATCH bpf-next v2 1/2] bpf: Add bpf_task_under_cgroup() kfunc Feng zhou
2023-04-21 18:20   ` Alexei Starovoitov
2023-04-21  9:04 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add testcase for bpf_task_under_cgroup Feng zhou
2023-04-21 18:24   ` Alexei Starovoitov
     [not found]     ` <f860cf89-74b2-9102-d28b-abec7d51f349@bytedance.com>
2023-04-23 16:22       ` [External] " Alexei Starovoitov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.