Re: [PATCH v2 2/2] ss: pretty-print BPF socket-local storage

[Martin KaFai Lau <martin.lau@linux.dev>]

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;
>   }