public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Stanislav Fomichev <sdf@fomichev.me>
To: Quentin Monnet <quentin.monnet@netronome.com>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>,
	Stanislav Fomichev <sdf@google.com>,
	netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
	Jiong Wang <jiong.wang@netronome.com>
Subject: Re: [PATCH bpf-next v2] bpftool: make libbfd optional
Date: Tue, 13 Nov 2018 07:45:31 -0800	[thread overview]
Message-ID: <20181113154531.dzdylro75jfdvqs3@mini-arch> (raw)
In-Reply-To: <c34fe18a-5f23-7b7d-7062-54bf9f7ad125@netronome.com>

On 11/13, Quentin Monnet wrote:
> 2018-11-12 14:02 UTC-0800 ~ Jakub Kicinski <jakub.kicinski@netronome.com>
> > On Mon, 12 Nov 2018 13:44:10 -0800, Stanislav Fomichev wrote:
> >> Make it possible to build bpftool without libbfd. libbfd and libopcodes are
> >> typically provided in dev/dbg packages (binutils-dev in debian) which we
> >> usually don't have installed on the fleet machines and we'd like a way to have
> >> bpftool version that works without installing any additional packages.
> >> This excludes support for disassembling jit-ted code and prints an error if
> >> the user tries to use these features.
> >>
> >> Tested by:
> >> cat > FEATURES_DUMP.bpftool <<EOF
> >> feature-libbfd=0
> >> feature-disassembler-four-args=1
> >> feature-reallocarray=0
> >> feature-libelf=1
> >> feature-libelf-mmap=1
> >> feature-bpf=1
> >> EOF
> >> FEATURES_DUMP=$PWD/FEATURES_DUMP.bpftool make
> >> ldd bpftool | grep libbfd
> >>
> >> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > 
> > Seems reasonable, thanks!
> > 
> > Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > 
> 
> Thanks Stanislav!
> 
> There is a problem with this patch on some distributions, Ubuntu at least.
> 
> Feature detection for libbfd has been used for perf before being also
> used with bpftool. Since commit 280e7c48c3b8 the feature needs libz and
> libiberty to be present on the system, otherwise the feature would not
> compile (and be detected) on OpenSuse.
> 
> On Ubuntu, libiberty is not needed (libbfd might be statically linked
> against it, if I remember correctly?), which means that we are able to
> build bpftool as long as binutils-dev has been installed, even if
> libiberty-dev has not been installed. The BFD feature, in that case,
> will appear as “undetected”. It is a bug. But since the Makefile does
> not stop compilation in that case (another bug), in the end we're good.
> 
> With your patch, the problem is that libbpf detection will fail on
> Ubuntu if libiberty-dev is not present, even though all the necessary
> libraries for using the JIT disassembler are available. And in that case
> it _will_ make a difference, since the Makefile will no more compile the
> libbfd-related bits.
> 
> So I'm not against the idea, but we have to fix libbfd detection first.
Yeah, libbfd feature detection looks broken on ubuntu, this patch just
exercises this brokenness :-). I can take a look somewhere this week,
thanks for spotting it!

I wonder, how does it work on opensuse currently if we link only against -lbfd
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/bpf/bpftool/Makefile#n56)?
It might be broken everywhere...

(Resent with proper formatting and without HTML).
> Thanks,
> Quentin

  reply	other threads:[~2018-11-14  1:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-12 21:44 [PATCH bpf-next v2] bpftool: make libbfd optional Stanislav Fomichev
2018-11-12 22:02 ` Jakub Kicinski
2018-11-13  6:03   ` Quentin Monnet
2018-11-13 15:45     ` Stanislav Fomichev [this message]
2018-11-16  0:42     ` Stanislav Fomichev
2018-11-17  4:47 ` Alexei Starovoitov
     [not found]   ` <CAKH8qBtbrNCNJkjqNH3vm_s_+x6vHbp8MFWSLgROMCF5A=xtjA@mail.gmail.com>
2018-11-17  4:57     ` Alexei Starovoitov
2018-11-17  5:06       ` Stanislav Fomichev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181113154531.dzdylro75jfdvqs3@mini-arch \
    --to=sdf@fomichev.me \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jakub.kicinski@netronome.com \
    --cc=jiong.wang@netronome.com \
    --cc=netdev@vger.kernel.org \
    --cc=quentin.monnet@netronome.com \
    --cc=sdf@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox