| Message ID | 20190820114706.18546-5-toke@redhat.com |
|---|---|
| State | RFC |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Convert iproute2 to use libbpf (WIP) | expand |
On 8/20/19 1:47 PM, Toke Høiland-Jørgensen wrote: > This adds a configure check for libbpf and renames functions to allow > lib/bpf.c to be compiled with it present. This makes it possible to > port functionality piecemeal to use libbpf. > > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> > --- > configure | 16 ++++++++++++++++ > include/bpf_util.h | 6 +++--- > ip/ipvrf.c | 4 ++-- > lib/bpf.c | 33 +++++++++++++++++++-------------- > 4 files changed, 40 insertions(+), 19 deletions(-) > > diff --git a/configure b/configure > index 45fcffb6..5a89ee9f 100755 > --- a/configure > +++ b/configure > @@ -238,6 +238,19 @@ check_elf() > fi > } > > +check_libbpf() > +{ > + if ${PKG_CONFIG} libbpf --exists; then > + echo "HAVE_LIBBPF:=y" >>$CONFIG > + echo "yes" > + > + echo 'CFLAGS += -DHAVE_LIBBPF' `${PKG_CONFIG} libbpf --cflags` >> $CONFIG > + echo 'LDLIBS += ' `${PKG_CONFIG} libbpf --libs` >>$CONFIG > + else > + echo "no" > + fi > +} > + > check_selinux() More of an implementation detail at this point in time, but want to make sure this doesn't get missed along the way: as discussed at bpfconf [0] best for iproute2 to handle libbpf support would be the same way of integration as pahole does, that is, to integrate it via submodule [1] to allow kernel and libbpf features to be in sync with iproute2 releases and therefore easily consume extensions we're adding to libbpf to aide iproute2 integration. Thanks, Daniel [0] http://vger.kernel.org/bpfconf2019.html#session-4 [1] https://git.kernel.org/pub/scm/devel/pahole/pahole.git/commit/?id=21507cd3e97bc5692d97201ee68df044c6767e9a
Daniel Borkmann <daniel@iogearbox.net> writes: > On 8/20/19 1:47 PM, Toke Høiland-Jørgensen wrote: >> This adds a configure check for libbpf and renames functions to allow >> lib/bpf.c to be compiled with it present. This makes it possible to >> port functionality piecemeal to use libbpf. >> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >> --- >> configure | 16 ++++++++++++++++ >> include/bpf_util.h | 6 +++--- >> ip/ipvrf.c | 4 ++-- >> lib/bpf.c | 33 +++++++++++++++++++-------------- >> 4 files changed, 40 insertions(+), 19 deletions(-) >> >> diff --git a/configure b/configure >> index 45fcffb6..5a89ee9f 100755 >> --- a/configure >> +++ b/configure >> @@ -238,6 +238,19 @@ check_elf() >> fi >> } >> >> +check_libbpf() >> +{ >> + if ${PKG_CONFIG} libbpf --exists; then >> + echo "HAVE_LIBBPF:=y" >>$CONFIG >> + echo "yes" >> + >> + echo 'CFLAGS += -DHAVE_LIBBPF' `${PKG_CONFIG} libbpf --cflags` >> $CONFIG >> + echo 'LDLIBS += ' `${PKG_CONFIG} libbpf --libs` >>$CONFIG >> + else >> + echo "no" >> + fi >> +} >> + >> check_selinux() > > More of an implementation detail at this point in time, but want to > make sure this doesn't get missed along the way: as discussed at > bpfconf [0] best for iproute2 to handle libbpf support would be the > same way of integration as pahole does, that is, to integrate it via > submodule [1] to allow kernel and libbpf features to be in sync with > iproute2 releases and therefore easily consume extensions we're adding > to libbpf to aide iproute2 integration. I can sorta see the point wrt keeping in sync with kernel features. But how will this work with distros that package libbpf as a regular library? Have you guys given up on regular library symbol versioning for libbpf? > [0] http://vger.kernel.org/bpfconf2019.html#session-4 Thanks for that link! Didn't manage to find any of the previous discussions on iproute2 compatibility. -Toke
On 8/22/19 12:43 PM, Toke Høiland-Jørgensen wrote: > Daniel Borkmann <daniel@iogearbox.net> writes: >> On 8/20/19 1:47 PM, Toke Høiland-Jørgensen wrote: >>> This adds a configure check for libbpf and renames functions to allow >>> lib/bpf.c to be compiled with it present. This makes it possible to >>> port functionality piecemeal to use libbpf. >>> >>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >>> --- >>> configure | 16 ++++++++++++++++ >>> include/bpf_util.h | 6 +++--- >>> ip/ipvrf.c | 4 ++-- >>> lib/bpf.c | 33 +++++++++++++++++++-------------- >>> 4 files changed, 40 insertions(+), 19 deletions(-) >>> >>> diff --git a/configure b/configure >>> index 45fcffb6..5a89ee9f 100755 >>> --- a/configure >>> +++ b/configure >>> @@ -238,6 +238,19 @@ check_elf() >>> fi >>> } >>> >>> +check_libbpf() >>> +{ >>> + if ${PKG_CONFIG} libbpf --exists; then >>> + echo "HAVE_LIBBPF:=y" >>$CONFIG >>> + echo "yes" >>> + >>> + echo 'CFLAGS += -DHAVE_LIBBPF' `${PKG_CONFIG} libbpf --cflags` >> $CONFIG >>> + echo 'LDLIBS += ' `${PKG_CONFIG} libbpf --libs` >>$CONFIG >>> + else >>> + echo "no" >>> + fi >>> +} >>> + >>> check_selinux() >> >> More of an implementation detail at this point in time, but want to >> make sure this doesn't get missed along the way: as discussed at >> bpfconf [0] best for iproute2 to handle libbpf support would be the >> same way of integration as pahole does, that is, to integrate it via >> submodule [1] to allow kernel and libbpf features to be in sync with >> iproute2 releases and therefore easily consume extensions we're adding >> to libbpf to aide iproute2 integration. > > I can sorta see the point wrt keeping in sync with kernel features. But > how will this work with distros that package libbpf as a regular > library? Have you guys given up on regular library symbol versioning for > libbpf? Not at all, and I hope you know that. ;-) The reason I added lib/bpf.c integration into iproute2 directly back then was exactly such that users can start consuming BPF for tc and XDP via iproute2 /everywhere/ with only a simple libelf dependency which is also available on all distros since pretty much forever. If it was an external library, we could have waited till hell freezes over and initial distro adoption would have pretty much taken forever: to pick one random example here wrt the pace of some downstream distros [0]. The main rationale is pretty much the same as with added kernel features that land complementary iproute2 patches for that kernel release and as libbpf is developed alongside it is reasonable to guarantee user expectations that iproute2 released for kernel version x can make use of BPF features added to kernel x with same loader support from x. [0] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1774815 >> [0] http://vger.kernel.org/bpfconf2019.html#session-4 > > Thanks for that link! Didn't manage to find any of the previous > discussions on iproute2 compatibility. > > -Toke >
Daniel Borkmann <daniel@iogearbox.net> writes: > On 8/22/19 12:43 PM, Toke Høiland-Jørgensen wrote: >> Daniel Borkmann <daniel@iogearbox.net> writes: >>> On 8/20/19 1:47 PM, Toke Høiland-Jørgensen wrote: >>>> This adds a configure check for libbpf and renames functions to allow >>>> lib/bpf.c to be compiled with it present. This makes it possible to >>>> port functionality piecemeal to use libbpf. >>>> >>>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >>>> --- >>>> configure | 16 ++++++++++++++++ >>>> include/bpf_util.h | 6 +++--- >>>> ip/ipvrf.c | 4 ++-- >>>> lib/bpf.c | 33 +++++++++++++++++++-------------- >>>> 4 files changed, 40 insertions(+), 19 deletions(-) >>>> >>>> diff --git a/configure b/configure >>>> index 45fcffb6..5a89ee9f 100755 >>>> --- a/configure >>>> +++ b/configure >>>> @@ -238,6 +238,19 @@ check_elf() >>>> fi >>>> } >>>> >>>> +check_libbpf() >>>> +{ >>>> + if ${PKG_CONFIG} libbpf --exists; then >>>> + echo "HAVE_LIBBPF:=y" >>$CONFIG >>>> + echo "yes" >>>> + >>>> + echo 'CFLAGS += -DHAVE_LIBBPF' `${PKG_CONFIG} libbpf --cflags` >> $CONFIG >>>> + echo 'LDLIBS += ' `${PKG_CONFIG} libbpf --libs` >>$CONFIG >>>> + else >>>> + echo "no" >>>> + fi >>>> +} >>>> + >>>> check_selinux() >>> >>> More of an implementation detail at this point in time, but want to >>> make sure this doesn't get missed along the way: as discussed at >>> bpfconf [0] best for iproute2 to handle libbpf support would be the >>> same way of integration as pahole does, that is, to integrate it via >>> submodule [1] to allow kernel and libbpf features to be in sync with >>> iproute2 releases and therefore easily consume extensions we're adding >>> to libbpf to aide iproute2 integration. >> >> I can sorta see the point wrt keeping in sync with kernel features. But >> how will this work with distros that package libbpf as a regular >> library? Have you guys given up on regular library symbol versioning for >> libbpf? > > Not at all, and I hope you know that. ;-) Good! Didn't really expect you had, just checking ;) > The reason I added lib/bpf.c integration into iproute2 directly back > then was exactly such that users can start consuming BPF for tc and > XDP via iproute2 /everywhere/ with only a simple libelf dependency > which is also available on all distros since pretty much forever. If > it was an external library, we could have waited till hell freezes > over and initial distro adoption would have pretty much taken forever: > to pick one random example here wrt the pace of some downstream > distros [0]. The main rationale is pretty much the same as with added > kernel features that land complementary iproute2 patches for that > kernel release and as libbpf is developed alongside it is reasonable > to guarantee user expectations that iproute2 released for kernel > version x can make use of BPF features added to kernel x with same > loader support from x. Well, for iproute2 I would expect this to be solved by version dependencies. I.e. iproute2 version X would depend on libbpf version Y+ (like, I dunno, the version of libbpf included in the same kernel source tree as the kernel version iproute2 is targeting? :)). If we vendor libbpf into every project using it, we'll end up with dozens of different versions statically linked into each binary, kinda defeating the purpose of having it as a shared library in the first place... -Toke
On 8/22/19 2:04 PM, Toke Høiland-Jørgensen wrote: > Daniel Borkmann <daniel@iogearbox.net> writes: >> On 8/22/19 12:43 PM, Toke Høiland-Jørgensen wrote: >>> Daniel Borkmann <daniel@iogearbox.net> writes: >>>> On 8/20/19 1:47 PM, Toke Høiland-Jørgensen wrote: >>>>> This adds a configure check for libbpf and renames functions to allow >>>>> lib/bpf.c to be compiled with it present. This makes it possible to >>>>> port functionality piecemeal to use libbpf. >>>>> >>>>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >>>>> --- >>>>> configure | 16 ++++++++++++++++ >>>>> include/bpf_util.h | 6 +++--- >>>>> ip/ipvrf.c | 4 ++-- >>>>> lib/bpf.c | 33 +++++++++++++++++++-------------- >>>>> 4 files changed, 40 insertions(+), 19 deletions(-) >>>>> >>>>> diff --git a/configure b/configure >>>>> index 45fcffb6..5a89ee9f 100755 >>>>> --- a/configure >>>>> +++ b/configure >>>>> @@ -238,6 +238,19 @@ check_elf() >>>>> fi >>>>> } >>>>> >>>>> +check_libbpf() >>>>> +{ >>>>> + if ${PKG_CONFIG} libbpf --exists; then >>>>> + echo "HAVE_LIBBPF:=y" >>$CONFIG >>>>> + echo "yes" >>>>> + >>>>> + echo 'CFLAGS += -DHAVE_LIBBPF' `${PKG_CONFIG} libbpf --cflags` >> $CONFIG >>>>> + echo 'LDLIBS += ' `${PKG_CONFIG} libbpf --libs` >>$CONFIG >>>>> + else >>>>> + echo "no" >>>>> + fi >>>>> +} >>>>> + >>>>> check_selinux() >>>> >>>> More of an implementation detail at this point in time, but want to >>>> make sure this doesn't get missed along the way: as discussed at >>>> bpfconf [0] best for iproute2 to handle libbpf support would be the >>>> same way of integration as pahole does, that is, to integrate it via >>>> submodule [1] to allow kernel and libbpf features to be in sync with >>>> iproute2 releases and therefore easily consume extensions we're adding >>>> to libbpf to aide iproute2 integration. >>> >>> I can sorta see the point wrt keeping in sync with kernel features. But >>> how will this work with distros that package libbpf as a regular >>> library? Have you guys given up on regular library symbol versioning for >>> libbpf? >> >> Not at all, and I hope you know that. ;-) > > Good! Didn't really expect you had, just checking ;) > >> The reason I added lib/bpf.c integration into iproute2 directly back >> then was exactly such that users can start consuming BPF for tc and >> XDP via iproute2 /everywhere/ with only a simple libelf dependency >> which is also available on all distros since pretty much forever. If >> it was an external library, we could have waited till hell freezes >> over and initial distro adoption would have pretty much taken forever: >> to pick one random example here wrt the pace of some downstream >> distros [0]. The main rationale is pretty much the same as with added >> kernel features that land complementary iproute2 patches for that >> kernel release and as libbpf is developed alongside it is reasonable >> to guarantee user expectations that iproute2 released for kernel >> version x can make use of BPF features added to kernel x with same >> loader support from x. > > Well, for iproute2 I would expect this to be solved by version > dependencies. I.e. iproute2 version X would depend on libbpf version Y+ > (like, I dunno, the version of libbpf included in the same kernel source > tree as the kernel version iproute2 is targeting? :)). This sounds nice in theory, but from what I've seen major (!) distros already seem to have a hard time releasing kernel x along with iproute2 package x, concrete example was that distro kernel was on 4.13 and its official iproute2 package on 4.9, adding yet another variable that needs to be in sync with kernel is simply impractical especially for a _core_ package like iproute2 that should have as little dependencies as possible. I also don't want to make a bet on whether libbpf will be available on every distro that also ships iproute2. Hence approach that pahole (and also bcc by the way) takes is most reasonable to have the best user experience. Thanks, Daniel
Daniel Borkmann <daniel@iogearbox.net> writes: > On 8/22/19 2:04 PM, Toke Høiland-Jørgensen wrote: >> Daniel Borkmann <daniel@iogearbox.net> writes: >>> On 8/22/19 12:43 PM, Toke Høiland-Jørgensen wrote: >>>> Daniel Borkmann <daniel@iogearbox.net> writes: >>>>> On 8/20/19 1:47 PM, Toke Høiland-Jørgensen wrote: >>>>>> This adds a configure check for libbpf and renames functions to allow >>>>>> lib/bpf.c to be compiled with it present. This makes it possible to >>>>>> port functionality piecemeal to use libbpf. >>>>>> >>>>>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >>>>>> --- >>>>>> configure | 16 ++++++++++++++++ >>>>>> include/bpf_util.h | 6 +++--- >>>>>> ip/ipvrf.c | 4 ++-- >>>>>> lib/bpf.c | 33 +++++++++++++++++++-------------- >>>>>> 4 files changed, 40 insertions(+), 19 deletions(-) >>>>>> >>>>>> diff --git a/configure b/configure >>>>>> index 45fcffb6..5a89ee9f 100755 >>>>>> --- a/configure >>>>>> +++ b/configure >>>>>> @@ -238,6 +238,19 @@ check_elf() >>>>>> fi >>>>>> } >>>>>> >>>>>> +check_libbpf() >>>>>> +{ >>>>>> + if ${PKG_CONFIG} libbpf --exists; then >>>>>> + echo "HAVE_LIBBPF:=y" >>$CONFIG >>>>>> + echo "yes" >>>>>> + >>>>>> + echo 'CFLAGS += -DHAVE_LIBBPF' `${PKG_CONFIG} libbpf --cflags` >> $CONFIG >>>>>> + echo 'LDLIBS += ' `${PKG_CONFIG} libbpf --libs` >>$CONFIG >>>>>> + else >>>>>> + echo "no" >>>>>> + fi >>>>>> +} >>>>>> + >>>>>> check_selinux() >>>>> >>>>> More of an implementation detail at this point in time, but want to >>>>> make sure this doesn't get missed along the way: as discussed at >>>>> bpfconf [0] best for iproute2 to handle libbpf support would be the >>>>> same way of integration as pahole does, that is, to integrate it via >>>>> submodule [1] to allow kernel and libbpf features to be in sync with >>>>> iproute2 releases and therefore easily consume extensions we're adding >>>>> to libbpf to aide iproute2 integration. >>>> >>>> I can sorta see the point wrt keeping in sync with kernel features. But >>>> how will this work with distros that package libbpf as a regular >>>> library? Have you guys given up on regular library symbol versioning for >>>> libbpf? >>> >>> Not at all, and I hope you know that. ;-) >> >> Good! Didn't really expect you had, just checking ;) >> >>> The reason I added lib/bpf.c integration into iproute2 directly back >>> then was exactly such that users can start consuming BPF for tc and >>> XDP via iproute2 /everywhere/ with only a simple libelf dependency >>> which is also available on all distros since pretty much forever. If >>> it was an external library, we could have waited till hell freezes >>> over and initial distro adoption would have pretty much taken forever: >>> to pick one random example here wrt the pace of some downstream >>> distros [0]. The main rationale is pretty much the same as with added >>> kernel features that land complementary iproute2 patches for that >>> kernel release and as libbpf is developed alongside it is reasonable >>> to guarantee user expectations that iproute2 released for kernel >>> version x can make use of BPF features added to kernel x with same >>> loader support from x. >> >> Well, for iproute2 I would expect this to be solved by version >> dependencies. I.e. iproute2 version X would depend on libbpf version Y+ >> (like, I dunno, the version of libbpf included in the same kernel source >> tree as the kernel version iproute2 is targeting? :)). > > This sounds nice in theory, but from what I've seen major (!) distros > already seem to have a hard time releasing kernel x along with iproute2 > package x, concrete example was that distro kernel was on 4.13 and its > official iproute2 package on 4.9, If the iproute2 package is not being updated at all I don't really see how it would make any difference whether libbpf is vendored or not? :) > adding yet another variable that needs to be in sync with kernel is > simply impractical especially for a _core_ package like iproute2 that > should have as little dependencies as possible. I also don't want to > make a bet on whether libbpf will be available on every distro that > also ships iproute2. Hence approach that pahole (and also bcc by the > way) takes is most reasonable to have the best user experience. Most users are going to get iproute2 from their distro packages anyway, so if distros are incompetent at packaging, my bet is that you're going to run into issues one way or another. But OK, if you think it is easier to work around bad distros by vendoring, you guys are the maintainers, so that's up to you. But can we at least put in the version dependency and let the build system pick up a system libbpf if it exists and is compatible? That way distros that *are* competent can still link it dynamically... -Toke
On 8/22/19 3:38 PM, Toke Høiland-Jørgensen wrote: > Daniel Borkmann <daniel@iogearbox.net> writes: >> On 8/22/19 2:04 PM, Toke Høiland-Jørgensen wrote: >>> Daniel Borkmann <daniel@iogearbox.net> writes: >>>> On 8/22/19 12:43 PM, Toke Høiland-Jørgensen wrote: >>>>> Daniel Borkmann <daniel@iogearbox.net> writes: >>>>>> On 8/20/19 1:47 PM, Toke Høiland-Jørgensen wrote: >>>>>>> This adds a configure check for libbpf and renames functions to allow >>>>>>> lib/bpf.c to be compiled with it present. This makes it possible to >>>>>>> port functionality piecemeal to use libbpf. >>>>>>> >>>>>>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >>>>>>> --- >>>>>>> configure | 16 ++++++++++++++++ >>>>>>> include/bpf_util.h | 6 +++--- >>>>>>> ip/ipvrf.c | 4 ++-- >>>>>>> lib/bpf.c | 33 +++++++++++++++++++-------------- >>>>>>> 4 files changed, 40 insertions(+), 19 deletions(-) >>>>>>> >>>>>>> diff --git a/configure b/configure >>>>>>> index 45fcffb6..5a89ee9f 100755 >>>>>>> --- a/configure >>>>>>> +++ b/configure >>>>>>> @@ -238,6 +238,19 @@ check_elf() >>>>>>> fi >>>>>>> } >>>>>>> >>>>>>> +check_libbpf() >>>>>>> +{ >>>>>>> + if ${PKG_CONFIG} libbpf --exists; then >>>>>>> + echo "HAVE_LIBBPF:=y" >>$CONFIG >>>>>>> + echo "yes" >>>>>>> + >>>>>>> + echo 'CFLAGS += -DHAVE_LIBBPF' `${PKG_CONFIG} libbpf --cflags` >> $CONFIG >>>>>>> + echo 'LDLIBS += ' `${PKG_CONFIG} libbpf --libs` >>$CONFIG >>>>>>> + else >>>>>>> + echo "no" >>>>>>> + fi >>>>>>> +} >>>>>>> + >>>>>>> check_selinux() >>>>>> >>>>>> More of an implementation detail at this point in time, but want to >>>>>> make sure this doesn't get missed along the way: as discussed at >>>>>> bpfconf [0] best for iproute2 to handle libbpf support would be the >>>>>> same way of integration as pahole does, that is, to integrate it via >>>>>> submodule [1] to allow kernel and libbpf features to be in sync with >>>>>> iproute2 releases and therefore easily consume extensions we're adding >>>>>> to libbpf to aide iproute2 integration. >>>>> >>>>> I can sorta see the point wrt keeping in sync with kernel features. But >>>>> how will this work with distros that package libbpf as a regular >>>>> library? Have you guys given up on regular library symbol versioning for >>>>> libbpf? >>>> >>>> Not at all, and I hope you know that. ;-) >>> >>> Good! Didn't really expect you had, just checking ;) >>> >>>> The reason I added lib/bpf.c integration into iproute2 directly back >>>> then was exactly such that users can start consuming BPF for tc and >>>> XDP via iproute2 /everywhere/ with only a simple libelf dependency >>>> which is also available on all distros since pretty much forever. If >>>> it was an external library, we could have waited till hell freezes >>>> over and initial distro adoption would have pretty much taken forever: >>>> to pick one random example here wrt the pace of some downstream >>>> distros [0]. The main rationale is pretty much the same as with added >>>> kernel features that land complementary iproute2 patches for that >>>> kernel release and as libbpf is developed alongside it is reasonable >>>> to guarantee user expectations that iproute2 released for kernel >>>> version x can make use of BPF features added to kernel x with same >>>> loader support from x. >>> >>> Well, for iproute2 I would expect this to be solved by version >>> dependencies. I.e. iproute2 version X would depend on libbpf version Y+ >>> (like, I dunno, the version of libbpf included in the same kernel source >>> tree as the kernel version iproute2 is targeting? :)). >> >> This sounds nice in theory, but from what I've seen major (!) distros >> already seem to have a hard time releasing kernel x along with iproute2 >> package x, concrete example was that distro kernel was on 4.13 and its >> official iproute2 package on 4.9, > > If the iproute2 package is not being updated at all I don't really see > how it would make any difference whether libbpf is vendored or not? :) > >> adding yet another variable that needs to be in sync with kernel is >> simply impractical especially for a _core_ package like iproute2 that >> should have as little dependencies as possible. I also don't want to >> make a bet on whether libbpf will be available on every distro that >> also ships iproute2. Hence approach that pahole (and also bcc by the >> way) takes is most reasonable to have the best user experience. > > Most users are going to get iproute2 from their distro packages anyway, > so if distros are incompetent at packaging, my bet is that you're going > to run into issues one way or another. > > But OK, if you think it is easier to work around bad distros by > vendoring, you guys are the maintainers, so that's up to you. But can we > at least put in the version dependency and let the build system pick up > a system libbpf if it exists and is compatible? That way distros that > *are* competent can still link it dynamically... Yeah that would be fine by me to use this as a fallback, and I think that iproute2's configure script should be able to easily handle this situation. That way we don't compromise on user experience. Thanks, Daniel
Daniel Borkmann <daniel@iogearbox.net> writes: > On 8/22/19 3:38 PM, Toke Høiland-Jørgensen wrote: >> Daniel Borkmann <daniel@iogearbox.net> writes: >>> On 8/22/19 2:04 PM, Toke Høiland-Jørgensen wrote: >>>> Daniel Borkmann <daniel@iogearbox.net> writes: >>>>> On 8/22/19 12:43 PM, Toke Høiland-Jørgensen wrote: >>>>>> Daniel Borkmann <daniel@iogearbox.net> writes: >>>>>>> On 8/20/19 1:47 PM, Toke Høiland-Jørgensen wrote: >>>>>>>> This adds a configure check for libbpf and renames functions to allow >>>>>>>> lib/bpf.c to be compiled with it present. This makes it possible to >>>>>>>> port functionality piecemeal to use libbpf. >>>>>>>> >>>>>>>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >>>>>>>> --- >>>>>>>> configure | 16 ++++++++++++++++ >>>>>>>> include/bpf_util.h | 6 +++--- >>>>>>>> ip/ipvrf.c | 4 ++-- >>>>>>>> lib/bpf.c | 33 +++++++++++++++++++-------------- >>>>>>>> 4 files changed, 40 insertions(+), 19 deletions(-) >>>>>>>> >>>>>>>> diff --git a/configure b/configure >>>>>>>> index 45fcffb6..5a89ee9f 100755 >>>>>>>> --- a/configure >>>>>>>> +++ b/configure >>>>>>>> @@ -238,6 +238,19 @@ check_elf() >>>>>>>> fi >>>>>>>> } >>>>>>>> >>>>>>>> +check_libbpf() >>>>>>>> +{ >>>>>>>> + if ${PKG_CONFIG} libbpf --exists; then >>>>>>>> + echo "HAVE_LIBBPF:=y" >>$CONFIG >>>>>>>> + echo "yes" >>>>>>>> + >>>>>>>> + echo 'CFLAGS += -DHAVE_LIBBPF' `${PKG_CONFIG} libbpf --cflags` >> $CONFIG >>>>>>>> + echo 'LDLIBS += ' `${PKG_CONFIG} libbpf --libs` >>$CONFIG >>>>>>>> + else >>>>>>>> + echo "no" >>>>>>>> + fi >>>>>>>> +} >>>>>>>> + >>>>>>>> check_selinux() >>>>>>> >>>>>>> More of an implementation detail at this point in time, but want to >>>>>>> make sure this doesn't get missed along the way: as discussed at >>>>>>> bpfconf [0] best for iproute2 to handle libbpf support would be the >>>>>>> same way of integration as pahole does, that is, to integrate it via >>>>>>> submodule [1] to allow kernel and libbpf features to be in sync with >>>>>>> iproute2 releases and therefore easily consume extensions we're adding >>>>>>> to libbpf to aide iproute2 integration. >>>>>> >>>>>> I can sorta see the point wrt keeping in sync with kernel features. But >>>>>> how will this work with distros that package libbpf as a regular >>>>>> library? Have you guys given up on regular library symbol versioning for >>>>>> libbpf? >>>>> >>>>> Not at all, and I hope you know that. ;-) >>>> >>>> Good! Didn't really expect you had, just checking ;) >>>> >>>>> The reason I added lib/bpf.c integration into iproute2 directly back >>>>> then was exactly such that users can start consuming BPF for tc and >>>>> XDP via iproute2 /everywhere/ with only a simple libelf dependency >>>>> which is also available on all distros since pretty much forever. If >>>>> it was an external library, we could have waited till hell freezes >>>>> over and initial distro adoption would have pretty much taken forever: >>>>> to pick one random example here wrt the pace of some downstream >>>>> distros [0]. The main rationale is pretty much the same as with added >>>>> kernel features that land complementary iproute2 patches for that >>>>> kernel release and as libbpf is developed alongside it is reasonable >>>>> to guarantee user expectations that iproute2 released for kernel >>>>> version x can make use of BPF features added to kernel x with same >>>>> loader support from x. >>>> >>>> Well, for iproute2 I would expect this to be solved by version >>>> dependencies. I.e. iproute2 version X would depend on libbpf version Y+ >>>> (like, I dunno, the version of libbpf included in the same kernel source >>>> tree as the kernel version iproute2 is targeting? :)). >>> >>> This sounds nice in theory, but from what I've seen major (!) distros >>> already seem to have a hard time releasing kernel x along with iproute2 >>> package x, concrete example was that distro kernel was on 4.13 and its >>> official iproute2 package on 4.9, >> >> If the iproute2 package is not being updated at all I don't really see >> how it would make any difference whether libbpf is vendored or not? :) >> >>> adding yet another variable that needs to be in sync with kernel is >>> simply impractical especially for a _core_ package like iproute2 that >>> should have as little dependencies as possible. I also don't want to >>> make a bet on whether libbpf will be available on every distro that >>> also ships iproute2. Hence approach that pahole (and also bcc by the >>> way) takes is most reasonable to have the best user experience. >> >> Most users are going to get iproute2 from their distro packages anyway, >> so if distros are incompetent at packaging, my bet is that you're going >> to run into issues one way or another. >> >> But OK, if you think it is easier to work around bad distros by >> vendoring, you guys are the maintainers, so that's up to you. But can we >> at least put in the version dependency and let the build system pick up >> a system libbpf if it exists and is compatible? That way distros that >> *are* competent can still link it dynamically... > > Yeah that would be fine by me to use this as a fallback, and I think that > iproute2's configure script should be able to easily handle this > situation. Cool, I can live with that :) -Toke
diff --git a/configure b/configure index 45fcffb6..5a89ee9f 100755 --- a/configure +++ b/configure @@ -238,6 +238,19 @@ check_elf() fi } +check_libbpf() +{ + if ${PKG_CONFIG} libbpf --exists; then + echo "HAVE_LIBBPF:=y" >>$CONFIG + echo "yes" + + echo 'CFLAGS += -DHAVE_LIBBPF' `${PKG_CONFIG} libbpf --cflags` >> $CONFIG + echo 'LDLIBS += ' `${PKG_CONFIG} libbpf --libs` >>$CONFIG + else + echo "no" + fi +} + check_selinux() # SELinux is a compile time option in the ss utility { @@ -386,6 +399,9 @@ check_selinux echo -n "ELF support: " check_elf +echo -n "libbpf support: " +check_libbpf + echo -n "libmnl support: " check_mnl diff --git a/include/bpf_util.h b/include/bpf_util.h index 63db07ca..72d3a32c 100644 --- a/include/bpf_util.h +++ b/include/bpf_util.h @@ -274,9 +274,9 @@ int bpf_trace_pipe(void); void bpf_print_ops(struct rtattr *bpf_ops, __u16 len); -int bpf_prog_load(enum bpf_prog_type type, const struct bpf_insn *insns, - size_t size_insns, const char *license, char *log, - size_t size_log); +int bpf_prog_load_buf(enum bpf_prog_type type, const struct bpf_insn *insns, + size_t size_insns, const char *license, char *log, + size_t size_log); int bpf_prog_attach_fd(int prog_fd, int target_fd, enum bpf_attach_type type); int bpf_prog_detach_fd(int target_fd, enum bpf_attach_type type); diff --git a/ip/ipvrf.c b/ip/ipvrf.c index 43366f6e..1d1aae6f 100644 --- a/ip/ipvrf.c +++ b/ip/ipvrf.c @@ -256,8 +256,8 @@ static int prog_load(int idx) BPF_EXIT_INSN(), }; - return bpf_prog_load(BPF_PROG_TYPE_CGROUP_SOCK, prog, sizeof(prog), - "GPL", bpf_log_buf, sizeof(bpf_log_buf)); + return bpf_prog_load_buf(BPF_PROG_TYPE_CGROUP_SOCK, prog, sizeof(prog), + "GPL", bpf_log_buf, sizeof(bpf_log_buf)); } static int vrf_configure_cgroup(const char *path, int ifindex) diff --git a/lib/bpf.c b/lib/bpf.c index 7d2a322f..c6e3bd0d 100644 --- a/lib/bpf.c +++ b/lib/bpf.c @@ -28,6 +28,11 @@ #include <gelf.h> #endif +#ifdef HAVE_LIBBPF +#include <bpf/libbpf.h> +#include <bpf/bpf.h> +#endif + #include <sys/types.h> #include <sys/stat.h> #include <sys/un.h> @@ -795,7 +800,7 @@ out: return mnt; } -static int bpf_obj_get(const char *pathname, enum bpf_prog_type type) +static int bpf_obj_get_path(const char *pathname, enum bpf_prog_type type) { union bpf_attr attr = {}; char tmp[PATH_MAX]; @@ -814,7 +819,7 @@ static int bpf_obj_get(const char *pathname, enum bpf_prog_type type) static int bpf_obj_pinned(const char *pathname, enum bpf_prog_type type) { - int prog_fd = bpf_obj_get(pathname, type); + int prog_fd = bpf_obj_get_path(pathname, type); if (prog_fd < 0) fprintf(stderr, "Couldn\'t retrieve pinned program \'%s\': %s\n", @@ -1036,7 +1041,7 @@ int bpf_graft_map(const char *map_path, uint32_t *key, int argc, char **argv) } } - map_fd = bpf_obj_get(map_path, cfg.type); + map_fd = bpf_obj_get_path(map_path, cfg.type); if (map_fd < 0) { fprintf(stderr, "Couldn\'t retrieve pinned map \'%s\': %s\n", map_path, strerror(errno)); @@ -1105,9 +1110,9 @@ static int bpf_prog_load_dev(enum bpf_prog_type type, return bpf(BPF_PROG_LOAD, &attr, sizeof(attr)); } -int bpf_prog_load(enum bpf_prog_type type, const struct bpf_insn *insns, - size_t size_insns, const char *license, char *log, - size_t size_log) +int bpf_prog_load_buf(enum bpf_prog_type type, const struct bpf_insn *insns, + size_t size_insns, const char *license, char *log, + size_t size_log) { return bpf_prog_load_dev(type, insns, size_insns, license, 0, log, size_log); @@ -1284,7 +1289,7 @@ static int bpf_btf_load(void *btf, size_t size_btf, return bpf(BPF_BTF_LOAD, &attr, sizeof(attr)); } -static int bpf_obj_pin(int fd, const char *pathname) +static int bpf_obj_pin_fd(int fd, const char *pathname) { union bpf_attr attr = {}; @@ -1433,7 +1438,7 @@ static int bpf_probe_pinned(const char *name, const struct bpf_elf_ctx *ctx, return 0; bpf_make_pathname(pathname, sizeof(pathname), name, ctx, pinning); - return bpf_obj_get(pathname, ctx->type); + return bpf_obj_get_path(pathname, ctx->type); } static int bpf_make_obj_path(const struct bpf_elf_ctx *ctx) @@ -1501,7 +1506,7 @@ static int bpf_place_pinned(int fd, const char *name, return ret; bpf_make_pathname(pathname, sizeof(pathname), name, ctx, pinning); - return bpf_obj_pin(fd, pathname); + return bpf_obj_pin_fd(fd, pathname); } static void bpf_prog_report(int fd, const char *section, @@ -1523,9 +1528,9 @@ static void bpf_prog_report(int fd, const char *section, bpf_dump_error(ctx, "Verifier analysis:\n\n"); } -static int bpf_prog_attach(const char *section, - const struct bpf_elf_prog *prog, - struct bpf_elf_ctx *ctx) +static int bpf_prog_attach_section(const char *section, + const struct bpf_elf_prog *prog, + struct bpf_elf_ctx *ctx) { int tries = 0, fd; retry: @@ -2347,7 +2352,7 @@ static int bpf_fetch_prog(struct bpf_elf_ctx *ctx, const char *section, prog.insns_num = prog.size / sizeof(struct bpf_insn); prog.insns = data.sec_data->d_buf; - fd = bpf_prog_attach(section, &prog, ctx); + fd = bpf_prog_attach_section(section, &prog, ctx); if (fd < 0) return fd; @@ -2513,7 +2518,7 @@ static int bpf_fetch_prog_relo(struct bpf_elf_ctx *ctx, const char *section, goto out; } - fd = bpf_prog_attach(section, prog, ctx); + fd = bpf_prog_attach_section(section, prog, ctx); free(prog->insns); if (fd < 0) { *lderr = true;
This adds a configure check for libbpf and renames functions to allow lib/bpf.c to be compiled with it present. This makes it possible to port functionality piecemeal to use libbpf. Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> --- configure | 16 ++++++++++++++++ include/bpf_util.h | 6 +++--- ip/ipvrf.c | 4 ++-- lib/bpf.c | 33 +++++++++++++++++++-------------- 4 files changed, 40 insertions(+), 19 deletions(-)