Re: [PATCH v1 bpf-next] bpf: Add __bpf_kfunc_{start,end}_defs macros

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 31, 2023 at 2:23 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Mon, Oct 30, 2023 at 10:56 PM Yafang Shao <laoar.shao@xxxxxxxxx> wrote:
> >
> > On Tue, Oct 31, 2023 at 5:07 AM Dave Marchevsky <davemarchevsky@xxxxxx> 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@xxxxxxxxxxxxxx/
> > >   [1]: https://lore.kernel.org/bpf/ZT+2qCc%2FaXep0%2FLf@krava/
> > >
> > > Signed-off-by: Dave Marchevsky <davemarchevsky@xxxxxx>
> > > Suggested-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > > Cc: Jiri Olsa <olsajiri@xxxxxxxxx>
> > > ---
> > >
> > > This patch was submitted earlier as part of task_vma
> > > iter series: https://lore.kernel.org/bpf/20231013204426.1074286-6-davemarchevsky@xxxxxx/
> > >
> > > 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@xxxxxxxxxx>
>
> > --
> > Regards
> > Yafang



-- 
Regards
Yafang





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux