| Message ID | 20201029151146.3810859-2-haliu@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | iproute2: add libbpf support | expand |
Hangbin Liu <haliu@redhat.com> writes: > This patch adds a check to see if we support libbpf. By default the > system libbpf will be used, but static linking against a custom libbpf > version can be achieved by passing LIBBPF_DIR to configure. FORCE_LIBBPF > can be set to force configure to abort if no suitable libbpf is found, > which is useful for automatic packaging that wants to enforce the > dependency. > > Signed-off-by: Hangbin Liu <haliu@redhat.com> With one nit below, feel free to add back my: Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com> > --- > v3: > Check function bpf_program__section_name() separately and only use it > on higher libbpf version. > > v2: > No update > --- > configure | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 94 insertions(+) > > diff --git a/configure b/configure > index 307912aa..58a7176e 100755 > --- a/configure > +++ b/configure > @@ -240,6 +240,97 @@ check_elf() > fi > } > > +have_libbpf_basic() > +{ > + cat >$TMPDIR/libbpf_test.c <<EOF > +#include <bpf/libbpf.h> > +int main(int argc, char **argv) { > + bpf_program__set_autoload(NULL, false); > + bpf_map__ifindex(NULL); > + bpf_map__set_pin_path(NULL, NULL); > + bpf_object__open_file(NULL, NULL); > + return 0; > +} > +EOF > + > + $CC -o $TMPDIR/libbpf_test $TMPDIR/libbpf_test.c $LIBBPF_CFLAGS $LIBBPF_LDLIBS >/dev/null 2>&1 > + local ret=$? > + > + rm -f $TMPDIR/libbpf_test.c $TMPDIR/libbpf_test > + return $ret > +} > + > +have_libbpf_sec_name() > +{ > + cat >$TMPDIR/libbpf_sec_test.c <<EOF > +#include <bpf/libbpf.h> > +int main(int argc, char **argv) { > + void *ptr; > + bpf_program__section_name(NULL); > + return 0; > +} > +EOF > + > + $CC -o $TMPDIR/libbpf_sec_test $TMPDIR/libbpf_sec_test.c $LIBBPF_CFLAGS $LIBBPF_LDLIBS >/dev/null 2>&1 > + local ret=$? > + > + rm -f $TMPDIR/libbpf_sec_test.c $TMPDIR/libbpf_sec_test > + return $ret > +} > + > +check_force_libbpf() > +{ > + # if set FORCE_LIBBPF but no libbpf support, just exist the config > + # process to make sure we don't build without libbpf. > + if [ -n "$FORCE_LIBBPF" ]; then > + echo "FORCE_LIBBPF set, but couldn't find a usable libbpf" > + exit 1 > + fi > +} > + > +check_libbpf() > +{ > + if ! ${PKG_CONFIG} libbpf --exists && [ -z "$LIBBPF_DIR" ] ; then > + echo "no" > + check_force_libbpf > + return > + fi > + > + if [ $(uname -m) == x86_64 ]; then > + local LIBSUBDIR=lib64 > + else > + local LIBSUBDIR=lib > + fi > + > + if [ -n "$LIBBPF_DIR" ]; then > + LIBBPF_CFLAGS="-I${LIBBPF_DIR}/include -L${LIBBPF_DIR}/${LIBSUBDIR}" > + LIBBPF_LDLIBS="${LIBBPF_DIR}/${LIBSUBDIR}/libbpf.a -lz -lelf" > + else > + LIBBPF_CFLAGS=$(${PKG_CONFIG} libbpf --cflags) > + LIBBPF_LDLIBS=$(${PKG_CONFIG} libbpf --libs) > + fi > + > + if ! have_libbpf_basic; then > + echo "no" > + echo " libbpf version is too low, please update it to at least 0.1.0" > + check_force_libbpf > + return > + else > + echo "HAVE_LIBBPF:=y" >>$CONFIG > + echo 'CFLAGS += -DHAVE_LIBBPF ' $LIBBPF_CFLAGS >> $CONFIG > + echo 'LDLIBS += ' $LIBBPF_LDLIBS >>$CONFIG > + fi > + > + # bpf_program__title() is deprecated since libbpf 0.2.0, use > + # bpf_program__section_name() instead if we support > + if have_libbpf_sec_name; then > + echo "HAVE_LIBBPF_SECTION_NAME:=y" >>$CONFIG > + echo 'CFLAGS += -DHAVE_LIBBPF_SECTION_NAME ' $LIBBPF_CFLAGS >> $CONFIG You already added $LIBBPF_CFLAGS above, so with this it ends up being duplicated, doesn't it? -Toke
On 10/29/20 9:11 AM, Hangbin Liu wrote: > This patch adds a check to see if we support libbpf. By default the > system libbpf will be used, but static linking against a custom libbpf > version can be achieved by passing LIBBPF_DIR to configure. FORCE_LIBBPF > can be set to force configure to abort if no suitable libbpf is found, > which is useful for automatic packaging that wants to enforce the > dependency. > Add an option to force libbpf off and use of the legacy code. i.e, yes it is installed, but don't use it. configure script really needs a usage to dump options like disabling libbpf.
On Mon, Nov 02, 2020 at 08:37:37AM -0700, David Ahern wrote: > On 10/29/20 9:11 AM, Hangbin Liu wrote: > > This patch adds a check to see if we support libbpf. By default the > > system libbpf will be used, but static linking against a custom libbpf > > version can be achieved by passing LIBBPF_DIR to configure. FORCE_LIBBPF > > can be set to force configure to abort if no suitable libbpf is found, > > which is useful for automatic packaging that wants to enforce the > > dependency. > > > > Add an option to force libbpf off and use of the legacy code. i.e, yes > it is installed, but don't use it. > > configure script really needs a usage to dump options like disabling libbpf. > Shouldn't we use libbpf by default if system support? The same like libmnl. There is no options to force libnml off. Thanks Hangbin
On 11/2/20 10:54 PM, Hangbin Liu wrote: > On Mon, Nov 02, 2020 at 08:37:37AM -0700, David Ahern wrote: >> On 10/29/20 9:11 AM, Hangbin Liu wrote: >>> This patch adds a check to see if we support libbpf. By default the >>> system libbpf will be used, but static linking against a custom libbpf >>> version can be achieved by passing LIBBPF_DIR to configure. FORCE_LIBBPF >>> can be set to force configure to abort if no suitable libbpf is found, >>> which is useful for automatic packaging that wants to enforce the >>> dependency. >>> >> >> Add an option to force libbpf off and use of the legacy code. i.e, yes >> it is installed, but don't use it. >> >> configure script really needs a usage to dump options like disabling libbpf. >> > > Shouldn't we use libbpf by default if system support? The same like libmnl. > There is no options to force libnml off. > configure scripts usually allow you to control options directly, overriding the autoprobe.
On Tue, Nov 03, 2020 at 10:32:37AM -0700, David Ahern wrote: > configure scripts usually allow you to control options directly, > overriding the autoprobe. What do you think of the follow update? It's a little rough and only controls libbpf. $ git diff diff --git a/configure b/configure index 711bb69c..be35c024 100755 --- a/configure +++ b/configure @@ -442,6 +442,35 @@ endif EOF } +usage() +{ + cat <<EOF +Usage: $0 [OPTIONS] + -h | --help Show this usage info + --no-libbpf build the package without libbpf + --libbpf-dir=DIR build the package with self defined libbpf dir +EOF + exit $1 +} + +while true; do + case "$1" in + --libbpf-dir) + LIBBPF_DIR="$2" + shift 2 ;; + --no-libbpf) + NO_LIBBPF_CHECK=1 + shift ;; + -h | --help) + usage 0 ;; + "") + break ;; + *) + usage 1 ;; + esac +done + + echo "# Generated config based on" $INCLUDE >$CONFIG quiet_config >> $CONFIG @@ -476,8 +505,10 @@ check_setns echo -n "SELinux support: " check_selinux -echo -n "libbpf support: " -check_libbpf +if [ -z $NO_LIBBPF_CHECK ]; then + echo -n "libbpf support: " + check_libbpf +fi echo -n "ELF support: " check_elf $ ./configure -h Usage: ./configure [OPTIONS] -h | --help Show this usage info --no-libbpf build the package without libbpf --libbpf-dir=DIR build the package with self defined libbpf dir Thanks Hangbin
Hangbin Liu <haliu@redhat.com> writes: > On Tue, Nov 03, 2020 at 10:32:37AM -0700, David Ahern wrote: >> configure scripts usually allow you to control options directly, >> overriding the autoprobe. > > What do you think of the follow update? It's a little rough and only controls > libbpf. > > $ git diff > diff --git a/configure b/configure > index 711bb69c..be35c024 100755 > --- a/configure > +++ b/configure > @@ -442,6 +442,35 @@ endif > EOF > } > > +usage() > +{ > + cat <<EOF > +Usage: $0 [OPTIONS] > + -h | --help Show this usage info > + --no-libbpf build the package without libbpf > + --libbpf-dir=DIR build the package with self defined libbpf dir > +EOF > + exit $1 > +} This would be the only command line arg that configure takes; all other options are passed via the environment. I think we should be consistent here; and since converting the whole configure script is probably out of scope for this patch, why not just use the existing FORCE_LIBBPF variable? I.e., FORCE_LIBBPF=on will fail if not libbpf is present, FORCE_LIBBPF=off will disable libbpf entirely, and if the variable is unset, libbpf will be used if found? Alternatively, keep them as two separate variables (FORCE_LIBBPF and DISABLE_LIBBPF?). I don't have any strong preference as to which of those is best, but I think they'd both be more consistent with the existing configure script logic... -Toke
On Wed, Nov 04, 2020 at 12:09:15PM +0100, Toke Høiland-Jørgensen wrote: > > +usage() > > +{ > > + cat <<EOF > > +Usage: $0 [OPTIONS] > > + -h | --help Show this usage info > > + --no-libbpf build the package without libbpf > > + --libbpf-dir=DIR build the package with self defined libbpf dir > > +EOF > > + exit $1 > > +} > > This would be the only command line arg that configure takes; all other > options are passed via the environment. I think we should be consistent > here; and since converting the whole configure script is probably out of > scope for this patch, why not just use the existing FORCE_LIBBPF > variable? Yes, converting the whole configure script should be split as another patch work. > > I.e., FORCE_LIBBPF=on will fail if not libbpf is present, > FORCE_LIBBPF=off will disable libbpf entirely, and if the variable is > unset, libbpf will be used if found? I like this one, with only one variable. I will check how to re-organize the script. > > Alternatively, keep them as two separate variables (FORCE_LIBBPF and > DISABLE_LIBBPF?). I don't have any strong preference as to which of > those is best, but I think they'd both be more consistent with the > existing configure script logic... Please tell me if others have any other ideas. Thanks Hnagbin
diff --git a/configure b/configure index 307912aa..58a7176e 100755 --- a/configure +++ b/configure @@ -240,6 +240,97 @@ check_elf() fi } +have_libbpf_basic() +{ + cat >$TMPDIR/libbpf_test.c <<EOF +#include <bpf/libbpf.h> +int main(int argc, char **argv) { + bpf_program__set_autoload(NULL, false); + bpf_map__ifindex(NULL); + bpf_map__set_pin_path(NULL, NULL); + bpf_object__open_file(NULL, NULL); + return 0; +} +EOF + + $CC -o $TMPDIR/libbpf_test $TMPDIR/libbpf_test.c $LIBBPF_CFLAGS $LIBBPF_LDLIBS >/dev/null 2>&1 + local ret=$? + + rm -f $TMPDIR/libbpf_test.c $TMPDIR/libbpf_test + return $ret +} + +have_libbpf_sec_name() +{ + cat >$TMPDIR/libbpf_sec_test.c <<EOF +#include <bpf/libbpf.h> +int main(int argc, char **argv) { + void *ptr; + bpf_program__section_name(NULL); + return 0; +} +EOF + + $CC -o $TMPDIR/libbpf_sec_test $TMPDIR/libbpf_sec_test.c $LIBBPF_CFLAGS $LIBBPF_LDLIBS >/dev/null 2>&1 + local ret=$? + + rm -f $TMPDIR/libbpf_sec_test.c $TMPDIR/libbpf_sec_test + return $ret +} + +check_force_libbpf() +{ + # if set FORCE_LIBBPF but no libbpf support, just exist the config + # process to make sure we don't build without libbpf. + if [ -n "$FORCE_LIBBPF" ]; then + echo "FORCE_LIBBPF set, but couldn't find a usable libbpf" + exit 1 + fi +} + +check_libbpf() +{ + if ! ${PKG_CONFIG} libbpf --exists && [ -z "$LIBBPF_DIR" ] ; then + echo "no" + check_force_libbpf + return + fi + + if [ $(uname -m) == x86_64 ]; then + local LIBSUBDIR=lib64 + else + local LIBSUBDIR=lib + fi + + if [ -n "$LIBBPF_DIR" ]; then + LIBBPF_CFLAGS="-I${LIBBPF_DIR}/include -L${LIBBPF_DIR}/${LIBSUBDIR}" + LIBBPF_LDLIBS="${LIBBPF_DIR}/${LIBSUBDIR}/libbpf.a -lz -lelf" + else + LIBBPF_CFLAGS=$(${PKG_CONFIG} libbpf --cflags) + LIBBPF_LDLIBS=$(${PKG_CONFIG} libbpf --libs) + fi + + if ! have_libbpf_basic; then + echo "no" + echo " libbpf version is too low, please update it to at least 0.1.0" + check_force_libbpf + return + else + echo "HAVE_LIBBPF:=y" >>$CONFIG + echo 'CFLAGS += -DHAVE_LIBBPF ' $LIBBPF_CFLAGS >> $CONFIG + echo 'LDLIBS += ' $LIBBPF_LDLIBS >>$CONFIG + fi + + # bpf_program__title() is deprecated since libbpf 0.2.0, use + # bpf_program__section_name() instead if we support + if have_libbpf_sec_name; then + echo "HAVE_LIBBPF_SECTION_NAME:=y" >>$CONFIG + echo 'CFLAGS += -DHAVE_LIBBPF_SECTION_NAME ' $LIBBPF_CFLAGS >> $CONFIG + fi + + echo "yes" +} + check_selinux() # SELinux is a compile time option in the ss utility { @@ -385,6 +476,9 @@ check_setns echo -n "SELinux support: " check_selinux +echo -n "libbpf support: " +check_libbpf + echo -n "ELF support: " check_elf
This patch adds a check to see if we support libbpf. By default the system libbpf will be used, but static linking against a custom libbpf version can be achieved by passing LIBBPF_DIR to configure. FORCE_LIBBPF can be set to force configure to abort if no suitable libbpf is found, which is useful for automatic packaging that wants to enforce the dependency. Signed-off-by: Hangbin Liu <haliu@redhat.com> --- v3: Check function bpf_program__section_name() separately and only use it on higher libbpf version. v2: No update --- configure | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+)