Messages in this thread Patch in this message | | | From | Joe Stringer <> | Date | Fri, 5 Oct 2018 10:19:47 -0700 | Subject | Re: [PATCH] bpf: fix building without CONFIG_INET |
| |
On Fri, 5 Oct 2018 at 09:16, Arnd Bergmann <arnd@arndb.de> wrote: > > The newly added TCP and UDP handling fails to link when CONFIG_INET > is disabled: > > net/core/filter.o: In function `sk_lookup': > filter.c:(.text+0x7ff8): undefined reference to `tcp_hashinfo' > filter.c:(.text+0x7ffc): undefined reference to `tcp_hashinfo' > filter.c:(.text+0x8020): undefined reference to `__inet_lookup_established' > filter.c:(.text+0x8058): undefined reference to `__inet_lookup_listener' > filter.c:(.text+0x8068): undefined reference to `udp_table' > filter.c:(.text+0x8070): undefined reference to `udp_table' > filter.c:(.text+0x808c): undefined reference to `__udp4_lib_lookup' > net/core/filter.o: In function `bpf_sk_release': > filter.c:(.text+0x82e8): undefined reference to `sock_gen_put' > > The compiler can optimize it out and avoid those references for > the most part, but we are missing a few steps here: > > - sk_lookup() should always have been marked 'static', this also > avoids a warning about a missing prototype when building with > 'make W=1'. > - The BPF_CALL_x() macro needs a little change to allow marking > the unneeded BPF call as 'static' and having the compiler > drop them. > - The reference to the bpf_func_proto must be made conditional. > > Fixes: 6acc9b432e67 ("bpf: Add helper to retrieve socket in BPF") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > ---
Thanks for the fix.
> include/linux/filter.h | 2 +- > net/core/filter.c | 18 +++++++++++------- > 2 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/include/linux/filter.h b/include/linux/filter.h > index 6791a0ac0139..d9ec9d908bbe 100644 > --- a/include/linux/filter.h > +++ b/include/linux/filter.h > @@ -428,9 +428,9 @@ struct sock_reuseport; > u64, __ur_3, u64, __ur_4, u64, __ur_5) > > #define BPF_CALL_x(x, name, ...) \ > + u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__)); \ > static __always_inline \ > u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__)); \ > - u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__)); \ > u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__)) \ > { \ > return ____##name(__BPF_MAP(x,__BPF_CAST,__BPF_N,__VA_ARGS__));\
For what it's worth, other similar cases in net/core/filter.c avoid this by just wrapping the relevant sections of the code in the #ifdef as well.
Might be a bit simpler to follow that style (only checked with make M=net/core W=1):
$ git di diff --git a/net/core/filter.c b/net/core/filter.c index 30c6b2d3ef16..4bbc6567fcb8 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -4817,8 +4817,9 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = { }; #endif /* CONFIG_IPV6_SEG6_BPF */
-struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple, - struct sk_buff *skb, u8 family, u8 proto) +#ifdef CONFIG_INET +static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple, + struct sk_buff *skb, u8 family, u8 proto) { int dif = skb->dev->ifindex; bool refcounted = false; @@ -4951,6 +4952,7 @@ static const struct bpf_func_proto bpf_sk_release_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_SOCKET, }; +#endif /* CONFIG_INET */
bool bpf_helper_changes_pkt_data(void *func) { @@ -5158,12 +5160,14 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) case BPF_FUNC_skb_ancestor_cgroup_id: return &bpf_skb_ancestor_cgroup_id_proto; #endif +#ifdef CONFIG_INET case BPF_FUNC_sk_lookup_tcp: return &bpf_sk_lookup_tcp_proto; case BPF_FUNC_sk_lookup_udp: return &bpf_sk_lookup_udp_proto; case BPF_FUNC_sk_release: return &bpf_sk_release_proto; +#endif default: return bpf_base_func_proto(func_id); } @@ -5264,12 +5268,14 @@ sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) return &bpf_sk_redirect_hash_proto; case BPF_FUNC_get_local_storage: return &bpf_get_local_storage_proto; +#ifdef CONFIG_INET case BPF_FUNC_sk_lookup_tcp: return &bpf_sk_lookup_tcp_proto; case BPF_FUNC_sk_lookup_udp: return &bpf_sk_lookup_udp_proto; case BPF_FUNC_sk_release: return &bpf_sk_release_proto; +#endif default: return bpf_base_func_proto(func_id); }
| |