| Message ID | 20220823222555.523590-1-sdf@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | bpf: expose bpf_{g,s}et_retval to more cgroup hooks | expand |
Hello: This series was applied to bpf/bpf-next.git (master) by Alexei Starovoitov <ast@kernel.org>: On Tue, 23 Aug 2022 15:25:50 -0700 you wrote: > 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: > > [...] Here is the summary with links: - [bpf-next,v5,1/5] bpf: Introduce cgroup_{common,current}_func_proto https://git.kernel.org/bpf/bpf-next/c/dea6a4e17013 - [bpf-next,v5,2/5] bpf: Use cgroup_{common,current}_func_proto in more hooks https://git.kernel.org/bpf/bpf-next/c/bed89185af0d - [bpf-next,v5,3/5] bpf: expose bpf_strtol and bpf_strtoul to all program types https://git.kernel.org/bpf/bpf-next/c/8a67f2de9b1d - [bpf-next,v5,4/5] bpf: update bpf_{g,s}et_retval documentation https://git.kernel.org/bpf/bpf-next/c/2172fb8007ea - [bpf-next,v5,5/5] selftests/bpf: Make sure bpf_{g,s}et_retval is exposed everywhere https://git.kernel.org/bpf/bpf-next/c/e7215f574079 You are awesome, thank you!
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. v5: - remove net/cls_cgroup.h include from patch 1/5 (Martin) - move endif changes from patch 1/5 to 3/5 (Martin) - don't define __weak protos, the ones in core.c suffice (Martin) v4: - don't touch existing helper.c helpers (Martin) - drop unneeded CONFIG_CGROUP_BPF in bpf_lsm_func_proto (Martin) v3: - expose strtol/strtoul everywhere (Martin) - move helpers declarations from bpf.h to bpf-cgroup.h (Martin) - revise bpf_{g,s}et_retval documentation (Martin) - don't expose bpf_{g,s}et_retval to cg_skb hooks (Martin) v2: - move everything into kernel/bpf/cgroup.c instead (Martin) - use cgroup_common_func_proto in lsm (Martin) Stanislav Fomichev (5): bpf: Introduce cgroup_{common,current}_func_proto bpf: Use cgroup_{common,current}_func_proto in more hooks bpf: expose bpf_strtol and bpf_strtoul to all program types bpf: update bpf_{g,s}et_retval documentation selftests/bpf: Make sure bpf_{g,s}et_retval is exposed everywhere include/linux/bpf-cgroup.h | 17 ++ include/linux/bpf.h | 1 + include/uapi/linux/bpf.h | 22 ++- kernel/bpf/bpf_lsm.c | 17 +- kernel/bpf/cgroup.c | 157 +++++++++++++++--- kernel/bpf/helpers.c | 40 +---- net/core/filter.c | 80 ++++----- tools/include/uapi/linux/bpf.h | 22 ++- 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 ++ 12 files changed, 322 insertions(+), 124 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