bpf: avoid VLAs in progs/test_xdp_dynptr.c

Commit Message

Jose E. Marchesi Jan. 23, 2024, 8:17 p.m. UTC

VLAs are not supported by either the BPF port of clang nor GCC.  The
selftest test_xdp_dynptr.c contains the following code:

  const size_t tcphdr_sz = sizeof(struct tcphdr);
  const size_t udphdr_sz = sizeof(struct udphdr);
  const size_t ethhdr_sz = sizeof(struct ethhdr);
  const size_t iphdr_sz = sizeof(struct iphdr);
  const size_t ipv6hdr_sz = sizeof(struct ipv6hdr);

  [...]

  static __always_inline int handle_ipv4(struct xdp_md *xdp, struct bpf_dynptr *xdp_ptr)
  {
	__u8 eth_buffer[ethhdr_sz + iphdr_sz + ethhdr_sz];
	__u8 iph_buffer_tcp[iphdr_sz + tcphdr_sz];
	__u8 iph_buffer_udp[iphdr_sz + udphdr_sz];
	[...]
  }

The eth_buffer, iph_buffer_tcp and other automatics are fixed size
only if the compiler optimizes away the constant global variables.
clang does this, but GCC does not, turning these automatics into
variable length arrays.

This patch removes the global variables and turns these values into
preprocessor constants.  This makes the selftest to build properly
with GCC.

Tested in bpf-next master.
No regressions.

Signed-off-by: Jose E. Marchesi <[e-mail]>
Cc: Yonghong Song <[e-mail]>
Cc: Eduard Zingerman <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
---
 tools/testing/selftests/bpf/progs/test_xdp_dynptr.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Yonghong Song Jan. 23, 2024, 9:06 p.m. UTC | #1

On 1/23/24 12:17 PM, Jose E. Marchesi wrote:
> VLAs are not supported by either the BPF port of clang nor GCC.  The
> selftest test_xdp_dynptr.c contains the following code:
>
>    const size_t tcphdr_sz = sizeof(struct tcphdr);
>    const size_t udphdr_sz = sizeof(struct udphdr);
>    const size_t ethhdr_sz = sizeof(struct ethhdr);
>    const size_t iphdr_sz = sizeof(struct iphdr);
>    const size_t ipv6hdr_sz = sizeof(struct ipv6hdr);
>
>    [...]
>
>    static __always_inline int handle_ipv4(struct xdp_md *xdp, struct bpf_dynptr *xdp_ptr)
>    {
> 	__u8 eth_buffer[ethhdr_sz + iphdr_sz + ethhdr_sz];
> 	__u8 iph_buffer_tcp[iphdr_sz + tcphdr_sz];
> 	__u8 iph_buffer_udp[iphdr_sz + udphdr_sz];
> 	[...]
>    }
>
> The eth_buffer, iph_buffer_tcp and other automatics are fixed size
> only if the compiler optimizes away the constant global variables.
> clang does this, but GCC does not, turning these automatics into
> variable length arrays.
>
> This patch removes the global variables and turns these values into
> preprocessor constants.  This makes the selftest to build properly
> with GCC.
>
> Tested in bpf-next master.
> No regressions.
>
> Signed-off-by: Jose E. Marchesi <[e-mail]>
> Cc: Yonghong Song <[e-mail]>
> Cc: Eduard Zingerman <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]

Acked-by: Yonghong Song <[e-mail]>

[e-mail] Jan. 24, 2024, midnight UTC | #2

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[e-mail]>:

On Tue, 23 Jan 2024 21:17:29 +0100 you wrote:
> VLAs are not supported by either the BPF port of clang nor GCC.  The
> selftest test_xdp_dynptr.c contains the following code:
> 
>   const size_t tcphdr_sz = sizeof(struct tcphdr);
>   const size_t udphdr_sz = sizeof(struct udphdr);
>   const size_t ethhdr_sz = sizeof(struct ethhdr);
>   const size_t iphdr_sz = sizeof(struct iphdr);
>   const size_t ipv6hdr_sz = sizeof(struct ipv6hdr);
> 
> [...]

Here is the summary with links:
  - bpf: avoid VLAs in progs/test_xdp_dynptr.c
    https://git.kernel.org/bpf/bpf-next/c/edb799035dd7

You are awesome, thank you!

Patch

diff --git a/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c b/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
index 78c368e71797..67a77944ef29 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
@@ -18,11 +18,11 @@ 
 #include "test_iptunnel_common.h"
 #include "bpf_kfuncs.h"
 
-const size_t tcphdr_sz = sizeof(struct tcphdr);
-const size_t udphdr_sz = sizeof(struct udphdr);
-const size_t ethhdr_sz = sizeof(struct ethhdr);
-const size_t iphdr_sz = sizeof(struct iphdr);
-const size_t ipv6hdr_sz = sizeof(struct ipv6hdr);
+#define tcphdr_sz sizeof(struct tcphdr)
+#define udphdr_sz sizeof(struct udphdr)
+#define ethhdr_sz sizeof(struct ethhdr)
+#define iphdr_sz sizeof(struct iphdr)
+#define ipv6hdr_sz sizeof(struct ipv6hdr)
 
 struct {
 	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);