Re: [PATCH v3 1/2] ss: add support for BPF socket-local storage

[David Ahern <dsahern@gmail.com>]

a few nits ...

On 12/20/23 8:23 AM, Quentin Deslandes wrote:
> +static struct rtattr *bpf_map_opts_alloc_rta(void)
> +{
> +	size_t total_size = RTA_LENGTH(RTA_LENGTH(sizeof(int)) * bpf_map_opts.nr_maps);

line is too long.

> +	struct rtattr *stgs_rta, *fd_rta;

move declaration here and ..

> +	unsigned int i;
> +	void *buf;
> +

set here.

> +	buf = malloc(total_size);
> +	if (!buf)
> +		return NULL;
> +
> +	stgs_rta = buf;
> +	stgs_rta->rta_type = INET_DIAG_REQ_SK_BPF_STORAGES | NLA_F_NESTED;
> +	stgs_rta->rta_len = total_size;
> +
> +	buf = RTA_DATA(stgs_rta);
> +	for (i = 0; i < bpf_map_opts.nr_maps; i++) {
> +		int *fd;
> +
> +		fd_rta = buf;
> +		fd_rta->rta_type = SK_DIAG_BPF_STORAGE_REQ_MAP_FD;
> +		fd_rta->rta_len = RTA_LENGTH(sizeof(int));
> +
> +		fd = RTA_DATA(fd_rta);
> +		*fd = bpf_map_opts.maps[i].fd;
> +
> +		buf += fd_rta->rta_len;
> +	}
> +
> +	return stgs_rta;
> +}
> +
> +static void show_sk_bpf_storages(struct rtattr *bpf_stgs)
> +{
> +	struct rtattr *tb[SK_DIAG_BPF_STORAGE_MAX + 1], *bpf_stg;
> +	unsigned int rem;
> +
> +	for (bpf_stg = RTA_DATA(bpf_stgs), rem = RTA_PAYLOAD(bpf_stgs);
> +		RTA_OK(bpf_stg, rem); bpf_stg = RTA_NEXT(bpf_stg, rem)) {
> +
> +		if ((bpf_stg->rta_type & NLA_TYPE_MASK) != SK_DIAG_BPF_STORAGE)
> +			continue;
> +
> +		parse_rtattr_nested(tb, SK_DIAG_BPF_STORAGE_MAX,
> +			(struct rtattr *)bpf_stg);
> +
> +		if (tb[SK_DIAG_BPF_STORAGE_MAP_ID]) {
> +			out("map_id:%u",
> +				rta_getattr_u32(tb[SK_DIAG_BPF_STORAGE_MAP_ID]));
> +		}
> +	}
> +}
> +
> +#endif
> +
>  static int inet_show_sock(struct nlmsghdr *nlh,
>  			  struct sockstat *s)
>  {
> @@ -3381,8 +3620,8 @@ static int inet_show_sock(struct nlmsghdr *nlh,
>  	struct inet_diag_msg *r = NLMSG_DATA(nlh);
>  	unsigned char v6only = 0;
>  
> -	parse_rtattr(tb, INET_DIAG_MAX, (struct rtattr *)(r+1),
> -		     nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
> +	parse_rtattr_flags(tb, INET_DIAG_MAX, (struct rtattr *)(r+1),
> +		nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)), NLA_F_NESTED);

column alignment and I think NESTED will need to be on the next line.

>  
>  	if (tb[INET_DIAG_PROTOCOL])
>  		s->type = rta_getattr_u8(tb[INET_DIAG_PROTOCOL]);


Also, please add a patch that updates the man page for all new options.