Re: [PATCH bpf-next 6/7] tools: bpftool: document and add bash completion for -L, -B options

[Quentin Monnet <quentin@isovalent.com>]

2021-07-30 11:59 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>
>> The -L|--use-loader option for using loader programs when loading, or
>> when generating a skeleton, did not have any documentation or bash
>> completion. Same thing goes for -B|--base-btf, used to pass a path to a
>> base BTF object for split BTF such as BTF for kernel modules.
>>
>> This patch documents and adds bash completion for those options.
>>
>> Fixes: 75fa1777694c ("tools/bpftool: Add bpftool support for split BTF")
>> Fixes: d510296d331a ("bpftool: Use syscall/loader program in "prog load" and "gen skeleton" command.")
>> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
>> ---
>> Note: The second example with base BTF in the BTF man page assumes that
>> dumping split BTF when objects are passed by id is supported. Support is
>> currently pending review in another PR.
>> ---
> 
> Not anymore :)
> 
> [...]
> 
>> @@ -73,6 +74,20 @@ OPTIONS
>>  =======
>>         .. include:: common_options.rst
>>
>> +       -B, --base-btf *FILE*
>> +                 Pass a base BTF object. Base BTF objects are typically used
>> +                 with BTF objects for kernel modules. To avoid duplicating
>> +                 all kernel symbols required by modules, BTF objects for
>> +                 modules are "split", they are built incrementally on top of
>> +                 the kernel (vmlinux) BTF object. So the base BTF reference
>> +                 should usually point to the kernel BTF.
>> +
>> +                 When the main BTF object to process (for example, the
>> +                 module BTF to dump) is passed as a *FILE*, bpftool attempts
>> +                 to autodetect the path for the base object, and passing
>> +                 this option is optional. When the main BTF object is passed
>> +                 through other handles, this option becomes necessary.
>> +
>>  EXAMPLES
>>  ========
>>  **# bpftool btf dump id 1226**
>> @@ -217,3 +232,34 @@ All the standard ways to specify map or program are supported:
>>  **# bpftool btf dump prog tag b88e0a09b1d9759d**
>>
>>  **# bpftool btf dump prog pinned /sys/fs/bpf/prog_name**
>> +
>> +|
>> +| **# bpftool btf dump file /sys/kernel/btf/i2c_smbus**
>> +| (or)
>> +| **# I2C_SMBUS_ID=$(bpftool btf show -p | jq '.[] | select(.name=="i2c_smbus").id')**
>> +| **# bpftool btf dump id ${I2C_SMBUS_ID} -B /sys/kernel/btf/vmlinux**
>> +
>> +::
>> +
>> +  [104848] STRUCT 'i2c_smbus_alert' size=40 vlen=2
>> +          'alert' type_id=393 bits_offset=0
>> +          'ara' type_id=56050 bits_offset=256
>> +  [104849] STRUCT 'alert_data' size=12 vlen=3
>> +          'addr' type_id=16 bits_offset=0
>> +          'type' type_id=56053 bits_offset=32
>> +          'data' type_id=7 bits_offset=64
>> +  [104850] PTR '(anon)' type_id=104848
>> +  [104851] PTR '(anon)' type_id=104849
>> +  [104852] FUNC 'i2c_register_spd' type_id=84745 linkage=static
>> +  [104853] FUNC 'smbalert_driver_init' type_id=1213 linkage=static
>> +  [104854] FUNC_PROTO '(anon)' ret_type_id=18 vlen=1
>> +          'ara' type_id=56050
>> +  [104855] FUNC 'i2c_handle_smbus_alert' type_id=104854 linkage=static
>> +  [104856] FUNC 'smbalert_remove' type_id=104854 linkage=static
>> +  [104857] FUNC_PROTO '(anon)' ret_type_id=18 vlen=2
>> +          'ara' type_id=56050
>> +          'id' type_id=56056
>> +  [104858] FUNC 'smbalert_probe' type_id=104857 linkage=static
>> +  [104859] FUNC 'smbalert_work' type_id=9695 linkage=static
>> +  [104860] FUNC 'smbus_alert' type_id=71367 linkage=static
>> +  [104861] FUNC 'smbus_do_alert' type_id=84827 linkage=static
> 
> This reminded be that it would be awesome to support "format c"
> use-case for dumping split BTF in a more sane way. I.e., instead of
> dumping all types from base and split BTF, only dump necessary (used)
> forward declarations from base BTF, and then full C dump of only new
> types from the split (module) BTF. This will become more important as
> people will start using module BTF more. It's an interesting add-on to
> libbpf's btf_dumper functionality. Not sure how hard that would be,
> but I'd imagine it shouldn't require much changes.
> 
> Just in case anyone wanted to challenge themselves with some more
> algorithmic patch for libbpf (*wink wink*)...

If you're addressing this to me, I'm not particularly looking for such
challenge at the moment :). In fact I already noted a few things that I
would like to fix or improve for bpftool, I will append this one to the
list. I should maybe start thinking of a tracker of some sort to list
and share this.