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

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

[Arnd Bergmann]

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 <arnd@arndb.de>
---
 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

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

[Arnd Bergmann]

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 <arnd@arndb.de>
---
 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

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

[Soheil Hassas Yeganeh]

On Mon, Jul 8, 2019 at 8:57 AM Arnd Bergmann <arnd@arndb.de> 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 <arnd@arndb.de>

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

> ---
>  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
>

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

[Yonghong Song]

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 <arnd@arndb.de>

FYI.

There is already a patch to fix the same issue,
https://lore.kernel.org/bpf/e9e489fe-feec-a211-82aa-5df0c6a308d1@huawei.com/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
> 

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

[Daniel Borkmann]

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 <arnd@arndb.de>
> 
> FYI.
> 
> There is already a patch to fix the same issue,
> https://lore.kernel.org/bpf/e9e489fe-feec-a211-82aa-5df0c6a308d1@huawei.com/T/#t
> 
> which has been acked and not merged yet.

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