Message ID | 20231030210638.2415306-1-davemarchevsky@fb.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | BPF |
Headers | show |
Series | [v1,bpf-next] bpf: Add __bpf_kfunc_{start,end}_defs macros | expand |
On Tue, Oct 31, 2023 at 5:07 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: > > BPF kfuncs are meant to be called from BPF programs. Accordingly, most > kfuncs are not called from anywhere in the kernel, which the > -Wmissing-prototypes warning is unhappy about. We've peppered > __diag_ignore_all("-Wmissing-prototypes", ... everywhere kfuncs are > defined in the codebase to suppress this warning. > > This patch adds two macros meant to bound one or many kfunc definitions. > All existing kfunc definitions which use these __diag calls to suppress > -Wmissing-prototypes are migrated to use the newly-introduced macros. > A new __diag_ignore_all - for "-Wmissing-declarations" - is added to the > __bpf_kfunc_start_defs macro based on feedback from Andrii on an earlier > version of this patch [0] and another recent mailing list thread [1]. > > In the future we might need to ignore different warnings or do other > kfunc-specific things. This change will make it easier to make such > modifications for all kfunc defs. > > [0]: https://lore.kernel.org/bpf/CAEf4BzaE5dRWtK6RPLnjTW-MW9sx9K3Fn6uwqCTChK2Dcb1Xig@mail.gmail.com/ > [1]: https://lore.kernel.org/bpf/ZT+2qCc%2FaXep0%2FLf@krava/ > > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > Cc: Jiri Olsa <olsajiri@gmail.com> > --- > > This patch was submitted earlier as part of task_vma > iter series: https://lore.kernel.org/bpf/20231013204426.1074286-6-davemarchevsky@fb.com/ > > This separate submission addresses Andrii's comments from > that thread. > > include/linux/btf.h | 9 +++++++++ > kernel/bpf/bpf_iter.c | 6 ++---- > kernel/bpf/cpumask.c | 6 ++---- > kernel/bpf/helpers.c | 6 ++---- > kernel/bpf/map_iter.c | 6 ++---- > kernel/bpf/task_iter.c | 6 ++---- > kernel/trace/bpf_trace.c | 6 ++---- > net/bpf/test_run.c | 7 +++---- > net/core/filter.c | 13 ++++--------- > net/core/xdp.c | 6 ++---- > net/ipv4/fou_bpf.c | 6 ++---- > net/netfilter/nf_conntrack_bpf.c | 6 ++---- > net/netfilter/nf_nat_bpf.c | 6 ++---- > net/xfrm/xfrm_interface_bpf.c | 6 ++---- > 14 files changed, 38 insertions(+), 57 deletions(-) > Thanks for your work. By using a simple grep for "__diag_ignore_all(\"-Wmissing-prototypes", it appears that the files net/socket.c, tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c, kernel/cgroup/rstat.c and Documentation/bpf/kfuncs.rst are missing. It seems that we should also update them.
On Mon, Oct 30, 2023 at 10:56 PM Yafang Shao <laoar.shao@gmail.com> wrote: > > On Tue, Oct 31, 2023 at 5:07 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: > > > > BPF kfuncs are meant to be called from BPF programs. Accordingly, most > > kfuncs are not called from anywhere in the kernel, which the > > -Wmissing-prototypes warning is unhappy about. We've peppered > > __diag_ignore_all("-Wmissing-prototypes", ... everywhere kfuncs are > > defined in the codebase to suppress this warning. > > > > This patch adds two macros meant to bound one or many kfunc definitions. > > All existing kfunc definitions which use these __diag calls to suppress > > -Wmissing-prototypes are migrated to use the newly-introduced macros. > > A new __diag_ignore_all - for "-Wmissing-declarations" - is added to the > > __bpf_kfunc_start_defs macro based on feedback from Andrii on an earlier > > version of this patch [0] and another recent mailing list thread [1]. > > > > In the future we might need to ignore different warnings or do other > > kfunc-specific things. This change will make it easier to make such > > modifications for all kfunc defs. > > > > [0]: https://lore.kernel.org/bpf/CAEf4BzaE5dRWtK6RPLnjTW-MW9sx9K3Fn6uwqCTChK2Dcb1Xig@mail.gmail.com/ > > [1]: https://lore.kernel.org/bpf/ZT+2qCc%2FaXep0%2FLf@krava/ > > > > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > > Cc: Jiri Olsa <olsajiri@gmail.com> > > --- > > > > This patch was submitted earlier as part of task_vma > > iter series: https://lore.kernel.org/bpf/20231013204426.1074286-6-davemarchevsky@fb.com/ > > > > This separate submission addresses Andrii's comments from > > that thread. > > > > include/linux/btf.h | 9 +++++++++ > > kernel/bpf/bpf_iter.c | 6 ++---- > > kernel/bpf/cpumask.c | 6 ++---- > > kernel/bpf/helpers.c | 6 ++---- > > kernel/bpf/map_iter.c | 6 ++---- > > kernel/bpf/task_iter.c | 6 ++---- > > kernel/trace/bpf_trace.c | 6 ++---- > > net/bpf/test_run.c | 7 +++---- > > net/core/filter.c | 13 ++++--------- > > net/core/xdp.c | 6 ++---- > > net/ipv4/fou_bpf.c | 6 ++---- > > net/netfilter/nf_conntrack_bpf.c | 6 ++---- > > net/netfilter/nf_nat_bpf.c | 6 ++---- > > net/xfrm/xfrm_interface_bpf.c | 6 ++---- > > 14 files changed, 38 insertions(+), 57 deletions(-) > > > > Thanks for your work. > > By using a simple grep for "__diag_ignore_all(\"-Wmissing-prototypes", > it appears that the files net/socket.c, > tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c, > kernel/cgroup/rstat.c and Documentation/bpf/kfuncs.rst are missing. It > seems that we should also update them. > rstat.c and net/socket.c don't have kfuncs, so those are not relevant here. But we are missing changes also in kernel/bpf/task_iter.c and kernel/bpf/cgroup_iter.c And let's update Documentation/bpf/kfuncs.rst to use your new set of macros? With the above addressed, please add my ack. Thanks! Acked-by: Andrii Nakryiko <andrii@kernel.org> > -- > Regards > Yafang
On Tue, Oct 31, 2023 at 2:23 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Mon, Oct 30, 2023 at 10:56 PM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > On Tue, Oct 31, 2023 at 5:07 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: > > > > > > BPF kfuncs are meant to be called from BPF programs. Accordingly, most > > > kfuncs are not called from anywhere in the kernel, which the > > > -Wmissing-prototypes warning is unhappy about. We've peppered > > > __diag_ignore_all("-Wmissing-prototypes", ... everywhere kfuncs are > > > defined in the codebase to suppress this warning. > > > > > > This patch adds two macros meant to bound one or many kfunc definitions. > > > All existing kfunc definitions which use these __diag calls to suppress > > > -Wmissing-prototypes are migrated to use the newly-introduced macros. > > > A new __diag_ignore_all - for "-Wmissing-declarations" - is added to the > > > __bpf_kfunc_start_defs macro based on feedback from Andrii on an earlier > > > version of this patch [0] and another recent mailing list thread [1]. > > > > > > In the future we might need to ignore different warnings or do other > > > kfunc-specific things. This change will make it easier to make such > > > modifications for all kfunc defs. > > > > > > [0]: https://lore.kernel.org/bpf/CAEf4BzaE5dRWtK6RPLnjTW-MW9sx9K3Fn6uwqCTChK2Dcb1Xig@mail.gmail.com/ > > > [1]: https://lore.kernel.org/bpf/ZT+2qCc%2FaXep0%2FLf@krava/ > > > > > > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > > > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > > > Cc: Jiri Olsa <olsajiri@gmail.com> > > > --- > > > > > > This patch was submitted earlier as part of task_vma > > > iter series: https://lore.kernel.org/bpf/20231013204426.1074286-6-davemarchevsky@fb.com/ > > > > > > This separate submission addresses Andrii's comments from > > > that thread. > > > > > > include/linux/btf.h | 9 +++++++++ > > > kernel/bpf/bpf_iter.c | 6 ++---- > > > kernel/bpf/cpumask.c | 6 ++---- > > > kernel/bpf/helpers.c | 6 ++---- > > > kernel/bpf/map_iter.c | 6 ++---- > > > kernel/bpf/task_iter.c | 6 ++---- > > > kernel/trace/bpf_trace.c | 6 ++---- > > > net/bpf/test_run.c | 7 +++---- > > > net/core/filter.c | 13 ++++--------- > > > net/core/xdp.c | 6 ++---- > > > net/ipv4/fou_bpf.c | 6 ++---- > > > net/netfilter/nf_conntrack_bpf.c | 6 ++---- > > > net/netfilter/nf_nat_bpf.c | 6 ++---- > > > net/xfrm/xfrm_interface_bpf.c | 6 ++---- > > > 14 files changed, 38 insertions(+), 57 deletions(-) > > > > > > > Thanks for your work. > > > > By using a simple grep for "__diag_ignore_all(\"-Wmissing-prototypes", > > it appears that the files net/socket.c, > > tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c, > > kernel/cgroup/rstat.c and Documentation/bpf/kfuncs.rst are missing. It > > seems that we should also update them. > > > > rstat.c and net/socket.c don't have kfuncs, so those are not relevant > here. The bpf_rstat_flush() and update_socket_protocol() can also trigger the -Wmissing-declarations. These two functions are for BPF only. Shouldn't we better include them as well ? > But we are missing changes also in kernel/bpf/task_iter.c and > kernel/bpf/cgroup_iter.c > > And let's update Documentation/bpf/kfuncs.rst to use your new set of macros? > > With the above addressed, please add my ack. Thanks! > > Acked-by: Andrii Nakryiko <andrii@kernel.org> > > > -- > > Regards > > Yafang
On 10/31/23 2:51 AM, Yafang Shao wrote: > On Tue, Oct 31, 2023 at 2:23 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: >> >> On Mon, Oct 30, 2023 at 10:56 PM Yafang Shao <laoar.shao@gmail.com> wrote: >>> >>> On Tue, Oct 31, 2023 at 5:07 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: >>>> >>>> BPF kfuncs are meant to be called from BPF programs. Accordingly, most >>>> kfuncs are not called from anywhere in the kernel, which the >>>> -Wmissing-prototypes warning is unhappy about. We've peppered >>>> __diag_ignore_all("-Wmissing-prototypes", ... everywhere kfuncs are >>>> defined in the codebase to suppress this warning. >>>> >>>> This patch adds two macros meant to bound one or many kfunc definitions. >>>> All existing kfunc definitions which use these __diag calls to suppress >>>> -Wmissing-prototypes are migrated to use the newly-introduced macros. >>>> A new __diag_ignore_all - for "-Wmissing-declarations" - is added to the >>>> __bpf_kfunc_start_defs macro based on feedback from Andrii on an earlier >>>> version of this patch [0] and another recent mailing list thread [1]. >>>> >>>> In the future we might need to ignore different warnings or do other >>>> kfunc-specific things. This change will make it easier to make such >>>> modifications for all kfunc defs. >>>> >>>> [0]: https://lore.kernel.org/bpf/CAEf4BzaE5dRWtK6RPLnjTW-MW9sx9K3Fn6uwqCTChK2Dcb1Xig@mail.gmail.com/ >>>> [1]: https://lore.kernel.org/bpf/ZT+2qCc%2FaXep0%2FLf@krava/ >>>> >>>> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> >>>> Suggested-by: Andrii Nakryiko <andrii@kernel.org> >>>> Cc: Jiri Olsa <olsajiri@gmail.com> >>>> --- >>>> >>>> This patch was submitted earlier as part of task_vma >>>> iter series: https://lore.kernel.org/bpf/20231013204426.1074286-6-davemarchevsky@fb.com/ >>>> >>>> This separate submission addresses Andrii's comments from >>>> that thread. >>>> >>>> include/linux/btf.h | 9 +++++++++ >>>> kernel/bpf/bpf_iter.c | 6 ++---- >>>> kernel/bpf/cpumask.c | 6 ++---- >>>> kernel/bpf/helpers.c | 6 ++---- >>>> kernel/bpf/map_iter.c | 6 ++---- >>>> kernel/bpf/task_iter.c | 6 ++---- >>>> kernel/trace/bpf_trace.c | 6 ++---- >>>> net/bpf/test_run.c | 7 +++---- >>>> net/core/filter.c | 13 ++++--------- >>>> net/core/xdp.c | 6 ++---- >>>> net/ipv4/fou_bpf.c | 6 ++---- >>>> net/netfilter/nf_conntrack_bpf.c | 6 ++---- >>>> net/netfilter/nf_nat_bpf.c | 6 ++---- >>>> net/xfrm/xfrm_interface_bpf.c | 6 ++---- >>>> 14 files changed, 38 insertions(+), 57 deletions(-) >>>> >>> >>> Thanks for your work. >>> >>> By using a simple grep for "__diag_ignore_all(\"-Wmissing-prototypes", >>> it appears that the files net/socket.c, >>> tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c, >>> kernel/cgroup/rstat.c and Documentation/bpf/kfuncs.rst are missing. It >>> seems that we should also update them. >>> >> >> rstat.c and net/socket.c don't have kfuncs, so those are not relevant >> here. > > The bpf_rstat_flush() and update_socket_protocol() can also trigger > the -Wmissing-declarations. > These two functions are for BPF only. Shouldn't we better include them as well ? > I had this conundrum when writing the patch as well. Since they're not kfuncs and the macros are meant to wrap kfunc definitions, I felt that it would be confusing to someone unfamiliar with BPF internals. But I agree that the current state isn't ideal either. How about either: * I use the __bpf_kfunc_{start,end}_defs macros in those two places, with comment describing that they're not wrapping kfunc def, but rather BPF hook point that throws the same warnings. * Two additional macros, __bpf_hook_{start,end} are added, just pointing to __bpf_kfunc_{start,end} for now. They're used for these two functions WDYT? >> But we are missing changes also in kernel/bpf/task_iter.c and >> kernel/bpf/cgroup_iter.c >> >> And let's update Documentation/bpf/kfuncs.rst to use your new set of macros? >> >> With the above addressed, please add my ack. Thanks! >> >> Acked-by: Andrii Nakryiko <andrii@kernel.org> >> >>> -- >>> Regards >>> Yafang > > >
On Tue, Oct 31, 2023 at 10:04 AM David Marchevsky <david.marchevsky@linux.dev> wrote: > > On 10/31/23 2:51 AM, Yafang Shao wrote: > > On Tue, Oct 31, 2023 at 2:23 PM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > >> > >> On Mon, Oct 30, 2023 at 10:56 PM Yafang Shao <laoar.shao@gmail.com> wrote: > >>> > >>> On Tue, Oct 31, 2023 at 5:07 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: > >>>> > >>>> BPF kfuncs are meant to be called from BPF programs. Accordingly, most > >>>> kfuncs are not called from anywhere in the kernel, which the > >>>> -Wmissing-prototypes warning is unhappy about. We've peppered > >>>> __diag_ignore_all("-Wmissing-prototypes", ... everywhere kfuncs are > >>>> defined in the codebase to suppress this warning. > >>>> > >>>> This patch adds two macros meant to bound one or many kfunc definitions. > >>>> All existing kfunc definitions which use these __diag calls to suppress > >>>> -Wmissing-prototypes are migrated to use the newly-introduced macros. > >>>> A new __diag_ignore_all - for "-Wmissing-declarations" - is added to the > >>>> __bpf_kfunc_start_defs macro based on feedback from Andrii on an earlier > >>>> version of this patch [0] and another recent mailing list thread [1]. > >>>> > >>>> In the future we might need to ignore different warnings or do other > >>>> kfunc-specific things. This change will make it easier to make such > >>>> modifications for all kfunc defs. > >>>> > >>>> [0]: https://lore.kernel.org/bpf/CAEf4BzaE5dRWtK6RPLnjTW-MW9sx9K3Fn6uwqCTChK2Dcb1Xig@mail.gmail.com/ > >>>> [1]: https://lore.kernel.org/bpf/ZT+2qCc%2FaXep0%2FLf@krava/ > >>>> > >>>> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > >>>> Suggested-by: Andrii Nakryiko <andrii@kernel.org> > >>>> Cc: Jiri Olsa <olsajiri@gmail.com> > >>>> --- > >>>> > >>>> This patch was submitted earlier as part of task_vma > >>>> iter series: https://lore.kernel.org/bpf/20231013204426.1074286-6-davemarchevsky@fb.com/ > >>>> > >>>> This separate submission addresses Andrii's comments from > >>>> that thread. > >>>> > >>>> include/linux/btf.h | 9 +++++++++ > >>>> kernel/bpf/bpf_iter.c | 6 ++---- > >>>> kernel/bpf/cpumask.c | 6 ++---- > >>>> kernel/bpf/helpers.c | 6 ++---- > >>>> kernel/bpf/map_iter.c | 6 ++---- > >>>> kernel/bpf/task_iter.c | 6 ++---- > >>>> kernel/trace/bpf_trace.c | 6 ++---- > >>>> net/bpf/test_run.c | 7 +++---- > >>>> net/core/filter.c | 13 ++++--------- > >>>> net/core/xdp.c | 6 ++---- > >>>> net/ipv4/fou_bpf.c | 6 ++---- > >>>> net/netfilter/nf_conntrack_bpf.c | 6 ++---- > >>>> net/netfilter/nf_nat_bpf.c | 6 ++---- > >>>> net/xfrm/xfrm_interface_bpf.c | 6 ++---- > >>>> 14 files changed, 38 insertions(+), 57 deletions(-) > >>>> > >>> > >>> Thanks for your work. > >>> > >>> By using a simple grep for "__diag_ignore_all(\"-Wmissing-prototypes", > >>> it appears that the files net/socket.c, > >>> tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c, > >>> kernel/cgroup/rstat.c and Documentation/bpf/kfuncs.rst are missing. It > >>> seems that we should also update them. > >>> > >> > >> rstat.c and net/socket.c don't have kfuncs, so those are not relevant > >> here. > > > > The bpf_rstat_flush() and update_socket_protocol() can also trigger > > the -Wmissing-declarations. > > These two functions are for BPF only. Shouldn't we better include them as well ? > > > > I had this conundrum when writing the patch as well. Since they're not kfuncs > and the macros are meant to wrap kfunc definitions, I felt that it would be yeah, I was going to say the same, it's misleading > confusing to someone unfamiliar with BPF internals. But I agree that the current > state isn't ideal either. > > How about either: > * I use the __bpf_kfunc_{start,end}_defs macros in those two places, > with comment describing that they're not wrapping kfunc def, but rather > BPF hook point that throws the same warnings. > * Two additional macros, __bpf_hook_{start,end} are added, just > pointing to __bpf_kfunc_{start,end} for now. They're used for > these two functions > > WDYT? I like the option #2 best > > >> But we are missing changes also in kernel/bpf/task_iter.c and > >> kernel/bpf/cgroup_iter.c > >> > >> And let's update Documentation/bpf/kfuncs.rst to use your new set of macros? > >> > >> With the above addressed, please add my ack. Thanks! > >> > >> Acked-by: Andrii Nakryiko <andrii@kernel.org> > >> > >>> -- > >>> Regards > >>> Yafang > > > > > >
diff --git a/include/linux/btf.h b/include/linux/btf.h index c2231c64d60b..dc5ce962f600 100644 --- a/include/linux/btf.h +++ b/include/linux/btf.h @@ -84,6 +84,15 @@ */ #define __bpf_kfunc __used noinline +#define __bpf_kfunc_start_defs() \ + __diag_push(); \ + __diag_ignore_all("-Wmissing-declarations", \ + "Global kfuncs as their definitions will be in BTF");\ + __diag_ignore_all("-Wmissing-prototypes", \ + "Global kfuncs as their definitions will be in BTF") + +#define __bpf_kfunc_end_defs() __diag_pop() + /* * Return the name of the passed struct, if exists, or halt the build if for * example the structure gets renamed. In this way, developers have to revisit diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c index 833faa04461b..0fae79164187 100644 --- a/kernel/bpf/bpf_iter.c +++ b/kernel/bpf/bpf_iter.c @@ -782,9 +782,7 @@ struct bpf_iter_num_kern { int end; /* final value, exclusive */ } __aligned(8); -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "Global functions as their definitions will be in vmlinux BTF"); +__bpf_kfunc_start_defs(); __bpf_kfunc int bpf_iter_num_new(struct bpf_iter_num *it, int start, int end) { @@ -843,4 +841,4 @@ __bpf_kfunc void bpf_iter_num_destroy(struct bpf_iter_num *it) s->cur = s->end = 0; } -__diag_pop(); +__bpf_kfunc_end_defs(); diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c index 6983af8e093c..e01c741e54e7 100644 --- a/kernel/bpf/cpumask.c +++ b/kernel/bpf/cpumask.c @@ -34,9 +34,7 @@ static bool cpu_valid(u32 cpu) return cpu < nr_cpu_ids; } -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "Global kfuncs as their definitions will be in BTF"); +__bpf_kfunc_start_defs(); /** * bpf_cpumask_create() - Create a mutable BPF cpumask. @@ -407,7 +405,7 @@ __bpf_kfunc u32 bpf_cpumask_any_and_distribute(const struct cpumask *src1, return cpumask_any_and_distribute(src1, src2); } -__diag_pop(); +__bpf_kfunc_end_defs(); BTF_SET8_START(cpumask_kfunc_btf_ids) BTF_ID_FLAGS(func, bpf_cpumask_create, KF_ACQUIRE | KF_RET_NULL) diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index e46ac288a108..0e4657606eaa 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1886,9 +1886,7 @@ void bpf_rb_root_free(const struct btf_field *field, void *rb_root, } } -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "Global functions as their definitions will be in vmlinux BTF"); +__bpf_kfunc_start_defs(); __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void *meta__ign) { @@ -2505,7 +2503,7 @@ __bpf_kfunc void bpf_throw(u64 cookie) WARN(1, "A call to BPF exception callback should never return\n"); } -__diag_pop(); +__bpf_kfunc_end_defs(); BTF_SET8_START(generic_btf_ids) #ifdef CONFIG_KEXEC_CORE diff --git a/kernel/bpf/map_iter.c b/kernel/bpf/map_iter.c index 6fc9dae9edc8..6abd7c5df4b3 100644 --- a/kernel/bpf/map_iter.c +++ b/kernel/bpf/map_iter.c @@ -193,9 +193,7 @@ static int __init bpf_map_iter_init(void) late_initcall(bpf_map_iter_init); -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "Global functions as their definitions will be in vmlinux BTF"); +__bpf_kfunc_start_defs(); __bpf_kfunc s64 bpf_map_sum_elem_count(const struct bpf_map *map) { @@ -213,7 +211,7 @@ __bpf_kfunc s64 bpf_map_sum_elem_count(const struct bpf_map *map) return ret; } -__diag_pop(); +__bpf_kfunc_end_defs(); BTF_SET8_START(bpf_map_iter_kfunc_ids) BTF_ID_FLAGS(func, bpf_map_sum_elem_count, KF_TRUSTED_ARGS) diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c index 59e747938bdb..19518f5ba7a3 100644 --- a/kernel/bpf/task_iter.c +++ b/kernel/bpf/task_iter.c @@ -824,9 +824,7 @@ struct bpf_iter_task_vma_kern { struct bpf_iter_task_vma_kern_data *data; } __attribute__((aligned(8))); -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "Global functions as their definitions will be in vmlinux BTF"); +__bpf_kfunc_start_defs(); __bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it, struct task_struct *task, u64 addr) @@ -892,7 +890,7 @@ __bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) } } -__diag_pop(); +__bpf_kfunc_end_defs(); struct bpf_iter_css_task { __u64 __opaque[1]; diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index df697c74d519..84e8a0f6e4e0 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1252,9 +1252,7 @@ static const struct bpf_func_proto bpf_get_func_arg_cnt_proto = { }; #ifdef CONFIG_KEYS -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "kfuncs which will be used in BPF programs"); +__bpf_kfunc_start_defs(); /** * bpf_lookup_user_key - lookup a key by its serial @@ -1404,7 +1402,7 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, } #endif /* CONFIG_SYSTEM_DATA_VERIFICATION */ -__diag_pop(); +__bpf_kfunc_end_defs(); BTF_SET8_START(key_sig_kfunc_set) BTF_ID_FLAGS(func, bpf_lookup_user_key, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE) diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index 0841f8d82419..c9fdcc5cdce1 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -503,9 +503,8 @@ static int bpf_test_finish(const union bpf_attr *kattr, * architecture dependent calling conventions. 7+ can be supported in the * future. */ -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "Global functions as their definitions will be in vmlinux BTF"); +__bpf_kfunc_start_defs(); + __bpf_kfunc int bpf_fentry_test1(int a) { return a + 1; @@ -605,7 +604,7 @@ __bpf_kfunc void bpf_kfunc_call_memb_release(struct prog_test_member *p) { } -__diag_pop(); +__bpf_kfunc_end_defs(); BTF_SET8_START(bpf_test_modify_return_ids) BTF_ID_FLAGS(func, bpf_modify_return_test) diff --git a/net/core/filter.c b/net/core/filter.c index 21d75108c2e9..383f96b0a1c7 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -11767,9 +11767,7 @@ bpf_sk_base_func_proto(enum bpf_func_id func_id) return func; } -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "Global functions as their definitions will be in vmlinux BTF"); +__bpf_kfunc_start_defs(); __bpf_kfunc int bpf_dynptr_from_skb(struct sk_buff *skb, u64 flags, struct bpf_dynptr_kern *ptr__uninit) { @@ -11816,7 +11814,7 @@ __bpf_kfunc int bpf_sock_addr_set_sun_path(struct bpf_sock_addr_kern *sa_kern, return 0; } -__diag_pop(); +__bpf_kfunc_end_defs(); int bpf_dynptr_from_skb_rdonly(struct sk_buff *skb, u64 flags, struct bpf_dynptr_kern *ptr__uninit) @@ -11879,10 +11877,7 @@ static int __init bpf_kfunc_init(void) } late_initcall(bpf_kfunc_init); -/* Disables missing prototype warnings */ -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "Global functions as their definitions will be in vmlinux BTF"); +__bpf_kfunc_start_defs(); /* bpf_sock_destroy: Destroy the given socket with ECONNABORTED error code. * @@ -11916,7 +11911,7 @@ __bpf_kfunc int bpf_sock_destroy(struct sock_common *sock) return sk->sk_prot->diag_destroy(sk, ECONNABORTED); } -__diag_pop() +__bpf_kfunc_end_defs(); BTF_SET8_START(bpf_sk_iter_kfunc_ids) BTF_ID_FLAGS(func, bpf_sock_destroy, KF_TRUSTED_ARGS) diff --git a/net/core/xdp.c b/net/core/xdp.c index df4789ab512d..b6f1d6dab3f2 100644 --- a/net/core/xdp.c +++ b/net/core/xdp.c @@ -696,9 +696,7 @@ struct xdp_frame *xdpf_clone(struct xdp_frame *xdpf) return nxdpf; } -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "Global functions as their definitions will be in vmlinux BTF"); +__bpf_kfunc_start_defs(); /** * bpf_xdp_metadata_rx_timestamp - Read XDP frame RX timestamp. @@ -738,7 +736,7 @@ __bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, u32 *hash, return -EOPNOTSUPP; } -__diag_pop(); +__bpf_kfunc_end_defs(); BTF_SET8_START(xdp_metadata_kfunc_ids) #define XDP_METADATA_KFUNC(_, __, name, ___) BTF_ID_FLAGS(func, name, KF_TRUSTED_ARGS) diff --git a/net/ipv4/fou_bpf.c b/net/ipv4/fou_bpf.c index 3760a14b6b57..4da03bf45c9b 100644 --- a/net/ipv4/fou_bpf.c +++ b/net/ipv4/fou_bpf.c @@ -22,9 +22,7 @@ enum bpf_fou_encap_type { FOU_BPF_ENCAP_GUE, }; -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "Global functions as their definitions will be in BTF"); +__bpf_kfunc_start_defs(); /* bpf_skb_set_fou_encap - Set FOU encap parameters * @@ -100,7 +98,7 @@ __bpf_kfunc int bpf_skb_get_fou_encap(struct __sk_buff *skb_ctx, return 0; } -__diag_pop() +__bpf_kfunc_end_defs(); BTF_SET8_START(fou_kfunc_set) BTF_ID_FLAGS(func, bpf_skb_set_fou_encap) diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c index b21799d468d2..475358ec8212 100644 --- a/net/netfilter/nf_conntrack_bpf.c +++ b/net/netfilter/nf_conntrack_bpf.c @@ -230,9 +230,7 @@ static int _nf_conntrack_btf_struct_access(struct bpf_verifier_log *log, return 0; } -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "Global functions as their definitions will be in nf_conntrack BTF"); +__bpf_kfunc_start_defs(); /* bpf_xdp_ct_alloc - Allocate a new CT entry * @@ -467,7 +465,7 @@ __bpf_kfunc int bpf_ct_change_status(struct nf_conn *nfct, u32 status) return nf_ct_change_status_common(nfct, status); } -__diag_pop() +__bpf_kfunc_end_defs(); BTF_SET8_START(nf_ct_kfunc_set) BTF_ID_FLAGS(func, bpf_xdp_ct_alloc, KF_ACQUIRE | KF_RET_NULL) diff --git a/net/netfilter/nf_nat_bpf.c b/net/netfilter/nf_nat_bpf.c index 141ee7783223..6e3b2f58855f 100644 --- a/net/netfilter/nf_nat_bpf.c +++ b/net/netfilter/nf_nat_bpf.c @@ -12,9 +12,7 @@ #include <net/netfilter/nf_conntrack_core.h> #include <net/netfilter/nf_nat.h> -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "Global functions as their definitions will be in nf_nat BTF"); +__bpf_kfunc_start_defs(); /* bpf_ct_set_nat_info - Set source or destination nat address * @@ -54,7 +52,7 @@ __bpf_kfunc int bpf_ct_set_nat_info(struct nf_conn___init *nfct, return nf_nat_setup_info(ct, &range, manip) == NF_DROP ? -ENOMEM : 0; } -__diag_pop() +__bpf_kfunc_end_defs(); BTF_SET8_START(nf_nat_kfunc_set) BTF_ID_FLAGS(func, bpf_ct_set_nat_info, KF_TRUSTED_ARGS) diff --git a/net/xfrm/xfrm_interface_bpf.c b/net/xfrm/xfrm_interface_bpf.c index d74f3fd20f2b..7d5e920141e9 100644 --- a/net/xfrm/xfrm_interface_bpf.c +++ b/net/xfrm/xfrm_interface_bpf.c @@ -27,9 +27,7 @@ struct bpf_xfrm_info { int link; }; -__diag_push(); -__diag_ignore_all("-Wmissing-prototypes", - "Global functions as their definitions will be in xfrm_interface BTF"); +__bpf_kfunc_start_defs(); /* bpf_skb_get_xfrm_info - Get XFRM metadata * @@ -93,7 +91,7 @@ __bpf_kfunc int bpf_skb_set_xfrm_info(struct __sk_buff *skb_ctx, const struct bp return 0; } -__diag_pop() +__bpf_kfunc_end_defs(); BTF_SET8_START(xfrm_ifc_kfunc_set) BTF_ID_FLAGS(func, bpf_skb_get_xfrm_info)
BPF kfuncs are meant to be called from BPF programs. Accordingly, most kfuncs are not called from anywhere in the kernel, which the -Wmissing-prototypes warning is unhappy about. We've peppered __diag_ignore_all("-Wmissing-prototypes", ... everywhere kfuncs are defined in the codebase to suppress this warning. This patch adds two macros meant to bound one or many kfunc definitions. All existing kfunc definitions which use these __diag calls to suppress -Wmissing-prototypes are migrated to use the newly-introduced macros. A new __diag_ignore_all - for "-Wmissing-declarations" - is added to the __bpf_kfunc_start_defs macro based on feedback from Andrii on an earlier version of this patch [0] and another recent mailing list thread [1]. In the future we might need to ignore different warnings or do other kfunc-specific things. This change will make it easier to make such modifications for all kfunc defs. [0]: https://lore.kernel.org/bpf/CAEf4BzaE5dRWtK6RPLnjTW-MW9sx9K3Fn6uwqCTChK2Dcb1Xig@mail.gmail.com/ [1]: https://lore.kernel.org/bpf/ZT+2qCc%2FaXep0%2FLf@krava/ Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> Suggested-by: Andrii Nakryiko <andrii@kernel.org> Cc: Jiri Olsa <olsajiri@gmail.com> --- This patch was submitted earlier as part of task_vma iter series: https://lore.kernel.org/bpf/20231013204426.1074286-6-davemarchevsky@fb.com/ This separate submission addresses Andrii's comments from that thread. include/linux/btf.h | 9 +++++++++ kernel/bpf/bpf_iter.c | 6 ++---- kernel/bpf/cpumask.c | 6 ++---- kernel/bpf/helpers.c | 6 ++---- kernel/bpf/map_iter.c | 6 ++---- kernel/bpf/task_iter.c | 6 ++---- kernel/trace/bpf_trace.c | 6 ++---- net/bpf/test_run.c | 7 +++---- net/core/filter.c | 13 ++++--------- net/core/xdp.c | 6 ++---- net/ipv4/fou_bpf.c | 6 ++---- net/netfilter/nf_conntrack_bpf.c | 6 ++---- net/netfilter/nf_nat_bpf.c | 6 ++---- net/xfrm/xfrm_interface_bpf.c | 6 ++---- 14 files changed, 38 insertions(+), 57 deletions(-)