diff mbox series

[bpf-next,01/19] bpf: rename BPF_STREAM_PARSER to BPF_SOCK_MAP

Message ID 20210203041636.38555-2-xiyou.wangcong@gmail.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series sock_map: add non-TCP and cross-protocol support | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count fail Series longer than 15 patches
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 10 maintainers not CCed: songliubraving@fb.com andrii@kernel.org yoshfuji@linux-ipv6.org edumazet@google.com kpsingh@kernel.org davem@davemloft.net ast@kernel.org kuba@kernel.org kafai@fb.com yhs@fb.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 12175 this patch: 12175
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 106 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 12823 this patch: 12823
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Cong Wang Feb. 3, 2021, 4:16 a.m. UTC
From: Cong Wang <cong.wang@bytedance.com>

Before we add non-TCP support, it is necessary to rename
BPF_STREAM_PARSER as it will be no longer specific to TCP,
and it does not have to be a parser either.

This patch renames BPF_STREAM_PARSER to BPF_SOCK_MAP, so
that sock_map.c hopefully would be protocol-independent.

Also, improve its Kconfig description to avoid confusion.

Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jakub Sitnicki <jakub@cloudflare.com>
Cc: Lorenz Bauer <lmb@cloudflare.com>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
---
 include/linux/bpf.h       |  4 ++--
 include/linux/bpf_types.h |  2 +-
 include/net/tcp.h         |  4 ++--
 include/net/udp.h         |  4 ++--
 net/Kconfig               | 13 ++++++-------
 net/core/Makefile         |  2 +-
 net/ipv4/Makefile         |  2 +-
 net/ipv4/tcp_bpf.c        |  4 ++--
 8 files changed, 17 insertions(+), 18 deletions(-)

Comments

Jakub Sitnicki Feb. 5, 2021, 10:32 a.m. UTC | #1
On Wed, Feb 03, 2021 at 05:16 AM CET, Cong Wang wrote:
> From: Cong Wang <cong.wang@bytedance.com>
>
> Before we add non-TCP support, it is necessary to rename
> BPF_STREAM_PARSER as it will be no longer specific to TCP,
> and it does not have to be a parser either.
>
> This patch renames BPF_STREAM_PARSER to BPF_SOCK_MAP, so
> that sock_map.c hopefully would be protocol-independent.
>
> Also, improve its Kconfig description to avoid confusion.
>
> Cc: John Fastabend <john.fastabend@gmail.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Jakub Sitnicki <jakub@cloudflare.com>
> Cc: Lorenz Bauer <lmb@cloudflare.com>
> Signed-off-by: Cong Wang <cong.wang@bytedance.com>
> ---
>  include/linux/bpf.h       |  4 ++--
>  include/linux/bpf_types.h |  2 +-
>  include/net/tcp.h         |  4 ++--
>  include/net/udp.h         |  4 ++--
>  net/Kconfig               | 13 ++++++-------
>  net/core/Makefile         |  2 +-
>  net/ipv4/Makefile         |  2 +-
>  net/ipv4/tcp_bpf.c        |  4 ++--
>  8 files changed, 17 insertions(+), 18 deletions(-)

We also have a couple of references to CONFIG_BPF_STREAM_PARSER in
tools/tests:

$ git grep -i bpf_stream_parser
...
tools/bpf/bpftool/feature.c:            { "CONFIG_BPF_STREAM_PARSER", },
tools/testing/selftests/bpf/config:CONFIG_BPF_STREAM_PARSER=y

[...]
John Fastabend Feb. 8, 2021, 8:21 a.m. UTC | #2
Cong Wang wrote:
> From: Cong Wang <cong.wang@bytedance.com>
> 
> Before we add non-TCP support, it is necessary to rename
> BPF_STREAM_PARSER as it will be no longer specific to TCP,
> and it does not have to be a parser either.
> 
> This patch renames BPF_STREAM_PARSER to BPF_SOCK_MAP, so
> that sock_map.c hopefully would be protocol-independent.
> 
> Also, improve its Kconfig description to avoid confusion.
> 
> Cc: John Fastabend <john.fastabend@gmail.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Jakub Sitnicki <jakub@cloudflare.com>
> Cc: Lorenz Bauer <lmb@cloudflare.com>
> Signed-off-by: Cong Wang <cong.wang@bytedance.com>
> ---

The BPF_STREAM_PARSER config was originally added because we need
the STREAM_PARSER define and wanted a way to get the 'depends on'
lines in Kconfig correct.

Rather than rename this, lets reduce its scope to just the set
of actions that need the STREAM_PARSER, this should be just the
stream parser programs. We probably should have done this sooner,
but doing it now will be fine.

I can probably draft a quick patch tomorrow if above is not clear.
It can go into bpf-next outside this series as well to reduce
the 19 patches a bit.

Thanks,
John
Lorenz Bauer Feb. 8, 2021, 9:50 a.m. UTC | #3
On Mon, 8 Feb 2021 at 08:21, John Fastabend <john.fastabend@gmail.com> wrote:
...
>
> Rather than rename this, lets reduce its scope to just the set
> of actions that need the STREAM_PARSER, this should be just the
> stream parser programs. We probably should have done this sooner,
> but doing it now will be fine.

So sockmap would not be hidden behind a CONFIG anymore? That
would be great.
Cong Wang Feb. 9, 2021, 1:40 a.m. UTC | #4
On Fri, Feb 5, 2021 at 2:32 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>
> On Wed, Feb 03, 2021 at 05:16 AM CET, Cong Wang wrote:
> > From: Cong Wang <cong.wang@bytedance.com>
> >
> > Before we add non-TCP support, it is necessary to rename
> > BPF_STREAM_PARSER as it will be no longer specific to TCP,
> > and it does not have to be a parser either.
> >
> > This patch renames BPF_STREAM_PARSER to BPF_SOCK_MAP, so
> > that sock_map.c hopefully would be protocol-independent.
> >
> > Also, improve its Kconfig description to avoid confusion.
> >
> > Cc: John Fastabend <john.fastabend@gmail.com>
> > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > Cc: Jakub Sitnicki <jakub@cloudflare.com>
> > Cc: Lorenz Bauer <lmb@cloudflare.com>
> > Signed-off-by: Cong Wang <cong.wang@bytedance.com>
> > ---
> >  include/linux/bpf.h       |  4 ++--
> >  include/linux/bpf_types.h |  2 +-
> >  include/net/tcp.h         |  4 ++--
> >  include/net/udp.h         |  4 ++--
> >  net/Kconfig               | 13 ++++++-------
> >  net/core/Makefile         |  2 +-
> >  net/ipv4/Makefile         |  2 +-
> >  net/ipv4/tcp_bpf.c        |  4 ++--
> >  8 files changed, 17 insertions(+), 18 deletions(-)
>
> We also have a couple of references to CONFIG_BPF_STREAM_PARSER in
> tools/tests:
>
> $ git grep -i bpf_stream_parser
> ...
> tools/bpf/bpftool/feature.c:            { "CONFIG_BPF_STREAM_PARSER", },
> tools/testing/selftests/bpf/config:CONFIG_BPF_STREAM_PARSER=y

I think I did a grep in the whole tree, but still missed these two.

Thanks for catching it!
Cong Wang Feb. 9, 2021, 1:45 a.m. UTC | #5
On Mon, Feb 8, 2021 at 12:21 AM John Fastabend <john.fastabend@gmail.com> wrote:
>
> Cong Wang wrote:
> > From: Cong Wang <cong.wang@bytedance.com>
> >
> > Before we add non-TCP support, it is necessary to rename
> > BPF_STREAM_PARSER as it will be no longer specific to TCP,
> > and it does not have to be a parser either.
> >
> > This patch renames BPF_STREAM_PARSER to BPF_SOCK_MAP, so
> > that sock_map.c hopefully would be protocol-independent.
> >
> > Also, improve its Kconfig description to avoid confusion.
> >
> > Cc: John Fastabend <john.fastabend@gmail.com>
> > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > Cc: Jakub Sitnicki <jakub@cloudflare.com>
> > Cc: Lorenz Bauer <lmb@cloudflare.com>
> > Signed-off-by: Cong Wang <cong.wang@bytedance.com>
> > ---
>
> The BPF_STREAM_PARSER config was originally added because we need
> the STREAM_PARSER define and wanted a way to get the 'depends on'
> lines in Kconfig correct.
>
> Rather than rename this, lets reduce its scope to just the set
> of actions that need the STREAM_PARSER, this should be just the
> stream parser programs. We probably should have done this sooner,
> but doing it now will be fine.

This makes sense, but we still need a Kconfig for the rest sockmap
code, right? At least for the dependency on NET_SOCK_MSG?

>
> I can probably draft a quick patch tomorrow if above is not clear.
> It can go into bpf-next outside this series as well to reduce
> the 19 patches a bit.

I can handle it in my next update too, like all other feedbacks.

Thanks.
John Fastabend Feb. 9, 2021, 6:48 a.m. UTC | #6
Cong Wang wrote:
> On Mon, Feb 8, 2021 at 12:21 AM John Fastabend <john.fastabend@gmail.com> wrote:
> >
> > Cong Wang wrote:
> > > From: Cong Wang <cong.wang@bytedance.com>
> > >
> > > Before we add non-TCP support, it is necessary to rename
> > > BPF_STREAM_PARSER as it will be no longer specific to TCP,
> > > and it does not have to be a parser either.
> > >
> > > This patch renames BPF_STREAM_PARSER to BPF_SOCK_MAP, so
> > > that sock_map.c hopefully would be protocol-independent.
> > >
> > > Also, improve its Kconfig description to avoid confusion.
> > >
> > > Cc: John Fastabend <john.fastabend@gmail.com>
> > > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > > Cc: Jakub Sitnicki <jakub@cloudflare.com>
> > > Cc: Lorenz Bauer <lmb@cloudflare.com>
> > > Signed-off-by: Cong Wang <cong.wang@bytedance.com>
> > > ---
> >
> > The BPF_STREAM_PARSER config was originally added because we need
> > the STREAM_PARSER define and wanted a way to get the 'depends on'
> > lines in Kconfig correct.
> >
> > Rather than rename this, lets reduce its scope to just the set
> > of actions that need the STREAM_PARSER, this should be just the
> > stream parser programs. We probably should have done this sooner,
> > but doing it now will be fine.
> 
> This makes sense, but we still need a Kconfig for the rest sockmap
> code, right? At least for the dependency on NET_SOCK_MSG?

Lets just enable NET_SOCK_MSG when CONFIG_BPF_SYSCALL is enabled. We
never put any of the other maps, devmap, cpumap, etc. behind an
explicit flag like this.

> 
> >
> > I can probably draft a quick patch tomorrow if above is not clear.
> > It can go into bpf-next outside this series as well to reduce
> > the 19 patches a bit.
> 
> I can handle it in my next update too, like all other feedbacks.

Great thanks. Especially because I haven't got to it yet today.

> 
> Thanks.
diff mbox series

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 321966fc35db..b5af6a4e9927 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1771,7 +1771,7 @@  static inline void bpf_map_offload_map_free(struct bpf_map *map)
 }
 #endif /* CONFIG_NET && CONFIG_BPF_SYSCALL */
 
-#if defined(CONFIG_BPF_STREAM_PARSER)
+#if defined(CONFIG_BPF_SOCK_MAP)
 int sock_map_prog_update(struct bpf_map *map, struct bpf_prog *prog,
 			 struct bpf_prog *old, u32 which);
 int sock_map_get_from_fd(const union bpf_attr *attr, struct bpf_prog *prog);
@@ -1804,7 +1804,7 @@  static inline int sock_map_update_elem_sys(struct bpf_map *map, void *key, void
 {
 	return -EOPNOTSUPP;
 }
-#endif /* CONFIG_BPF_STREAM_PARSER */
+#endif /* CONFIG_BPF_SOCK_MAP */
 
 #if defined(CONFIG_INET) && defined(CONFIG_BPF_SYSCALL)
 void bpf_sk_reuseport_detach(struct sock *sk);
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 99f7fd657d87..6e27726ae578 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -103,7 +103,7 @@  BPF_MAP_TYPE(BPF_MAP_TYPE_HASH_OF_MAPS, htab_of_maps_map_ops)
 BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
 BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP_HASH, dev_map_hash_ops)
 BPF_MAP_TYPE(BPF_MAP_TYPE_SK_STORAGE, sk_storage_map_ops)
-#if defined(CONFIG_BPF_STREAM_PARSER)
+#if defined(CONFIG_BPF_SOCK_MAP)
 BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
 BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKHASH, sock_hash_ops)
 #endif
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 4bb42fb19711..be66571ad122 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2207,14 +2207,14 @@  void tcp_update_ulp(struct sock *sk, struct proto *p,
 struct sk_msg;
 struct sk_psock;
 
-#ifdef CONFIG_BPF_STREAM_PARSER
+#ifdef CONFIG_BPF_SOCK_MAP
 struct proto *tcp_bpf_get_proto(struct sock *sk, struct sk_psock *psock);
 void tcp_bpf_clone(const struct sock *sk, struct sock *newsk);
 #else
 static inline void tcp_bpf_clone(const struct sock *sk, struct sock *newsk)
 {
 }
-#endif /* CONFIG_BPF_STREAM_PARSER */
+#endif /* CONFIG_BPF_SOCK_MAP */
 
 #ifdef CONFIG_NET_SOCK_MSG
 int tcp_bpf_sendmsg_redir(struct sock *sk, struct sk_msg *msg, u32 bytes,
diff --git a/include/net/udp.h b/include/net/udp.h
index 877832bed471..0ff921e6b866 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -511,9 +511,9 @@  static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
 	return segs;
 }
 
-#ifdef CONFIG_BPF_STREAM_PARSER
+#ifdef CONFIG_BPF_SOCK_MAP
 struct sk_psock;
 struct proto *udp_bpf_get_proto(struct sock *sk, struct sk_psock *psock);
-#endif /* BPF_STREAM_PARSER */
+#endif /* CONFIG_BPF_SOCK_MAP */
 
 #endif	/* _UDP_H */
diff --git a/net/Kconfig b/net/Kconfig
index f4c32d982af6..0cc0805a8127 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -305,20 +305,19 @@  config BPF_JIT
 	  /proc/sys/net/core/bpf_jit_harden   (optional)
 	  /proc/sys/net/core/bpf_jit_kallsyms (optional)
 
-config BPF_STREAM_PARSER
-	bool "enable BPF STREAM_PARSER"
+config BPF_SOCK_MAP
+	bool "enable BPF socket maps"
 	depends on INET
 	depends on BPF_SYSCALL
 	depends on CGROUP_BPF
 	select STREAM_PARSER
 	select NET_SOCK_MSG
 	help
-	  Enabling this allows a stream parser to be used with
-	  BPF_MAP_TYPE_SOCKMAP.
+	  Enabling this allows skb parser and verdict to be used with
+	  BPF_MAP_TYPE_SOCKMAP or BPF_MAP_TYPE_SOCKHASH.
 
-	  BPF_MAP_TYPE_SOCKMAP provides a map type to use with network sockets.
-	  It can be used to enforce socket policy, implement socket redirects,
-	  etc.
+	  This provides a BPF map type to use with network sockets. It can
+	  be used to enforce socket policy, implement socket redirects, etc.
 
 config NET_FLOW_LIMIT
 	bool
diff --git a/net/core/Makefile b/net/core/Makefile
index 3e2c378e5f31..e7c1bdaadefd 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -28,7 +28,7 @@  obj-$(CONFIG_CGROUP_NET_PRIO) += netprio_cgroup.o
 obj-$(CONFIG_CGROUP_NET_CLASSID) += netclassid_cgroup.o
 obj-$(CONFIG_LWTUNNEL) += lwtunnel.o
 obj-$(CONFIG_LWTUNNEL_BPF) += lwt_bpf.o
-obj-$(CONFIG_BPF_STREAM_PARSER) += sock_map.o
+obj-$(CONFIG_BPF_SOCK_MAP) += sock_map.o
 obj-$(CONFIG_DST_CACHE) += dst_cache.o
 obj-$(CONFIG_HWBM) += hwbm.o
 obj-$(CONFIG_NET_DEVLINK) += devlink.o
diff --git a/net/ipv4/Makefile b/net/ipv4/Makefile
index 5b77a46885b9..f72f84d1b982 100644
--- a/net/ipv4/Makefile
+++ b/net/ipv4/Makefile
@@ -62,7 +62,7 @@  obj-$(CONFIG_TCP_CONG_LP) += tcp_lp.o
 obj-$(CONFIG_TCP_CONG_YEAH) += tcp_yeah.o
 obj-$(CONFIG_TCP_CONG_ILLINOIS) += tcp_illinois.o
 obj-$(CONFIG_NET_SOCK_MSG) += tcp_bpf.o
-obj-$(CONFIG_BPF_STREAM_PARSER) += udp_bpf.o
+obj-$(CONFIG_BPF_SOCK_MAP) += udp_bpf.o
 obj-$(CONFIG_NETLABEL) += cipso_ipv4.o
 
 obj-$(CONFIG_XFRM) += xfrm4_policy.o xfrm4_state.o xfrm4_input.o \
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index bc7d2a586e18..2252f1d90676 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -229,7 +229,7 @@  int tcp_bpf_sendmsg_redir(struct sock *sk, struct sk_msg *msg,
 }
 EXPORT_SYMBOL_GPL(tcp_bpf_sendmsg_redir);
 
-#ifdef CONFIG_BPF_STREAM_PARSER
+#ifdef CONFIG_BPF_SOCK_MAP
 static bool tcp_bpf_stream_read(const struct sock *sk)
 {
 	struct sk_psock *psock;
@@ -629,4 +629,4 @@  void tcp_bpf_clone(const struct sock *sk, struct sock *newsk)
 	if (prot == &tcp_bpf_prots[family][TCP_BPF_BASE])
 		newsk->sk_prot = sk->sk_prot_creator;
 }
-#endif /* CONFIG_BPF_STREAM_PARSER */
+#endif /* CONFIG_BPF_SOCK_MAP */