> On 4/24/23 2:09 AM, Xueming Feng wrote:
>> When using `bpftool map dump` in plain format, it is usually
>> more convenient to show the inner map id instead of raw value.
>> Changing this behavior would help with quick debugging with
>> `bpftool`, without disrupting scripted behavior. Since user
>> could dump the inner map with id, and need to convert value.
>>
>> Signed-off-by: Xueming Feng <kuro@xxxxxxxx>
>> ---
>> Changes in v2:
>> - Fix commit message grammar.
>> - Change `print_uint` to only print to stdout, make `arg` const, and rename
>> `n` to `arg_size`.
>> - Make `print_uint` able to take any size of argument up to `unsigned long`,
>> and print it as unsigned decimal.
>>
>> Thanks for the review and suggestions! I have changed my patch accordingly.
>> There is a possibility that `arg_size` is larger than `unsigned long`,
>> but previous review suggested that it should be up to the caller function to
>> set `arg_size` correctly. So I didn't add check for that, should I?
>>
>> tools/bpf/bpftool/main.c | 15 +++++++++++++++
>> tools/bpf/bpftool/main.h | 1 +
>> tools/bpf/bpftool/map.c | 9 +++++++--
>> 3 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
>> index 08d0ac543c67..810c0dc10ecb 100644
>> --- a/tools/bpf/bpftool/main.c
>> +++ b/tools/bpf/bpftool/main.c
>> @@ -251,6 +251,21 @@ int detect_common_prefix(const char *arg, ...)
>> return 0;
>> }
>>
>> +void print_uint(const void *arg, unsigned int arg_size)
>> +{
>> + const unsigned char *data = arg;
>> + unsigned long val = 0ul;
>> +
>> + #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
>> + memcpy(&val, data, arg_size);
>> + #else
>> + memcpy((unsigned char *)&val + sizeof(val) - arg_size,
>> + data, arg_size);
>> + #endif
>> +
>> + fprintf(stdout, "%lu", val);
>> +}
>> +
>> void fprint_hex(FILE *f, void *arg, unsigned int n, const char *sep)
>> {
>> unsigned char *data = arg;
>> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
>> index 0ef373cef4c7..0de671423431 100644
>> --- a/tools/bpf/bpftool/main.h
>> +++ b/tools/bpf/bpftool/main.h
>> @@ -90,6 +90,7 @@ void __printf(1, 2) p_info(const char *fmt, ...);
>>
>> bool is_prefix(const char *pfx, const char *str);
>> int detect_common_prefix(const char *arg, ...);
>> +void print_uint(const void *arg, unsigned int arg_size);
>> void fprint_hex(FILE *f, void *arg, unsigned int n, const char *sep);
>> void usage(void) __noreturn;
>>
>> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
>> index aaeb8939e137..f5be4c0564cf 100644
>> --- a/tools/bpf/bpftool/map.c
>> +++ b/tools/bpf/bpftool/map.c
>> @@ -259,8 +259,13 @@ static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
>> }
>>
>> if (info->value_size) {
>> - printf("value:%c", break_names ? '\n' : ' ');
>> - fprint_hex(stdout, value, info->value_size, " ");
>> + if (map_is_map_of_maps(info->type)) {
>> + printf("id:%c", break_names ? '\n' : ' ');
>1> + print_uint(value, info->value_size);
On Mon, 24 Apr 2023 18:07:27 -0700, Yonghong Song wrote:
> For all map_in_map types, the inner map value size is 32bit int which
> represents a fd (for map creation) and a id (for map info), e.g., in
> show_prog_maps() in prog.c. So maybe we can simplify the code as below:
> printf("id: %u", *(unsigned int *)value);
That is true, maybe the "id" could also be changed to "map_id" to follow the
convention. Do you think that `print_uint` could be useful in the future?
If that is the case, should I keep using it here as an example usage, and to
avoid dead code? Or should I just remove it?
>> + } else {
>> + printf("value:%c", break_names ? '\n' : ' ');
>> + fprint_hex(stdout, value, info->value_size, " ");
>> + }
>> }
>>
>> printf("\n");
