[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f730b229-4851-4021-9800-f0aa265a729f@linux.dev>
Date: Fri, 12 Jan 2024 18:12:17 -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 v4 1/3] ss: add support for BPF socket-local storage
On 1/12/24 6:04 AM, Quentin Deslandes wrote:
> +static struct rtattr *bpf_map_opts_alloc_rta(void)
> +{
> + struct rtattr *stgs_rta, *fd_rta;
> + size_t total_size;
> + unsigned int i;
> + void *buf;
> +
> + /* If bpf_map_opts.show_all == true, then bpf_map_opts.nr_maps == 0. We
> + * will send an empty message to the kernel, which will return all the
> + * socket-local data attached to a socket, no matter their map ID. */
> + total_size = RTA_LENGTH(RTA_LENGTH(sizeof(int)) * bpf_map_opts.nr_maps);
I have been trying the patch in some heavier traffic machines because I am over
excited :)
The "--bpf-maps" result is pretty flaky. It does not always print all the
sk_storage_map.
This line has a bug when using with the "--bpf-maps" cmd opts. The nr_maps will
become non-zero and will end up not printing all sk_storage_map. Take a look at
the inet_show_netlink() and there is a "goto again" case.
It really has to test with the bpf_map_opts.show_all 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;
> +}
> +
[ ... ]
> @@ -3564,13 +3767,14 @@ static int sockdiag_send(int family, int fd, int protocol, struct filter *f)
> {
> struct sockaddr_nl nladdr = { .nl_family = AF_NETLINK };
> DIAG_REQUEST(req, struct inet_diag_req_v2 r);
> + struct rtattr *bpf_stgs_rta = NULL;
> char *bc = NULL;
> int bclen;
> __u32 proto;
> struct msghdr msg;
> struct rtattr rta_bc;
> struct rtattr rta_proto;
> - struct iovec iov[5];
> + struct iovec iov[6];
> int iovlen = 1;
>
> if (family == PF_UNSPEC)
> @@ -3623,6 +3827,19 @@ static int sockdiag_send(int family, int fd, int protocol, struct filter *f)
> iovlen += 2;
> }
>
> +#ifdef HAVE_LIBBPF
> + if (bpf_map_opts_is_enabled()) {
> + bpf_stgs_rta = bpf_map_opts_alloc_rta();
> + if (!bpf_stgs_rta) {
> + fprintf(stderr, "ss: cannot alloc request for --bpf-map\n");
> + return -1;
> + }
> +
> + iov[iovlen++] = (struct iovec){ bpf_stgs_rta, bpf_stgs_rta->rta_len };
> + req.nlh.nlmsg_len += bpf_stgs_rta->rta_len;
> + }
> +#endif
> +
> msg = (struct msghdr) {
> .msg_name = (void *)&nladdr,
> .msg_namelen = sizeof(nladdr),
> @@ -3631,10 +3848,13 @@ static int sockdiag_send(int family, int fd, int protocol, struct filter *f)
> };
>
> if (sendmsg(fd, &msg, 0) < 0) {
> + free(bpf_stgs_rta);
> close(fd);
> return -1;
> }
>
> + free(bpf_stgs_rta);
> +
> return 0;
> }
Powered by blists - more mailing lists