diff mbox series

[suricata,1/3] suricata/ebpf: take clang -target bpf include issue of stdint.h into account

Message ID 151804207444.30000.12666757505388007849.stgit@firesoul
State Not Applicable, archived
Delegated to: David Miller
Headers show
Series Suricata cleanup makefile | expand

Commit Message

Jesper Dangaard Brouer Feb. 7, 2018, 10:21 p.m. UTC 
From: Jesper Dangaard Brouer <netoptimizer@brouer.com>

This patch prepares code before enabling the clang -target bpf.

The clang compiler does not like #include <stdint.h> when
using '-target bpf' it will fail with:

 fatal error: 'gnu/stubs-32.h' file not found

This is because using clang -target bpf, then clang will have '__bpf__'
defined instead of '__x86_64__' hence the gnu/stubs-32.h include
attempt as /usr/include/gnu/stubs.h contains, on x86_64:

  #if !defined __x86_64__
  # include <gnu/stubs-32.h>
  #endif
  #if defined __x86_64__ && defined __LP64__
  # include <gnu/stubs-64.h>
  #endif
  #if defined __x86_64__ && defined __ILP32__
  # include <gnu/stubs-x32.h>
  #endif

This can be worked around by installing the 32-bit version of
glibc-devel.i686 on your distribution.

But the BPF programs does not really need to include stdint.h,
if converting:
  uint64_t -> __u64
  uint32_t -> __u32
  uint16_t -> __u16
  uint8_t  -> __u8

This patch does this type syntax conversion.

Signed-off-by: Jesper Dangaard Brouer <netoptimizer@brouer.com>
---
 ebpf/bypass_filter.c |   27 +++++++++++++--------------
 ebpf/filter.c        |    3 +--
 ebpf/hash_func01.h   |   12 ++++++------
 ebpf/lb.c            |   11 +++++------
 ebpf/vlan_filter.c   |    5 ++---
 ebpf/xdp_filter.c    |   42 ++++++++++++++++++++----------------------
 6 files changed, 47 insertions(+), 53 deletions(-)

Comments

Eric Leblond Feb. 7, 2018, 11:52 p.m. UTC | #1 
Hi,

On Wed, 2018-02-07 at 23:21 +0100, Jesper Dangaard Brouer wrote:
> From: Jesper Dangaard Brouer <netoptimizer@brouer.com>
> 
> This patch prepares code before enabling the clang -target bpf.
> 
> The clang compiler does not like #include <stdint.h> when
> using '-target bpf' it will fail with:
> 
>  fatal error: 'gnu/stubs-32.h' file not found
...
> This can be worked around by installing the 32-bit version of
> glibc-devel.i686 on your distribution.
> 
> But the BPF programs does not really need to include stdint.h,
> if converting:
>   uint64_t -> __u64
>   uint32_t -> __u32
>   uint16_t -> __u16
>   uint8_t  -> __u8
> 
> This patch does this type syntax conversion.

There is an issue for system like Debian because they don't have a
asm/types.h in the include path if the architecture is not defined
which is the case due to target bpf. This results in:

clang-5.0 -Wall -Iinclude -O2 \
	-D__KERNEL__ -D__ASM_SYSREG_H \
	-target bpf -S -emit-llvm vlan_filter.c -o vlan_filter.ll
In file included from vlan_filter.c:19:
In file included from include/linux/bpf.h:11:
/usr/include/linux/types.h:5:10: fatal error: 'asm/types.h' file not
found
#include <asm/types.h>
         ^~~~~~~~~~~~~
1 error generated.
Makefile:523: recipe for target 'vlan_filter.bpf' failed

To go into details, the Debian package providing the 'asm/typs.h'
include is the the headers or linux-libc-dev. But this package comes
with a flavor and thus we have a prefix: 
 linux-libc-dev:amd64: /usr/include/x86_64-linux-gnu/asm/types.h

"Fun" part here is that if you build a debian package of the via make
in Linux tree then the linux-libc-dev package is correct.

So I propose the following patch that fixes the issue for me:

diff --git a/ebpf/Makefile.am b/ebpf/Makefile.am
index 89a3304e9..712b05343 100644
--- a/ebpf/Makefile.am
+++ b/ebpf/Makefile.am
@@ -16,6 +16,7 @@ all: $(BPF_TARGETS)
 $(BPF_TARGETS): %.bpf: %.c
 #      From C-code to LLVM-IR format suffix .ll (clang -S -emit-llvm)
        ${CLANG} -Wall $(BPF_CFLAGS) -O2 \
+               -I/usr/include/$(host_cpu)-$(host_os)/ \
                -D__KERNEL__ -D__ASM_SYSREG_H \
                -target bpf -S -emit-llvm $< -o ${@:.bpf=.ll}
 #      From LLVM-IR to BPF-bytecode in ELF-obj file

Let me know if it is ok for you.

Best regards,
Jesper Dangaard Brouer Feb. 8, 2018, 8:42 a.m. UTC | #2 
On Thu, 08 Feb 2018 00:52:09 +0100 Eric Leblond <eric@regit.org> wrote:

> Hi,
> 
> On Wed, 2018-02-07 at 23:21 +0100, Jesper Dangaard Brouer wrote:
> > From: Jesper Dangaard Brouer <netoptimizer@brouer.com>
> > 
> > This patch prepares code before enabling the clang -target bpf.
> > 
> > The clang compiler does not like #include <stdint.h> when
> > using '-target bpf' it will fail with:
> > 
> >  fatal error: 'gnu/stubs-32.h' file not found  
> ...
> > This can be worked around by installing the 32-bit version of
> > glibc-devel.i686 on your distribution.
> > 
> > But the BPF programs does not really need to include stdint.h,
> > if converting:
> >   uint64_t -> __u64
> >   uint32_t -> __u32
> >   uint16_t -> __u16
> >   uint8_t  -> __u8
> > 
> > This patch does this type syntax conversion.  
> 
> There is an issue for system like Debian because they don't have a
> asm/types.h in the include path if the architecture is not defined
> which is the case due to target bpf. This results in:
> 
> clang-5.0 -Wall -Iinclude -O2 \
> 	-D__KERNEL__ -D__ASM_SYSREG_H \
> 	-target bpf -S -emit-llvm vlan_filter.c -o vlan_filter.ll
> In file included from vlan_filter.c:19:
> In file included from include/linux/bpf.h:11:
> /usr/include/linux/types.h:5:10: fatal error: 'asm/types.h' file not
> found
> #include <asm/types.h>
>          ^~~~~~~~~~~~~
> 1 error generated.
> Makefile:523: recipe for target 'vlan_filter.bpf' failed
> 
> To go into details, the Debian package providing the 'asm/typs.h'
> include is the the headers or linux-libc-dev. But this package comes
> with a flavor and thus we have a prefix: 
>  linux-libc-dev:amd64: /usr/include/x86_64-linux-gnu/asm/types.h

Oh, the joy of distro choices.

> "Fun" part here is that if you build a debian package of the via make
> in Linux tree then the linux-libc-dev package is correct.
> 
> So I propose the following patch that fixes the issue for me:
> 
> diff --git a/ebpf/Makefile.am b/ebpf/Makefile.am
> index 89a3304e9..712b05343 100644
> --- a/ebpf/Makefile.am
> +++ b/ebpf/Makefile.am
> @@ -16,6 +16,7 @@ all: $(BPF_TARGETS)
>  $(BPF_TARGETS): %.bpf: %.c
>  #      From C-code to LLVM-IR format suffix .ll (clang -S -emit-llvm)
>         ${CLANG} -Wall $(BPF_CFLAGS) -O2 \
> +               -I/usr/include/$(host_cpu)-$(host_os)/ \

Cool solution. These variables originate from configure/automake.

Would it be more technical correct to use(?): $(build_cpu)-$(build_os)

I verified that the variables are the same (notice 'make -p' trick):

$ make -p | egrep '_os'
build_os = linux-gnu
host_os = linux-gnu

$ make -p | egrep '_cpu'
host_cpu = x86_64
build_cpu = x86_64



>                 -D__KERNEL__ -D__ASM_SYSREG_H \
>                 -target bpf -S -emit-llvm $< -o ${@:.bpf=.ll}
>  #      From LLVM-IR to BPF-bytecode in ELF-obj file
> 
> Let me know if it is ok for you.

I'm fine with this fix.

I wonder if we should check other distros?
diff mbox series

Patch

diff --git a/ebpf/bypass_filter.c b/ebpf/bypass_filter.c
index d2ce12aa1cd5..be81032b16cf 100644
--- a/ebpf/bypass_filter.c
+++ b/ebpf/bypass_filter.c
@@ -15,7 +15,6 @@ 
  * 02110-1301, USA.
  */
 
-#include <stdint.h>
 #include <stddef.h>
 #include <linux/bpf.h>
 
@@ -51,9 +50,9 @@  struct flowv6_keys {
 } __attribute__((__aligned__(8)));
 
 struct pair {
-    uint64_t time;
-    uint64_t packets;
-    uint64_t bytes;
+    __u64 time;
+    __u64 packets;
+    __u64 bytes;
 } __attribute__((__aligned__(8)));
 
 struct bpf_map_def SEC("maps") flow_table_v4 = {
@@ -77,10 +76,10 @@  struct bpf_map_def SEC("maps") flow_table_v6 = {
  */
 static __always_inline int ipv4_filter(struct __sk_buff *skb)
 {
-    uint32_t nhoff, verlen;
+    __u32 nhoff, verlen;
     struct flowv4_keys tuple;
     struct pair *value;
-    uint16_t port;
+    __u16 port;
 
     nhoff = skb->cb[0];
 
@@ -107,8 +106,8 @@  static __always_inline int ipv4_filter(struct __sk_buff *skb)
 #if 0
     if ((tuple.port16[0] == 22) || (tuple.port16[1] == 22))
     {
-        uint16_t sp = tuple.port16[0];
-        //uint16_t dp = tuple.port16[1];
+        __u16 sp = tuple.port16[0];
+        //__u16 dp = tuple.port16[1];
         char fmt[] = "Parsed SSH flow: %u %d -> %u\n";
         bpf_trace_printk(fmt, sizeof(fmt), tuple.src, sp, tuple.dst);
     }
@@ -118,8 +117,8 @@  static __always_inline int ipv4_filter(struct __sk_buff *skb)
     if (value) {
 #if 0
         {
-            uint16_t sp = tuple.port16[0];
-            //uint16_t dp = tuple.port16[1];
+            __u16 sp = tuple.port16[0];
+            //__u16 dp = tuple.port16[1];
             char bfmt[] = "Found flow: %u %d -> %u\n";
             bpf_trace_printk(bfmt, sizeof(bfmt), tuple.src, sp, tuple.dst);
         }
@@ -139,11 +138,11 @@  static __always_inline int ipv4_filter(struct __sk_buff *skb)
  */
 static __always_inline int ipv6_filter(struct __sk_buff *skb)
 {
-    uint32_t nhoff;
-    uint8_t nhdr;
+    __u32 nhoff;
+    __u8 nhdr;
     struct flowv6_keys tuple;
     struct pair *value;
-    uint16_t port;
+    __u16 port;
 
     nhoff = skb->cb[0];
 
@@ -223,4 +222,4 @@  int SEC("filter") hashfilter(struct __sk_buff *skb) {
 
 char __license[] SEC("license") = "GPL";
 
-uint32_t __version SEC("version") = LINUX_VERSION_CODE;
+__u32 __version SEC("version") = LINUX_VERSION_CODE;
diff --git a/ebpf/filter.c b/ebpf/filter.c
index 1976ffcf188f..4fe95d4fb005 100644
--- a/ebpf/filter.c
+++ b/ebpf/filter.c
@@ -15,7 +15,6 @@ 
  * 02110-1301, USA.
  */
 
-#include <stdint.h>
 #include <stddef.h>
 #include <linux/bpf.h>
 
@@ -56,4 +55,4 @@  int SEC("filter") hashfilter(struct __sk_buff *skb) {
 
 char __license[] SEC("license") = "GPL";
 
-uint32_t __version SEC("version") = LINUX_VERSION_CODE;
+__u32 __version SEC("version") = LINUX_VERSION_CODE;
diff --git a/ebpf/hash_func01.h b/ebpf/hash_func01.h
index 060346f67a6a..38255812e376 100644
--- a/ebpf/hash_func01.h
+++ b/ebpf/hash_func01.h
@@ -4,12 +4,12 @@ 
  * From: http://www.azillionmonkeys.com/qed/hash.html
  */
 
-#define get16bits(d) (*((const uint16_t *) (d)))
+#define get16bits(d) (*((const __u16 *) (d)))
 
 static __always_inline
-uint32_t SuperFastHash (const char *data, int len, uint32_t initval) {
-	uint32_t hash = initval;
-	uint32_t tmp;
+__u32 SuperFastHash (const char *data, int len, __u32 initval) {
+	__u32 hash = initval;
+	__u32 tmp;
 	int rem;
 
 	if (len <= 0 || data == NULL) return 0;
@@ -23,7 +23,7 @@  uint32_t SuperFastHash (const char *data, int len, uint32_t initval) {
 		hash  += get16bits (data);
 		tmp    = (get16bits (data+2) << 11) ^ hash;
 		hash   = (hash << 16) ^ tmp;
-		data  += 2*sizeof (uint16_t);
+		data  += 2*sizeof (__u16);
 		hash  += hash >> 11;
 	}
 
@@ -31,7 +31,7 @@  uint32_t SuperFastHash (const char *data, int len, uint32_t initval) {
 	switch (rem) {
         case 3: hash += get16bits (data);
                 hash ^= hash << 16;
-                hash ^= ((signed char)data[sizeof (uint16_t)]) << 18;
+                hash ^= ((signed char)data[sizeof (__u16)]) << 18;
                 hash += hash >> 11;
                 break;
         case 2: hash += get16bits (data);
diff --git a/ebpf/lb.c b/ebpf/lb.c
index 14974784d925..7551781c5f80 100644
--- a/ebpf/lb.c
+++ b/ebpf/lb.c
@@ -15,7 +15,6 @@ 
  * 02110-1301, USA.
  */
 
-#include <stdint.h>
 #include <stddef.h>
 #include <linux/bpf.h>
 
@@ -36,8 +35,8 @@ 
 
 static __always_inline int ipv4_hash(struct __sk_buff *skb)
 {
-    uint32_t nhoff;
-    uint32_t src, dst;
+    __u32 nhoff;
+    __u32 src, dst;
 
     nhoff = skb->cb[0];
     src = load_word(skb, nhoff + offsetof(struct iphdr, saddr));
@@ -54,8 +53,8 @@  static __always_inline int ipv4_hash(struct __sk_buff *skb)
 
 static __always_inline int ipv6_hash(struct __sk_buff *skb)
 {
-    uint32_t nhoff;
-    uint32_t src, dst, hash;
+    __u32 nhoff;
+    __u32 src, dst, hash;
 
     nhoff = skb->cb[0];
     hash = 0;
@@ -107,4 +106,4 @@  char __license[] __section("license") = "GPL";
 
 /* libbpf needs version section to check sync of eBPF code and kernel
  * but socket filter don't need it */
-uint32_t __version __section("version") = LINUX_VERSION_CODE;
+__u32 __version __section("version") = LINUX_VERSION_CODE;
diff --git a/ebpf/vlan_filter.c b/ebpf/vlan_filter.c
index 5c3865273d3c..d797b94bfbd5 100644
--- a/ebpf/vlan_filter.c
+++ b/ebpf/vlan_filter.c
@@ -15,7 +15,6 @@ 
  * 02110-1301, USA.
  */
 
-#include <stdint.h>
 #include <stddef.h>
 #include <linux/bpf.h>
 
@@ -24,7 +23,7 @@ 
 #define LINUX_VERSION_CODE 263682
 
 int SEC("filter") hashfilter(struct __sk_buff *skb) {
-    uint16_t vlan_id = skb->vlan_tci & 0x0fff;
+    __u16 vlan_id = skb->vlan_tci & 0x0fff;
     /* accept VLAN 2 and 4 and drop the rest */
     switch (vlan_id) {
         case 2:
@@ -38,4 +37,4 @@  int SEC("filter") hashfilter(struct __sk_buff *skb) {
 
 char __license[] SEC("license") = "GPL";
 
-uint32_t __version SEC("version") = LINUX_VERSION_CODE;
+__u32 __version SEC("version") = LINUX_VERSION_CODE;
diff --git a/ebpf/xdp_filter.c b/ebpf/xdp_filter.c
index 9d3ff0910c30..3ee9dd0b114c 100644
--- a/ebpf/xdp_filter.c
+++ b/ebpf/xdp_filter.c
@@ -16,8 +16,6 @@ 
  */
 
 #define KBUILD_MODNAME "foo"
-#include <stdint.h>
-#include <string.h>
 #include <stddef.h>
 #include <linux/bpf.h>
 
@@ -70,9 +68,9 @@  struct flowv6_keys {
 } __attribute__((__aligned__(8)));
 
 struct pair {
-    uint64_t time;
-    uint64_t packets;
-    uint64_t bytes;
+    __u64 time;
+    __u64 packets;
+    __u64 bytes;
 } __attribute__((__aligned__(8)));
 
 struct bpf_map_def SEC("maps") flow_table_v4 = {
@@ -128,7 +126,7 @@  struct bpf_map_def SEC("maps") tx_peer_int = {
 };
 
 static __always_inline int get_sport(void *trans_data, void *data_end,
-        uint8_t protocol)
+        __u8 protocol)
 {
     struct tcphdr *th;
     struct udphdr *uh;
@@ -150,7 +148,7 @@  static __always_inline int get_sport(void *trans_data, void *data_end,
 }
 
 static __always_inline int get_dport(void *trans_data, void *data_end,
-        uint8_t protocol)
+        __u8 protocol)
 {
     struct tcphdr *th;
     struct udphdr *uh;
@@ -178,12 +176,12 @@  static int __always_inline filter_ipv4(void *data, __u64 nh_off, void *data_end)
     int sport;
     struct flowv4_keys tuple;
     struct pair *value;
-    uint32_t key0 = 0;
+    __u32 key0 = 0;
 #if BUILD_CPUMAP
-    uint32_t cpu_dest;
-    uint32_t *cpu_max = bpf_map_lookup_elem(&cpus_count, &key0);
-    uint32_t *cpu_selected;
-    uint32_t cpu_hash;
+    __u32 cpu_dest;
+    __u32 *cpu_max = bpf_map_lookup_elem(&cpus_count, &key0);
+    __u32 *cpu_selected;
+    __u32 cpu_hash;
 #endif
     int *iface_peer;
     int tx_port = 0;
@@ -191,7 +189,7 @@  static int __always_inline filter_ipv4(void *data, __u64 nh_off, void *data_end)
     if ((void *)(iph + 1) > data_end)
         return XDP_PASS;
 
-    tuple.ip_proto = (uint32_t) iph->protocol;
+    tuple.ip_proto = (__u32) iph->protocol;
     tuple.src = iph->saddr;
     tuple.dst = iph->daddr;
 
@@ -203,8 +201,8 @@  static int __always_inline filter_ipv4(void *data, __u64 nh_off, void *data_end)
     if (sport == -1)
         return XDP_PASS;
 
-    tuple.port16[0] = (uint16_t)sport;
-    tuple.port16[1] = (uint16_t)dport;
+    tuple.port16[0] = (__u16)sport;
+    tuple.port16[1] = (__u16)dport;
     value = bpf_map_lookup_elem(&flow_table_v4, &tuple);
 #if 0
     {
@@ -260,12 +258,12 @@  static int __always_inline filter_ipv6(void *data, __u64 nh_off, void *data_end)
     int sport;
     struct flowv6_keys tuple;
     struct pair *value;
-    uint32_t key0 = 0;
+    __u32 key0 = 0;
 #if BUILD_CPUMAP
-    uint32_t cpu_dest;
+    __u32 cpu_dest;
     int *cpu_max = bpf_map_lookup_elem(&cpus_count, &key0);
-    uint32_t *cpu_selected;
-    uint32_t cpu_hash;
+    __u32 *cpu_selected;
+    __u32 cpu_hash;
 #endif
     int tx_port = 0;
     int *iface_peer;
@@ -336,8 +334,8 @@  int SEC("xdp") xdp_hashfilter(struct xdp_md *ctx)
     void *data = (void *)(long)ctx->data;
     struct ethhdr *eth = data;
     int rc = XDP_PASS;
-    uint16_t h_proto;
-	uint64_t nh_off;
+    __u16 h_proto;
+    __u64 nh_off;
 
 	nh_off = sizeof(*eth);
 	if (data + nh_off > data_end)
@@ -376,4 +374,4 @@  int SEC("xdp") xdp_hashfilter(struct xdp_md *ctx)
 
 char __license[] SEC("license") = "GPL";
 
-uint32_t __version SEC("version") = LINUX_VERSION_CODE;
+__u32 __version SEC("version") = LINUX_VERSION_CODE;