[PATCH net v2] selftests/bpf: fix broken build due to types.h

[PATCH net v2] selftests/bpf: fix broken build due to types.h

[Yonghong Song]

Commit 0a5539f66133 ("bpf: Provide a linux/types.h override
for bpf selftests.") caused a build failure for tools/testing/selftest/bpf
because of some missing types:
    $ make -C tools/testing/selftests/bpf/
    ...
    In file included from /home/yhs/work/net-next/tools/testing/selftests/bpf/test_pkt_access.c:8:
    ../../../include/uapi/linux/bpf.h:170:3: error: unknown type name '__aligned_u64'
                    __aligned_u64   key;
    ...
    /usr/include/linux/swab.h:160:8: error: unknown type name '__always_inline'
    static __always_inline __u16 __swab16p(const __u16 *p)
    ...
The type __aligned_u64 is defined in linux:include/uapi/linux/types.h.

The fix is to copy missing type definition into
tools/testing/selftests/bpf/include/uapi/linux/types.h.
Adding additional include "string.h" resolves __always_inline issue.

Fixes: 0a5539f66133 ("bpf: Provide a linux/types.h override for bpf selftests.")
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/testing/selftests/bpf/include/uapi/linux/types.h | 16 ++++++++++++++++
 tools/testing/selftests/bpf/test_pkt_access.c          |  1 +
 2 files changed, 17 insertions(+)

diff --git a/tools/testing/selftests/bpf/include/uapi/linux/types.h b/tools/testing/selftests/bpf/include/uapi/linux/types.h
index fbd16a7..5184184 100644
--- a/tools/testing/selftests/bpf/include/uapi/linux/types.h
+++ b/tools/testing/selftests/bpf/include/uapi/linux/types.h
@@ -3,4 +3,20 @@
 
 #include <asm-generic/int-ll64.h>
 
+/* copied from linux:include/uapi/linux/types.h */
+#define __bitwise
+typedef __u16 __bitwise __le16;
+typedef __u16 __bitwise __be16;
+typedef __u32 __bitwise __le32;
+typedef __u32 __bitwise __be32;
+typedef __u64 __bitwise __le64;
+typedef __u64 __bitwise __be64;
+
+typedef __u16 __bitwise __sum16;
+typedef __u32 __bitwise __wsum;
+
+#define __aligned_u64 __u64 __attribute__((aligned(8)))
+#define __aligned_be64 __be64 __attribute__((aligned(8)))
+#define __aligned_le64 __le64 __attribute__((aligned(8)))
+
 #endif /* _UAPI_LINUX_TYPES_H */
diff --git a/tools/testing/selftests/bpf/test_pkt_access.c b/tools/testing/selftests/bpf/test_pkt_access.c
index 39387bb..6e11ba1 100644
--- a/tools/testing/selftests/bpf/test_pkt_access.c
+++ b/tools/testing/selftests/bpf/test_pkt_access.c
@@ -5,6 +5,7 @@
  * License as published by the Free Software Foundation.
  */
 #include <stddef.h>
+#include <string.h>
 #include <linux/bpf.h>
 #include <linux/if_ether.h>
 #include <linux/if_packet.h>
-- 
2.9.3

Re: [PATCH net v2] selftests/bpf: fix broken build due to types.h

[David Miller]

From: Yonghong Song <yhs@fb.com>
Date: Wed, 17 May 2017 15:18:05 -0700

> Commit 0a5539f66133 ("bpf: Provide a linux/types.h override
> for bpf selftests.") caused a build failure for tools/testing/selftest/bpf
> because of some missing types:
>     $ make -C tools/testing/selftests/bpf/
>     ...
>     In file included from /home/yhs/work/net-next/tools/testing/selftests/bpf/test_pkt_access.c:8:
>     ../../../include/uapi/linux/bpf.h:170:3: error: unknown type name '__aligned_u64'
>                     __aligned_u64   key;
>     ...
>     /usr/include/linux/swab.h:160:8: error: unknown type name '__always_inline'
>     static __always_inline __u16 __swab16p(const __u16 *p)
>     ...
> The type __aligned_u64 is defined in linux:include/uapi/linux/types.h.
> 
> The fix is to copy missing type definition into
> tools/testing/selftests/bpf/include/uapi/linux/types.h.
> Adding additional include "string.h" resolves __always_inline issue.
> 
> Fixes: 0a5539f66133 ("bpf: Provide a linux/types.h override for bpf selftests.")
> Signed-off-by: Yonghong Song <yhs@fb.com>

Applied, thank you.

Re: [PATCH net v2] selftests/bpf: fix broken build due to types.h

[Daniel Borkmann]

On 05/18/2017 12:18 AM, Yonghong Song wrote:
> Commit 0a5539f66133 ("bpf: Provide a linux/types.h override
> for bpf selftests.") caused a build failure for tools/testing/selftest/bpf
> because of some missing types:
>      $ make -C tools/testing/selftests/bpf/
>      ...
>      In file included from /home/yhs/work/net-next/tools/testing/selftests/bpf/test_pkt_access.c:8:
>      ../../../include/uapi/linux/bpf.h:170:3: error: unknown type name '__aligned_u64'
>                      __aligned_u64   key;
>      ...
>      /usr/include/linux/swab.h:160:8: error: unknown type name '__always_inline'
>      static __always_inline __u16 __swab16p(const __u16 *p)
>      ...
> The type __aligned_u64 is defined in linux:include/uapi/linux/types.h.
>
> The fix is to copy missing type definition into
> tools/testing/selftests/bpf/include/uapi/linux/types.h.
> Adding additional include "string.h" resolves __always_inline issue.
>
> Fixes: 0a5539f66133 ("bpf: Provide a linux/types.h override for bpf selftests.")
> Signed-off-by: Yonghong Song <yhs@fb.com>

Can you elaborate why string.h specifically? Can't we define the
__always_inline ourselves?

Thanks,
Daniel

Re: [PATCH net v2] selftests/bpf: fix broken build due to types.h

[David Miller]

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Thu, 18 May 2017 00:57:04 +0200

> On 05/18/2017 12:18 AM, Yonghong Song wrote:
>> Commit 0a5539f66133 ("bpf: Provide a linux/types.h override
>> for bpf selftests.") caused a build failure for
>> tools/testing/selftest/bpf
>> because of some missing types:
>>      $ make -C tools/testing/selftests/bpf/
>>      ...
>>      In file included from
>>      /home/yhs/work/net-next/tools/testing/selftests/bpf/test_pkt_access.c:8:
>>      ../../../include/uapi/linux/bpf.h:170:3: error: unknown type name
>>      '__aligned_u64'
>>                      __aligned_u64   key;
>>      ...
>>      /usr/include/linux/swab.h:160:8: error: unknown type name
>>      '__always_inline'
>>      static __always_inline __u16 __swab16p(const __u16 *p)
>>      ...
>> The type __aligned_u64 is defined in linux:include/uapi/linux/types.h.
>>
>> The fix is to copy missing type definition into
>> tools/testing/selftests/bpf/include/uapi/linux/types.h.
>> Adding additional include "string.h" resolves __always_inline issue.
>>
>> Fixes: 0a5539f66133 ("bpf: Provide a linux/types.h override for bpf
>> selftests.")
>> Signed-off-by: Yonghong Song <yhs@fb.com>
> 
> Can you elaborate why string.h specifically? Can't we define the
> __always_inline ourselves?

That way it comes from compiler.h

Probably it would have been better to have the BPF linux/types.h bring
it in.

Sorry I applied this so quickly, I wanted this regression fixed as fast
as possible.

Re: [PATCH net v2] selftests/bpf: fix broken build due to types.h

[Daniel Borkmann]

On 05/18/2017 01:01 AM, David Miller wrote:
> From: Daniel Borkmann <daniel@iogearbox.net>
> Date: Thu, 18 May 2017 00:57:04 +0200
>
>> On 05/18/2017 12:18 AM, Yonghong Song wrote:
>>> Commit 0a5539f66133 ("bpf: Provide a linux/types.h override
>>> for bpf selftests.") caused a build failure for
>>> tools/testing/selftest/bpf
>>> because of some missing types:
>>>       $ make -C tools/testing/selftests/bpf/
>>>       ...
>>>       In file included from
>>>       /home/yhs/work/net-next/tools/testing/selftests/bpf/test_pkt_access.c:8:
>>>       ../../../include/uapi/linux/bpf.h:170:3: error: unknown type name
>>>       '__aligned_u64'
>>>                       __aligned_u64   key;
>>>       ...
>>>       /usr/include/linux/swab.h:160:8: error: unknown type name
>>>       '__always_inline'
>>>       static __always_inline __u16 __swab16p(const __u16 *p)
>>>       ...
>>> The type __aligned_u64 is defined in linux:include/uapi/linux/types.h.
>>>
>>> The fix is to copy missing type definition into
>>> tools/testing/selftests/bpf/include/uapi/linux/types.h.
>>> Adding additional include "string.h" resolves __always_inline issue.
>>>
>>> Fixes: 0a5539f66133 ("bpf: Provide a linux/types.h override for bpf
>>> selftests.")
>>> Signed-off-by: Yonghong Song <yhs@fb.com>
>>
>> Can you elaborate why string.h specifically? Can't we define the
>> __always_inline ourselves?
>
> That way it comes from compiler.h
>
> Probably it would have been better to have the BPF linux/types.h bring
> it in.

Would have made it a bit more clear at least.

> Sorry I applied this so quickly, I wanted this regression fixed as fast
> as possible.

Ok, no problem.

Btw, 0day kernel testing bot is from now on running BPF selftests
as well as part of their regression tests. Fengguang confirmed this
today after integrating this.

Re: [PATCH net v2] selftests/bpf: fix broken build due to types.h

[David Miller]

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Thu, 18 May 2017 01:15:43 +0200

> Btw, 0day kernel testing bot is from now on running BPF selftests
> as well as part of their regression tests. Fengguang confirmed this
> today after integrating this.

That's great news.

Re: [PATCH net v2] selftests/bpf: fix broken build due to types.h

[Yonghong Song]

On 5/17/17 3:57 PM, Daniel Borkmann wrote:
> On 05/18/2017 12:18 AM, Yonghong Song wrote:
>> Commit 0a5539f66133 ("bpf: Provide a linux/types.h override
>> for bpf selftests.") caused a build failure for 
>> tools/testing/selftest/bpf
>> because of some missing types:
>>      $ make -C tools/testing/selftests/bpf/
>>      ...
>>      In file included from 
>> /home/yhs/work/net-next/tools/testing/selftests/bpf/test_pkt_access.c:8:
>>      ../../../include/uapi/linux/bpf.h:170:3: error: unknown type name 
>> '__aligned_u64'
>>                      __aligned_u64   key;
>>      ...
>>      /usr/include/linux/swab.h:160:8: error: unknown type name 
>> '__always_inline'
>>      static __always_inline __u16 __swab16p(const __u16 *p)
>>      ...
>> The type __aligned_u64 is defined in linux:include/uapi/linux/types.h.
>>
>> The fix is to copy missing type definition into
>> tools/testing/selftests/bpf/include/uapi/linux/types.h.
>> Adding additional include "string.h" resolves __always_inline issue.
>>
>> Fixes: 0a5539f66133 ("bpf: Provide a linux/types.h override for bpf 
>> selftests.")
>> Signed-off-by: Yonghong Song <yhs@fb.com>
> 
> Can you elaborate why string.h specifically? Can't we define the
> __always_inline ourselves?

Actually, yes, we can define __always_inline ourselves in each bpf 
program or in some common header files used by all selftests bpf
programs.

I use string.h because several other bpf programs
(test_xdp.c test_l4lb.c test_tcp_estats.c) they all have
__always_inline and string.h, so they do not have issue.
I discovered this pattern so I just add string.h into test_pkt_access.c
as well.

string.h provides memset/memcpy prototypes which is used in bpf program 
(test_xdp.c and test_tcp_estats.c, but not test_l4lb.c and 
test_pkt_access.c).

Thanks,

Yonghong

> 
> Thanks,
> Daniel

Re: [PATCH net v2] selftests/bpf: fix broken build due to types.h

[Daniel Borkmann]

On 05/18/2017 01:48 AM, Yonghong Song wrote:
> On 5/17/17 3:57 PM, Daniel Borkmann wrote:
>> On 05/18/2017 12:18 AM, Yonghong Song wrote:
>>> Commit 0a5539f66133 ("bpf: Provide a linux/types.h override
>>> for bpf selftests.") caused a build failure for tools/testing/selftest/bpf
>>> because of some missing types:
>>>      $ make -C tools/testing/selftests/bpf/
>>>      ...
>>>      In file included from /home/yhs/work/net-next/tools/testing/selftests/bpf/test_pkt_access.c:8:
>>>      ../../../include/uapi/linux/bpf.h:170:3: error: unknown type name '__aligned_u64'
>>>                      __aligned_u64   key;
>>>      ...
>>>      /usr/include/linux/swab.h:160:8: error: unknown type name '__always_inline'
>>>      static __always_inline __u16 __swab16p(const __u16 *p)
>>>      ...
>>> The type __aligned_u64 is defined in linux:include/uapi/linux/types.h.
>>>
>>> The fix is to copy missing type definition into
>>> tools/testing/selftests/bpf/include/uapi/linux/types.h.
>>> Adding additional include "string.h" resolves __always_inline issue.
>>>
>>> Fixes: 0a5539f66133 ("bpf: Provide a linux/types.h override for bpf selftests.")
>>> Signed-off-by: Yonghong Song <yhs@fb.com>
>>
>> Can you elaborate why string.h specifically? Can't we define the
>> __always_inline ourselves?
>
> Actually, yes, we can define __always_inline ourselves in each bpf program or in some common header files used by all selftests bpf
> programs.
>
> I use string.h because several other bpf programs
> (test_xdp.c test_l4lb.c test_tcp_estats.c) they all have
> __always_inline and string.h, so they do not have issue.
> I discovered this pattern so I just add string.h into test_pkt_access.c
> as well.

Ok, thanks for clarifying.

Re: [PATCH net v2] selftests/bpf: fix broken build due to types.h

[Yonghong Song]

On 5/17/17 4:01 PM, David Miller wrote:
> From: Daniel Borkmann <daniel@iogearbox.net>
> Date: Thu, 18 May 2017 00:57:04 +0200
> 
>> On 05/18/2017 12:18 AM, Yonghong Song wrote:
>>> Commit 0a5539f66133 ("bpf: Provide a linux/types.h override
>>> for bpf selftests.") caused a build failure for
>>> tools/testing/selftest/bpf
>>> because of some missing types:
>>>       $ make -C tools/testing/selftests/bpf/
>>>       ...
>>>       In file included from
>>>       /home/yhs/work/net-next/tools/testing/selftests/bpf/test_pkt_access.c:8:
>>>       ../../../include/uapi/linux/bpf.h:170:3: error: unknown type name
>>>       '__aligned_u64'
>>>                       __aligned_u64   key;
>>>       ...
>>>       /usr/include/linux/swab.h:160:8: error: unknown type name
>>>       '__always_inline'
>>>       static __always_inline __u16 __swab16p(const __u16 *p)
>>>       ...
>>> The type __aligned_u64 is defined in linux:include/uapi/linux/types.h.
>>>
>>> The fix is to copy missing type definition into
>>> tools/testing/selftests/bpf/include/uapi/linux/types.h.
>>> Adding additional include "string.h" resolves __always_inline issue.
>>>
>>> Fixes: 0a5539f66133 ("bpf: Provide a linux/types.h override for bpf
>>> selftests.")
>>> Signed-off-by: Yonghong Song <yhs@fb.com>
>>
>> Can you elaborate why string.h specifically? Can't we define the
>> __always_inline ourselves?
> 
> That way it comes from compiler.h

Just a little bit correction. The __always_inline is not from 
compiler.h. The compiler.h is inside kernel source tree. Currently,
programs in selftests do not directly referencing kernel header
files (except test_verifier trying to do with alignment)

======
#ifdef HAVE_GENHDR
# include "autoconf.h"
#else
# if defined(__i386) || defined(__x86_64) || defined(__s390x__) || 
defined(__aarch64__)
#  define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS 1
# endif
#endif
=====
(The above part may be gone soon with recent alignment tracking patch)

[yhs@localhost include]$ pwd
/usr/include
[yhs@localhost include]$ find . -name "compiler.h"
[yhs@localhost include]$

The __always_inline comes from sys/cdefs.h
sys/cdefs.h:# define __always_inline __inline __attribute__ 
((__always_inline))

The following is the include chain leading to __always_inline:
string.h
    features.h
       sys/cdefs.h

Yes, it is deeply embedded in chain of header files and hard
to figure out intuitively....

Yonghong

> 
> Probably it would have been better to have the BPF linux/types.h bring
> it in.
> 
> Sorry I applied this so quickly, I wanted this regression fixed as fast
> as possible.
>