Re: [suricata PATCH 1/3] suricata/ebpf: take clang -target bpf include issue of stdint.h into account
On Thu, 08 Feb 2018 00:52:09 +0100 Eric Leblond wrote: > Hi, > > On Wed, 2018-02-07 at 23:21 +0100, Jesper Dangaard Brouer wrote: > > From: Jesper Dangaard Brouer > > > > This patch prepares code before enabling the clang -target bpf. > > > > The clang compiler does not like #include 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 > ^ > 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? -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
Re: [suricata PATCH 1/3] suricata/ebpf: take clang -target bpf include issue of stdint.h into account
Hi, On Wed, 2018-02-07 at 23:21 +0100, Jesper Dangaard Brouer wrote: > From: Jesper Dangaard Brouer > > This patch prepares code before enabling the clang -target bpf. > > The clang compiler does not like #include 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 ^ 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, -- Eric Leblond
[suricata PATCH 1/3] suricata/ebpf: take clang -target bpf include issue of stdint.h into account
From: Jesper Dangaard Brouer This patch prepares code before enabling the clang -target bpf. The clang compiler does not like #include 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 #endif #if defined __x86_64__ && defined __LP64__ # include #endif #if defined __x86_64__ && defined __ILP32__ # include #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 --- 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(-) 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 #include #include @@ -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 #include #include @@ -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); + d