Re: [PATCH net-next V3 2/3] tools: bpftool: show filenames of pinned objects
From: Jakub Kicinski <hidden>
Date: 2017-11-07 12:37:07
On Mon, 6 Nov 2017 15:06:31 +0900, Prashant Bhole wrote:
Added support to show filenames of pinned objects. ... Signed-off-by: Prashant Bhole <redacted>
Thanks for the changes, a couple more nit picks, sorry for not spotting them earlier.
quoted hunk ↗ jump to hunk
v2: - Dynamically identify bpf-fs moutpoint - Close files descriptors before returning on error - Fixed line break for proper output formatting - Code style: wrapped lines > 80, used reverse christmastree style v3: - Handle multiple bpffs mountpoints - Code style: fixed line break indentation tools/bpf/bpftool/common.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++ tools/bpf/bpftool/main.c | 8 +++++ tools/bpf/bpftool/main.h | 17 ++++++++++ tools/bpf/bpftool/map.c | 21 ++++++++++++ tools/bpf/bpftool/prog.c | 24 +++++++++++++ 5 files changed, 155 insertions(+)diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c index 4556947709ee..152c5bdbe2e9 100644 --- a/tools/bpf/bpftool/common.c +++ b/tools/bpf/bpftool/common.c@@ -45,6 +45,8 @@ #include <sys/mount.h> #include <sys/types.h> #include <sys/vfs.h> +#include <mntent.h> +#include <fts.h>
Please try to keep includes in an alphabetical order.
quoted hunk ↗ jump to hunk
#include <bpf.h>@@ -290,3 +292,86 @@ void print_hex_data_json(uint8_t *data, size_t len) jsonw_printf(json_wtr, "\"0x%02hhx\"", data[i]); jsonw_end_array(json_wtr); } + +int build_pinned_obj_table(struct pinned_obj_table *tab, + enum bpf_obj_type type) +{ + struct bpf_prog_info pinned_info = {}; + __u32 len = sizeof(pinned_info); + struct pinned_obj *obj_node = NULL; + enum bpf_obj_type objtype; + struct mntent *mntent = NULL;
Please try to order variable declarations longest to shortest.
+ FILE *mntfile = NULL;
+ FTSENT *ftse = NULL;
+ FTS *fts = NULL;
+ int fd, err;
+
+ mntfile = setmntent("/proc/mounts", "r");
+ if (!mntfile)
+ return -1;
+
+ while ((mntent = getmntent(mntfile)) != NULL) {Please try to avoid comparisons to NULL, writing: if (ptr) is more intuitive to most C programmers than: if (ptr != NULL)
+ char *path[] = {mntent->mnt_dir, 0};Shouldn't there be spaces after and before the curly braces? Does checkpatch --strict not warn about this?
+
+ if (strncmp(mntent->mnt_type, "bpf", 3) != 0)
+ continue;
+
+ fts = fts_open(path, 0, NULL);
+ if (!fts)
+ continue;
+
+ while ((ftse = fts_read(fts)) != NULL) {
+ if (!(ftse->fts_info & FTS_F))
+ continue;
+ fd = open_obj_pinned(ftse->fts_path);
+ 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);
+ }
+ fclose(mntfile);
+ return 0;
+}
+
+void delete_pinned_obj_table(struct pinned_obj_table *tab)
+{
+ struct pinned_obj *obj;
+ struct hlist_node *tmp;
+ unsigned int bkt;
+
+ if (hash_empty(tab->table))
+ return;is this necessary? Does hash_for_each_safe() not work with empty table?
+ hash_for_each_safe(tab->table, bkt, tmp, obj, hash) {
+ hash_del(&obj->hash);
+ free(obj->path);
+ free(obj);
+ }
+}