BPF List
 help / color / mirror / Atom feed
From: Tony Ambardar <tony.ambardar@gmail.com>
To: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>
Cc: Tony Ambardar <Tony.Ambardar@gmail.com>,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	Quentin Monnet <quentin@isovalent.com>
Subject: [PATCH bpf-next v2] bpftool: use only nftw for file tree parsing
Date: Wed, 15 Jul 2020 22:29:26 -0700	[thread overview]
Message-ID: <20200716052926.10933-1-Tony.Ambardar@gmail.com> (raw)
In-Reply-To: <20200715051214.28099-1-Tony.Ambardar () gmail ! com>

The bpftool sources include code to walk file trees, but use multiple
frameworks to do so: nftw and fts. While nftw conforms to POSIX/SUSv3 and
is widely available, fts is not conformant and less common, especially on
non-glibc systems. The inconsistent framework usage hampers maintenance
and portability of bpftool, in particular for embedded systems.

Standardize code usage by rewriting one fts-based function to use nftw.
Clean up related function warnings by using "const char *" arguments and
fixing an unsafe call to dirname().

These changes help in building bpftool against musl for OpenWrt.

Signed-off-by: Tony Ambardar <Tony.Ambardar@gmail.com>
---

V2:
* use _GNU_SOURCE to pull in getpagesize(), getline(), nftw() definitions
* use "const char *" in open_obj_pinned() and open_obj_pinned_any()
* make dirname() safely act on a string copy

---
 tools/bpf/bpftool/common.c | 129 +++++++++++++++++++++----------------
 tools/bpf/bpftool/main.h   |   4 +-
 2 files changed, 76 insertions(+), 57 deletions(-)

diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 29f4e7611ae8..7c2e52fc5784 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -1,10 +1,11 @@
 // SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
 /* Copyright (C) 2017-2018 Netronome Systems, Inc. */
 
+#define _GNU_SOURCE
 #include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
-#include <fts.h>
+#include <ftw.h>
 #include <libgen.h>
 #include <mntent.h>
 #include <stdbool.h>
@@ -160,24 +161,36 @@ int mount_tracefs(const char *target)
 	return err;
 }
 
-int open_obj_pinned(char *path, bool quiet)
+int open_obj_pinned(const char *path, bool quiet)
 {
-	int fd;
+	char *pname;
+	int fd = -1;
+
+	pname = strdup(path);
+	if (pname == NULL) {
+		if (!quiet)
+			p_err("bpf obj get (%s): %s", path, strerror(errno));
+		goto out_ret;
+	}
+
 
-	fd = bpf_obj_get(path);
+	fd = bpf_obj_get(pname);
 	if (fd < 0) {
 		if (!quiet)
-			p_err("bpf obj get (%s): %s", path,
-			      errno == EACCES && !is_bpffs(dirname(path)) ?
+			p_err("bpf obj get (%s): %s", pname,
+			      errno == EACCES && !is_bpffs(dirname(pname)) ?
 			    "directory not in bpf file system (bpffs)" :
 			    strerror(errno));
-		return -1;
+		goto out_free;
 	}
 
+out_free:
+	free(pname);
+out_ret:
 	return fd;
 }
 
-int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
+int open_obj_pinned_any(const char *path, enum bpf_obj_type exp_type)
 {
 	enum bpf_obj_type type;
 	int fd;
@@ -367,68 +380,74 @@ void print_hex_data_json(uint8_t *data, size_t len)
 	jsonw_end_array(json_wtr);
 }
 
-int build_pinned_obj_table(struct pinned_obj_table *tab,
-			   enum bpf_obj_type type)
+static struct pinned_obj_table *build_fn_table; /* params for nftw cb*/
+static enum bpf_obj_type build_fn_type;
+
+static int do_build_table_cb(const char *fpath, const struct stat *sb,
+			    int typeflag, struct FTW *ftwbuf)
 {
 	struct bpf_prog_info pinned_info = {};
 	struct pinned_obj *obj_node = NULL;
 	__u32 len = sizeof(pinned_info);
-	struct mntent *mntent = NULL;
 	enum bpf_obj_type objtype;
+	int fd, err = 0;
+
+	if (typeflag != FTW_F)
+		goto out_ret;
+	fd = open_obj_pinned(fpath, true);
+	if (fd < 0)
+		goto out_ret;
+
+	objtype = get_fd_type(fd);
+	if (objtype != build_fn_type)
+		goto out_close;
+
+	memset(&pinned_info, 0, sizeof(pinned_info));
+	if (bpf_obj_get_info_by_fd(fd, &pinned_info, &len)) {
+		p_err("can't get obj info: %s", strerror(errno));
+		goto out_close;
+	}
+
+	obj_node = malloc(sizeof(*obj_node));
+	if (!obj_node) {
+		p_err("mem alloc failed");
+		err = -1;
+		goto out_close;
+	}
+
+	memset(obj_node, 0, sizeof(*obj_node));
+	obj_node->id = pinned_info.id;
+	obj_node->path = strdup(fpath);
+	hash_add(build_fn_table->table, &obj_node->hash, obj_node->id);
+
+out_close:
+	close(fd);
+out_ret:
+	return err;
+}
+
+int build_pinned_obj_table(struct pinned_obj_table *tab,
+			   enum bpf_obj_type type)
+{
+	struct mntent *mntent = NULL;
 	FILE *mntfile = NULL;
-	FTSENT *ftse = NULL;
-	FTS *fts = NULL;
-	int fd, err;
+	int flags = FTW_PHYS;
+	int nopenfd = 16;
 
 	mntfile = setmntent("/proc/mounts", "r");
 	if (!mntfile)
 		return -1;
 
+	build_fn_table = tab;
+	build_fn_type = type;
+
 	while ((mntent = getmntent(mntfile))) {
-		char *path[] = { mntent->mnt_dir, NULL };
+		char *path = mntent->mnt_dir;
 
 		if (strncmp(mntent->mnt_type, "bpf", 3) != 0)
 			continue;
-
-		fts = fts_open(path, 0, NULL);
-		if (!fts)
-			continue;
-
-		while ((ftse = fts_read(fts))) {
-			if (!(ftse->fts_info & FTS_F))
-				continue;
-			fd = open_obj_pinned(ftse->fts_path, true);
-			if (fd < 0)
-				continue;
-
-			objtype = get_fd_type(fd);
-			if (objtype != type) {
-				close(fd);
-				continue;
-			}
-			memset(&pinned_info, 0, sizeof(pinned_info));
-			err = bpf_obj_get_info_by_fd(fd, &pinned_info, &len);
-			if (err) {
-				close(fd);
-				continue;
-			}
-
-			obj_node = malloc(sizeof(*obj_node));
-			if (!obj_node) {
-				close(fd);
-				fts_close(fts);
-				fclose(mntfile);
-				return -1;
-			}
-
-			memset(obj_node, 0, sizeof(*obj_node));
-			obj_node->id = pinned_info.id;
-			obj_node->path = strdup(ftse->fts_path);
-			hash_add(tab->table, &obj_node->hash, obj_node->id);
-
-			close(fd);
-		}
-		fts_close(fts);
+		if (nftw(path, do_build_table_cb, nopenfd, flags) == -1)
+			break;
 	}
 	fclose(mntfile);
 	return 0;
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 78d34e860713..e3a79b5a9960 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -152,8 +152,8 @@ int cmd_select(const struct cmd *cmds, int argc, char **argv,
 int get_fd_type(int fd);
 const char *get_fd_type_name(enum bpf_obj_type type);
 char *get_fdinfo(int fd, const char *key);
-int open_obj_pinned(char *path, bool quiet);
-int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type);
+int open_obj_pinned(const char *path, bool quiet);
+int open_obj_pinned_any(const char *path, enum bpf_obj_type exp_type);
 int mount_bpffs_for_pin(const char *name);
 int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(int *, char ***));
 int do_pin_fd(int fd, const char *name);
-- 
2.17.1


       reply	other threads:[~2020-07-16  5:29 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200715051214.28099-1-Tony.Ambardar () gmail ! com>
2020-07-16  5:29 ` Tony Ambardar [this message]
2020-07-16  9:14   ` [PATCH bpf-next v2] bpftool: use only nftw for file tree parsing Quentin Monnet
2020-07-17 22:43     ` Tony Ambardar

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=20200716052926.10933-1-Tony.Ambardar@gmail.com \
    --to=tony.ambardar@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=netdev@vger.kernel.org \
    --cc=quentin@isovalent.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox