[net-next PATCH 0/2] bpf: sockmap build fixes

[net-next PATCH 0/2] bpf: sockmap build fixes

[John Fastabend]

Two build fixes for sockmap, this should resolve the build errors
and warnings that were reported. Thanks everyone.

---

John Fastabend (2):
      bpf: sockmap state change warning fix
      bpf: sock_map fixes for !CONFIG_BPF_SYSCALL and !STREAM_PARSER


 include/linux/bpf.h       |   10 +++++++++-
 include/linux/bpf_types.h |    2 ++
 kernel/bpf/Makefile       |    5 ++++-
 kernel/bpf/core.c         |    1 +
 kernel/bpf/sockmap.c      |    3 +++
 5 files changed, 19 insertions(+), 2 deletions(-)

[net-next PATCH 1/2] bpf: sockmap state change warning fix

[John Fastabend]

psock will uninitialized in default case we need to do the same psock lookup
and check as in other branch. Fixes compile warning below.

kernel/bpf/sockmap.c: In function ‘smap_state_change’:
kernel/bpf/sockmap.c:156:21: warning: ‘psock’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  struct smap_psock *psock;

Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
Reported-by: David Miller <davem@davemloft.net>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 kernel/bpf/sockmap.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 792f0ad..f7e5e6c 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -188,6 +188,9 @@ static void smap_state_change(struct sock *sk)
 			smap_release_sock(sk);
 		break;
 	default:
+		psock = smap_psock_sk(sk);
+		if (unlikely(!psock))
+			break;
 		smap_report_sk_error(psock, EPIPE);
 		break;
 	}

Re: [net-next PATCH 1/2] bpf: sockmap state change warning fix

[Daniel Borkmann]

On 08/17/2017 12:02 AM, John Fastabend wrote:
> psock will uninitialized in default case we need to do the same psock lookup
> and check as in other branch. Fixes compile warning below.
>
> kernel/bpf/sockmap.c: In function ‘smap_state_change’:
> kernel/bpf/sockmap.c:156:21: warning: ‘psock’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>    struct smap_psock *psock;
>
> Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
> Reported-by: David Miller <davem@davemloft.net>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>

Thanks for fixing up quickly!

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

[net-next PATCH 2/2] bpf: sock_map fixes for !CONFIG_BPF_SYSCALL and !STREAM_PARSER

[John Fastabend]

Resolve issues with !CONFIG_BPF_SYSCALL and !STREAM_PARSER

net/core/filter.c: In function ‘do_sk_redirect_map’:
net/core/filter.c:1881:3: error: implicit declaration of function ‘__sock_map_lookup_elem’ [-Werror=implicit-function-declaration]
   sk = __sock_map_lookup_elem(ri->map, ri->ifindex);
   ^
net/core/filter.c:1881:6: warning: assignment makes pointer from integer without a cast [enabled by default]
   sk = __sock_map_lookup_elem(ri->map, ri->ifindex);

Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 include/linux/bpf.h       |   10 +++++++++-
 include/linux/bpf_types.h |    2 ++
 kernel/bpf/Makefile       |    5 ++++-
 kernel/bpf/core.c         |    1 +
 4 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a4145e9..1cc6c5f 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -313,7 +313,6 @@ static inline void bpf_long_memcpy(void *dst, const void *src, u32 size)
 
 /* Map specifics */
 struct net_device  *__dev_map_lookup_elem(struct bpf_map *map, u32 key);
-struct sock  *__sock_map_lookup_elem(struct bpf_map *map, u32 key);
 void __dev_map_insert_ctx(struct bpf_map *map, u32 index);
 void __dev_map_flush(struct bpf_map *map);
 
@@ -377,6 +376,15 @@ static inline void __dev_map_flush(struct bpf_map *map)
 }
 #endif /* CONFIG_BPF_SYSCALL */
 
+#if defined(CONFIG_STREAM_PARSER) && defined(CONFIG_BPF_SYSCALL)
+struct sock  *__sock_map_lookup_elem(struct bpf_map *map, u32 key);
+#else
+static inline struct sock  *__sock_map_lookup_elem(struct bpf_map *map, u32 key)
+{
+	return NULL;
+}
+#endif
+
 /* verifier prototypes for helper functions called from eBPF programs */
 extern const struct bpf_func_proto bpf_map_lookup_elem_proto;
 extern const struct bpf_func_proto bpf_map_update_elem_proto;
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index fa80507..6f1a567 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -38,5 +38,7 @@
 BPF_MAP_TYPE(BPF_MAP_TYPE_HASH_OF_MAPS, htab_of_maps_map_ops)
 #ifdef CONFIG_NET
 BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
+#ifdef CONFIG_STREAM_PARSER
 BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
 #endif
+#endif
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index aa24287..897daa0 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -3,7 +3,10 @@ obj-y := core.o
 obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o
 obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o
 ifeq ($(CONFIG_NET),y)
-obj-$(CONFIG_BPF_SYSCALL) += devmap.o sockmap.o
+obj-$(CONFIG_BPF_SYSCALL) += devmap.o
+ifeq ($(CONFIG_STREAM_PARSER),y)
+obj-$(CONFIG_BPF_SYSCALL) += sockmap.o
+endif
 endif
 ifeq ($(CONFIG_PERF_EVENTS),y)
 obj-$(CONFIG_BPF_SYSCALL) += stackmap.o
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index c69e7f5..917cc04 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1438,6 +1438,7 @@ void bpf_user_rnd_init_once(void)
 const struct bpf_func_proto bpf_get_current_pid_tgid_proto __weak;
 const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
 const struct bpf_func_proto bpf_get_current_comm_proto __weak;
+const struct bpf_func_proto bpf_sock_map_update_proto __weak;
 
 const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
 {

Re: [net-next PATCH 2/2] bpf: sock_map fixes for !CONFIG_BPF_SYSCALL and !STREAM_PARSER

[David Ahern]

On 8/16/17 4:02 PM, John Fastabend wrote:
> diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
> index aa24287..897daa0 100644
> --- a/kernel/bpf/Makefile
> +++ b/kernel/bpf/Makefile
> @@ -3,7 +3,10 @@ obj-y := core.o
>  obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o
>  obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o
>  ifeq ($(CONFIG_NET),y)
> -obj-$(CONFIG_BPF_SYSCALL) += devmap.o sockmap.o
> +obj-$(CONFIG_BPF_SYSCALL) += devmap.o
> +ifeq ($(CONFIG_STREAM_PARSER),y)
> +obj-$(CONFIG_BPF_SYSCALL) += sockmap.o
> +endif
>  endif

sockmap requires KCM? I thought it just needed STREAM_PARSER. Can't
STREAM_PARSER be used outside of KCM? If so why not make STREAM_PARSER
enabled if sockmap is wanted vs requiring KCM to be compiled in to get
stream parser to get sockmap?

Re: [net-next PATCH 2/2] bpf: sock_map fixes for !CONFIG_BPF_SYSCALL and !STREAM_PARSER

[John Fastabend]

On 08/16/2017 03:06 PM, David Ahern wrote:
> On 8/16/17 4:02 PM, John Fastabend wrote:
>> diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
>> index aa24287..897daa0 100644
>> --- a/kernel/bpf/Makefile
>> +++ b/kernel/bpf/Makefile
>> @@ -3,7 +3,10 @@ obj-y := core.o
>>  obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o
>>  obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o
>>  ifeq ($(CONFIG_NET),y)
>> -obj-$(CONFIG_BPF_SYSCALL) += devmap.o sockmap.o
>> +obj-$(CONFIG_BPF_SYSCALL) += devmap.o
>> +ifeq ($(CONFIG_STREAM_PARSER),y)
>> +obj-$(CONFIG_BPF_SYSCALL) += sockmap.o
>> +endif
>>  endif
> 
> sockmap requires KCM? I thought it just needed STREAM_PARSER. Can't
> STREAM_PARSER be used outside of KCM? If so why not make STREAM_PARSER
> enabled if sockmap is wanted vs requiring KCM to be compiled in to get
> stream parser to get sockmap?
> 

KCM is not required all sockmap needs is STREAM_PARSER. STREAM_PARSER can
be used outside of KCM so no problem there.

I didn't want to require all users of BPF maps to automatically get
STREAM_PARSER however. I'll add a Kconfig option for STREAM_PARSER so we
can pull it in without KCM easily. This is very similar to how the cgroup
BPF build logic works as well with CONFIG_CGROUP_BPF.

I wanted to get the build fix out though as soon as possible rather than
wait for me to write the STREAM_PARSER patch and test it.

Thanks,
John

Re: [net-next PATCH 2/2] bpf: sock_map fixes for !CONFIG_BPF_SYSCALL and !STREAM_PARSER

[Daniel Borkmann]

On 08/17/2017 12:02 AM, John Fastabend wrote:
> Resolve issues with !CONFIG_BPF_SYSCALL and !STREAM_PARSER
>
> net/core/filter.c: In function ‘do_sk_redirect_map’:
> net/core/filter.c:1881:3: error: implicit declaration of function ‘__sock_map_lookup_elem’ [-Werror=implicit-function-declaration]
>     sk = __sock_map_lookup_elem(ri->map, ri->ifindex);
>     ^
> net/core/filter.c:1881:6: warning: assignment makes pointer from integer without a cast [enabled by default]
>     sk = __sock_map_lookup_elem(ri->map, ri->ifindex);
>
> Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
> Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Re: [net-next PATCH 0/2] bpf: sockmap build fixes

[David Miller]

From: John Fastabend <john.fastabend@gmail.com>
Date: Wed, 16 Aug 2017 15:01:52 -0700

> Two build fixes for sockmap, this should resolve the build errors
> and warnings that were reported. Thanks everyone.

Series applied, thanks John.

Re: [net-next PATCH 0/2] bpf: sockmap build fixes

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Wed, 16 Aug 2017 15:35:16 -0700 (PDT)

> From: John Fastabend <john.fastabend@gmail.com>
> Date: Wed, 16 Aug 2017 15:01:52 -0700
> 
>> Two build fixes for sockmap, this should resolve the build errors
>> and warnings that were reported. Thanks everyone.
> 
> Series applied, thanks John.

Actually, now there is a new problem:

ERROR: "tcp_sendpage_locked" [net/ipv6/ipv6.ko] undefined!
ERROR: "tcp_sendmsg_locked" [net/ipv6/ipv6.ko] undefined!
make[1]: *** [scripts/Makefile.modpost:91: __modpost] Error 1
make: *** [Makefile:1217: modules] Error 2

I added GPL exports for these, but please do an allmodconfig
build before you submit anything to me when there are weird
inter-module dependencies like this.

Thanks.

Re: [net-next PATCH 0/2] bpf: sockmap build fixes

[John Fastabend]

On 08/16/2017 03:42 PM, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 16 Aug 2017 15:35:16 -0700 (PDT)
> 
>> From: John Fastabend <john.fastabend@gmail.com>
>> Date: Wed, 16 Aug 2017 15:01:52 -0700
>>
>>> Two build fixes for sockmap, this should resolve the build errors
>>> and warnings that were reported. Thanks everyone.
>>
>> Series applied, thanks John.
> 
> Actually, now there is a new problem:
> 
> ERROR: "tcp_sendpage_locked" [net/ipv6/ipv6.ko] undefined!
> ERROR: "tcp_sendmsg_locked" [net/ipv6/ipv6.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:91: __modpost] Error 1
> make: *** [Makefile:1217: modules] Error 2
> 
> I added GPL exports for these, but please do an allmodconfig
> build before you submit anything to me when there are weird
> inter-module dependencies like this.
> 
> Thanks.
> 

Thanks and allmodconfig noted.