| Message ID | 20230421101154.23690-1-kuro@kuroa.me (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Dump map id instead of value for map_of_maps types | expand |
On Fri, 21 Apr 2023 at 11:12, Xueming Feng <kuro@kuroa.me> 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 disruption scripted behavior. Since user s/disruption/disrupting/ ? > could dump the inner map with id, but need to convert value. > > Signed-off-by: Xueming Feng <kuro@kuroa.me> Thanks for this patch! It looks good, with some minor comments below. > --- > tools/bpf/bpftool/main.c | 16 ++++++++++++++++ > tools/bpf/bpftool/main.h | 1 + > tools/bpf/bpftool/map.c | 9 +++++++-- > 3 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c > index 08d0ac543c67..d297200c91f7 100644 > --- a/tools/bpf/bpftool/main.c > +++ b/tools/bpf/bpftool/main.c > @@ -251,6 +251,22 @@ int detect_common_prefix(const char *arg, ...) > return 0; > } > > +void fprint_uint(FILE *f, void *arg, unsigned int n) I suppose you based this function on fprint_hex(). But for your function, let's remove the first argument? We always print to stdout. We can always turn it to a "fprint" version again if we need to print elsewhere in the future. Also, "arg" should probably be a "const"? Can you please rename "n" to "arg_size" or something similar? > +{ > + unsigned char *data = arg; > + unsigned int data_uint = 0; > + > + for (unsigned int i = 0; i < n && i < 4; i++) { Please move the declaration for "i" to the top of the function, for consistency. Also, why stop at i == 4? Couldn't this function be used for 8-byte long integers too? It should be up to the caller function to set "n" correctly. > + #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ > + data_uint |= data[i] << (i * 8); > + #else > + data_uint |= data[i] << ((n - i - 1) * 8); > + #endif > + } > + > + fprintf(f, "%d", data_uint); > +} > + > 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..7488ef38e7a9 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 fprint_uint(FILE *f, void *arg, unsigned int n); > 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..638bd8de8135 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' : ' '); > + fprint_uint(stdout, value, info->value_size); > + } else { > + printf("value:%c", break_names ? '\n' : ' '); > + fprint_hex(stdout, value, info->value_size, " "); > + } > } > > printf("\n"); > -- > 2.37.1 (Apple Git-137.1) >
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c index 08d0ac543c67..d297200c91f7 100644 --- a/tools/bpf/bpftool/main.c +++ b/tools/bpf/bpftool/main.c @@ -251,6 +251,22 @@ int detect_common_prefix(const char *arg, ...) return 0; } +void fprint_uint(FILE *f, void *arg, unsigned int n) +{ + unsigned char *data = arg; + unsigned int data_uint = 0; + + for (unsigned int i = 0; i < n && i < 4; i++) { + #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ + data_uint |= data[i] << (i * 8); + #else + data_uint |= data[i] << ((n - i - 1) * 8); + #endif + } + + fprintf(f, "%d", data_uint); +} + 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..7488ef38e7a9 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 fprint_uint(FILE *f, void *arg, unsigned int n); 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..638bd8de8135 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' : ' '); + fprint_uint(stdout, value, info->value_size); + } else { + printf("value:%c", break_names ? '\n' : ' '); + fprint_hex(stdout, value, info->value_size, " "); + } } printf("\n");
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 disruption scripted behavior. Since user could dump the inner map with id, but need to convert value. Signed-off-by: Xueming Feng <kuro@kuroa.me> --- tools/bpf/bpftool/main.c | 16 ++++++++++++++++ tools/bpf/bpftool/main.h | 1 + tools/bpf/bpftool/map.c | 9 +++++++-- 3 files changed, 24 insertions(+), 2 deletions(-)