On Sat, Oct 5, 2019 at 6:01 PM Alexei Starovoitov <ast@xxxxxx> wrote: > > On 10/5/19 11:24 AM, Andrii Nakryiko wrote: > > On Sat, Oct 5, 2019 at 10:10 AM Alexei Starovoitov <ast@xxxxxx> wrote: > >> > >> On 10/5/19 12:59 AM, Andrii Nakryiko wrote: > >>> Get rid of list of BPF helpers in bpf_helpers.h (irony...) and > >>> auto-generate it into bpf_helpers_defs.h, which is now included from > >>> bpf_helpers.h. > >>> > >>> Suggested-by: Alexei Starovoitov<ast@xxxxxx> > >>> Signed-off-by: Andrii Nakryiko<andriin@xxxxxx> > >>> --- > >>> tools/lib/bpf/Makefile | 8 +- > >>> tools/lib/bpf/bpf_helpers.h | 264 +-- > >>> tools/lib/bpf/bpf_helpers_defs.h | 2677 ++++++++++++++++++++++++++++++ > >>> 3 files changed, 2685 insertions(+), 264 deletions(-) > >>> create mode 100644 tools/lib/bpf/bpf_helpers_defs.h > >> > >> Approach looks good to me. > >> imo that's better than messing with macros. > >> > >> Using bpf_helpers_doc.py as part of build will help man pages too. > >> I think we were sloppy documenting helpers, since only Quentin > >> was running that script regularly. > > > > Yep, I agree, I had to fix few things, as well as (char *) vs (void *) > > vs (__u8 *) differences were causing some extra warnings. > > Please check the list of type translations whether they make sense. > > yes. These conversions make sense. cool, thanks for checking! > Only one sk_buff->__sk_buff won't work for too long. > Once my btf vmlinux stuff lands both structs will be possible. I guess it will have to be sk_buff -> void conversion then, but we'll do it later. > > >> > >> Only question is what is the reason to commit generated .h into git? > > > > So originally I didn't want to depend on system UAPI headers during > > Github build. But now I recalled that we do have latest UAPI synced > > into Github's include/ subdir, so that's not an obstacle really. We'll > > just need to re-license bpf_helpers_doc.py (I don't think Quentin will > > mind) and start syncing it to Github (not a big deal at all). > > github sync script can run make, grab generated .h, > and check it in into github. Haven't considered that. That will work as well, a bit more work for me on sync script side, but that's ok. > > > > > There is still a benefit in having it checked in: easy to spot if > > script does something wrong and double-check the changes (after > > initial big commit, of course). > > > > If you think that's not reason enough, let me know and I can drop it in v2. > > I don't remember too many cases when generated files were committed. > My preference to keep it out of kernel git. > github/libbpf is a pure mirror. That .h can be committed there. > The whole thing is automatic, so we don't need to move .py > into github too. > Yeah, makes sense, will submit v2 w/ bpf_helpers_defs.h added to .gitignore