All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dominique Martinet <asmadeus@codewreck.org>
To: bpf@vger.kernel.org
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	KP Singh <kpsingh@kernel.org>,
	John Fastabend <john.fastabend@gmail.com>,
	Yonghong Song <yhs@fb.com>, Song Liu <songliubraving@fb.com>,
	Martin KaFai Lau <kafai@fb.com>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>,
	Dominique Martinet <asmadeus@codewreck.org>
Subject: [PATCH 3/4] tools/bpf: musl compat: replace nftw with FTW_ACTIONRETVAL
Date: Sun, 24 Apr 2022 14:10:21 +0900	[thread overview]
Message-ID: <20220424051022.2619648-4-asmadeus@codewreck.org> (raw)
In-Reply-To: <20220424051022.2619648-1-asmadeus@codewreck.org>

musl nftw implementation does not support FTW_ACTIONRETVAL.

There have been multiple attempts at pushing the feature in musl
upstream but it has been refused or ignored all the times:
https://www.openwall.com/lists/musl/2021/03/26/1
https://www.openwall.com/lists/musl/2022/01/22/1

In this case we only care about /proc/<pid>/fd/<fd>, so it's not
too difficult to reimplement directly instead, and the new
implementation makes 'bpftool perf' slightly faster because it doesn't
needlessly stat/readdir unneeded directories (54ms -> 13ms on my machine)

Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---

Alternatively alpine has one package where they reimplemented nftw with
FTW_ACTIONRETVAL support locally, so if reaaallly needed we could do the
same here.. But honestly doing two readdirs is probably just as simple
for this particular case.

 tools/bpf/bpftool/perf.c | 116 ++++++++++++++++++++-------------------
 1 file changed, 59 insertions(+), 57 deletions(-)

diff --git a/tools/bpf/bpftool/perf.c b/tools/bpf/bpftool/perf.c
index 50de087b0db7..de793872544e 100644
--- a/tools/bpf/bpftool/perf.c
+++ b/tools/bpf/bpftool/perf.c
@@ -11,7 +11,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
-#include <ftw.h>
+#include <dirent.h>
 
 #include <bpf/bpf.h>
 
@@ -147,81 +147,83 @@ static void print_perf_plain(int pid, int fd, __u32 prog_id, __u32 fd_type,
 	}
 }
 
-static int show_proc(const char *fpath, const struct stat *sb,
-		     int tflag, struct FTW *ftwbuf)
+static int show_proc(void)
 {
 	__u64 probe_offset, probe_addr;
 	__u32 len, prog_id, fd_type;
-	int err, pid = 0, fd = 0;
+	int err, pid, fd;
+	DIR *proc, *pid_fd;
+	struct dirent *proc_de, *pid_fd_de;
 	const char *pch;
 	char buf[4096];
 
-	/* prefix always /proc */
-	pch = fpath + 5;
-	if (*pch == '\0')
-		return 0;
+	proc = opendir("/proc");
+	if (!proc)
+		return -1;
+	while ((proc_de = readdir(proc))) {
+		pid = 0;
+		pch = proc_de->d_name;
 
-	/* pid should be all numbers */
-	pch++;
-	while (isdigit(*pch)) {
-		pid = pid * 10 + *pch - '0';
-		pch++;
+		/* pid should be all numbers */
+		while (isdigit(*pch)) {
+			pid = pid * 10 + *pch - '0';
+			pch++;
+		}
+		if (*pch != '\0')
+			continue;
+
+		err = snprintf(buf, sizeof(buf), "/proc/%s/fd", proc_de->d_name);
+		if (err < 0 || err >= (int)sizeof(buf))
+			continue;
+
+		pid_fd = opendir(buf);
+		if (!pid_fd)
+			continue;
+
+		while ((pid_fd_de = readdir(pid_fd))) {
+			fd = 0;
+			pch = pid_fd_de->d_name;
+
+			/* fd should be all numbers */
+			while (isdigit(*pch)) {
+				fd = fd * 10 + *pch - '0';
+				pch++;
+			}
+			if (*pch != '\0')
+				continue;
+
+			/* query (pid, fd) for potential perf events */
+			len = sizeof(buf);
+			err = bpf_task_fd_query(pid, fd, 0, buf, &len, &prog_id, &fd_type,
+						&probe_offset, &probe_addr);
+			if (err < 0)
+				continue;
+
+			if (json_output)
+				print_perf_json(pid, fd, prog_id, fd_type, buf, probe_offset,
+						probe_addr);
+			else
+				print_perf_plain(pid, fd, prog_id, fd_type, buf, probe_offset,
+						 probe_addr);
+		}
+		closedir(pid_fd);
 	}
-	if (*pch == '\0')
-		return 0;
-	if (*pch != '/')
-		return FTW_SKIP_SUBTREE;
-
-	/* check /proc/<pid>/fd directory */
-	pch++;
-	if (strncmp(pch, "fd", 2))
-		return FTW_SKIP_SUBTREE;
-	pch += 2;
-	if (*pch == '\0')
-		return 0;
-	if (*pch != '/')
-		return FTW_SKIP_SUBTREE;
-
-	/* check /proc/<pid>/fd/<fd_num> */
-	pch++;
-	while (isdigit(*pch)) {
-		fd = fd * 10 + *pch - '0';
-		pch++;
-	}
-	if (*pch != '\0')
-		return FTW_SKIP_SUBTREE;
-
-	/* query (pid, fd) for potential perf events */
-	len = sizeof(buf);
-	err = bpf_task_fd_query(pid, fd, 0, buf, &len, &prog_id, &fd_type,
-				&probe_offset, &probe_addr);
-	if (err < 0)
-		return 0;
-
-	if (json_output)
-		print_perf_json(pid, fd, prog_id, fd_type, buf, probe_offset,
-				probe_addr);
-	else
-		print_perf_plain(pid, fd, prog_id, fd_type, buf, probe_offset,
-				 probe_addr);
-
+	closedir(proc);
 	return 0;
 }
 
 static int do_show(int argc, char **argv)
 {
-	int flags = FTW_ACTIONRETVAL | FTW_PHYS;
-	int err = 0, nopenfd = 16;
+	int err;
 
 	if (!has_perf_query_support())
 		return -1;
 
 	if (json_output)
 		jsonw_start_array(json_wtr);
-	if (nftw("/proc", show_proc, nopenfd, flags) == -1) {
-		p_err("%s", strerror(errno));
-		err = -1;
-	}
+
+	err = show_proc();
+
 	if (json_output)
 		jsonw_end_array(json_wtr);
 
-- 
2.35.1


  parent reply	other threads:[~2022-04-24  5:11 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-24  5:10 [PATCH 0/4] tools/bpf: allow building with musl Dominique Martinet
2022-04-24  5:10 ` [PATCH 1/4] tools/bpf/runqslower: musl compat: explicitly link with libargp if found Dominique Martinet
2022-04-24  6:58   ` Dominique Martinet
2022-04-25 21:35     ` Daniel Borkmann
2022-04-25 22:33       ` Dominique Martinet
2022-04-24  5:10 ` [PATCH 2/4] tools/bpf: musl compat: do not use DEFFILEMODE Dominique Martinet
2022-04-24  5:10 ` Dominique Martinet [this message]
2022-04-25 21:24   ` [PATCH 3/4] tools/bpf: musl compat: replace nftw with FTW_ACTIONRETVAL Quentin Monnet
2022-04-24  5:10 ` [PATCH 4/4] tools/bpf: replace sys/fcntl.h by fcntl.h Dominique Martinet
2022-04-25 21:25   ` Quentin Monnet
2022-04-25 21:30 ` [PATCH 0/4] tools/bpf: allow building with musl patchwork-bot+netdevbpf

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=20220424051022.2619648-4-asmadeus@codewreck.org \
    --to=asmadeus@codewreck.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.