Message ID | 1556822018-75282-1-git-send-email-u9012063@gmail.com |
---|---|
State | Accepted |
Delegated to: | BPF Maintainers |
Headers | show |
Series | [bpf] libbpf: add libbpf_util.h to header install. | expand |
On Thu, May 2, 2019 at 11:34 AM William Tu <u9012063@gmail.com> wrote: > > The libbpf_util.h is used by xsk.h, so add it to > the install headers. Can we try to change code a little bit to avoid exposing libbpf_util.h? Originally libbpf_util.h is considered as libbpf internal. I am not strongly against this patch. But would really like to see whether we have an alternative not exposing libbpf_util.h. > > Reported-by: Ben Pfaff <blp@ovn.org> > Signed-off-by: William Tu <u9012063@gmail.com> > --- > tools/lib/bpf/Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile > index c6c06bc6683c..f91639bf5650 100644 > --- a/tools/lib/bpf/Makefile > +++ b/tools/lib/bpf/Makefile > @@ -230,6 +230,7 @@ install_headers: > $(call do_install,bpf.h,$(prefix)/include/bpf,644); \ > $(call do_install,libbpf.h,$(prefix)/include/bpf,644); \ > $(call do_install,btf.h,$(prefix)/include/bpf,644); \ > + $(call do_install,libbpf_util.h,$(prefix)/include/bpf,644); \ > $(call do_install,xsk.h,$(prefix)/include/bpf,644); > > install_pkgconfig: $(PC_FILE) > -- > 2.7.4 >
On Thu, May 2, 2019 at 1:18 PM Y Song <ys114321@gmail.com> wrote: > > On Thu, May 2, 2019 at 11:34 AM William Tu <u9012063@gmail.com> wrote: > > > > The libbpf_util.h is used by xsk.h, so add it to > > the install headers. > > Can we try to change code a little bit to avoid exposing libbpf_util.h? > Originally libbpf_util.h is considered as libbpf internal. > I am not strongly against this patch. But would really like to see > whether we have an alternative not exposing libbpf_util.h. > The commit b7e3a28019c92ff ("libbpf: remove dependency on barrier.h in xsk.h") adds the dependency of libbpf_util.h to xsk.h. How about we move the libbpf_smp_* into the xsk.h, since they are used only by xsk.h. Regards, William > > > > Reported-by: Ben Pfaff <blp@ovn.org> > > Signed-off-by: William Tu <u9012063@gmail.com> > > --- > > tools/lib/bpf/Makefile | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile > > index c6c06bc6683c..f91639bf5650 100644 > > --- a/tools/lib/bpf/Makefile > > +++ b/tools/lib/bpf/Makefile > > @@ -230,6 +230,7 @@ install_headers: > > $(call do_install,bpf.h,$(prefix)/include/bpf,644); \ > > $(call do_install,libbpf.h,$(prefix)/include/bpf,644); \ > > $(call do_install,btf.h,$(prefix)/include/bpf,644); \ > > + $(call do_install,libbpf_util.h,$(prefix)/include/bpf,644); \ > > $(call do_install,xsk.h,$(prefix)/include/bpf,644); > > > > install_pkgconfig: $(PC_FILE) > > -- > > 2.7.4 > >
On Fri, May 3, 2019 at 12:54 PM William Tu <u9012063@gmail.com> wrote: > > On Thu, May 2, 2019 at 1:18 PM Y Song <ys114321@gmail.com> wrote: > > > > On Thu, May 2, 2019 at 11:34 AM William Tu <u9012063@gmail.com> wrote: > > > > > > The libbpf_util.h is used by xsk.h, so add it to > > > the install headers. > > > > Can we try to change code a little bit to avoid exposing libbpf_util.h? > > Originally libbpf_util.h is considered as libbpf internal. > > I am not strongly against this patch. But would really like to see > > whether we have an alternative not exposing libbpf_util.h. > > > > The commit b7e3a28019c92ff ("libbpf: remove dependency on barrier.h in xsk.h") > adds the dependency of libbpf_util.h to xsk.h. > How about we move the libbpf_smp_* into the xsk.h, since they are > used only by xsk.h. Okay. Looks like the libbpf_smp_* is used in some static inline functions which are also API functions. Probably having libbpf_smp_* in libbpf_util.h is a better choice as these primitives can be used by other .c files in tools/lib/bpf. On the other hand, exposing macros pr_warning(), pr_info() and pr_debug() may not be a bad thing as user can use them with the same debug level used by libbpf itself. Ack your original patch: Acked-by: Yonghong Song <yhs@fb.com> > > Regards, > William > > > > > > > Reported-by: Ben Pfaff <blp@ovn.org> > > > Signed-off-by: William Tu <u9012063@gmail.com> > > > --- > > > tools/lib/bpf/Makefile | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile > > > index c6c06bc6683c..f91639bf5650 100644 > > > --- a/tools/lib/bpf/Makefile > > > +++ b/tools/lib/bpf/Makefile > > > @@ -230,6 +230,7 @@ install_headers: > > > $(call do_install,bpf.h,$(prefix)/include/bpf,644); \ > > > $(call do_install,libbpf.h,$(prefix)/include/bpf,644); \ > > > $(call do_install,btf.h,$(prefix)/include/bpf,644); \ > > > + $(call do_install,libbpf_util.h,$(prefix)/include/bpf,644); \ > > > $(call do_install,xsk.h,$(prefix)/include/bpf,644); > > > > > > install_pkgconfig: $(PC_FILE) > > > -- > > > 2.7.4 > > >
On Fri, May 3, 2019 at 2:09 PM Y Song <ys114321@gmail.com> wrote: > > On Fri, May 3, 2019 at 12:54 PM William Tu <u9012063@gmail.com> wrote: > > > > On Thu, May 2, 2019 at 1:18 PM Y Song <ys114321@gmail.com> wrote: > > > > > > On Thu, May 2, 2019 at 11:34 AM William Tu <u9012063@gmail.com> wrote: > > > > > > > > The libbpf_util.h is used by xsk.h, so add it to > > > > the install headers. > > > > > > Can we try to change code a little bit to avoid exposing libbpf_util.h? > > > Originally libbpf_util.h is considered as libbpf internal. > > > I am not strongly against this patch. But would really like to see > > > whether we have an alternative not exposing libbpf_util.h. > > > > > > > The commit b7e3a28019c92ff ("libbpf: remove dependency on barrier.h in xsk.h") > > adds the dependency of libbpf_util.h to xsk.h. > > How about we move the libbpf_smp_* into the xsk.h, since they are > > used only by xsk.h. > > Okay. Looks like the libbpf_smp_* is used in some static inline functions > which are also API functions. > > Probably having libbpf_smp_* in libbpf_util.h is a better choice as these > primitives can be used by other .c files in tools/lib/bpf. > > On the other hand, exposing macros pr_warning(), pr_info() and > pr_debug() may not > be a bad thing as user can use them with the same debug level used by > libbpf itself. > > Ack your original patch: > Acked-by: Yonghong Song <yhs@fb.com> Applied. Thanks
diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile index c6c06bc6683c..f91639bf5650 100644 --- a/tools/lib/bpf/Makefile +++ b/tools/lib/bpf/Makefile @@ -230,6 +230,7 @@ install_headers: $(call do_install,bpf.h,$(prefix)/include/bpf,644); \ $(call do_install,libbpf.h,$(prefix)/include/bpf,644); \ $(call do_install,btf.h,$(prefix)/include/bpf,644); \ + $(call do_install,libbpf_util.h,$(prefix)/include/bpf,644); \ $(call do_install,xsk.h,$(prefix)/include/bpf,644); install_pkgconfig: $(PC_FILE)
The libbpf_util.h is used by xsk.h, so add it to the install headers. Reported-by: Ben Pfaff <blp@ovn.org> Signed-off-by: William Tu <u9012063@gmail.com> --- tools/lib/bpf/Makefile | 1 + 1 file changed, 1 insertion(+)