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

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

On 1/12/24 6:04 AM, Quentin Deslandes wrote:
> @@ -3445,8 +3478,16 @@ static int bpf_map_opts_load_info(unsigned int map_id)
>   		return -1;
>   	}
>   
> +	r = bpf_maps_opts_load_btf(&info, &btf);
> +	if (r) {
> +		close(fd);
> +		return -1;
> +	}
> +
>   	bpf_map_opts.maps[bpf_map_opts.nr_maps].id = map_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;
>   }
> @@ -3469,6 +3510,29 @@ static struct bpf_sk_storage_map_info *bpf_map_opts_get_info(
>   	return &bpf_map_opts.maps[bpf_map_opts.nr_maps - 1];
>   }
>   
> +static void out_bpf_sk_storage_print_fn(void *ctx, const char *fmt, va_list args)
> +{
> +	vout(fmt, args);
> +}
> +
> +static struct btf_dump *bpf_map_opts_get_btf_dump(
> +	struct bpf_sk_storage_map_info *map_info)
> +{
> +	struct btf_dump_opts dopts = {
> +		.sz = sizeof(struct btf_dump_opts)
> +	};
> +
> +	if (!map_info->dump) {
> +		map_info->dump = btf_dump__new(map_info->btf,
> +					       out_bpf_sk_storage_print_fn,
> +					       NULL, &dopts);

A nit/simplification for the consideration. May be initialize the map_info->dump 
in the bpf_map_opts_load_info() also together with other map_info->* 
initialization? It is likely map_info->dump will be needed anyway.

Acked-by: Martin KaFai Lau <martin.lau@kernel.org>


> +		if (!map_info->dump)
> +			fprintf(stderr, "Failed to create btf_dump object\n");
> +	}
> +
> +	return map_info->dump;
> +}
> +

[ ... ]

> +static void out_bpf_sk_storage(int map_id, const void *data, size_t len)
> +{
> +	uint32_t type_id;
> +	struct bpf_sk_storage_map_info *map_info;
> +	struct btf_dump *dump;
> +	struct btf_dump_type_data_opts opts = {
> +		.sz = sizeof(struct btf_dump_type_data_opts),
> +		.indent_str = SK_STORAGE_INDENT_STR,
> +		.indent_level = 2,
> +		.emit_zeroes = 1
> +	};
> +	int r;
> +
> +	map_info = bpf_map_opts_get_info(map_id);
> +	if (!map_info) {
> +		/* The kernel might return a map we can't get info for, skip
> +		 * it but print the other ones. */
> +		out(SK_STORAGE_INDENT_STR "map_id: %d failed to fetch info, skipping\n",
> +		    map_id);
> +		return;
> +	}
> +
> +	if (map_info->info.value_size != len) {
> +		fprintf(stderr, "map_id: %d: invalid value size, expecting %u, got %lu\n",
> +			map_id, map_info->info.value_size, len);
> +		return;
> +	}
> +
> +	type_id = map_info->info.btf_value_type_id;
> +
> +	dump = bpf_map_opts_get_btf_dump(map_info);
> +	if (!dump)
> +		return;
> +
> +	out(SK_STORAGE_INDENT_STR "map_id: %d [\n", map_id);
> +	r = btf_dump__dump_type_data(dump, type_id, data, len, &opts);
> +	if (r < 0)
> +		out(SK_STORAGE_INDENT_STR SK_STORAGE_INDENT_STR "failed to dump data: %d", r);
> +	out("\n" SK_STORAGE_INDENT_STR "]");
> +}
> +