* [PATCH bpf-next 0/3] bpf: expose bpf_{g,s}et_retval to more cgroup hooks
@ 2022-08-12 19:02 Stanislav Fomichev
2022-08-12 19:02 ` [PATCH bpf-next 1/3] bpf: introduce cgroup_{common,current}_func_proto Stanislav Fomichev
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2022-08-12 19:02 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa
Apparently, only a small subset of cgroup hooks actually falls
back to cgroup_base_func_proto. This leads to unexpected result
where not all cgroup helpers have bpf_{g,s}et_retval.
It's getting harder and harder to manage which helpers are exported
to which hooks. We now have the following call chains:
- cg_skb_func_proto
- sk_filter_func_proto
- bpf_sk_base_func_proto
- bpf_base_func_proto
So by looking at cg_skb_func_proto it's pretty hard to understand
what's going on.
For cgroup helpers, I'm proposing we do the following instead:
func_proto = cgroup_common_func_proto();
if (func_proto) return func_proto;
/* optional, if hook has 'current' */
func_proto = cgroup_current_func_proto();
if (func_proto) return func_proto;
...
switch (func_id) {
/* hook specific helpers */
case BPF_FUNC_hook_specific_helper:
return &xyz;
default:
/* always fall back to plain bpf_base_func_proto */
bpf_base_func_proto(func_id);
}
If this turns out more workable, we can follow up with converting
the rest to the same pattern.
Stanislav Fomichev (3):
bpf: introduce cgroup_{common,current}_func_proto
bpf: use cgroup_{common,current}_func_proto in more hooks
selftests/bpf: make sure bpf_{g,s}et_retval is exposed everywhere
include/linux/bpf.h | 16 +++
kernel/bpf/cgroup.c | 82 +++++++--------
kernel/bpf/helpers.c | 99 ++++++++++++++++++-
net/core/filter.c | 78 ++++++---------
tools/testing/selftests/bpf/Makefile | 1 +
.../bpf/cgroup_getset_retval_hooks.h | 25 +++++
.../bpf/prog_tests/cgroup_getset_retval.c | 48 +++++++++
.../bpf/progs/cgroup_getset_retval_hooks.c | 16 +++
8 files changed, 270 insertions(+), 95 deletions(-)
create mode 100644 tools/testing/selftests/bpf/cgroup_getset_retval_hooks.h
create mode 100644 tools/testing/selftests/bpf/progs/cgroup_getset_retval_hooks.c
--
2.37.1.595.g718a3a8f04-goog
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH bpf-next 1/3] bpf: introduce cgroup_{common,current}_func_proto
2022-08-12 19:02 [PATCH bpf-next 0/3] bpf: expose bpf_{g,s}et_retval to more cgroup hooks Stanislav Fomichev
@ 2022-08-12 19:02 ` Stanislav Fomichev
2022-08-15 21:26 ` Martin KaFai Lau
2022-08-12 19:02 ` [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks Stanislav Fomichev
2022-08-12 19:02 ` [PATCH bpf-next 3/3] selftests/bpf: make sure bpf_{g,s}et_retval is exposed everywhere Stanislav Fomichev
2 siblings, 1 reply; 10+ messages in thread
From: Stanislav Fomichev @ 2022-08-12 19:02 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa
Split cgroup_base_func_proto into the following:
* cgroup_common_func_proto - common helpers for all cgroup hooks
* cgroup_current_func_proto - common helpers for all cgroup hooks
running in the process context (== have meaningful 'current').
Move bpf_{g,s}et_retval into kernel/bpf/helpers.c so they are more
usable from the core parts.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
include/linux/bpf.h | 16 +++++++++
kernel/bpf/cgroup.c | 82 +++++++++++++++++++-------------------------
kernel/bpf/helpers.c | 67 ++++++++++++++++++++++++++++++++++--
3 files changed, 116 insertions(+), 49 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a627a02cf8ab..cdb295c6c728 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1948,6 +1948,10 @@ struct bpf_prog *bpf_prog_by_id(u32 id);
struct bpf_link *bpf_link_by_id(u32 id);
const struct bpf_func_proto *bpf_base_func_proto(enum bpf_func_id func_id);
+const struct bpf_func_proto *
+cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog);
+const struct bpf_func_proto *
+cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog);
void bpf_task_storage_free(struct task_struct *task);
bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog);
const struct btf_func_model *
@@ -2154,6 +2158,18 @@ bpf_base_func_proto(enum bpf_func_id func_id)
return NULL;
}
+static inline const struct bpf_func_proto *
+cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+ return NULL;
+}
+
+static inline const struct bpf_func_proto *
+cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+ return NULL;
+}
+
static inline void bpf_task_storage_free(struct task_struct *task)
{
}
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 59b7eb60d5b4..cc0b33b7d4cc 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1527,63 +1527,27 @@ int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
return ret;
}
-BPF_CALL_0(bpf_get_retval)
-{
- struct bpf_cg_run_ctx *ctx =
- container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
-
- return ctx->retval;
-}
-
-const struct bpf_func_proto bpf_get_retval_proto = {
- .func = bpf_get_retval,
- .gpl_only = false,
- .ret_type = RET_INTEGER,
-};
-
-BPF_CALL_1(bpf_set_retval, int, retval)
+static const struct bpf_func_proto *
+cgroup_dev_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
- struct bpf_cg_run_ctx *ctx =
- container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
+ const struct bpf_func_proto *func_proto;
- ctx->retval = retval;
- return 0;
-}
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
-const struct bpf_func_proto bpf_set_retval_proto = {
- .func = bpf_set_retval,
- .gpl_only = false,
- .ret_type = RET_INTEGER,
- .arg1_type = ARG_ANYTHING,
-};
+ func_proto = cgroup_current_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
-static const struct bpf_func_proto *
-cgroup_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
-{
switch (func_id) {
- case BPF_FUNC_get_current_uid_gid:
- return &bpf_get_current_uid_gid_proto;
- case BPF_FUNC_get_local_storage:
- return &bpf_get_local_storage_proto;
- case BPF_FUNC_get_current_cgroup_id:
- return &bpf_get_current_cgroup_id_proto;
case BPF_FUNC_perf_event_output:
return &bpf_event_output_data_proto;
- case BPF_FUNC_get_retval:
- return &bpf_get_retval_proto;
- case BPF_FUNC_set_retval:
- return &bpf_set_retval_proto;
default:
return bpf_base_func_proto(func_id);
}
}
-static const struct bpf_func_proto *
-cgroup_dev_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
-{
- return cgroup_base_func_proto(func_id, prog);
-}
-
static bool cgroup_dev_is_valid_access(int off, int size,
enum bpf_access_type type,
const struct bpf_prog *prog,
@@ -2096,6 +2060,16 @@ static const struct bpf_func_proto bpf_sysctl_set_new_value_proto = {
static const struct bpf_func_proto *
sysctl_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+ const struct bpf_func_proto *func_proto;
+
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
+ func_proto = cgroup_current_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
switch (func_id) {
case BPF_FUNC_strtol:
return &bpf_strtol_proto;
@@ -2111,8 +2085,10 @@ sysctl_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sysctl_set_new_value_proto;
case BPF_FUNC_ktime_get_coarse_ns:
return &bpf_ktime_get_coarse_ns_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_event_output_data_proto;
default:
- return cgroup_base_func_proto(func_id, prog);
+ return bpf_base_func_proto(func_id);
}
}
@@ -2233,6 +2209,16 @@ static const struct bpf_func_proto bpf_get_netns_cookie_sockopt_proto = {
static const struct bpf_func_proto *
cg_sockopt_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+ const struct bpf_func_proto *func_proto;
+
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
+ func_proto = cgroup_current_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
switch (func_id) {
#ifdef CONFIG_NET
case BPF_FUNC_get_netns_cookie:
@@ -2254,8 +2240,10 @@ cg_sockopt_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
case BPF_FUNC_tcp_sock:
return &bpf_tcp_sock_proto;
#endif
+ case BPF_FUNC_perf_event_output:
+ return &bpf_event_output_data_proto;
default:
- return cgroup_base_func_proto(func_id, prog);
+ return bpf_base_func_proto(func_id);
}
}
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 3c1b9bbcf971..de7d2fabb06d 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -429,7 +429,6 @@ const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto = {
};
#ifdef CONFIG_CGROUP_BPF
-
BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
{
/* flags argument is not used now,
@@ -460,7 +459,37 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
.arg1_type = ARG_CONST_MAP_PTR,
.arg2_type = ARG_ANYTHING,
};
-#endif
+
+BPF_CALL_0(bpf_get_retval)
+{
+ struct bpf_cg_run_ctx *ctx =
+ container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
+
+ return ctx->retval;
+}
+
+const struct bpf_func_proto bpf_get_retval_proto = {
+ .func = bpf_get_retval,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+};
+
+BPF_CALL_1(bpf_set_retval, int, retval)
+{
+ struct bpf_cg_run_ctx *ctx =
+ container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
+
+ ctx->retval = retval;
+ return 0;
+}
+
+const struct bpf_func_proto bpf_set_retval_proto = {
+ .func = bpf_set_retval,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_ANYTHING,
+};
+#endif /* CONFIG_CGROUP_BPF */
#define BPF_STRTOX_BASE_MASK 0x1F
@@ -1726,6 +1755,40 @@ bpf_base_func_proto(enum bpf_func_id func_id)
}
}
+/* Common helpers for cgroup hooks. */
+const struct bpf_func_proto *
+cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+ switch (func_id) {
+#ifdef CONFIG_CGROUP_BPF
+ case BPF_FUNC_get_local_storage:
+ return &bpf_get_local_storage_proto;
+ case BPF_FUNC_get_retval:
+ return &bpf_get_retval_proto;
+ case BPF_FUNC_set_retval:
+ return &bpf_set_retval_proto;
+#endif
+ default:
+ return NULL;
+ }
+}
+
+/* Common helpers for cgroup hooks with valid process context. */
+const struct bpf_func_proto *
+cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+ switch (func_id) {
+#ifdef CONFIG_CGROUPS
+ case BPF_FUNC_get_current_uid_gid:
+ return &bpf_get_current_uid_gid_proto;
+ case BPF_FUNC_get_current_cgroup_id:
+ return &bpf_get_current_cgroup_id_proto;
+#endif
+ default:
+ return NULL;
+ }
+}
+
BTF_SET8_START(tracing_btf_ids)
#ifdef CONFIG_KEXEC_CORE
BTF_ID_FLAGS(func, crash_kexec, KF_DESTRUCTIVE)
--
2.37.1.595.g718a3a8f04-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks
2022-08-12 19:02 [PATCH bpf-next 0/3] bpf: expose bpf_{g,s}et_retval to more cgroup hooks Stanislav Fomichev
2022-08-12 19:02 ` [PATCH bpf-next 1/3] bpf: introduce cgroup_{common,current}_func_proto Stanislav Fomichev
@ 2022-08-12 19:02 ` Stanislav Fomichev
2022-08-13 6:25 ` kernel test robot
` (2 more replies)
2022-08-12 19:02 ` [PATCH bpf-next 3/3] selftests/bpf: make sure bpf_{g,s}et_retval is exposed everywhere Stanislav Fomichev
2 siblings, 3 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2022-08-12 19:02 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa
The following hooks are per-cgroup hooks but they are not
using cgroup_{common,current}_func_proto, fix it:
* BPF_PROG_TYPE_CGROUP_SKB (cg_skb)
* BPF_PROG_TYPE_CGROUP_SOCK_ADDR (cg_sock_addr)
* BPF_PROG_TYPE_CGROUP_SOCK (cg_sock)
Also:
* move common func_proto's into cgroup func_proto handlers
* make sure bpf_{g,s}et_retval are not accessible from recvmsg,
getpeername and getsockname (return/errno is ignore in these
places)
* as a side effect, expose get_current_pid_tgid, get_current_comm_proto,
get_current_ancestor_cgroup_id, get_cgroup_classid to more cgroup
hooks
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
kernel/bpf/helpers.c | 36 ++++++++++++++++++--
net/core/filter.c | 78 ++++++++++++++++++--------------------------
2 files changed, 66 insertions(+), 48 deletions(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index de7d2fabb06d..87ce47b13b22 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1764,9 +1764,31 @@ cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
case BPF_FUNC_get_local_storage:
return &bpf_get_local_storage_proto;
case BPF_FUNC_get_retval:
- return &bpf_get_retval_proto;
+ switch (prog->expected_attach_type) {
+ case BPF_CGROUP_SOCK_OPS:
+ case BPF_CGROUP_UDP4_RECVMSG:
+ case BPF_CGROUP_UDP6_RECVMSG:
+ case BPF_CGROUP_INET4_GETPEERNAME:
+ case BPF_CGROUP_INET6_GETPEERNAME:
+ case BPF_CGROUP_INET4_GETSOCKNAME:
+ case BPF_CGROUP_INET6_GETSOCKNAME:
+ return NULL;
+ default:
+ return &bpf_get_retval_proto;
+ }
case BPF_FUNC_set_retval:
- return &bpf_set_retval_proto;
+ switch (prog->expected_attach_type) {
+ case BPF_CGROUP_SOCK_OPS:
+ case BPF_CGROUP_UDP4_RECVMSG:
+ case BPF_CGROUP_UDP6_RECVMSG:
+ case BPF_CGROUP_INET4_GETPEERNAME:
+ case BPF_CGROUP_INET6_GETPEERNAME:
+ case BPF_CGROUP_INET4_GETSOCKNAME:
+ case BPF_CGROUP_INET6_GETSOCKNAME:
+ return NULL;
+ default:
+ return &bpf_set_retval_proto;
+ }
#endif
default:
return NULL;
@@ -1781,8 +1803,18 @@ cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
#ifdef CONFIG_CGROUPS
case BPF_FUNC_get_current_uid_gid:
return &bpf_get_current_uid_gid_proto;
+ case BPF_FUNC_get_current_pid_tgid:
+ return &bpf_get_current_pid_tgid_proto;
+ case BPF_FUNC_get_current_comm:
+ return &bpf_get_current_comm_proto;
case BPF_FUNC_get_current_cgroup_id:
return &bpf_get_current_cgroup_id_proto;
+ case BPF_FUNC_get_current_ancestor_cgroup_id:
+ return &bpf_get_current_ancestor_cgroup_id_proto;
+#endif
+#ifdef CONFIG_CGROUP_NET_CLASSID
+ case BPF_FUNC_get_cgroup_classid:
+ return &bpf_get_cgroup_classid_curr_proto;
#endif
default:
return NULL;
diff --git a/net/core/filter.c b/net/core/filter.c
index 5669248aff25..6731e60621e9 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7663,34 +7663,23 @@ const struct bpf_func_proto bpf_sk_storage_get_cg_sock_proto __weak;
static const struct bpf_func_proto *
sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+ const struct bpf_func_proto *func_proto;
+
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
+ func_proto = cgroup_current_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
switch (func_id) {
- /* inet and inet6 sockets are created in a process
- * context so there is always a valid uid/gid
- */
- case BPF_FUNC_get_current_uid_gid:
- return &bpf_get_current_uid_gid_proto;
- case BPF_FUNC_get_local_storage:
- return &bpf_get_local_storage_proto;
case BPF_FUNC_get_socket_cookie:
return &bpf_get_socket_cookie_sock_proto;
case BPF_FUNC_get_netns_cookie:
return &bpf_get_netns_cookie_sock_proto;
case BPF_FUNC_perf_event_output:
return &bpf_event_output_data_proto;
- case BPF_FUNC_get_current_pid_tgid:
- return &bpf_get_current_pid_tgid_proto;
- case BPF_FUNC_get_current_comm:
- return &bpf_get_current_comm_proto;
-#ifdef CONFIG_CGROUPS
- case BPF_FUNC_get_current_cgroup_id:
- return &bpf_get_current_cgroup_id_proto;
- case BPF_FUNC_get_current_ancestor_cgroup_id:
- return &bpf_get_current_ancestor_cgroup_id_proto;
-#endif
-#ifdef CONFIG_CGROUP_NET_CLASSID
- case BPF_FUNC_get_cgroup_classid:
- return &bpf_get_cgroup_classid_curr_proto;
-#endif
case BPF_FUNC_sk_storage_get:
return &bpf_sk_storage_get_cg_sock_proto;
case BPF_FUNC_ktime_get_coarse_ns:
@@ -7703,12 +7692,17 @@ sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
static const struct bpf_func_proto *
sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+ const struct bpf_func_proto *func_proto;
+
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
+ func_proto = cgroup_current_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
switch (func_id) {
- /* inet and inet6 sockets are created in a process
- * context so there is always a valid uid/gid
- */
- case BPF_FUNC_get_current_uid_gid:
- return &bpf_get_current_uid_gid_proto;
case BPF_FUNC_bind:
switch (prog->expected_attach_type) {
case BPF_CGROUP_INET4_CONNECT:
@@ -7721,24 +7715,8 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_get_socket_cookie_sock_addr_proto;
case BPF_FUNC_get_netns_cookie:
return &bpf_get_netns_cookie_sock_addr_proto;
- case BPF_FUNC_get_local_storage:
- return &bpf_get_local_storage_proto;
case BPF_FUNC_perf_event_output:
return &bpf_event_output_data_proto;
- case BPF_FUNC_get_current_pid_tgid:
- return &bpf_get_current_pid_tgid_proto;
- case BPF_FUNC_get_current_comm:
- return &bpf_get_current_comm_proto;
-#ifdef CONFIG_CGROUPS
- case BPF_FUNC_get_current_cgroup_id:
- return &bpf_get_current_cgroup_id_proto;
- case BPF_FUNC_get_current_ancestor_cgroup_id:
- return &bpf_get_current_ancestor_cgroup_id_proto;
-#endif
-#ifdef CONFIG_CGROUP_NET_CLASSID
- case BPF_FUNC_get_cgroup_classid:
- return &bpf_get_cgroup_classid_curr_proto;
-#endif
#ifdef CONFIG_INET
case BPF_FUNC_sk_lookup_tcp:
return &bpf_sock_addr_sk_lookup_tcp_proto;
@@ -7819,9 +7797,13 @@ const struct bpf_func_proto bpf_sk_storage_delete_proto __weak;
static const struct bpf_func_proto *
cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+ const struct bpf_func_proto *func_proto;
+
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
switch (func_id) {
- case BPF_FUNC_get_local_storage:
- return &bpf_get_local_storage_proto;
case BPF_FUNC_sk_fullsock:
return &bpf_sk_fullsock_proto;
case BPF_FUNC_sk_storage_get:
@@ -8061,6 +8043,12 @@ const struct bpf_func_proto bpf_sock_hash_update_proto __weak;
static const struct bpf_func_proto *
sock_ops_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+ const struct bpf_func_proto *func_proto;
+
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
switch (func_id) {
case BPF_FUNC_setsockopt:
return &bpf_sock_ops_setsockopt_proto;
@@ -8074,8 +8062,6 @@ sock_ops_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sock_hash_update_proto;
case BPF_FUNC_get_socket_cookie:
return &bpf_get_socket_cookie_sock_ops_proto;
- case BPF_FUNC_get_local_storage:
- return &bpf_get_local_storage_proto;
case BPF_FUNC_perf_event_output:
return &bpf_event_output_data_proto;
case BPF_FUNC_sk_storage_get:
--
2.37.1.595.g718a3a8f04-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf-next 3/3] selftests/bpf: make sure bpf_{g,s}et_retval is exposed everywhere
2022-08-12 19:02 [PATCH bpf-next 0/3] bpf: expose bpf_{g,s}et_retval to more cgroup hooks Stanislav Fomichev
2022-08-12 19:02 ` [PATCH bpf-next 1/3] bpf: introduce cgroup_{common,current}_func_proto Stanislav Fomichev
2022-08-12 19:02 ` [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks Stanislav Fomichev
@ 2022-08-12 19:02 ` Stanislav Fomichev
2 siblings, 0 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2022-08-12 19:02 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa
For each hook, have a simple bpf_set_retval(bpf_get_retval) program
and make sure it loads for the hooks we want. The exceptions are
the hooks which don't propagate the error to the callers:
- sockops
- recvmsg
- getpeername
- getsockname
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/Makefile | 1 +
.../bpf/cgroup_getset_retval_hooks.h | 25 ++++++++++
.../bpf/prog_tests/cgroup_getset_retval.c | 48 +++++++++++++++++++
.../bpf/progs/cgroup_getset_retval_hooks.c | 16 +++++++
4 files changed, 90 insertions(+)
create mode 100644 tools/testing/selftests/bpf/cgroup_getset_retval_hooks.h
create mode 100644 tools/testing/selftests/bpf/progs/cgroup_getset_retval_hooks.c
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 8d59ec7f4c2d..eecad99f1735 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -323,6 +323,7 @@ $(OUTPUT)/test_l4lb_noinline.o: BPF_CFLAGS += -fno-inline
$(OUTPUT)/test_xdp_noinline.o: BPF_CFLAGS += -fno-inline
$(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h
+$(OUTPUT)/cgroup_getset_retval_hooks.o: cgroup_getset_retval_hooks.h
# Build BPF object using Clang
# $1 - input .c file
diff --git a/tools/testing/selftests/bpf/cgroup_getset_retval_hooks.h b/tools/testing/selftests/bpf/cgroup_getset_retval_hooks.h
new file mode 100644
index 000000000000..4a8d2063163d
--- /dev/null
+++ b/tools/testing/selftests/bpf/cgroup_getset_retval_hooks.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+BPF_RETVAL_HOOK(ingress, "cgroup_skb/ingress", __sk_buff, 0)
+BPF_RETVAL_HOOK(egress, "cgroup_skb/egress", __sk_buff, 0)
+BPF_RETVAL_HOOK(sock_create, "cgroup/sock_create", bpf_sock, 0)
+BPF_RETVAL_HOOK(sock_ops, "sockops", bpf_sock_ops, -EINVAL)
+BPF_RETVAL_HOOK(dev, "cgroup/dev", bpf_cgroup_dev_ctx, 0)
+BPF_RETVAL_HOOK(bind4, "cgroup/bind4", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(bind6, "cgroup/bind6", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(connect4, "cgroup/connect4", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(connect6, "cgroup/connect6", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(post_bind4, "cgroup/post_bind4", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(post_bind6, "cgroup/post_bind6", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(sendmsg4, "cgroup/sendmsg4", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(sendmsg6, "cgroup/sendmsg6", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(sysctl, "cgroup/sysctl", bpf_sysctl, 0)
+BPF_RETVAL_HOOK(recvmsg4, "cgroup/recvmsg4", bpf_sock_addr, -EINVAL)
+BPF_RETVAL_HOOK(recvmsg6, "cgroup/recvmsg6", bpf_sock_addr, -EINVAL)
+BPF_RETVAL_HOOK(getsockopt, "cgroup/getsockopt", bpf_sockopt, 0)
+BPF_RETVAL_HOOK(setsockopt, "cgroup/setsockopt", bpf_sockopt, 0)
+BPF_RETVAL_HOOK(getpeername4, "cgroup/getpeername4", bpf_sock_addr, -EINVAL)
+BPF_RETVAL_HOOK(getpeername6, "cgroup/getpeername6", bpf_sock_addr, -EINVAL)
+BPF_RETVAL_HOOK(getsockname4, "cgroup/getsockname4", bpf_sock_addr, -EINVAL)
+BPF_RETVAL_HOOK(getsockname6, "cgroup/getsockname6", bpf_sock_addr, -EINVAL)
+BPF_RETVAL_HOOK(sock_release, "cgroup/sock_release", bpf_sock, 0)
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_getset_retval.c b/tools/testing/selftests/bpf/prog_tests/cgroup_getset_retval.c
index 0b47c3c000c7..4d2fa99273d8 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_getset_retval.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_getset_retval.c
@@ -10,6 +10,7 @@
#include "cgroup_getset_retval_setsockopt.skel.h"
#include "cgroup_getset_retval_getsockopt.skel.h"
+#include "cgroup_getset_retval_hooks.skel.h"
#define SOL_CUSTOM 0xdeadbeef
@@ -433,6 +434,50 @@ static void test_getsockopt_retval_sync(int cgroup_fd, int sock_fd)
cgroup_getset_retval_getsockopt__destroy(obj);
}
+struct exposed_hook {
+ const char *name;
+ int expected_err;
+} exposed_hooks[] = {
+
+#define BPF_RETVAL_HOOK(NAME, SECTION, CTX, EXPECTED_ERR) \
+ { \
+ .name = #NAME, \
+ .expected_err = EXPECTED_ERR, \
+ },
+
+#include "cgroup_getset_retval_hooks.h"
+
+#undef BPF_RETVAL_HOOK
+};
+
+static void test_exposed_hooks(int cgroup_fd, int sock_fd)
+{
+ struct cgroup_getset_retval_hooks *skel;
+ struct bpf_program *prog;
+ int err;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(exposed_hooks); i++) {
+ skel = cgroup_getset_retval_hooks__open();
+ if (!ASSERT_OK_PTR(skel, "cgroup_getset_retval_hooks__open"))
+ continue;
+
+ prog = bpf_object__find_program_by_name(skel->obj, exposed_hooks[i].name);
+ if (!ASSERT_NEQ(prog, NULL, "bpf_object__find_program_by_name"))
+ goto close_skel;
+
+ err = bpf_program__set_autoload(prog, true);
+ if (!ASSERT_OK(err, "bpf_program__set_autoload"))
+ goto close_skel;
+
+ err = cgroup_getset_retval_hooks__load(skel);
+ ASSERT_EQ(err, exposed_hooks[i].expected_err, "expected_err");
+
+close_skel:
+ cgroup_getset_retval_hooks__destroy(skel);
+ }
+}
+
void test_cgroup_getset_retval(void)
{
int cgroup_fd = -1;
@@ -476,6 +521,9 @@ void test_cgroup_getset_retval(void)
if (test__start_subtest("getsockopt-retval_sync"))
test_getsockopt_retval_sync(cgroup_fd, sock_fd);
+ if (test__start_subtest("exposed_hooks"))
+ test_exposed_hooks(cgroup_fd, sock_fd);
+
close_fd:
close(cgroup_fd);
}
diff --git a/tools/testing/selftests/bpf/progs/cgroup_getset_retval_hooks.c b/tools/testing/selftests/bpf/progs/cgroup_getset_retval_hooks.c
new file mode 100644
index 000000000000..13dfb4bbfd28
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/cgroup_getset_retval_hooks.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+#define BPF_RETVAL_HOOK(name, section, ctx, expected_err) \
+ __attribute__((__section__("?" section))) \
+ int name(struct ctx *_ctx) \
+ { \
+ bpf_set_retval(bpf_get_retval()); \
+ return 1; \
+ }
+
+#include "cgroup_getset_retval_hooks.h"
+
+#undef BPF_RETVAL_HOOK
--
2.37.1.595.g718a3a8f04-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks
2022-08-12 19:02 ` [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks Stanislav Fomichev
@ 2022-08-13 6:25 ` kernel test robot
2022-08-13 18:55 ` kernel test robot
2022-08-15 21:57 ` Martin KaFai Lau
2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2022-08-13 6:25 UTC (permalink / raw)
To: Stanislav Fomichev, bpf
Cc: kbuild-all, ast, daniel, andrii, martin.lau, song, yhs,
john.fastabend, kpsingh, sdf, haoluo, jolsa
Hi Stanislav,
I love your patch! Yet something to improve:
[auto build test ERROR on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Stanislav-Fomichev/bpf-expose-bpf_-g-s-et_retval-to-more-cgroup-hooks/20220813-030615
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-randconfig-a013 (https://download.01.org/0day-ci/archive/20220813/202208131415.CkdRZBrY-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/0429824054f7a843ee976d48432e825e493a0a7e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Stanislav-Fomichev/bpf-expose-bpf_-g-s-et_retval-to-more-cgroup-hooks/20220813-030615
git checkout 0429824054f7a843ee976d48432e825e493a0a7e
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
kernel/bpf/helpers.c: In function 'cgroup_current_func_proto':
>> kernel/bpf/helpers.c:1817:25: error: 'bpf_get_cgroup_classid_curr_proto' undeclared (first use in this function)
1817 | return &bpf_get_cgroup_classid_curr_proto;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
kernel/bpf/helpers.c:1817:25: note: each undeclared identifier is reported only once for each function it appears in
vim +/bpf_get_cgroup_classid_curr_proto +1817 kernel/bpf/helpers.c
1797
1798 /* Common helpers for cgroup hooks with valid process context. */
1799 const struct bpf_func_proto *
1800 cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
1801 {
1802 switch (func_id) {
1803 #ifdef CONFIG_CGROUPS
1804 case BPF_FUNC_get_current_uid_gid:
1805 return &bpf_get_current_uid_gid_proto;
1806 case BPF_FUNC_get_current_pid_tgid:
1807 return &bpf_get_current_pid_tgid_proto;
1808 case BPF_FUNC_get_current_comm:
1809 return &bpf_get_current_comm_proto;
1810 case BPF_FUNC_get_current_cgroup_id:
1811 return &bpf_get_current_cgroup_id_proto;
1812 case BPF_FUNC_get_current_ancestor_cgroup_id:
1813 return &bpf_get_current_ancestor_cgroup_id_proto;
1814 #endif
1815 #ifdef CONFIG_CGROUP_NET_CLASSID
1816 case BPF_FUNC_get_cgroup_classid:
> 1817 return &bpf_get_cgroup_classid_curr_proto;
1818 #endif
1819 default:
1820 return NULL;
1821 }
1822 }
1823
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks
2022-08-12 19:02 ` [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks Stanislav Fomichev
2022-08-13 6:25 ` kernel test robot
@ 2022-08-13 18:55 ` kernel test robot
2022-08-15 21:57 ` Martin KaFai Lau
2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2022-08-13 18:55 UTC (permalink / raw)
To: Stanislav Fomichev, bpf
Cc: llvm, kbuild-all, ast, daniel, andrii, martin.lau, song, yhs,
john.fastabend, kpsingh, sdf, haoluo, jolsa
Hi Stanislav,
I love your patch! Yet something to improve:
[auto build test ERROR on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Stanislav-Fomichev/bpf-expose-bpf_-g-s-et_retval-to-more-cgroup-hooks/20220813-030615
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: i386-randconfig-a004 (https://download.01.org/0day-ci/archive/20220814/202208140239.yjzPuMmv-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 3329cec2f79185bafd678f310fafadba2a8c76d2)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/0429824054f7a843ee976d48432e825e493a0a7e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Stanislav-Fomichev/bpf-expose-bpf_-g-s-et_retval-to-more-cgroup-hooks/20220813-030615
git checkout 0429824054f7a843ee976d48432e825e493a0a7e
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> kernel/bpf/helpers.c:1817:11: error: use of undeclared identifier 'bpf_get_cgroup_classid_curr_proto'
return &bpf_get_cgroup_classid_curr_proto;
^
1 error generated.
vim +/bpf_get_cgroup_classid_curr_proto +1817 kernel/bpf/helpers.c
1797
1798 /* Common helpers for cgroup hooks with valid process context. */
1799 const struct bpf_func_proto *
1800 cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
1801 {
1802 switch (func_id) {
1803 #ifdef CONFIG_CGROUPS
1804 case BPF_FUNC_get_current_uid_gid:
1805 return &bpf_get_current_uid_gid_proto;
1806 case BPF_FUNC_get_current_pid_tgid:
1807 return &bpf_get_current_pid_tgid_proto;
1808 case BPF_FUNC_get_current_comm:
1809 return &bpf_get_current_comm_proto;
1810 case BPF_FUNC_get_current_cgroup_id:
1811 return &bpf_get_current_cgroup_id_proto;
1812 case BPF_FUNC_get_current_ancestor_cgroup_id:
1813 return &bpf_get_current_ancestor_cgroup_id_proto;
1814 #endif
1815 #ifdef CONFIG_CGROUP_NET_CLASSID
1816 case BPF_FUNC_get_cgroup_classid:
> 1817 return &bpf_get_cgroup_classid_curr_proto;
1818 #endif
1819 default:
1820 return NULL;
1821 }
1822 }
1823
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next 1/3] bpf: introduce cgroup_{common,current}_func_proto
2022-08-12 19:02 ` [PATCH bpf-next 1/3] bpf: introduce cgroup_{common,current}_func_proto Stanislav Fomichev
@ 2022-08-15 21:26 ` Martin KaFai Lau
2022-08-16 17:19 ` Stanislav Fomichev
0 siblings, 1 reply; 10+ messages in thread
From: Martin KaFai Lau @ 2022-08-15 21:26 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa
On Fri, Aug 12, 2022 at 12:02:39PM -0700, Stanislav Fomichev wrote:
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 3c1b9bbcf971..de7d2fabb06d 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -429,7 +429,6 @@ const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto = {
> };
>
> #ifdef CONFIG_CGROUP_BPF
> -
> BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
> {
> /* flags argument is not used now,
> @@ -460,7 +459,37 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
> .arg1_type = ARG_CONST_MAP_PTR,
> .arg2_type = ARG_ANYTHING,
> };
> -#endif
> +
> +BPF_CALL_0(bpf_get_retval)
> +{
> + struct bpf_cg_run_ctx *ctx =
> + container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
> +
> + return ctx->retval;
> +}
> +
> +const struct bpf_func_proto bpf_get_retval_proto = {
> + .func = bpf_get_retval,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> +};
> +
> +BPF_CALL_1(bpf_set_retval, int, retval)
> +{
> + struct bpf_cg_run_ctx *ctx =
> + container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
> +
> + ctx->retval = retval;
> + return 0;
> +}
> +
> +const struct bpf_func_proto bpf_set_retval_proto = {
> + .func = bpf_set_retval,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_ANYTHING,
> +};
> +#endif /* CONFIG_CGROUP_BPF */
>
> #define BPF_STRTOX_BASE_MASK 0x1F
>
> @@ -1726,6 +1755,40 @@ bpf_base_func_proto(enum bpf_func_id func_id)
> }
> }
>
> +/* Common helpers for cgroup hooks. */
> +const struct bpf_func_proto *
> +cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> +{
> + switch (func_id) {
> +#ifdef CONFIG_CGROUP_BPF
> + case BPF_FUNC_get_local_storage:
> + return &bpf_get_local_storage_proto;
> + case BPF_FUNC_get_retval:
> + return &bpf_get_retval_proto;
> + case BPF_FUNC_set_retval:
> + return &bpf_set_retval_proto;
> +#endif
> + default:
> + return NULL;
> + }
> +}
> +
> +/* Common helpers for cgroup hooks with valid process context. */
> +const struct bpf_func_proto *
> +cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> +{
> + switch (func_id) {
> +#ifdef CONFIG_CGROUPS
> + case BPF_FUNC_get_current_uid_gid:
> + return &bpf_get_current_uid_gid_proto;
> + case BPF_FUNC_get_current_cgroup_id:
> + return &bpf_get_current_cgroup_id_proto;
> +#endif
> + default:
> + return NULL;
> + }
> +}
Does it make sense to move all these changes to kernel/bpf/cgroup.c
instead such that there is no need to do 'ifdef CONFIG_CGROUPS*'.
bpf_get_local_storage probably needs to move to kernel/bpf/cgroup.c
also.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks
2022-08-12 19:02 ` [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks Stanislav Fomichev
2022-08-13 6:25 ` kernel test robot
2022-08-13 18:55 ` kernel test robot
@ 2022-08-15 21:57 ` Martin KaFai Lau
2022-08-16 17:19 ` Stanislav Fomichev
2 siblings, 1 reply; 10+ messages in thread
From: Martin KaFai Lau @ 2022-08-15 21:57 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa
On Fri, Aug 12, 2022 at 12:02:40PM -0700, Stanislav Fomichev wrote:
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index de7d2fabb06d..87ce47b13b22 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -1764,9 +1764,31 @@ cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> case BPF_FUNC_get_local_storage:
> return &bpf_get_local_storage_proto;
> case BPF_FUNC_get_retval:
> - return &bpf_get_retval_proto;
> + switch (prog->expected_attach_type) {
> + case BPF_CGROUP_SOCK_OPS:
> + case BPF_CGROUP_UDP4_RECVMSG:
> + case BPF_CGROUP_UDP6_RECVMSG:
> + case BPF_CGROUP_INET4_GETPEERNAME:
> + case BPF_CGROUP_INET6_GETPEERNAME:
> + case BPF_CGROUP_INET4_GETSOCKNAME:
> + case BPF_CGROUP_INET6_GETSOCKNAME:
> + return NULL;
> + default:
> + return &bpf_get_retval_proto;
> + }
> case BPF_FUNC_set_retval:
> - return &bpf_set_retval_proto;
> + switch (prog->expected_attach_type) {
> + case BPF_CGROUP_SOCK_OPS:
> + case BPF_CGROUP_UDP4_RECVMSG:
> + case BPF_CGROUP_UDP6_RECVMSG:
> + case BPF_CGROUP_INET4_GETPEERNAME:
> + case BPF_CGROUP_INET6_GETPEERNAME:
> + case BPF_CGROUP_INET4_GETSOCKNAME:
> + case BPF_CGROUP_INET6_GETSOCKNAME:
> + return NULL;
> + default:
> + return &bpf_set_retval_proto;
> + }
Does it make sense to have bpf_lsm_func_proto() calling
cgroup_common_func_proto() also?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next 1/3] bpf: introduce cgroup_{common,current}_func_proto
2022-08-15 21:26 ` Martin KaFai Lau
@ 2022-08-16 17:19 ` Stanislav Fomichev
0 siblings, 0 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2022-08-16 17:19 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa
On Mon, Aug 15, 2022 at 2:27 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Fri, Aug 12, 2022 at 12:02:39PM -0700, Stanislav Fomichev wrote:
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index 3c1b9bbcf971..de7d2fabb06d 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -429,7 +429,6 @@ const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto = {
> > };
> >
> > #ifdef CONFIG_CGROUP_BPF
> > -
> > BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
> > {
> > /* flags argument is not used now,
> > @@ -460,7 +459,37 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
> > .arg1_type = ARG_CONST_MAP_PTR,
> > .arg2_type = ARG_ANYTHING,
> > };
> > -#endif
> > +
> > +BPF_CALL_0(bpf_get_retval)
> > +{
> > + struct bpf_cg_run_ctx *ctx =
> > + container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
> > +
> > + return ctx->retval;
> > +}
> > +
> > +const struct bpf_func_proto bpf_get_retval_proto = {
> > + .func = bpf_get_retval,
> > + .gpl_only = false,
> > + .ret_type = RET_INTEGER,
> > +};
> > +
> > +BPF_CALL_1(bpf_set_retval, int, retval)
> > +{
> > + struct bpf_cg_run_ctx *ctx =
> > + container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
> > +
> > + ctx->retval = retval;
> > + return 0;
> > +}
> > +
> > +const struct bpf_func_proto bpf_set_retval_proto = {
> > + .func = bpf_set_retval,
> > + .gpl_only = false,
> > + .ret_type = RET_INTEGER,
> > + .arg1_type = ARG_ANYTHING,
> > +};
> > +#endif /* CONFIG_CGROUP_BPF */
> >
> > #define BPF_STRTOX_BASE_MASK 0x1F
> >
> > @@ -1726,6 +1755,40 @@ bpf_base_func_proto(enum bpf_func_id func_id)
> > }
> > }
> >
> > +/* Common helpers for cgroup hooks. */
> > +const struct bpf_func_proto *
> > +cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > +{
> > + switch (func_id) {
> > +#ifdef CONFIG_CGROUP_BPF
> > + case BPF_FUNC_get_local_storage:
> > + return &bpf_get_local_storage_proto;
> > + case BPF_FUNC_get_retval:
> > + return &bpf_get_retval_proto;
> > + case BPF_FUNC_set_retval:
> > + return &bpf_set_retval_proto;
> > +#endif
> > + default:
> > + return NULL;
> > + }
> > +}
> > +
> > +/* Common helpers for cgroup hooks with valid process context. */
> > +const struct bpf_func_proto *
> > +cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > +{
> > + switch (func_id) {
> > +#ifdef CONFIG_CGROUPS
> > + case BPF_FUNC_get_current_uid_gid:
> > + return &bpf_get_current_uid_gid_proto;
> > + case BPF_FUNC_get_current_cgroup_id:
> > + return &bpf_get_current_cgroup_id_proto;
> > +#endif
> > + default:
> > + return NULL;
> > + }
> > +}
> Does it make sense to move all these changes to kernel/bpf/cgroup.c
> instead such that there is no need to do 'ifdef CONFIG_CGROUPS*'.
> bpf_get_local_storage probably needs to move to kernel/bpf/cgroup.c
> also.
Hm, didn't think about it, let me try moving everything under
bpf/cgroup.c instead..
(saw the build failures but decided not to spam with a v2)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks
2022-08-15 21:57 ` Martin KaFai Lau
@ 2022-08-16 17:19 ` Stanislav Fomichev
0 siblings, 0 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2022-08-16 17:19 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa
On Mon, Aug 15, 2022 at 2:58 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Fri, Aug 12, 2022 at 12:02:40PM -0700, Stanislav Fomichev wrote:
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index de7d2fabb06d..87ce47b13b22 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -1764,9 +1764,31 @@ cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > case BPF_FUNC_get_local_storage:
> > return &bpf_get_local_storage_proto;
> > case BPF_FUNC_get_retval:
> > - return &bpf_get_retval_proto;
> > + switch (prog->expected_attach_type) {
> > + case BPF_CGROUP_SOCK_OPS:
> > + case BPF_CGROUP_UDP4_RECVMSG:
> > + case BPF_CGROUP_UDP6_RECVMSG:
> > + case BPF_CGROUP_INET4_GETPEERNAME:
> > + case BPF_CGROUP_INET6_GETPEERNAME:
> > + case BPF_CGROUP_INET4_GETSOCKNAME:
> > + case BPF_CGROUP_INET6_GETSOCKNAME:
> > + return NULL;
> > + default:
> > + return &bpf_get_retval_proto;
> > + }
> > case BPF_FUNC_set_retval:
> > - return &bpf_set_retval_proto;
> > + switch (prog->expected_attach_type) {
> > + case BPF_CGROUP_SOCK_OPS:
> > + case BPF_CGROUP_UDP4_RECVMSG:
> > + case BPF_CGROUP_UDP6_RECVMSG:
> > + case BPF_CGROUP_INET4_GETPEERNAME:
> > + case BPF_CGROUP_INET6_GETPEERNAME:
> > + case BPF_CGROUP_INET4_GETSOCKNAME:
> > + case BPF_CGROUP_INET6_GETSOCKNAME:
> > + return NULL;
> > + default:
> > + return &bpf_set_retval_proto;
> > + }
> Does it make sense to have bpf_lsm_func_proto() calling
> cgroup_common_func_proto() also?
Oh, sure, will do, thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-08-16 17:19 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-12 19:02 [PATCH bpf-next 0/3] bpf: expose bpf_{g,s}et_retval to more cgroup hooks Stanislav Fomichev
2022-08-12 19:02 ` [PATCH bpf-next 1/3] bpf: introduce cgroup_{common,current}_func_proto Stanislav Fomichev
2022-08-15 21:26 ` Martin KaFai Lau
2022-08-16 17:19 ` Stanislav Fomichev
2022-08-12 19:02 ` [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks Stanislav Fomichev
2022-08-13 6:25 ` kernel test robot
2022-08-13 18:55 ` kernel test robot
2022-08-15 21:57 ` Martin KaFai Lau
2022-08-16 17:19 ` Stanislav Fomichev
2022-08-12 19:02 ` [PATCH bpf-next 3/3] selftests/bpf: make sure bpf_{g,s}et_retval is exposed everywhere Stanislav Fomichev
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).