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.
