Re: [PATCH bpf-next v2 6/8] bpftool: Add LLVM as default library for disassembling JIT-ed programs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Yonghong,

Tue Sep 20 2022 05:08:04 GMT+0100 (British Summer Time) ~ Yonghong Song
<yhs@xxxxxx>
> 
> 
> On 9/16/22 2:09 PM, Daniel Borkmann wrote:
>> On 9/11/22 10:14 PM, Quentin Monnet wrote:
>>> To disassemble instructions for JIT-ed programs, bpftool has relied on
>>> the libbfd library. This has been problematic in the past: libbfd's
>>> interface is not meant to be stable and has changed several times. For
>>> building bpftool, we have to detect how the libbfd version on the system
>>> behaves, which is why we have to handle features disassembler-four-args
>>> and disassembler-init-styled in the Makefile. When it comes to shipping
>>> bpftool, this has also caused issues with several distribution
>>> maintainers unwilling to support the feature (see for example Debian's
>>> page for binutils-dev, which ships libbfd: "Note that building Debian
>>> packages which depend on the shared libbfd is Not Allowed." [0]).
>>>
>>> For these reasons, we add support for LLVM as an alternative to libbfd
>>> for disassembling instructions of JIT-ed programs. Thanks to the
>>> preparation work in the previous commits, it's easy to add the library
>>> by passing the relevant compilation options in the Makefile, and by
>>> adding the functions for setting up the LLVM disassembler in file
>>> jit_disasm.c.
>>
>> Could you add more context around the LLVM lib? The motivation is that
>> libbfd's
>> interface is not meant to be stable and has changed several times. How
>> does this
>> look on the LLVM's library side? Also, for the 2nd part, what is
>> Debian's stance
>> related to the LLVM lib? Would be good if both is explained in the
>> commit message.
>> Right now it mainly reads 'that libbfd has all these issues, so we're
>> moving to
>> something else', so would be good to provide more context to the ready
>> why the
>> 'something else' is better than current one.
> 
> It will be good to mention that e.g., llvm development package
> (e.g., llvm-devel for fedora) is needed for bpftool build with llvm.

Right. I didn't mention it because I noticed that on my distro, llvm-dev
came as a dependency for llvm (so simply having llvm installed is enough
to ensure the development package is present too). But this may not be
the case everywhere (do you know for Fedora?), so I'll add it to the
commit log for the next version.

Thanks for the review!
Quentin




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux