netdev - Re: [bpf-next v2 8/9] bpf: Provide helper to do forwarding lookups in kernel FIB table

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]

Date:   Wed, 9 May 2018 22:44:00 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     David Ahern <dsahern@...il.com>, netdev@...r.kernel.org,
        borkmann@...earbox.net, ast@...nel.org
Cc:     davem@...emloft.net, shm@...ulusnetworks.com,
        roopa@...ulusnetworks.com, brouer@...hat.com, toke@...e.dk,
        john.fastabend@...il.com
Subject: Re: [bpf-next v2 8/9] bpf: Provide helper to do forwarding lookups in
 kernel FIB table

On 05/09/2018 06:05 PM, David Ahern wrote:
> On 5/9/18 2:15 AM, Daniel Borkmann wrote:
>>
>> Ohh well, this is causing allmodconfig build warnings (e.g. on x86) as reported today:
> 
> lovely.
> 
>> In file included from include/linux/dma-mapping.h:5:0,
>>                  from include/linux/skbuff.h:34,
>>                  from include/linux/if_ether.h:23,
>>                  from include/uapi/linux/bpf.h:13,
>>                  from include/linux/bpf-cgroup.h:6,
>>                  from include/linux/cgroup-defs.h:22,
>>                  from include/linux/cgroup.h:28,
>>                  from include/linux/perf_event.h:57,
>>                  from include/linux/trace_events.h:10,
>>                  from include/trace/trace_events.h:20,
>>                  from include/trace/define_trace.h:96,
>>                  from drivers/android/binder_trace.h:387,
>>                  from drivers/android/binder.c:5702:
>> include/linux/sizes.h:24:0: warning: "SZ_1K" redefined
>>  #define SZ_1K    0x00000400
>> drivers/android/binder.c:116:0: note: this is the location of the previous definition
>>  #define SZ_1K                               0x400
> 
> binder.c has very few recent commits to it. Are you ok with me
> submitting the change with the others in this set (with proper cc's of
> course)?
> 
>> fs/ecryptfs/miscdev.c:206:0: warning: "PKT_TYPE_OFFSET" redefined
>>  #define PKT_TYPE_OFFSET  0
>> In file included from include/linux/if_ether.h:23:0,
>>                  from include/uapi/linux/bpf.h:13,
>>                  from include/linux/bpf-cgroup.h:6,
>>                  from include/linux/cgroup-defs.h:22,
>>                  from include/linux/cgroup.h:28,
>>                  from include/linux/writeback.h:183,
>>                  from include/linux/backing-dev.h:16,
>>                  from fs/ecryptfs/ecryptfs_kernel.h:41,
>>                  from fs/ecryptfs/miscdev.c:30:
>> include/linux/skbuff.h:753:0: note: this is the location of the previous definition
>>  #define PKT_TYPE_OFFSET() offsetof(struct sk_buff, __pkt_type_offset)
> 
> And this one I renamed to SKB_PKT_TYPE_OFFSET
> 
> With that it compiles cleanly.

Generally, no objection. However, could we get rid of the two extra includes altogether
to avoid running into any such dependency issue? Right now the only includes we have in
the bpf uapi header is linux/types.h and linux/bpf_common.h (latter has no extra deps
by itself). Both the ETH_ALEN and struct in6_addr are in uapi and therefore never allowed
to change so we can e.g. avoid to use ETH_ALEN and just have the value instead. In the
other places of the header we use __u32 remote_ipv6[4], __u32 src_ip6[4] etc to denote
a v6 address, we could do the same here and should be all good then.