netdev - Re: [PATCH bpf-next] xsk: introduce padding between ring pointers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <43b0605d-f0c9-b81c-4d16-344a7832e083@iogearbox.net>
Date:   Fri, 9 Oct 2020 17:03:44 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Magnus Karlsson <magnus.karlsson@...il.com>,
        magnus.karlsson@...el.com, bjorn.topel@...el.com, ast@...nel.org,
        netdev@...r.kernel.org, jonathan.lemon@...il.com
Cc:     bpf@...r.kernel.org, john.fastabend@...il.com
Subject: Re: [PATCH bpf-next] xsk: introduce padding between ring pointers

On 10/8/20 4:12 PM, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@...el.com>
> 
> Introduce one cache line worth of padding between the producer and
> consumer pointers in all the lockless rings. This so that the HW
> adjacency prefetcher will not prefetch the consumer pointer when the
> producer pointer is used and vice versa. This improves throughput
> performance for the l2fwd sample app with 2% on my machine with HW
> prefetching turned on.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@...el.com>

Applied, thanks!

>   net/xdp/xsk_queue.h | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
> index dc1dd5e..3c235d2 100644
> --- a/net/xdp/xsk_queue.h
> +++ b/net/xdp/xsk_queue.h
> @@ -15,6 +15,10 @@
>   
>   struct xdp_ring {
>   	u32 producer ____cacheline_aligned_in_smp;
> +	/* Hinder the adjacent cache prefetcher to prefetch the consumer pointer if the producer
> +	 * pointer is touched and vice versa.
> +	 */
> +	u32 pad ____cacheline_aligned_in_smp;
>   	u32 consumer ____cacheline_aligned_in_smp;
>   	u32 flags;
>   };
> 

I was wondering whether we should even generalize this further for reuse
elsewhere e.g. ...

diff --git a/include/linux/cache.h b/include/linux/cache.h
index 1aa8009f6d06..5521dab01649 100644
--- a/include/linux/cache.h
+++ b/include/linux/cache.h
@@ -85,4 +85,17 @@
  #define cache_line_size()      L1_CACHE_BYTES
  #endif

+/*
+ * Dummy element for use in structs in order to pad a cacheline
+ * aligned element with an extra cacheline to hinder the adjacent
+ * cache prefetcher to prefetch the subsequent struct element.
+ */
+#ifndef ____cacheline_padding_in_smp
+# ifdef CONFIG_SMP
+#  define ____cacheline_padding_in_smp u8 :8 ____cacheline_aligned_in_smp
+# else
+#  define ____cacheline_padding_in_smp
+# endif /* CONFIG_SMP */
+#endif
+
  #endif /* __LINUX_CACHE_H */
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index cdb9cf3cd136..1da36423e779 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -15,11 +15,9 @@

  struct xdp_ring {
         u32 producer ____cacheline_aligned_in_smp;
-       /* Hinder the adjacent cache prefetcher to prefetch the consumer
-        * pointer if the producer pointer is touched and vice versa.
-        */
-       u32 pad ____cacheline_aligned_in_smp;
+       ____cacheline_padding_in_smp;
         u32 consumer ____cacheline_aligned_in_smp;
+       ____cacheline_padding_in_smp;
         u32 flags;
  };

... was there any improvement to also pad after the consumer given the struct
xdp_ring is also embedded into other structs?

Thanks,
Daniel