[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 12 Dec 2023 14:58:01 -0800
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Quentin Deslandes <qde@...cy.de>
Cc: David Ahern <dsahern@...il.com>, Martin KaFai Lau
<martin.lau@...nel.org>, kernel-team@...a.com, netdev@...r.kernel.org
Subject: Re: [PATCH v2 2/2] ss: pretty-print BPF socket-local storage
On 12/8/23 6:57 AM, Quentin Deslandes wrote:
> @@ -1039,11 +1044,10 @@ static int buf_update(int len)
> }
>
> /* Append content to buffer as part of the current field */
> -__attribute__((format(printf, 1, 2)))
> -static void out(const char *fmt, ...)
> +static void vout(const char *fmt, va_list args)
> {
> struct column *f = current_field;
> - va_list args;
> + va_list _args;
> char *pos;
> int len;
>
> @@ -1054,18 +1058,27 @@ static void out(const char *fmt, ...)
> buffer.head = buf_chunk_new();
>
> again: /* Append to buffer: if we have a new chunk, print again */
> + va_copy(_args, args);
>
> pos = buffer.cur->data + buffer.cur->len;
> - va_start(args, fmt);
>
> /* Limit to tail room. If we hit the limit, buf_update() will tell us */
> len = vsnprintf(pos, buf_chunk_avail(buffer.tail), fmt, args);
hmm... based on the changes made in this function, I am pretty sure the
intention is to pass the "_args" here instead of passing the "args". Please
double check.
> - va_end(args);
>
> if (buf_update(len))
> goto again;
> }
>
> +__attribute__((format(printf, 1, 2)))
> +static void out(const char *fmt, ...)
> +{
> + va_list args;
> +
> + va_start(args, fmt);
> + vout(fmt, args);
> + va_end(args);
> +}
> +
> static int print_left_spacing(struct column *f, int stored, int printed)
> {
> int s;
> @@ -1213,6 +1226,9 @@ static void render_calc_width(void)
> */
> c->width = min(c->width, screen_width);
>
> + if (c == &columns[COL_SKSTOR])
> + c->width = 1;
> +
> if (c->width)
> first = 0;
> }
> @@ -3392,6 +3408,8 @@ static struct bpf_map_opts {
> struct bpf_sk_storage_map_info {
> unsigned int id;
> int fd;
> + struct bpf_map_info info;
> + struct btf *btf;
> } maps[MAX_NR_BPF_MAP_ID_OPTS];
> bool show_all;
> struct btf *kernel_btf;
> @@ -3403,6 +3421,22 @@ static void bpf_map_opts_mixed_error(void)
> "ss: --bpf-maps and --bpf-map-id cannot be used together\n");
> }
>
> +static int bpf_maps_opts_load_btf(struct bpf_map_info *info, struct btf **btf)
> +{
> + if (info->btf_value_type_id) {
> + *btf = btf__load_from_kernel_by_id(info->btf_id);
> + if (!*btf) {
> + fprintf(stderr, "ss: failed to load BTF for map ID %u\n",
> + info->id);
> + return -1;
> + }
> + } else {
> + *btf = NULL;
> + }
> +
> + return 0;
> +}
> +
> static int bpf_map_opts_add_all(void)
> {
> unsigned int i;
> @@ -3418,6 +3452,7 @@ static int bpf_map_opts_add_all(void)
> while (1) {
> struct bpf_map_info info = {};
> uint32_t len = sizeof(info);
> + struct btf *btf;
>
> r = bpf_map_get_next_id(id, &id);
> if (r) {
> @@ -3462,8 +3497,18 @@ static int bpf_map_opts_add_all(void)
> continue;
> }
>
> + r = bpf_maps_opts_load_btf(&info, &btf);
> + if (r) {
> + fprintf(stderr, "ss: failed to get BTF data for BPF map ID: %u\n",
> + id);
> + close(fd);
> + goto err;
> + }
> +
> bpf_map_opts.maps[bpf_map_opts.nr_maps].id = id;
> - bpf_map_opts.maps[bpf_map_opts.nr_maps++].fd = fd;
> + bpf_map_opts.maps[bpf_map_opts.nr_maps].fd = fd;
> + bpf_map_opts.maps[bpf_map_opts.nr_maps].info = info;
> + bpf_map_opts.maps[bpf_map_opts.nr_maps++].btf = btf;
The "err:" path of this function needs a btf__free().
> }
>
> bpf_map_opts.show_all = true;
> @@ -3482,6 +3527,7 @@ static int bpf_map_opts_add_id(const char *optarg)
> struct bpf_map_info info = {};
> uint32_t len = sizeof(info);
> size_t optarg_len;
> + struct btf *btf;
> unsigned long id;
> unsigned int i;
> char *end;
> @@ -3539,8 +3585,17 @@ static int bpf_map_opts_add_id(const char *optarg)
> return -1;
> }
>
> + r = bpf_maps_opts_load_btf(&info, &btf);
> + if (r) {
> + fprintf(stderr, "ss: failed to get BTF data for BPF map ID: %lu\n",
> + id);
close(fd);
> + return -1;
> + }
> +
> bpf_map_opts.maps[bpf_map_opts.nr_maps].id = id;
> - bpf_map_opts.maps[bpf_map_opts.nr_maps++].fd = fd;
> + bpf_map_opts.maps[bpf_map_opts.nr_maps].fd = fd;
> + bpf_map_opts.maps[bpf_map_opts.nr_maps].info = info;
> + bpf_map_opts.maps[bpf_map_opts.nr_maps++].btf = btf;
>
> return 0;
> }
Powered by blists - more mailing lists