[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 12 Dec 2023 14:49:28 -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 1/2] ss: add support for BPF socket-local storage
On 12/8/23 6:57 AM, Quentin Deslandes wrote:
> +static inline bool bpf_map_opts_is_enabled(void)
nit.
This is the only "inline" usage in this file. I would avoid it and depend on the
compiler to decide.
> +{
> + return bpf_map_opts.nr_maps;
> +}
> +
[ ... ]
> 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);
>
> if (tb[INET_DIAG_PROTOCOL])
> s->type = rta_getattr_u8(tb[INET_DIAG_PROTOCOL]);
> @@ -3479,6 +3718,11 @@ static int inet_show_sock(struct nlmsghdr *nlh,
> }
> sctp_ino = s->ino;
>
> + if (tb[INET_DIAG_SK_BPF_STORAGES]) {
> + field_set(COL_SKSTOR);
> + show_sk_bpf_storages(tb[INET_DIAG_SK_BPF_STORAGES]);
> + }
> +
> return 0;
> }
>
> @@ -3560,13 +3804,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)
> @@ -3619,6 +3864,17 @@ static int sockdiag_send(int family, int fd, int protocol, struct filter *f)
> iovlen += 2;
> }
>
> + if (bpf_map_opts_is_enabled()) {
This will have compiler error when HAVE_LIBBPF is not set.
> + 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;
> + }
> +
> msg = (struct msghdr) {
> .msg_name = (void *)&nladdr,
> .msg_namelen = sizeof(nladdr),
[ ... ]
> @@ -5712,6 +5982,16 @@ int main(int argc, char *argv[])
> case OPT_INET_SOCKOPT:
> show_inet_sockopt = 1;
> break;
> +#ifdef HAVE_LIBBPF
> + case OPT_BPF_MAPS:
> + if (bpf_map_opts_add_all())
> + exit(1);
> + break;
> + case OPT_BPF_MAP_ID:
> + if (bpf_map_opts_add_id(optarg))
> + exit(1);
> + break;
> +#endif
> case 'h':
> help();
> case '?':
> @@ -5810,6 +6090,9 @@ int main(int argc, char *argv[])
> if (!(current_filter.states & (current_filter.states - 1)))
> columns[COL_STATE].disabled = 1;
>
> + if (bpf_map_opts.nr_maps)
same here when HAVE_LIBBPF is not set
> + columns[COL_SKSTOR].disabled = 0;
> +
> if (show_header)
> print_header();
>
> @@ -5845,6 +6128,7 @@ int main(int argc, char *argv[])
>
> if (show_processes || show_threads || show_proc_ctx || show_sock_ctx)
> user_ent_destroy();
> + bpf_map_opts_destroy();
same here.
A #ifdef is needed.
Another option is to create an empty or always-return-false function.
>
> render();
>
Powered by blists - more mailing lists