[PATCH bpf-next 0/2] libbpf/xsk cleanups

STALE1970d

7 messages, 4 authors, 2021-03-11 · open the first message on its own page

[PATCH bpf-next 0/2] libbpf/xsk cleanups

From: Björn Töpel <hidden>
Date: 2021-03-10 08:10:38

This series removes a header dependency from xsk.h, and moves
libbpf_util.h into xsk.h.

More details in each commit!


Thank you,
Björn

Björn Töpel (2):
  libbpf: xsk: remove linux/compiler.h header
  libbpf: xsk: move barriers from libbpf_util.h to xsk.h

 tools/lib/bpf/Makefile      |  1 -
 tools/lib/bpf/libbpf_util.h | 75 -------------------------------------
 tools/lib/bpf/xsk.h         | 68 ++++++++++++++++++++++++++++++++-
 3 files changed, 67 insertions(+), 77 deletions(-)
 delete mode 100644 tools/lib/bpf/libbpf_util.h


base-commit: 32f91529e2bdbe0d92edb3ced41dfba4beffa84a
-- 
2.27.0

[PATCH bpf-next 1/2] libbpf: xsk: remove linux/compiler.h header

From: Björn Töpel <hidden>
Date: 2021-03-10 08:10:38

From: Björn Töpel <redacted>

In commit 291471dd1559 ("libbpf, xsk: Add libbpf_smp_store_release
libbpf_smp_load_acquire") linux/compiler.h was added as a dependency
to xsk.h, which is the user-facing API. This makes it harder for
userspace application to consume the library. Here the header
inclusion is removed, and instead {READ,WRITE}_ONCE() is added
explicitly.

Signed-off-by: Björn Töpel <redacted>
---
 tools/lib/bpf/libbpf_util.h | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/tools/lib/bpf/libbpf_util.h b/tools/lib/bpf/libbpf_util.h
index cfbcfc063c81..954da9b34a34 100644
--- a/tools/lib/bpf/libbpf_util.h
+++ b/tools/lib/bpf/libbpf_util.h

@@ -5,25 +5,30 @@
 #define __LIBBPF_LIBBPF_UTIL_H
 
 #include <stdbool.h>
-#include <linux/compiler.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
-/* Use these barrier functions instead of smp_[rw]mb() when they are
- * used in a libbpf header file. That way they can be built into the
- * application that uses libbpf.
+/* Load-Acquire Store-Release barriers used by the XDP socket
+ * library. The following macros should *NOT* be considered part of
+ * the xsk.h API, and is subject to change anytime.
+ *
+ * LIBRARY INTERNAL
  */
+
+#define __XSK_READ_ONCE(x) (*(volatile typeof(x) *)&x)
+#define __XSK_WRITE_ONCE(x, v) (*(volatile typeof(x) *)&x) = (v)
+
 #if defined(__i386__) || defined(__x86_64__)
 # define libbpf_smp_store_release(p, v)					\
 	do {								\
 		asm volatile("" : : : "memory");			\
-		WRITE_ONCE(*p, v);					\
+		__XSK_WRITE_ONCE(*p, v);				\
 	} while (0)
 # define libbpf_smp_load_acquire(p)					\
 	({								\
-		typeof(*p) ___p1 = READ_ONCE(*p);			\
+		typeof(*p) ___p1 = __XSK_READ_ONCE(*p);			\
 		asm volatile("" : : : "memory");			\
 		___p1;							\
 	})

@@ -41,11 +46,11 @@ extern "C" {
 # define libbpf_smp_store_release(p, v)					\
 	do {								\
 		asm volatile ("fence rw,w" : : : "memory");		\
-		WRITE_ONCE(*p, v);					\
+		__XSK_WRITE_ONCE(*p, v);				\
 	} while (0)
 # define libbpf_smp_load_acquire(p)					\
 	({								\
-		typeof(*p) ___p1 = READ_ONCE(*p);			\
+		typeof(*p) ___p1 = __XSK_READ_ONCE(*p);			\
 		asm volatile ("fence r,rw" : : : "memory");		\
 		___p1;							\
 	})

@@ -55,19 +60,21 @@ extern "C" {
 #define libbpf_smp_store_release(p, v)					\
 	do {								\
 		__sync_synchronize();					\
-		WRITE_ONCE(*p, v);					\
+		__XSK_WRITE_ONCE(*p, v);				\
 	} while (0)
 #endif
 
 #ifndef libbpf_smp_load_acquire
 #define libbpf_smp_load_acquire(p)					\
 	({								\
-		typeof(*p) ___p1 = READ_ONCE(*p);			\
+		typeof(*p) ___p1 = __XSK_READ_ONCE(*p);			\
 		__sync_synchronize();					\
 		___p1;							\
 	})
 #endif
 
+/* LIBRARY INTERNAL -- END */
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif

-- 
2.27.0

[PATCH bpf-next 2/2] libbpf: xsk: move barriers from libbpf_util.h to xsk.h

From: Björn Töpel <hidden>
Date: 2021-03-10 08:10:38

From: Björn Töpel <redacted>

The only user of libbpf_util.h is xsk.h. Move the barriers to xsk.h,
and remove libbpf_util.h. The barriers are used as an implementation
detail, and should not be considered part of the stable API.

Signed-off-by: Björn Töpel <redacted>
---
 tools/lib/bpf/Makefile      |  1 -
 tools/lib/bpf/libbpf_util.h | 82 -------------------------------------
 tools/lib/bpf/xsk.h         | 68 +++++++++++++++++++++++++++++-
 3 files changed, 67 insertions(+), 84 deletions(-)
 delete mode 100644 tools/lib/bpf/libbpf_util.h

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 8170f88e8ea6..f45bacbaa3d5 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile

@@ -228,7 +228,6 @@ install_headers: $(BPF_HELPER_DEFS)
 		$(call do_install,bpf.h,$(prefix)/include/bpf,644); \
 		$(call do_install,libbpf.h,$(prefix)/include/bpf,644); \
 		$(call do_install,btf.h,$(prefix)/include/bpf,644); \
-		$(call do_install,libbpf_util.h,$(prefix)/include/bpf,644); \
 		$(call do_install,libbpf_common.h,$(prefix)/include/bpf,644); \
 		$(call do_install,xsk.h,$(prefix)/include/bpf,644); \
 		$(call do_install,bpf_helpers.h,$(prefix)/include/bpf,644); \

diff --git a/tools/lib/bpf/libbpf_util.h b/tools/lib/bpf/libbpf_util.h
deleted file mode 100644
index 954da9b34a34..000000000000
--- a/tools/lib/bpf/libbpf_util.h
+++ /dev/null

@@ -1,82 +0,0 @@
-/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
-/* Copyright (c) 2019 Facebook */
-
-#ifndef __LIBBPF_LIBBPF_UTIL_H
-#define __LIBBPF_LIBBPF_UTIL_H
-
-#include <stdbool.h>
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-/* Load-Acquire Store-Release barriers used by the XDP socket
- * library. The following macros should *NOT* be considered part of
- * the xsk.h API, and is subject to change anytime.
- *
- * LIBRARY INTERNAL
- */
-
-#define __XSK_READ_ONCE(x) (*(volatile typeof(x) *)&x)
-#define __XSK_WRITE_ONCE(x, v) (*(volatile typeof(x) *)&x) = (v)
-
-#if defined(__i386__) || defined(__x86_64__)
-# define libbpf_smp_store_release(p, v)					\
-	do {								\
-		asm volatile("" : : : "memory");			\
-		__XSK_WRITE_ONCE(*p, v);				\
-	} while (0)
-# define libbpf_smp_load_acquire(p)					\
-	({								\
-		typeof(*p) ___p1 = __XSK_READ_ONCE(*p);			\
-		asm volatile("" : : : "memory");			\
-		___p1;							\
-	})
-#elif defined(__aarch64__)
-# define libbpf_smp_store_release(p, v)					\
-		asm volatile ("stlr %w1, %0" : "=Q" (*p) : "r" (v) : "memory")
-# define libbpf_smp_load_acquire(p)					\
-	({								\
-		typeof(*p) ___p1;					\
-		asm volatile ("ldar %w0, %1"				\
-			      : "=r" (___p1) : "Q" (*p) : "memory");	\
-		___p1;							\
-	})
-#elif defined(__riscv)
-# define libbpf_smp_store_release(p, v)					\
-	do {								\
-		asm volatile ("fence rw,w" : : : "memory");		\
-		__XSK_WRITE_ONCE(*p, v);				\
-	} while (0)
-# define libbpf_smp_load_acquire(p)					\
-	({								\
-		typeof(*p) ___p1 = __XSK_READ_ONCE(*p);			\
-		asm volatile ("fence r,rw" : : : "memory");		\
-		___p1;							\
-	})
-#endif
-
-#ifndef libbpf_smp_store_release
-#define libbpf_smp_store_release(p, v)					\
-	do {								\
-		__sync_synchronize();					\
-		__XSK_WRITE_ONCE(*p, v);				\
-	} while (0)
-#endif
-
-#ifndef libbpf_smp_load_acquire
-#define libbpf_smp_load_acquire(p)					\
-	({								\
-		typeof(*p) ___p1 = __XSK_READ_ONCE(*p);			\
-		__sync_synchronize();					\
-		___p1;							\
-	})
-#endif
-
-/* LIBRARY INTERNAL -- END */
-
-#ifdef __cplusplus
-} /* extern "C" */
-#endif
-
-#endif

diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index a9fdea87b5cd..1d846a419d21 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h

@@ -4,6 +4,7 @@
  * AF_XDP user-space access library.
  *
  * Copyright(c) 2018 - 2019 Intel Corporation.
+ * Copyright (c) 2019 Facebook
  *
  * Author(s): Magnus Karlsson <magnus.karlsson@intel.com>
  */

@@ -13,15 +14,80 @@
 
 #include <stdio.h>
 #include <stdint.h>
+#include <stdbool.h>
 #include <linux/if_xdp.h>
 
 #include "libbpf.h"
-#include "libbpf_util.h"
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+/* Load-Acquire Store-Release barriers used by the XDP socket
+ * library. The following macros should *NOT* be considered part of
+ * the xsk.h API, and is subject to change anytime.
+ *
+ * LIBRARY INTERNAL
+ */
+
+#define __XSK_READ_ONCE(x) (*(volatile typeof(x) *)&x)
+#define __XSK_WRITE_ONCE(x, v) (*(volatile typeof(x) *)&x) = (v)
+
+#if defined(__i386__) || defined(__x86_64__)
+# define libbpf_smp_store_release(p, v)					\
+	do {								\
+		asm volatile("" : : : "memory");			\
+		__XSK_WRITE_ONCE(*p, v);				\
+	} while (0)
+# define libbpf_smp_load_acquire(p)					\
+	({								\
+		typeof(*p) ___p1 = __XSK_READ_ONCE(*p);			\
+		asm volatile("" : : : "memory");			\
+		___p1;							\
+	})
+#elif defined(__aarch64__)
+# define libbpf_smp_store_release(p, v)					\
+		asm volatile ("stlr %w1, %0" : "=Q" (*p) : "r" (v) : "memory")
+# define libbpf_smp_load_acquire(p)					\
+	({								\
+		typeof(*p) ___p1;					\
+		asm volatile ("ldar %w0, %1"				\
+			      : "=r" (___p1) : "Q" (*p) : "memory");	\
+		___p1;							\
+	})
+#elif defined(__riscv)
+# define libbpf_smp_store_release(p, v)					\
+	do {								\
+		asm volatile ("fence rw,w" : : : "memory");		\
+		__XSK_WRITE_ONCE(*p, v);				\
+	} while (0)
+# define libbpf_smp_load_acquire(p)					\
+	({								\
+		typeof(*p) ___p1 = __XSK_READ_ONCE(*p);			\
+		asm volatile ("fence r,rw" : : : "memory");		\
+		___p1;							\
+	})
+#endif
+
+#ifndef libbpf_smp_store_release
+#define libbpf_smp_store_release(p, v)					\
+	do {								\
+		__sync_synchronize();					\
+		__XSK_WRITE_ONCE(*p, v);				\
+	} while (0)
+#endif
+
+#ifndef libbpf_smp_load_acquire
+#define libbpf_smp_load_acquire(p)					\
+	({								\
+		typeof(*p) ___p1 = __XSK_READ_ONCE(*p);			\
+		__sync_synchronize();					\
+		___p1;							\
+	})
+#endif
+
+/* LIBRARY INTERNAL -- END */
+
 /* Do not access these members directly. Use the functions below. */
 #define DEFINE_XSK_RING(name) \
 struct name { \

-- 
2.27.0

Re: [PATCH bpf-next 2/2] libbpf: xsk: move barriers from libbpf_util.h to xsk.h

From: Andrii Nakryiko <hidden>
Date: 2021-03-10 22:01:37

On Wed, Mar 10, 2021 at 12:09 AM Björn Töpel [off-list ref] wrote:

From: Björn Töpel <redacted>

The only user of libbpf_util.h is xsk.h. Move the barriers to xsk.h,
and remove libbpf_util.h. The barriers are used as an implementation
detail, and should not be considered part of the stable API.

Signed-off-by: Björn Töpel <redacted>
---
 tools/lib/bpf/Makefile      |  1 -
 tools/lib/bpf/libbpf_util.h | 82 -------------------------------------
 tools/lib/bpf/xsk.h         | 68 +++++++++++++++++++++++++++++-
 3 files changed, 67 insertions(+), 84 deletions(-)
 delete mode 100644 tools/lib/bpf/libbpf_util.h

[...]

quoted hunk

diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index a9fdea87b5cd..1d846a419d21 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h

@@ -4,6 +4,7 @@
  * AF_XDP user-space access library.
  *
  * Copyright(c) 2018 - 2019 Intel Corporation.

Couldn't unsee ^this mismatch. Added space there.

Also, removing libbpf_util.h caused incremental build failure for the
kernel due to cached dependency files. make clean solved the issue. So
just FYI for everyone.

Applied to bpf-next, thanks.

quoted hunk

+ * Copyright (c) 2019 Facebook
  *
  * Author(s): Magnus Karlsson [off-list ref]
  */

@@ -13,15 +14,80 @@

 #include <stdio.h>
 #include <stdint.h>
+#include <stdbool.h>
 #include <linux/if_xdp.h>

 #include "libbpf.h"
-#include "libbpf_util.h"

 #ifdef __cplusplus
 extern "C" {
 #endif

[...]

Re: [PATCH bpf-next 2/2] libbpf: xsk: move barriers from libbpf_util.h to xsk.h

From: Jonathan Lemon <hidden>
Date: 2021-03-11 00:13:25

On Wed, Mar 10, 2021 at 09:09:29AM +0100, Björn Töpel wrote:

From: Björn Töpel <redacted>

The only user of libbpf_util.h is xsk.h. Move the barriers to xsk.h,
and remove libbpf_util.h. The barriers are used as an implementation
detail, and should not be considered part of the stable API.

Does that mean that anything else which uses the same type of
shared rings (bpf ringbuffer, io_uring, zctap) have to implement
the same primitives that xsk.h has?
-- 
Jonathan

Re: [PATCH bpf-next 2/2] libbpf: xsk: move barriers from libbpf_util.h to xsk.h

From: Björn Töpel <hidden>
Date: 2021-03-11 07:00:55

On 2021-03-11 01:06, Jonathan Lemon wrote:

On Wed, Mar 10, 2021 at 09:09:29AM +0100, Björn Töpel wrote:

quoted

From: Björn Töpel <redacted>

The only user of libbpf_util.h is xsk.h. Move the barriers to xsk.h,
and remove libbpf_util.h. The barriers are used as an implementation
detail, and should not be considered part of the stable API.

Does that mean that anything else which uses the same type of
shared rings (bpf ringbuffer, io_uring, zctap) have to implement
the same primitives that xsk.h has?

Jonathan, there's a longer explanation on back-/forward-compatibility in
the commit message [1]. Again, this is for the XDP socket rings, so I
wont comment on the other rings. I would not assume compatibility
between different rings (e.g. the bpf ringbuffer and XDP sockets rings),
not even prior the barrier change.


Björn

[1] 
https://lore.kernel.org/bpf/20210305094113.413544-2-bjorn.topel@gmail.com/

Re: [PATCH bpf-next 2/2] libbpf: xsk: move barriers from libbpf_util.h to xsk.h

From: Andrii Nakryiko <hidden>
Date: 2021-03-11 18:58:54

On Wed, Mar 10, 2021 at 10:59 PM Björn Töpel [off-list ref] wrote:

On 2021-03-11 01:06, Jonathan Lemon wrote:

quoted

On Wed, Mar 10, 2021 at 09:09:29AM +0100, Björn Töpel wrote:

quoted

From: Björn Töpel <redacted>

The only user of libbpf_util.h is xsk.h. Move the barriers to xsk.h,
and remove libbpf_util.h. The barriers are used as an implementation
detail, and should not be considered part of the stable API.

Does that mean that anything else which uses the same type of
shared rings (bpf ringbuffer, io_uring, zctap) have to implement
the same primitives that xsk.h has?

Jonathan, there's a longer explanation on back-/forward-compatibility in
the commit message [1]. Again, this is for the XDP socket rings, so I
wont comment on the other rings. I would not assume compatibility
between different rings (e.g. the bpf ringbuffer and XDP sockets rings),
not even prior the barrier change.

BPF ringbuf is using smp_store_release()/smp_load_acquire(), which are
coming from asm/barrier.h. But libbpf abstracts all the low-level
details, so users don't have to use such low-level primitives
directly.

Björn

[1]
https://lore.kernel.org/bpf/20210305094113.413544-2-bjorn.topel@gmail.com/

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help