Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
---
tools/include/tools/opts.h | 68 +++++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/bpf.c | 1 +
tools/lib/bpf/btf.c | 1 +
tools/lib/bpf/btf_dump.c | 1 +
tools/lib/bpf/libbpf.c | 3 +-
tools/lib/bpf/libbpf_internal.h | 48 -----------------------------
tools/lib/bpf/linker.c | 1 +
tools/lib/bpf/netlink.c | 1 +
tools/lib/bpf/ringbuf.c | 1 +
9 files changed, 76 insertions(+), 49 deletions(-)
diff --git a/tools/include/tools/opts.h b/tools/include/tools/opts.h
new file mode 100644
index 000000000000..42b4c1a66cad
--- /dev/null
+++ b/tools/include/tools/opts.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
+
+/*
+ * Options helpers.
+ *
+ * Originally in tools/lib/bpf/libbpf_internal.h
+ */
+
+#ifndef __TOOLS_OPTS_H
+#define __TOOLS_OPTS_H
+
+#include <stdint.h>
+#include <stdio.h>
+
+#ifndef offsetofend
+#define offsetofend(TYPE, MEMBER) \
+ (offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER)))
+#endif
+
+static inline bool lib_is_mem_zeroed(const char *p, ssize_t len)
+{
+ while (len > 0) {
+ if (*p)
+ return false;
+ p++;
+ len--;
+ }
+ return true;
+}
+
+static inline bool lib_validate_opts(const char *opts,
+ size_t opts_sz, size_t user_sz,
+ const char *type_name)
+{
+ if (user_sz < sizeof(size_t)) {
+ fprintf(stderr, "%s size (%zu) is too small\n", type_name, user_sz);
+ return false;
+ }
+ if (!lib_is_mem_zeroed(opts + opts_sz, (ssize_t)user_sz - opts_sz)) {
+ fprintf(stderr, "%s has non-zero extra bytes\n", type_name);
+ return false;
+ }
+ return true;
+}
+
+#define OPTS_VALID(opts, type) \
+ (!(opts) || lib_validate_opts((const char *)opts, \
+ offsetofend(struct type, \
+ type##__last_field), \
+ (opts)->sz, #type))
+#define OPTS_HAS(opts, field) \
+ ((opts) && opts->sz >= offsetofend(typeof(*(opts)), field))
+#define OPTS_GET(opts, field, fallback_value) \
+ (OPTS_HAS(opts, field) ? (opts)->field : fallback_value)
+#define OPTS_SET(opts, field, value) \
+ do { \
+ if (OPTS_HAS(opts, field)) \
+ (opts)->field = value; \
+ } while (0)
+
+#define OPTS_ZEROED(opts, last_nonzero_field) \
+({ \
+ ssize_t __off = offsetofend(typeof(*(opts)), last_nonzero_field); \
+ !(opts) || lib_is_mem_zeroed((const void *)opts + __off, \
+ (opts)->sz - __off); \
+})
+
+#endif /* __TOOLS_OPTS_H */
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 2a4c71501a17..089f0e0be3a2 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -32,6 +32,7 @@
#include <linux/kernel.h>
#include <limits.h>
#include <sys/resource.h>
+#include <tools/opts.h>
#include "bpf.h"
#include "libbpf.h"
#include "libbpf_internal.h"
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 2d0840ef599a..e03974de2f02 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -16,6 +16,7 @@
#include <linux/err.h>
#include <linux/btf.h>
#include <gelf.h>
+#include <tools/opts.h>
#include "btf.h"
#include "bpf.h"
#include "libbpf.h"
diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index 5dbca76b953f..877479228954 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -17,6 +17,7 @@
#include <linux/err.h>
#include <linux/btf.h>
#include <linux/kernel.h>
+#include <tools/opts.h>
#include "btf.h"
#include "hashmap.h"
#include "libbpf.h"
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5401f2df463d..97a47a3d4e51 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -43,6 +43,7 @@
#include <sys/vfs.h>
#include <sys/utsname.h>
#include <sys/resource.h>
+#include <tools/opts.h>
#include <libelf.h>
#include <gelf.h>
#include <zlib.h>
@@ -1146,7 +1147,7 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
kern_member = find_member_by_name(kern_btf, kern_type, mname);
if (!kern_member) {
- if (!libbpf_is_mem_zeroed(mdata, msize)) {
+ if (!lib_is_mem_zeroed(mdata, msize)) {
pr_warn("struct_ops init_kern %s: Cannot find member %s in kernel BTF\n",
map->name, mname);
return -ENOTSUP;
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index a0dcfb82e455..033b852ed9a7 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -283,54 +283,6 @@ void *libbpf_add_mem(void **data, size_t *cap_cnt, size_t elem_sz,
size_t cur_cnt, size_t max_cnt, size_t add_cnt);
int libbpf_ensure_mem(void **data, size_t *cap_cnt, size_t elem_sz, size_t need_cnt);
-static inline bool libbpf_is_mem_zeroed(const char *p, ssize_t len)
-{
- while (len > 0) {
- if (*p)
- return false;
- p++;
- len--;
- }
- return true;
-}
-
-static inline bool libbpf_validate_opts(const char *opts,
- size_t opts_sz, size_t user_sz,
- const char *type_name)
-{
- if (user_sz < sizeof(size_t)) {
- pr_warn("%s size (%zu) is too small\n", type_name, user_sz);
- return false;
- }
- if (!libbpf_is_mem_zeroed(opts + opts_sz, (ssize_t)user_sz - opts_sz)) {
- pr_warn("%s has non-zero extra bytes\n", type_name);
- return false;
- }
- return true;
-}
-
-#define OPTS_VALID(opts, type) \
- (!(opts) || libbpf_validate_opts((const char *)opts, \
- offsetofend(struct type, \
- type##__last_field), \
- (opts)->sz, #type))
-#define OPTS_HAS(opts, field) \
- ((opts) && opts->sz >= offsetofend(typeof(*(opts)), field))
-#define OPTS_GET(opts, field, fallback_value) \
- (OPTS_HAS(opts, field) ? (opts)->field : fallback_value)
-#define OPTS_SET(opts, field, value) \
- do { \
- if (OPTS_HAS(opts, field)) \
- (opts)->field = value; \
- } while (0)
-
-#define OPTS_ZEROED(opts, last_nonzero_field) \
-({ \
- ssize_t __off = offsetofend(typeof(*(opts)), last_nonzero_field); \
- !(opts) || libbpf_is_mem_zeroed((const void *)opts + __off, \
- (opts)->sz - __off); \
-})
-
enum kern_feature_id {
/* v4.14: kernel support for program & map names. */
FEAT_PROG_NAME,
diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
index 0d4be829551b..e6fb12ba396c 100644
--- a/tools/lib/bpf/linker.c
+++ b/tools/lib/bpf/linker.c
@@ -16,6 +16,7 @@
#include <elf.h>
#include <libelf.h>
#include <fcntl.h>
+#include <tools/opts.h>
#include "libbpf.h"
#include "btf.h"
#include "libbpf_internal.h"
diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
index 68a2def17175..786a4f6dc3ab 100644
--- a/tools/lib/bpf/netlink.c
+++ b/tools/lib/bpf/netlink.c
@@ -11,6 +11,7 @@
#include <linux/rtnetlink.h>
#include <linux/netdev.h>
#include <sys/socket.h>
+#include <tools/opts.h>
#include <errno.h>
#include <time.h>
diff --git a/tools/lib/bpf/ringbuf.c b/tools/lib/bpf/ringbuf.c
index bfd8dac4c0cc..547781cde26d 100644
--- a/tools/lib/bpf/ringbuf.c
+++ b/tools/lib/bpf/ringbuf.c
@@ -16,6 +16,7 @@
#include <asm/barrier.h>
#include <sys/mman.h>
#include <sys/epoll.h>
+#include <tools/opts.h>
#include <time.h>
#include "libbpf.h"
--
2.44.0On Mon, Jul 29, 2024 at 9:46 AM Charlie Jenkins <charlie@rivosinc.com> wrote: > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > --- > tools/include/tools/opts.h | 68 +++++++++++++++++++++++++++++++++++++++++ > tools/lib/bpf/bpf.c | 1 + > tools/lib/bpf/btf.c | 1 + > tools/lib/bpf/btf_dump.c | 1 + > tools/lib/bpf/libbpf.c | 3 +- > tools/lib/bpf/libbpf_internal.h | 48 ----------------------------- > tools/lib/bpf/linker.c | 1 + > tools/lib/bpf/netlink.c | 1 + > tools/lib/bpf/ringbuf.c | 1 + > 9 files changed, 76 insertions(+), 49 deletions(-) > Nope, sorry, I don't think I want to do this for libbpf. This will just make Github synchronization trickier, and I don't really see a point. I'm totally fine with libperf making a copy of these helpers, though (this is not complicated or tricky code). I also don't think it will change much, so there is little risk of any sort of divergence. [...]
On Mon, Jul 29, 2024 at 10:01:05AM -0700, Andrii Nakryiko wrote: > On Mon, Jul 29, 2024 at 9:46 AM Charlie Jenkins <charlie@rivosinc.com> wrote: > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > --- > > tools/include/tools/opts.h | 68 +++++++++++++++++++++++++++++++++++++++++ > > tools/lib/bpf/bpf.c | 1 + > > tools/lib/bpf/btf.c | 1 + > > tools/lib/bpf/btf_dump.c | 1 + > > tools/lib/bpf/libbpf.c | 3 +- > > tools/lib/bpf/libbpf_internal.h | 48 ----------------------------- > > tools/lib/bpf/linker.c | 1 + > > tools/lib/bpf/netlink.c | 1 + > > tools/lib/bpf/ringbuf.c | 1 + > > 9 files changed, 76 insertions(+), 49 deletions(-) > > > > Nope, sorry, I don't think I want to do this for libbpf. This will > just make Github synchronization trickier, and I don't really see a > point. > > I'm totally fine with libperf making a copy of these helpers, though > (this is not complicated or tricky code). I also don't think it will > change much, so there is little risk of any sort of divergence. I did this because there were two comments on the previous version of this patch that asked to change the functions that were copied over. I had a couple of choices, have the implementations diverge, not change the implementation in perf to keep it the same as bpf, update both perf and bpf, or share the implementations. I figured the last option was the best to avoid immediate divergence. However, both of the comments can be safely ignored, and also perhaps divergence doesn't matter. - Charlie > > [...]
On Mon, Jul 29, 2024 at 10:55 AM Charlie Jenkins <charlie@rivosinc.com> wrote: > > On Mon, Jul 29, 2024 at 10:01:05AM -0700, Andrii Nakryiko wrote: > > On Mon, Jul 29, 2024 at 9:46 AM Charlie Jenkins <charlie@rivosinc.com> wrote: > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > --- > > > tools/include/tools/opts.h | 68 +++++++++++++++++++++++++++++++++++++++++ > > > tools/lib/bpf/bpf.c | 1 + > > > tools/lib/bpf/btf.c | 1 + > > > tools/lib/bpf/btf_dump.c | 1 + > > > tools/lib/bpf/libbpf.c | 3 +- > > > tools/lib/bpf/libbpf_internal.h | 48 ----------------------------- > > > tools/lib/bpf/linker.c | 1 + > > > tools/lib/bpf/netlink.c | 1 + > > > tools/lib/bpf/ringbuf.c | 1 + > > > 9 files changed, 76 insertions(+), 49 deletions(-) > > > > > > > Nope, sorry, I don't think I want to do this for libbpf. This will > > just make Github synchronization trickier, and I don't really see a > > point. > > > > I'm totally fine with libperf making a copy of these helpers, though > > (this is not complicated or tricky code). I also don't think it will > > change much, so there is little risk of any sort of divergence. > > I did this because there were two comments on the previous version of > this patch that asked to change the functions that were copied over. I > had a couple of choices, have the implementations diverge, not change > the implementation in perf to keep it the same as bpf, update both perf > and bpf, or share the implementations. I figured the last option was the > best to avoid immediate divergence. However, both of the comments can be > safely ignored, and also perhaps divergence doesn't matter. > I mean, feel free to diverge. First and foremost the code has to make sense to specific library and specific use case. If libperf has some extra things that it needs to enforce or check, by all means. I just want to avoid unnecessary code sharing, given the code isn't tricky or complicated, but will complicate libbpf's sync story to Github (libbpf kind of lives in two places, kernel repo and Github repo). > - Charlie > > > > > [...]
On Mon, Jul 29, 2024 at 11:59:47AM -0700, Andrii Nakryiko wrote: > On Mon, Jul 29, 2024 at 10:55 AM Charlie Jenkins <charlie@rivosinc.com> wrote: > > > > On Mon, Jul 29, 2024 at 10:01:05AM -0700, Andrii Nakryiko wrote: > > > On Mon, Jul 29, 2024 at 9:46 AM Charlie Jenkins <charlie@rivosinc.com> wrote: > > > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > > --- > > > > tools/include/tools/opts.h | 68 +++++++++++++++++++++++++++++++++++++++++ > > > > tools/lib/bpf/bpf.c | 1 + > > > > tools/lib/bpf/btf.c | 1 + > > > > tools/lib/bpf/btf_dump.c | 1 + > > > > tools/lib/bpf/libbpf.c | 3 +- > > > > tools/lib/bpf/libbpf_internal.h | 48 ----------------------------- > > > > tools/lib/bpf/linker.c | 1 + > > > > tools/lib/bpf/netlink.c | 1 + > > > > tools/lib/bpf/ringbuf.c | 1 + > > > > 9 files changed, 76 insertions(+), 49 deletions(-) > > > > > > > > > > Nope, sorry, I don't think I want to do this for libbpf. This will > > > just make Github synchronization trickier, and I don't really see a > > > point. > > > > > > I'm totally fine with libperf making a copy of these helpers, though > > > (this is not complicated or tricky code). I also don't think it will > > > change much, so there is little risk of any sort of divergence. > > > > I did this because there were two comments on the previous version of > > this patch that asked to change the functions that were copied over. I > > had a couple of choices, have the implementations diverge, not change > > the implementation in perf to keep it the same as bpf, update both perf > > and bpf, or share the implementations. I figured the last option was the > > best to avoid immediate divergence. However, both of the comments can be > > safely ignored, and also perhaps divergence doesn't matter. > > > > I mean, feel free to diverge. First and foremost the code has to make > sense to specific library and specific use case. If libperf has some > extra things that it needs to enforce or check, by all means. I just > want to avoid unnecessary code sharing, given the code isn't tricky or > complicated, but will complicate libbpf's sync story to Github (libbpf > kind of lives in two places, kernel repo and Github repo). Alright, I like to avoid copy-pasting code but if that's what is required I will do that. > > > - Charlie > > > > > > > > [...]
© 2016 - 2026 Red Hat, Inc.