| 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 |
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 [...]
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
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.
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!
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.
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 --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 */