[PATCH bpf 0/2] libbpf: remove two dependencies on Linux kernel headers and improve performance as a bonus

[PATCH bpf 0/2] libbpf: remove two dependencies on Linux kernel headers and improve performance as a bonus

[Magnus Karlsson]

This patch set removes two dependencies on Linux kernel headers
from the XDP socket code in libbpf. A number of people have pointed
out that these two dependencies make it hard to build the XDP socket
part of libbpf without any kernel header dependencies. The two removed
dependecies are:

* Remove the usage of likely and unlikely (compiler.h) in xsk.h. It
  has been reported that the use of these actually decreases the
  performance of the ring access code due to an increase in
  instruction cache misses, so let us just remove these.

* Remove the dependency on barrier.h as it brings in a lot of kernel
  headers. As the XDP socket code only uses two simple functions from
  it, we can reimplement these. As a bonus, the new implementation is
  faster as it uses the same barrier primitives as the kernel does
  when the same code is compiled there. Without this patch, the user
  land code uses lfence and sfence on x86, which are unecessarily
  harsh/thorough.

Do the ARM barriers look sane? Have not tested these.

/Magnus

Magnus Karlsson (2):
  libbpf: remove likely/unlikely in xsk.h
  libbpf: remove dependency on barrier.h in xsk.h

 tools/lib/bpf/xsk.h | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

--
2.7.4

[PATCH bpf 1/2] libbpf: remove likely/unlikely in xsk.h

[Magnus Karlsson]

This patch removes the use of likely and unlikely in xsk.h since they
create a dependency on Linux headers as reported by several
users. There have also been reports that the use of these decreases
performance as the compiler puts the code on two different cache lines
instead of on a single one. All in all, I think we are better off
without them.

Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/lib/bpf/xsk.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index a497f00..3638147 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -105,7 +105,7 @@ static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, __u32 nb)
 static inline size_t xsk_ring_prod__reserve(struct xsk_ring_prod *prod,
 					    size_t nb, __u32 *idx)
 {
-	if (unlikely(xsk_prod_nb_free(prod, nb) < nb))
+	if (xsk_prod_nb_free(prod, nb) < nb)
 		return 0;
 
 	*idx = prod->cached_prod;
@@ -129,7 +129,7 @@ static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
 {
 	size_t entries = xsk_cons_nb_avail(cons, nb);
 
-	if (likely(entries > 0)) {
+	if (entries > 0) {
 		/* Make sure we do not speculatively read the data before
 		 * we have received the packet buffers from the ring.
 		 */
-- 
2.7.4

[PATCH bpf 2/2] libbpf: remove dependency on barrier.h in xsk.h

[Magnus Karlsson]

The use of smp_rmb() and smp_wmb() creates a Linux header dependency
on barrier.h that is uneccessary in most parts. This patch implements
the two small defines that are needed from barrier.h. As a bonus, the
new implementations are faster than the default ones as they default
to sfence and lfence for x86, while we only need a compiler barrier in
our case. Just as it is when the same ring access code is compiled in
the kernel.

Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/lib/bpf/xsk.h | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 3638147..317b44f 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -39,6 +39,21 @@ DEFINE_XSK_RING(xsk_ring_cons);
 struct xsk_umem;
 struct xsk_socket;
 
+#if !defined bpf_smp_rmb && !defined bpf_smp_wmb
+# if defined(__i386__) || defined(__x86_64__)
+#  define bpf_smp_rmb() asm volatile("" : : : "memory")
+#  define bpf_smp_wmb() asm volatile("" : : : "memory")
+# elif defined(__aarch64__)
+#  define bpf_smp_rmb() asm volatile("dmb ishld" : : : "memory")
+#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
+# elif defined(__arm__)
+#  define bpf_smp_rmb() asm volatile("dmb ish" : : : "memory")
+#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
+# else
+#  error Architecture not supported by the XDP socket code in libbpf.
+# endif
+#endif
+
 static inline __u64 *xsk_ring_prod__fill_addr(struct xsk_ring_prod *fill,
 					      __u32 idx)
 {
@@ -119,7 +134,7 @@ static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
 	/* Make sure everything has been written to the ring before signalling
 	 * this to the kernel.
 	 */
-	smp_wmb();
+	bpf_smp_wmb();
 
 	*prod->producer += nb;
 }
@@ -133,7 +148,7 @@ static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
 		/* Make sure we do not speculatively read the data before
 		 * we have received the packet buffers from the ring.
 		 */
-		smp_rmb();
+		bpf_smp_rmb();
 
 		*idx = cons->cached_cons;
 		cons->cached_cons += entries;
-- 
2.7.4

Re: [PATCH bpf 2/2] libbpf: remove dependency on barrier.h in xsk.h

[Daniel Borkmann]

On 04/09/2019 08:44 AM, Magnus Karlsson wrote:
> The use of smp_rmb() and smp_wmb() creates a Linux header dependency
> on barrier.h that is uneccessary in most parts. This patch implements
> the two small defines that are needed from barrier.h. As a bonus, the
> new implementations are faster than the default ones as they default
> to sfence and lfence for x86, while we only need a compiler barrier in
> our case. Just as it is when the same ring access code is compiled in
> the kernel.
> 
> Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>  tools/lib/bpf/xsk.h | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> index 3638147..317b44f 100644
> --- a/tools/lib/bpf/xsk.h
> +++ b/tools/lib/bpf/xsk.h
> @@ -39,6 +39,21 @@ DEFINE_XSK_RING(xsk_ring_cons);
>  struct xsk_umem;
>  struct xsk_socket;
>  
> +#if !defined bpf_smp_rmb && !defined bpf_smp_wmb
> +# if defined(__i386__) || defined(__x86_64__)
> +#  define bpf_smp_rmb() asm volatile("" : : : "memory")
> +#  define bpf_smp_wmb() asm volatile("" : : : "memory")
> +# elif defined(__aarch64__)
> +#  define bpf_smp_rmb() asm volatile("dmb ishld" : : : "memory")
> +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
> +# elif defined(__arm__)
> +#  define bpf_smp_rmb() asm volatile("dmb ish" : : : "memory")
> +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
> +# else
> +#  error Architecture not supported by the XDP socket code in libbpf.
> +# endif
> +#endif

Hmm, bit unfortunate, but I can see why it's needed inside the header here
as it's installed into the system and needs to be included by applications.
Fine with me. In any case, I still think we should also fix it for tooling
infra to avoid others running into the same issue, lemme resurrect [0] from
back in the days.

One question though regarding above arm32 implementation. You match these
barriers against the ones from af_xdp in kernel. So smp_* variants from the
arch/arm/include/asm/barrier.h header need to be matched. This selects the
dmb(ish) and dmb(ishst) variant. The implementation of dmb() however is
quite CPU dependent, the above assumes to match with __LINUX_ARM_ARCH__ >= 7.
Perhaps this assumption needs at minimum a comment to make it clear for
developers.

  [0] https://lore.kernel.org/netdev/20181017144156.16639-2-daniel@iogearbox.net/

>  static inline __u64 *xsk_ring_prod__fill_addr(struct xsk_ring_prod *fill,
>  					      __u32 idx)
>  {
> @@ -119,7 +134,7 @@ static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
>  	/* Make sure everything has been written to the ring before signalling
>  	 * this to the kernel.
>  	 */
> -	smp_wmb();
> +	bpf_smp_wmb();
>  
>  	*prod->producer += nb;
>  }
> @@ -133,7 +148,7 @@ static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
>  		/* Make sure we do not speculatively read the data before
>  		 * we have received the packet buffers from the ring.
>  		 */
> -		smp_rmb();
> +		bpf_smp_rmb();
>  
>  		*idx = cons->cached_cons;
>  		cons->cached_cons += entries;
> 

Re: [PATCH bpf 2/2] libbpf: remove dependency on barrier.h in xsk.h

[Magnus Karlsson]

On Tue, Apr 9, 2019 at 11:11 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 04/09/2019 08:44 AM, Magnus Karlsson wrote:
> > The use of smp_rmb() and smp_wmb() creates a Linux header dependency
> > on barrier.h that is uneccessary in most parts. This patch implements
> > the two small defines that are needed from barrier.h. As a bonus, the
> > new implementations are faster than the default ones as they default
> > to sfence and lfence for x86, while we only need a compiler barrier in
> > our case. Just as it is when the same ring access code is compiled in
> > the kernel.
> >
> > Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >  tools/lib/bpf/xsk.h | 19 +++++++++++++++++--
> >  1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> > index 3638147..317b44f 100644
> > --- a/tools/lib/bpf/xsk.h
> > +++ b/tools/lib/bpf/xsk.h
> > @@ -39,6 +39,21 @@ DEFINE_XSK_RING(xsk_ring_cons);
> >  struct xsk_umem;
> >  struct xsk_socket;
> >
> > +#if !defined bpf_smp_rmb && !defined bpf_smp_wmb
> > +# if defined(__i386__) || defined(__x86_64__)
> > +#  define bpf_smp_rmb() asm volatile("" : : : "memory")
> > +#  define bpf_smp_wmb() asm volatile("" : : : "memory")
> > +# elif defined(__aarch64__)
> > +#  define bpf_smp_rmb() asm volatile("dmb ishld" : : : "memory")
> > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
> > +# elif defined(__arm__)
> > +#  define bpf_smp_rmb() asm volatile("dmb ish" : : : "memory")
> > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
> > +# else
> > +#  error Architecture not supported by the XDP socket code in libbpf.
> > +# endif
> > +#endif
>
> Hmm, bit unfortunate, but I can see why it's needed inside the header here
> as it's installed into the system and needs to be included by applications.
> Fine with me. In any case, I still think we should also fix it for tooling
> infra to avoid others running into the same issue, lemme resurrect [0] from
> back in the days.
>
> One question though regarding above arm32 implementation. You match these
> barriers against the ones from af_xdp in kernel. So smp_* variants from the
> arch/arm/include/asm/barrier.h header need to be matched. This selects the
> dmb(ish) and dmb(ishst) variant. The implementation of dmb() however is
> quite CPU dependent, the above assumes to match with __LINUX_ARM_ARCH__ >= 7.
> Perhaps this assumption needs at minimum a comment to make it clear for
> developers.

Good idea. I will put a comment in there to clarify this.

/Magnus

>   [0] https://lore.kernel.org/netdev/20181017144156.16639-2-daniel@iogearbox.net/
>
> >  static inline __u64 *xsk_ring_prod__fill_addr(struct xsk_ring_prod *fill,
> >                                             __u32 idx)
> >  {
> > @@ -119,7 +134,7 @@ static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
> >       /* Make sure everything has been written to the ring before signalling
> >        * this to the kernel.
> >        */
> > -     smp_wmb();
> > +     bpf_smp_wmb();
> >
> >       *prod->producer += nb;
> >  }
> > @@ -133,7 +148,7 @@ static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
> >               /* Make sure we do not speculatively read the data before
> >                * we have received the packet buffers from the ring.
> >                */
> > -             smp_rmb();
> > +             bpf_smp_rmb();
> >
> >               *idx = cons->cached_cons;
> >               cons->cached_cons += entries;
> >
>

Re: [PATCH bpf 2/2] libbpf: remove dependency on barrier.h in xsk.h

[Georg Müller]

Am 09.04.19 um 13:29 schrieb Magnus Karlsson:
> On Tue, Apr 9, 2019 at 11:11 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>
>> On 04/09/2019 08:44 AM, Magnus Karlsson wrote:
>>> The use of smp_rmb() and smp_wmb() creates a Linux header dependency
>>> on barrier.h that is uneccessary in most parts. This patch implements
>>> the two small defines that are needed from barrier.h. As a bonus, the
>>> new implementations are faster than the default ones as they default
>>> to sfence and lfence for x86, while we only need a compiler barrier in
>>> our case. Just as it is when the same ring access code is compiled in
>>> the kernel.
>>>
>>> Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
>>> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
>>> ---
>>>  tools/lib/bpf/xsk.h | 19 +++++++++++++++++--
>>>  1 file changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
>>> index 3638147..317b44f 100644
>>> --- a/tools/lib/bpf/xsk.h
>>> +++ b/tools/lib/bpf/xsk.h
>>> @@ -39,6 +39,21 @@ DEFINE_XSK_RING(xsk_ring_cons);
>>>  struct xsk_umem;
>>>  struct xsk_socket;
>>>
>>> +#if !defined bpf_smp_rmb && !defined bpf_smp_wmb
>>> +# if defined(__i386__) || defined(__x86_64__)
>>> +#  define bpf_smp_rmb() asm volatile("" : : : "memory")
>>> +#  define bpf_smp_wmb() asm volatile("" : : : "memory")
>>> +# elif defined(__aarch64__)
>>> +#  define bpf_smp_rmb() asm volatile("dmb ishld" : : : "memory")
>>> +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
>>> +# elif defined(__arm__)
>>> +#  define bpf_smp_rmb() asm volatile("dmb ish" : : : "memory")
>>> +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
>>> +# else
>>> +#  error Architecture not supported by the XDP socket code in libbpf.
>>> +# endif
>>> +#endif
>>
>> Hmm, bit unfortunate, but I can see why it's needed inside the header here
>> as it's installed into the system and needs to be included by applications.
>> Fine with me. In any case, I still think we should also fix it for tooling
>> infra to avoid others running into the same issue, lemme resurrect [0] from
>> back in the days.
>>
>> One question though regarding above arm32 implementation. You match these
>> barriers against the ones from af_xdp in kernel. So smp_* variants from the
>> arch/arm/include/asm/barrier.h header need to be matched. This selects the
>> dmb(ish) and dmb(ishst) variant. The implementation of dmb() however is
>> quite CPU dependent, the above assumes to match with __LINUX_ARM_ARCH__ >= 7.
>> Perhaps this assumption needs at minimum a comment to make it clear for
>> developers.
>
> Good idea. I will put a comment in there to clarify this.
>
> /Magnus


Since this is a userspace library, what about using the C11 atomic_thread_fence()
from stdatomic.h - wouldn't that be sufficient?


>>   [0] https://lore.kernel.org/netdev/20181017144156.16639-2-daniel@iogearbox.net/
>>
>>>  static inline __u64 *xsk_ring_prod__fill_addr(struct xsk_ring_prod *fill,
>>>                                             __u32 idx)
>>>  {
>>> @@ -119,7 +134,7 @@ static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
>>>       /* Make sure everything has been written to the ring before signalling
>>>        * this to the kernel.
>>>        */
>>> -     smp_wmb();
>>> +     bpf_smp_wmb();
>>>
>>>       *prod->producer += nb;
>>>  }
>>> @@ -133,7 +148,7 @@ static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
>>>               /* Make sure we do not speculatively read the data before
>>>                * we have received the packet buffers from the ring.
>>>                */
>>> -             smp_rmb();
>>> +             bpf_smp_rmb();
>>>
>>>               *idx = cons->cached_cons;
>>>               cons->cached_cons += entries;
>>>
>>

Re: [PATCH bpf 2/2] libbpf: remove dependency on barrier.h in xsk.h

[Daniel Borkmann]

On 04/10/2019 12:28 AM, Georg Müller wrote:
> Am 09.04.19 um 13:29 schrieb Magnus Karlsson:
>> On Tue, Apr 9, 2019 at 11:11 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>> On 04/09/2019 08:44 AM, Magnus Karlsson wrote:
>>>> The use of smp_rmb() and smp_wmb() creates a Linux header dependency
>>>> on barrier.h that is uneccessary in most parts. This patch implements
>>>> the two small defines that are needed from barrier.h. As a bonus, the
>>>> new implementations are faster than the default ones as they default
>>>> to sfence and lfence for x86, while we only need a compiler barrier in
>>>> our case. Just as it is when the same ring access code is compiled in
>>>> the kernel.
>>>>
>>>> Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
>>>> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
>>>> ---
>>>>  tools/lib/bpf/xsk.h | 19 +++++++++++++++++--
>>>>  1 file changed, 17 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
>>>> index 3638147..317b44f 100644
>>>> --- a/tools/lib/bpf/xsk.h
>>>> +++ b/tools/lib/bpf/xsk.h
>>>> @@ -39,6 +39,21 @@ DEFINE_XSK_RING(xsk_ring_cons);
>>>>  struct xsk_umem;
>>>>  struct xsk_socket;
>>>>
>>>> +#if !defined bpf_smp_rmb && !defined bpf_smp_wmb
>>>> +# if defined(__i386__) || defined(__x86_64__)
>>>> +#  define bpf_smp_rmb() asm volatile("" : : : "memory")
>>>> +#  define bpf_smp_wmb() asm volatile("" : : : "memory")
>>>> +# elif defined(__aarch64__)
>>>> +#  define bpf_smp_rmb() asm volatile("dmb ishld" : : : "memory")
>>>> +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
>>>> +# elif defined(__arm__)
>>>> +#  define bpf_smp_rmb() asm volatile("dmb ish" : : : "memory")
>>>> +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
>>>> +# else
>>>> +#  error Architecture not supported by the XDP socket code in libbpf.
>>>> +# endif
>>>> +#endif
>>>
>>> Hmm, bit unfortunate, but I can see why it's needed inside the header here
>>> as it's installed into the system and needs to be included by applications.
>>> Fine with me. In any case, I still think we should also fix it for tooling
>>> infra to avoid others running into the same issue, lemme resurrect [0] from
>>> back in the days.
>>>
>>> One question though regarding above arm32 implementation. You match these
>>> barriers against the ones from af_xdp in kernel. So smp_* variants from the
>>> arch/arm/include/asm/barrier.h header need to be matched. This selects the
>>> dmb(ish) and dmb(ishst) variant. The implementation of dmb() however is
>>> quite CPU dependent, the above assumes to match with __LINUX_ARM_ARCH__ >= 7.
>>> Perhaps this assumption needs at minimum a comment to make it clear for
>>> developers.
>>
>> Good idea. I will put a comment in there to clarify this.
> 
> Since this is a userspace library, what about using the C11 atomic_thread_fence()
> from stdatomic.h - wouldn't that be sufficient?

These are paired with smp_{w,r}mb() in AF_XDP's kernel code, therefore they
would need the exact same counterpart in user space. We had similar discussion
earlier wrt perf ring buffer, conclusion was: the C11 memory model doesn't
generally align with the kernel's, but even if it would emit the same code,
it's still implementation dependent from compiler side, so one would need to
have a guarantee that the mapping from supported compilers must be compatible
at all times to the kernels for the various supported archs.

>>>   [0] https://lore.kernel.org/netdev/20181017144156.16639-2-daniel@iogearbox.net/
>>>
>>>>  static inline __u64 *xsk_ring_prod__fill_addr(struct xsk_ring_prod *fill,
>>>>                                             __u32 idx)
>>>>  {
>>>> @@ -119,7 +134,7 @@ static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
>>>>       /* Make sure everything has been written to the ring before signalling
>>>>        * this to the kernel.
>>>>        */
>>>> -     smp_wmb();
>>>> +     bpf_smp_wmb();
>>>>
>>>>       *prod->producer += nb;
>>>>  }
>>>> @@ -133,7 +148,7 @@ static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
>>>>               /* Make sure we do not speculatively read the data before
>>>>                * we have received the packet buffers from the ring.
>>>>                */
>>>> -             smp_rmb();
>>>> +             bpf_smp_rmb();
>>>>
>>>>               *idx = cons->cached_cons;
>>>>               cons->cached_cons += entries;
>>>>
>>>

Re: [PATCH bpf 2/2] libbpf: remove dependency on barrier.h in xsk.h

[Yauheni Kaliuta]

Hi, Magnus!

>>>>> On Tue,  9 Apr 2019 08:44:13 +0200, Magnus Karlsson  wrote:

 > The use of smp_rmb() and smp_wmb() creates a Linux header dependency
 > on barrier.h that is uneccessary in most parts. This patch implements
 > the two small defines that are needed from barrier.h. As a bonus, the
 > new implementations are faster than the default ones as they default
 > to sfence and lfence for x86, while we only need a compiler barrier in
 > our case. Just as it is when the same ring access code is compiled in
 > the kernel.

 > Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
 > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
 > ---
 >  tools/lib/bpf/xsk.h | 19 +++++++++++++++++--
 >  1 file changed, 17 insertions(+), 2 deletions(-)

 > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
 > index 3638147..317b44f 100644
 > --- a/tools/lib/bpf/xsk.h
 > +++ b/tools/lib/bpf/xsk.h
 > @@ -39,6 +39,21 @@ DEFINE_XSK_RING(xsk_ring_cons);
 >  struct xsk_umem;
 >  struct xsk_socket;
 
 > +#if !defined bpf_smp_rmb && !defined bpf_smp_wmb
 > +# if defined(__i386__) || defined(__x86_64__)
 > +#  define bpf_smp_rmb() asm volatile("" : : : "memory")
 > +#  define bpf_smp_wmb() asm volatile("" : : : "memory")
 > +# elif defined(__aarch64__)
 > +#  define bpf_smp_rmb() asm volatile("dmb ishld" : : : "memory")
 > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
 > +# elif defined(__arm__)
 > +#  define bpf_smp_rmb() asm volatile("dmb ish" : : : "memory")
 > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
 > +# else
 > +#  error Architecture not supported by the XDP socket code in libbpf.
 > +# endif
 > +#endif
 > +

What about other architectures then?


 >  static inline __u64 *xsk_ring_prod__fill_addr(struct xsk_ring_prod *fill,
 >  					      __u32 idx)
 >  {
 > @@ -119,7 +134,7 @@ static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
 >  	/* Make sure everything has been written to the ring before signalling
 >  	 * this to the kernel.
 >  	 */
 > -	smp_wmb();
 > +	bpf_smp_wmb();
 
 >  	*prod->producer += nb;
 >  }
 > @@ -133,7 +148,7 @@ static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
 >  		/* Make sure we do not speculatively read the data before
 >  		 * we have received the packet buffers from the ring.
 >  		 */
 > -		smp_rmb();
 > +		bpf_smp_rmb();
 
 >  		*idx = cons->cached_cons;
 cons-> cached_cons += entries;
 > -- 
 > 2.7.4


-- 
WBR,
Yauheni Kaliuta

Re: [PATCH bpf 2/2] libbpf: remove dependency on barrier.h in xsk.h

[Magnus Karlsson]

On Wed, Sep 4, 2019 at 7:32 AM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
>
> Hi, Magnus!
>
> >>>>> On Tue,  9 Apr 2019 08:44:13 +0200, Magnus Karlsson  wrote:
>
>  > The use of smp_rmb() and smp_wmb() creates a Linux header dependency
>  > on barrier.h that is uneccessary in most parts. This patch implements
>  > the two small defines that are needed from barrier.h. As a bonus, the
>  > new implementations are faster than the default ones as they default
>  > to sfence and lfence for x86, while we only need a compiler barrier in
>  > our case. Just as it is when the same ring access code is compiled in
>  > the kernel.
>
>  > Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
>  > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
>  > ---
>  >  tools/lib/bpf/xsk.h | 19 +++++++++++++++++--
>  >  1 file changed, 17 insertions(+), 2 deletions(-)
>
>  > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
>  > index 3638147..317b44f 100644
>  > --- a/tools/lib/bpf/xsk.h
>  > +++ b/tools/lib/bpf/xsk.h
>  > @@ -39,6 +39,21 @@ DEFINE_XSK_RING(xsk_ring_cons);
>  >  struct xsk_umem;
>  >  struct xsk_socket;
>
>  > +#if !defined bpf_smp_rmb && !defined bpf_smp_wmb
>  > +# if defined(__i386__) || defined(__x86_64__)
>  > +#  define bpf_smp_rmb() asm volatile("" : : : "memory")
>  > +#  define bpf_smp_wmb() asm volatile("" : : : "memory")
>  > +# elif defined(__aarch64__)
>  > +#  define bpf_smp_rmb() asm volatile("dmb ishld" : : : "memory")
>  > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
>  > +# elif defined(__arm__)
>  > +#  define bpf_smp_rmb() asm volatile("dmb ish" : : : "memory")
>  > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
>  > +# else
>  > +#  error Architecture not supported by the XDP socket code in libbpf.
>  > +# endif
>  > +#endif
>  > +
>
> What about other architectures then?

AF_XDP has not been tested on anything else, as far as I know. But
contributions that extend it to more archs are very welcome.

/Magnus

>
>  >  static inline __u64 *xsk_ring_prod__fill_addr(struct xsk_ring_prod *fill,
>  >                                            __u32 idx)
>  >  {
>  > @@ -119,7 +134,7 @@ static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
>  >      /* Make sure everything has been written to the ring before signalling
>  >       * this to the kernel.
>  >       */
>  > -    smp_wmb();
>  > +    bpf_smp_wmb();
>
>  >      *prod->producer += nb;
>  >  }
>  > @@ -133,7 +148,7 @@ static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
>  >              /* Make sure we do not speculatively read the data before
>  >               * we have received the packet buffers from the ring.
>  >               */
>  > -            smp_rmb();
>  > +            bpf_smp_rmb();
>
>  >              *idx = cons->cached_cons;
>  cons-> cached_cons += entries;
>  > --
>  > 2.7.4
>
>
> --
> WBR,
> Yauheni Kaliuta

Re: [PATCH bpf 2/2] libbpf: remove dependency on barrier.h in xsk.h

[Yauheni Kaliuta]

Hi, Magnus!

>>>>> On Wed, 4 Sep 2019 08:39:24 +0200, Magnus Karlsson  wrote:

 > On Wed, Sep 4, 2019 at 7:32 AM Yauheni Kaliuta
 > <yauheni.kaliuta@redhat.com> wrote:
 >> 
 >> Hi, Magnus!
 >> 
 >> >>>>> On Tue,  9 Apr 2019 08:44:13 +0200, Magnus Karlsson  wrote:
 >> 
 >> > The use of smp_rmb() and smp_wmb() creates a Linux header dependency
 >> > on barrier.h that is uneccessary in most parts. This patch implements
 >> > the two small defines that are needed from barrier.h. As a bonus, the
 >> > new implementations are faster than the default ones as they default
 >> > to sfence and lfence for x86, while we only need a compiler barrier in
 >> > our case. Just as it is when the same ring access code is compiled in
 >> > the kernel.
 >> 
 >> > Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
 >> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
 >> > ---
 >> >  tools/lib/bpf/xsk.h | 19 +++++++++++++++++--
 >> >  1 file changed, 17 insertions(+), 2 deletions(-)
 >> 
 >> > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
 >> > index 3638147..317b44f 100644
 >> > --- a/tools/lib/bpf/xsk.h
 >> > +++ b/tools/lib/bpf/xsk.h
 >> > @@ -39,6 +39,21 @@ DEFINE_XSK_RING(xsk_ring_cons);
 >> >  struct xsk_umem;
 >> >  struct xsk_socket;
 >> 
 >> > +#if !defined bpf_smp_rmb && !defined bpf_smp_wmb
 >> > +# if defined(__i386__) || defined(__x86_64__)
 >> > +#  define bpf_smp_rmb() asm volatile("" : : : "memory")
 >> > +#  define bpf_smp_wmb() asm volatile("" : : : "memory")
 >> > +# elif defined(__aarch64__)
 >> > +#  define bpf_smp_rmb() asm volatile("dmb ishld" : : : "memory")
 >> > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
 >> > +# elif defined(__arm__)
 >> > +#  define bpf_smp_rmb() asm volatile("dmb ish" : : : "memory")
 >> > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
 >> > +# else
 >> > +#  error Architecture not supported by the XDP socket code in libbpf.
 >> > +# endif
 >> > +#endif
 >> > +
 >> 
 >> What about other architectures then?

 > AF_XDP has not been tested on anything else, as far as I
 > know. But contributions that extend it to more archs are very
 > welcome.

Well, I'll may be try to fetch something from barrier.h's (since
I cannot consider myself as a specialist in the area), but at the
moment the patch breaks the build on that arches.

 > /Magnus

 >> 
 >> >  static inline __u64 *xsk_ring_prod__fill_addr(struct xsk_ring_prod *fill,
 >> >                                            __u32 idx)
 >> >  {
 >> > @@ -119,7 +134,7 @@ static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
 >> >      /* Make sure everything has been written to the ring before signalling
 >> >       * this to the kernel.
 >> >       */
 >> > -    smp_wmb();
 >> > +    bpf_smp_wmb();
 >> 
 >> >      *prod->producer += nb;
 >> >  }
 >> > @@ -133,7 +148,7 @@ static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
 >> >              /* Make sure we do not speculatively read the data before
 >> >               * we have received the packet buffers from the ring.
 >> >               */
 >> > -            smp_rmb();
 >> > +            bpf_smp_rmb();
 >> 
 >> >              *idx = cons->cached_cons;
 cons-> cached_cons += entries;
 >> > --
 >> > 2.7.4
 >> 
 >> 
 >> --
 >> WBR,
 >> Yauheni Kaliuta

-- 
WBR,
Yauheni Kaliuta

Re: [PATCH bpf 2/2] libbpf: remove dependency on barrier.h in xsk.h

[Magnus Karlsson]

On Wed, Sep 4, 2019 at 8:56 AM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
>
> Hi, Magnus!
>
> >>>>> On Wed, 4 Sep 2019 08:39:24 +0200, Magnus Karlsson  wrote:
>
>  > On Wed, Sep 4, 2019 at 7:32 AM Yauheni Kaliuta
>  > <yauheni.kaliuta@redhat.com> wrote:
>  >>
>  >> Hi, Magnus!
>  >>
>  >> >>>>> On Tue,  9 Apr 2019 08:44:13 +0200, Magnus Karlsson  wrote:
>  >>
>  >> > The use of smp_rmb() and smp_wmb() creates a Linux header dependency
>  >> > on barrier.h that is uneccessary in most parts. This patch implements
>  >> > the two small defines that are needed from barrier.h. As a bonus, the
>  >> > new implementations are faster than the default ones as they default
>  >> > to sfence and lfence for x86, while we only need a compiler barrier in
>  >> > our case. Just as it is when the same ring access code is compiled in
>  >> > the kernel.
>  >>
>  >> > Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
>  >> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
>  >> > ---
>  >> >  tools/lib/bpf/xsk.h | 19 +++++++++++++++++--
>  >> >  1 file changed, 17 insertions(+), 2 deletions(-)
>  >>
>  >> > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
>  >> > index 3638147..317b44f 100644
>  >> > --- a/tools/lib/bpf/xsk.h
>  >> > +++ b/tools/lib/bpf/xsk.h
>  >> > @@ -39,6 +39,21 @@ DEFINE_XSK_RING(xsk_ring_cons);
>  >> >  struct xsk_umem;
>  >> >  struct xsk_socket;
>  >>
>  >> > +#if !defined bpf_smp_rmb && !defined bpf_smp_wmb
>  >> > +# if defined(__i386__) || defined(__x86_64__)
>  >> > +#  define bpf_smp_rmb() asm volatile("" : : : "memory")
>  >> > +#  define bpf_smp_wmb() asm volatile("" : : : "memory")
>  >> > +# elif defined(__aarch64__)
>  >> > +#  define bpf_smp_rmb() asm volatile("dmb ishld" : : : "memory")
>  >> > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
>  >> > +# elif defined(__arm__)
>  >> > +#  define bpf_smp_rmb() asm volatile("dmb ish" : : : "memory")
>  >> > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
>  >> > +# else
>  >> > +#  error Architecture not supported by the XDP socket code in libbpf.
>  >> > +# endif
>  >> > +#endif
>  >> > +
>  >>
>  >> What about other architectures then?
>
>  > AF_XDP has not been tested on anything else, as far as I
>  > know. But contributions that extend it to more archs are very
>  > welcome.
>
> Well, I'll may be try to fetch something from barrier.h's (since
> I cannot consider myself as a specialist in the area), but at the
> moment the patch breaks the build on that arches.

Do you have a specific architecture in mind and do you have some
board/server (of that architecture) you could test AF_XDP on?

>  > /Magnus
>
>  >>
>  >> >  static inline __u64 *xsk_ring_prod__fill_addr(struct xsk_ring_prod *fill,
>  >> >                                            __u32 idx)
>  >> >  {
>  >> > @@ -119,7 +134,7 @@ static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
>  >> >      /* Make sure everything has been written to the ring before signalling
>  >> >       * this to the kernel.
>  >> >       */
>  >> > -    smp_wmb();
>  >> > +    bpf_smp_wmb();
>  >>
>  >> >      *prod->producer += nb;
>  >> >  }
>  >> > @@ -133,7 +148,7 @@ static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
>  >> >              /* Make sure we do not speculatively read the data before
>  >> >               * we have received the packet buffers from the ring.
>  >> >               */
>  >> > -            smp_rmb();
>  >> > +            bpf_smp_rmb();
>  >>
>  >> >              *idx = cons->cached_cons;
>  cons-> cached_cons += entries;
>  >> > --
>  >> > 2.7.4
>  >>
>  >>
>  >> --
>  >> WBR,
>  >> Yauheni Kaliuta
>
> --
> WBR,
> Yauheni Kaliuta

Re: [PATCH bpf 2/2] libbpf: remove dependency on barrier.h in xsk.h

[Yauheni Kaliuta]

Hi, Magnus!

>>>>> On Wed, 4 Sep 2019 12:25:13 +0200, Magnus Karlsson  wrote:
 > On Wed, Sep 4, 2019 at 8:56 AM Yauheni Kaliuta
 > <yauheni.kaliuta@redhat.com> wrote:
 >> 
 >> Hi, Magnus!
 >> 
 >> >>>>> On Wed, 4 Sep 2019 08:39:24 +0200, Magnus Karlsson  wrote:
 >> 
 >> > On Wed, Sep 4, 2019 at 7:32 AM Yauheni Kaliuta
 >> > <yauheni.kaliuta@redhat.com> wrote:
 >> >>
 >> >> Hi, Magnus!
 >> >>
 >> >> >>>>> On Tue,  9 Apr 2019 08:44:13 +0200, Magnus Karlsson  wrote:
 >> >>
 >> >> > The use of smp_rmb() and smp_wmb() creates a Linux header dependency
 >> >> > on barrier.h that is uneccessary in most parts. This patch implements
 >> >> > the two small defines that are needed from barrier.h. As a bonus, the
 >> >> > new implementations are faster than the default ones as they default
 >> >> > to sfence and lfence for x86, while we only need a compiler barrier in
 >> >> > our case. Just as it is when the same ring access code is compiled in
 >> >> > the kernel.
 >> >>
 >> >> > Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
 >> >> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
 >> >> > ---
 >> >> >  tools/lib/bpf/xsk.h | 19 +++++++++++++++++--
 >> >> >  1 file changed, 17 insertions(+), 2 deletions(-)
 >> >>
 >> >> > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
 >> >> > index 3638147..317b44f 100644
 >> >> > --- a/tools/lib/bpf/xsk.h
 >> >> > +++ b/tools/lib/bpf/xsk.h
 >> >> > @@ -39,6 +39,21 @@ DEFINE_XSK_RING(xsk_ring_cons);
 >> >> >  struct xsk_umem;
 >> >> >  struct xsk_socket;
 >> >>
 >> >> > +#if !defined bpf_smp_rmb && !defined bpf_smp_wmb
 >> >> > +# if defined(__i386__) || defined(__x86_64__)
 >> >> > +#  define bpf_smp_rmb() asm volatile("" : : : "memory")
 >> >> > +#  define bpf_smp_wmb() asm volatile("" : : : "memory")
 >> >> > +# elif defined(__aarch64__)
 >> >> > +#  define bpf_smp_rmb() asm volatile("dmb ishld" : : : "memory")
 >> >> > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
 >> >> > +# elif defined(__arm__)
 >> >> > +#  define bpf_smp_rmb() asm volatile("dmb ish" : : : "memory")
 >> >> > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
 >> >> > +# else
 >> >> > +#  error Architecture not supported by the XDP socket code in libbpf.
 >> >> > +# endif
 >> >> > +#endif
 >> >> > +
 >> >>
 >> >> What about other architectures then?
 >> 
 >> > AF_XDP has not been tested on anything else, as far as I
 >> > know. But contributions that extend it to more archs are
 >> > very welcome.
 >> 
 >> Well, I'll may be try to fetch something from barrier.h's
 >> (since I cannot consider myself as a specialist in the area),
 >> but at the moment the patch breaks the build on that arches.

 > Do you have a specific architecture in mind and do you have
 > some board/server (of that architecture) you could test AF_XDP
 > on?

I do care about s390 and ppc64 and I can run tests for them.


[...]

-- 
WBR,
Yauheni Kaliuta

Re: [PATCH bpf 2/2] libbpf: remove dependency on barrier.h in xsk.h

[Magnus Karlsson]

On Wed, Sep 4, 2019 at 2:19 PM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
>
> Hi, Magnus!
>
> >>>>> On Wed, 4 Sep 2019 12:25:13 +0200, Magnus Karlsson  wrote:
>  > On Wed, Sep 4, 2019 at 8:56 AM Yauheni Kaliuta
>  > <yauheni.kaliuta@redhat.com> wrote:
>  >>
>  >> Hi, Magnus!
>  >>
>  >> >>>>> On Wed, 4 Sep 2019 08:39:24 +0200, Magnus Karlsson  wrote:
>  >>
>  >> > On Wed, Sep 4, 2019 at 7:32 AM Yauheni Kaliuta
>  >> > <yauheni.kaliuta@redhat.com> wrote:
>  >> >>
>  >> >> Hi, Magnus!
>  >> >>
>  >> >> >>>>> On Tue,  9 Apr 2019 08:44:13 +0200, Magnus Karlsson  wrote:
>  >> >>
>  >> >> > The use of smp_rmb() and smp_wmb() creates a Linux header dependency
>  >> >> > on barrier.h that is uneccessary in most parts. This patch implements
>  >> >> > the two small defines that are needed from barrier.h. As a bonus, the
>  >> >> > new implementations are faster than the default ones as they default
>  >> >> > to sfence and lfence for x86, while we only need a compiler barrier in
>  >> >> > our case. Just as it is when the same ring access code is compiled in
>  >> >> > the kernel.
>  >> >>
>  >> >> > Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
>  >> >> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
>  >> >> > ---
>  >> >> >  tools/lib/bpf/xsk.h | 19 +++++++++++++++++--
>  >> >> >  1 file changed, 17 insertions(+), 2 deletions(-)
>  >> >>
>  >> >> > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
>  >> >> > index 3638147..317b44f 100644
>  >> >> > --- a/tools/lib/bpf/xsk.h
>  >> >> > +++ b/tools/lib/bpf/xsk.h
>  >> >> > @@ -39,6 +39,21 @@ DEFINE_XSK_RING(xsk_ring_cons);
>  >> >> >  struct xsk_umem;
>  >> >> >  struct xsk_socket;
>  >> >>
>  >> >> > +#if !defined bpf_smp_rmb && !defined bpf_smp_wmb
>  >> >> > +# if defined(__i386__) || defined(__x86_64__)
>  >> >> > +#  define bpf_smp_rmb() asm volatile("" : : : "memory")
>  >> >> > +#  define bpf_smp_wmb() asm volatile("" : : : "memory")
>  >> >> > +# elif defined(__aarch64__)
>  >> >> > +#  define bpf_smp_rmb() asm volatile("dmb ishld" : : : "memory")
>  >> >> > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
>  >> >> > +# elif defined(__arm__)
>  >> >> > +#  define bpf_smp_rmb() asm volatile("dmb ish" : : : "memory")
>  >> >> > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
>  >> >> > +# else
>  >> >> > +#  error Architecture not supported by the XDP socket code in libbpf.
>  >> >> > +# endif
>  >> >> > +#endif
>  >> >> > +
>  >> >>
>  >> >> What about other architectures then?
>  >>
>  >> > AF_XDP has not been tested on anything else, as far as I
>  >> > know. But contributions that extend it to more archs are
>  >> > very welcome.
>  >>
>  >> Well, I'll may be try to fetch something from barrier.h's
>  >> (since I cannot consider myself as a specialist in the area),
>  >> but at the moment the patch breaks the build on that arches.
>
>  > Do you have a specific architecture in mind and do you have
>  > some board/server (of that architecture) you could test AF_XDP
>  > on?
>
> I do care about s390 and ppc64 and I can run tests for them.

Perfect!. Thanks.

/Magnus

>
> [...]
>
> --
> WBR,
> Yauheni Kaliuta