From: Martin KaFai Lau <martin.lau@linux•dev>
To: Quentin Deslandes <qde@naccy•de>
Cc: David Ahern <dsahern@gmail•com>,
Martin KaFai Lau <martin.lau@kernel•org>,
netdev@vger•kernel.org
Subject: Re: [PATCH 2/3] ss: add support for BPF socket-local storage
Date: Tue, 28 Nov 2023 15:35:27 -0800 [thread overview]
Message-ID: <9f1b0310-25c5-4791-a825-e67cd59fea18@linux.dev> (raw)
In-Reply-To: <20231128023058.53546-3-qde@naccy.de>
On 11/27/23 6:30 PM, Quentin Deslandes wrote:
> diff --git a/misc/ss.c b/misc/ss.c
> index 09dc1f37..5b255ce3 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -51,6 +51,11 @@
> #include <linux/tls.h>
> #include <linux/mptcp.h>
>
> +#ifdef HAVE_LIBBPF
> +#include <bpf/bpf.h>
> +#include <bpf/libbpf.h>
> +#endif
> +
> #if HAVE_RPC
> #include <rpc/rpc.h>
> #include <rpc/xdr.h>
> @@ -101,6 +106,7 @@ enum col_id {
> COL_RADDR,
> COL_RSERV,
> COL_PROC,
> + COL_SKSTOR,
> COL_EXT,
> COL_MAX
> };
> @@ -130,6 +136,7 @@ static struct column columns[] = {
> { ALIGN_RIGHT, "Peer Address:", " ", 0, 0, 0 },
> { ALIGN_LEFT, "Port", "", 0, 0, 0 },
> { ALIGN_LEFT, "Process", "", 0, 0, 0 },
> + { ALIGN_LEFT, "Socket storage", "", 1, 0, 0 },
> { ALIGN_LEFT, "", "", 0, 0, 0 },
> };
>
> @@ -3368,6 +3375,222 @@ static void parse_diag_msg(struct nlmsghdr *nlh, struct sockstat *s)
> memcpy(s->remote.data, r->id.idiag_dst, s->local.bytelen);
> }
>
> +#ifdef HAVE_LIBBPF
> +
> +#define MAX_NR_BPF_MAP_ID_OPTS 32
> +
> +struct btf;
> +
> +static struct bpf_map_opts {
> + unsigned int nr_maps;
> + struct bpf_sk_storage_map_info {
> + unsigned int id;
> + int fd;
> + } maps[MAX_NR_BPF_MAP_ID_OPTS];
> + bool show_all;
> + struct btf *kernel_btf;
> +} bpf_map_opts;
> +
> +static void bpf_map_opts_mixed_error(void)
> +{
> + fprintf(stderr,
> + "ss: --bpf-maps and --bpf-map-id cannot be used together\n");
> +}
> +
> +static int bpf_map_opts_add_all(void)
> +{
> + unsigned int i;
> + unsigned int fd;
> + uint32_t id = 0;
> + int r;
> +
> + if (bpf_map_opts.nr_maps) {
> + bpf_map_opts_mixed_error();
> + return -1;
> + }
> +
> + while (1) {
> + struct bpf_map_info info = {};
> + uint32_t len = sizeof(info);
> +
> + r = bpf_map_get_next_id(id, &id);
> + if (r) {
> + if (errno == ENOENT)
> + break;
> +
> + fprintf(stderr, "ss: failed to fetch BPF map ID\n");
> + goto err;
> + }
> +
> + fd = bpf_map_get_fd_by_id(id);
> + if (fd == -1) {
The map might be gone. Check for errno == -ENOENT and "continue;" instead of
"goto err;".
> + fprintf(stderr, "ss: cannot get fd for BPF map ID %u%s\n",
> + id, errno == EPERM ?
> + ": missing root permissions, CAP_BPF, or CAP_SYS_ADMIN" : "");
> + goto err;
> + }
> +
> + r = bpf_obj_get_info_by_fd(fd, &info, &len);
> + if (r) {
> + fprintf(stderr, "ss: failed to get info for BPF map ID %u\n",
> + id);
> + close(fd);
> + goto err;
> + }
> +
> + if (info.type != BPF_MAP_TYPE_SK_STORAGE) {
> + close(fd);
> + continue;
> + }
> +
> + if (bpf_map_opts.nr_maps == MAX_NR_BPF_MAP_ID_OPTS) {
> + fprintf(stderr, "ss: too many (> %u) BPF socket-local storage maps found, skipping map ID %u\n",
> + MAX_NR_BPF_MAP_ID_OPTS, id);
> + close(fd);
> + continue;
> + }
> +
> + bpf_map_opts.maps[bpf_map_opts.nr_maps].id = id;
> + bpf_map_opts.maps[bpf_map_opts.nr_maps++].fd = fd;
Not sure how the ss takes care of the fd/memory resources before process exit.
May be the fd(s) need a close() at some point?
> + }
> +
> + bpf_map_opts.show_all = true;
> +
> + return 0;
> +
> +err:
> + for (i = 0; i < bpf_map_opts.nr_maps; ++i)
> + close(bpf_map_opts.maps[i].fd);
> +
> + return -1;
> +}
> +
> +static int bpf_map_opts_add_id(const char *optarg)
> +{
> + struct bpf_map_info info = {};
> + uint32_t len = sizeof(info);
> + size_t optarg_len;
> + unsigned long id;
> + unsigned int i;
> + char *end;
> + int fd;
> + int r;
> +
> + if (bpf_map_opts.show_all) {
> + bpf_map_opts_mixed_error();
> + return -1;
> + }
> +
> + optarg_len = strlen(optarg);
> + id = strtoul(optarg, &end, 0);
> + if (end != optarg + optarg_len || id == 0 || id > UINT32_MAX) {
id >= INT32_MAX
> + fprintf(stderr, "ss: invalid BPF map ID %s\n", optarg);
> + return -1;
> + }
> +
> + for (i = 0; i < bpf_map_opts.nr_maps; i++) {
> + if (bpf_map_opts.maps[i].id == id)
> + return 0;
> + }
> +
> + if (bpf_map_opts.nr_maps == MAX_NR_BPF_MAP_ID_OPTS) {
> + fprintf(stderr, "ss: too many (> %u) BPF socket-local storage maps found, skipping map ID %lu\n",
> + MAX_NR_BPF_MAP_ID_OPTS, id);
> + return 0;
> + }
> +
> + fd = bpf_map_get_fd_by_id(id);
> + if (fd == -1) {
> + fprintf(stderr, "ss: cannot get fd for BPF map ID %lu%s\n",
> + id, errno == EPERM ?
> + ": missing root permissions, CAP_BPF, or CAP_SYS_ADMIN" : "");
> + return -1;
> + }
> +
> + r = bpf_obj_get_info_by_fd(fd, &info, &len);
> + if (r) {
> + fprintf(stderr, "ss: failed to get info for BPF map ID %lu\n", id);
> + close(fd);
> + return -1;
> + }
> +
> + if (info.type != BPF_MAP_TYPE_SK_STORAGE) {
> + fprintf(stderr, "ss: BPF map with ID %s has type '%s', expecting 'sk_storage'\n",
> + optarg, libbpf_bpf_map_type_str(info.type));
> + 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;
> +
> + return 0;
> +}
> +
> +static inline bool bpf_map_opts_is_enabled(void)
> +{
> + return bpf_map_opts.nr_maps;
> +}
> +
> +static struct rtattr *bpf_map_opts_alloc_rta(void)
> +{
> + size_t total_size = RTA_LENGTH(RTA_LENGTH(sizeof(int)) * bpf_map_opts.nr_maps);
> + struct rtattr *stgs_rta, *fd_rta;
> + unsigned int i;
> + void *buf;
> +
> + stgs_rta = malloc(RTA_LENGTH(0));
stgs_rta is malloc()-ed here.
> + stgs_rta->rta_len = RTA_LENGTH(0);
> + stgs_rta->rta_type = INET_DIAG_REQ_SK_BPF_STORAGES | NLA_F_NESTED;
> +
> + buf = malloc(total_size);
> + if (!buf)
> + return NULL;
> +
> + stgs_rta = buf;
and then overwriteen by buf. doesn't look right.
> + 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;
> +}
next prev parent reply other threads:[~2023-11-28 23:35 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-28 2:30 [PATCH 0/3] ss: pretty-printing BPF socket-local storage Quentin Deslandes
2023-11-28 2:30 ` [PATCH 1/3] ss: prevent "Process" column from being printed unless requested Quentin Deslandes
2023-11-29 0:20 ` David Ahern
2023-11-28 2:30 ` [PATCH 2/3] ss: add support for BPF socket-local storage Quentin Deslandes
2023-11-28 23:35 ` Martin KaFai Lau [this message]
2023-11-28 2:30 ` [PATCH 3/3] ss: pretty-print " Quentin Deslandes
2023-11-28 23:42 ` Martin KaFai Lau
2023-11-28 22:43 ` [PATCH 0/3] ss: pretty-printing " Stephen Hemminger
2023-12-08 15:01 ` Quentin Deslandes
2023-12-08 17:27 ` Stephen Hemminger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9f1b0310-25c5-4791-a825-e67cd59fea18@linux.dev \
--to=martin.lau@linux$(echo .)dev \
--cc=dsahern@gmail$(echo .)com \
--cc=martin.lau@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=qde@naccy$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox