2019-07-08 17:50:32

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH net-next 1/2] bpf: skip sockopt hooks without CONFIG_NET

When CONFIG_NET is disabled, we get a link error:

kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_setsockopt':
cgroup.c:(.text+0x3010): undefined reference to `lock_sock_nested'
cgroup.c:(.text+0x3258): undefined reference to `release_sock'
kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_getsockopt':
cgroup.c:(.text+0x3568): undefined reference to `lock_sock_nested'
cgroup.c:(.text+0x3870): undefined reference to `release_sock'
kernel/bpf/cgroup.o: In function `cg_sockopt_func_proto':
cgroup.c:(.text+0x41d8): undefined reference to `bpf_sk_storage_delete_proto'

None of this code is useful in this configuration anyway, so we can
simply hide it in an appropriate #ifdef.

Fixes: 0d01da6afc54 ("bpf: implement getsockopt and setsockopt hooks")
Signed-off-by: Arnd Bergmann <[email protected]>
---
include/linux/bpf_types.h | 2 ++
kernel/bpf/cgroup.c | 6 ++++++
2 files changed, 8 insertions(+)

diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index eec5aeeeaf92..3c7222b2db96 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -30,8 +30,10 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE, raw_tracepoint_writable)
#ifdef CONFIG_CGROUP_BPF
BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_DEVICE, cg_dev)
BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SYSCTL, cg_sysctl)
+#ifdef CONFIG_NET
BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCKOPT, cg_sockopt)
#endif
+#endif
#ifdef CONFIG_BPF_LIRC_MODE2
BPF_PROG_TYPE(BPF_PROG_TYPE_LIRC_MODE2, lirc_mode2)
#endif
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 76fa0076f20d..7be44460bd93 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -590,6 +590,7 @@ int cgroup_bpf_prog_query(const union bpf_attr *attr,
return ret;
}

+#ifdef CONFIG_NET
/**
* __cgroup_bpf_run_filter_skb() - Run a program for packet filtering
* @sk: The socket sending or receiving traffic
@@ -750,6 +751,7 @@ int __cgroup_bpf_run_filter_sock_ops(struct sock *sk,
return ret == 1 ? 0 : -EPERM;
}
EXPORT_SYMBOL(__cgroup_bpf_run_filter_sock_ops);
+#endif

int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
short access, enum bpf_attach_type type)
@@ -939,6 +941,7 @@ int __cgroup_bpf_run_filter_sysctl(struct ctl_table_header *head,
}
EXPORT_SYMBOL(__cgroup_bpf_run_filter_sysctl);

+#ifdef CONFIG_NET
static bool __cgroup_bpf_prog_array_is_empty(struct cgroup *cgrp,
enum bpf_attach_type attach_type)
{
@@ -1120,6 +1123,7 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
return ret;
}
EXPORT_SYMBOL(__cgroup_bpf_run_filter_getsockopt);
+#endif

static ssize_t sysctl_cpy_dir(const struct ctl_dir *dir, char **bufp,
size_t *lenp)
@@ -1382,6 +1386,7 @@ const struct bpf_verifier_ops cg_sysctl_verifier_ops = {
const struct bpf_prog_ops cg_sysctl_prog_ops = {
};

+#ifdef CONFIG_NET
static const struct bpf_func_proto *
cg_sockopt_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
@@ -1531,3 +1536,4 @@ const struct bpf_verifier_ops cg_sockopt_verifier_ops = {

const struct bpf_prog_ops cg_sockopt_prog_ops = {
};
+#endif
--
2.20.0


2019-07-08 17:50:34

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH net-next 2/2] bpf: avoid unused variable warning in tcp_bpf_rtt()

When CONFIG_BPF is disabled, we get a warning for an unused
variable:

In file included from drivers/target/target_core_device.c:26:
include/net/tcp.h:2226:19: error: unused variable 'tp' [-Werror,-Wunused-variable]
struct tcp_sock *tp = tcp_sk(sk);

The variable is only used in one place, so it can be
replaced with its value there to avoid the warning.

Fixes: 23729ff23186 ("bpf: add BPF_CGROUP_SOCK_OPS callback that is executed on every RTT")
Signed-off-by: Arnd Bergmann <[email protected]>
---
include/net/tcp.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index e16d8a3fd3b4..cca3c59b98bf 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2223,9 +2223,7 @@ static inline bool tcp_bpf_ca_needs_ecn(struct sock *sk)

static inline void tcp_bpf_rtt(struct sock *sk)
{
- struct tcp_sock *tp = tcp_sk(sk);
-
- if (BPF_SOCK_OPS_TEST_FLAG(tp, BPF_SOCK_OPS_RTT_CB_FLAG))
+ if (BPF_SOCK_OPS_TEST_FLAG(tcp_sk(sk), BPF_SOCK_OPS_RTT_CB_FLAG))
tcp_call_bpf(sk, BPF_SOCK_OPS_RTT_CB, 0, NULL);
}

--
2.20.0

2019-07-08 19:03:37

by Soheil Hassas Yeganeh

[permalink] [raw]
Subject: Re: [PATCH net-next 2/2] bpf: avoid unused variable warning in tcp_bpf_rtt()

On Mon, Jul 8, 2019 at 8:57 AM Arnd Bergmann <[email protected]> wrote:
>
> When CONFIG_BPF is disabled, we get a warning for an unused
> variable:
>
> In file included from drivers/target/target_core_device.c:26:
> include/net/tcp.h:2226:19: error: unused variable 'tp' [-Werror,-Wunused-variable]
> struct tcp_sock *tp = tcp_sk(sk);
>
> The variable is only used in one place, so it can be
> replaced with its value there to avoid the warning.
>
> Fixes: 23729ff23186 ("bpf: add BPF_CGROUP_SOCK_OPS callback that is executed on every RTT")
> Signed-off-by: Arnd Bergmann <[email protected]>

Acked-by: Soheil Hassas Yeganeh <[email protected]>

> ---
> include/net/tcp.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index e16d8a3fd3b4..cca3c59b98bf 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2223,9 +2223,7 @@ static inline bool tcp_bpf_ca_needs_ecn(struct sock *sk)
>
> static inline void tcp_bpf_rtt(struct sock *sk)
> {
> - struct tcp_sock *tp = tcp_sk(sk);
> -
> - if (BPF_SOCK_OPS_TEST_FLAG(tp, BPF_SOCK_OPS_RTT_CB_FLAG))
> + if (BPF_SOCK_OPS_TEST_FLAG(tcp_sk(sk), BPF_SOCK_OPS_RTT_CB_FLAG))
> tcp_call_bpf(sk, BPF_SOCK_OPS_RTT_CB, 0, NULL);
> }
>
> --
> 2.20.0
>

2019-07-08 19:28:38

by Yonghong Song

[permalink] [raw]
Subject: Re: [PATCH net-next 1/2] bpf: skip sockopt hooks without CONFIG_NET



On 7/8/19 5:57 AM, Arnd Bergmann wrote:
> When CONFIG_NET is disabled, we get a link error:
>
> kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_setsockopt':
> cgroup.c:(.text+0x3010): undefined reference to `lock_sock_nested'
> cgroup.c:(.text+0x3258): undefined reference to `release_sock'
> kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_getsockopt':
> cgroup.c:(.text+0x3568): undefined reference to `lock_sock_nested'
> cgroup.c:(.text+0x3870): undefined reference to `release_sock'
> kernel/bpf/cgroup.o: In function `cg_sockopt_func_proto':
> cgroup.c:(.text+0x41d8): undefined reference to `bpf_sk_storage_delete_proto'
>
> None of this code is useful in this configuration anyway, so we can
> simply hide it in an appropriate #ifdef.
>
> Fixes: 0d01da6afc54 ("bpf: implement getsockopt and setsockopt hooks")
> Signed-off-by: Arnd Bergmann <[email protected]>

FYI.

There is already a patch to fix the same issue,
https://lore.kernel.org/bpf/[email protected]/T/#t

which has been acked and not merged yet.

> ---
> include/linux/bpf_types.h | 2 ++
> kernel/bpf/cgroup.c | 6 ++++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index eec5aeeeaf92..3c7222b2db96 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -30,8 +30,10 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE, raw_tracepoint_writable)
> #ifdef CONFIG_CGROUP_BPF
> BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_DEVICE, cg_dev)
> BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SYSCTL, cg_sysctl)
> +#ifdef CONFIG_NET
> BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCKOPT, cg_sockopt)
> #endif
> +#endif
> #ifdef CONFIG_BPF_LIRC_MODE2
> BPF_PROG_TYPE(BPF_PROG_TYPE_LIRC_MODE2, lirc_mode2)
> #endif
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 76fa0076f20d..7be44460bd93 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -590,6 +590,7 @@ int cgroup_bpf_prog_query(const union bpf_attr *attr,
> return ret;
> }
>
> +#ifdef CONFIG_NET
> /**
> * __cgroup_bpf_run_filter_skb() - Run a program for packet filtering
> * @sk: The socket sending or receiving traffic
> @@ -750,6 +751,7 @@ int __cgroup_bpf_run_filter_sock_ops(struct sock *sk,
> return ret == 1 ? 0 : -EPERM;
> }
> EXPORT_SYMBOL(__cgroup_bpf_run_filter_sock_ops);
> +#endif
>
> int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
> short access, enum bpf_attach_type type)
> @@ -939,6 +941,7 @@ int __cgroup_bpf_run_filter_sysctl(struct ctl_table_header *head,
> }
> EXPORT_SYMBOL(__cgroup_bpf_run_filter_sysctl);
>
> +#ifdef CONFIG_NET
> static bool __cgroup_bpf_prog_array_is_empty(struct cgroup *cgrp,
> enum bpf_attach_type attach_type)
> {
> @@ -1120,6 +1123,7 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
> return ret;
> }
> EXPORT_SYMBOL(__cgroup_bpf_run_filter_getsockopt);
> +#endif
>
> static ssize_t sysctl_cpy_dir(const struct ctl_dir *dir, char **bufp,
> size_t *lenp)
> @@ -1382,6 +1386,7 @@ const struct bpf_verifier_ops cg_sysctl_verifier_ops = {
> const struct bpf_prog_ops cg_sysctl_prog_ops = {
> };
>
> +#ifdef CONFIG_NET
> static const struct bpf_func_proto *
> cg_sockopt_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> {
> @@ -1531,3 +1536,4 @@ const struct bpf_verifier_ops cg_sockopt_verifier_ops = {
>
> const struct bpf_prog_ops cg_sockopt_prog_ops = {
> };
> +#endif
>

2019-07-08 22:41:07

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH net-next 1/2] bpf: skip sockopt hooks without CONFIG_NET

On 07/08/2019 05:06 PM, Yonghong Song wrote:
> On 7/8/19 5:57 AM, Arnd Bergmann wrote:
>> When CONFIG_NET is disabled, we get a link error:
>>
>> kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_setsockopt':
>> cgroup.c:(.text+0x3010): undefined reference to `lock_sock_nested'
>> cgroup.c:(.text+0x3258): undefined reference to `release_sock'
>> kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_getsockopt':
>> cgroup.c:(.text+0x3568): undefined reference to `lock_sock_nested'
>> cgroup.c:(.text+0x3870): undefined reference to `release_sock'
>> kernel/bpf/cgroup.o: In function `cg_sockopt_func_proto':
>> cgroup.c:(.text+0x41d8): undefined reference to `bpf_sk_storage_delete_proto'
>>
>> None of this code is useful in this configuration anyway, so we can
>> simply hide it in an appropriate #ifdef.
>>
>> Fixes: 0d01da6afc54 ("bpf: implement getsockopt and setsockopt hooks")
>> Signed-off-by: Arnd Bergmann <[email protected]>
>
> FYI.
>
> There is already a patch to fix the same issue,
> https://lore.kernel.org/bpf/[email protected]/T/#t
>
> which has been acked and not merged yet.

Done now, and I've also applied patch 2/2 from here, thanks.