Re: [PATCH bpf-next 3/7] skmsg: introduce sk_psock_hooks

[Jakub Sitnicki <jakub@cloudflare.com>]

On Tue, Feb 25, 2020 at 02:56 PM CET, Lorenz Bauer wrote:
> The sockmap works by overriding some of the callbacks in sk->sk_prot, while
> leaving others untouched. This means that we need access to the struct proto
> for any protocol we want to support. For IPv4 this is trivial, since both
> TCP and UDP are always compiled in. IPv6 may be disabled or compiled as a
> module, so the existing TCP sockmap hooks use some trickery to lazily
> initialize the modified struct proto for TCPv6.
>
> Pull this logic into a standalone struct sk_psock_hooks, so that it can be
> re-used by UDP sockmap.
>
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> ---

I've been looking at how to simplify this a bit. One thing that seems
like an easy win is to fold sk_psock_hooks_init into its callers. Then
we can go back to using spinlock initializer macros. Patch below.

This highlights some inconsistency in naming instances of
sk_psock_hooks, that is tcp_bpf_hooks vs udp_psock_proto.

---
 include/linux/skmsg.h | 7 -------
 net/ipv4/tcp_bpf.c    | 3 ++-
 net/ipv4/udp_bpf.c    | 3 ++-
 3 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index 174c76c725fb..4566724dc0c9 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -425,13 +425,6 @@ static inline void psock_progs_drop(struct sk_psock_progs *progs)
 	psock_set_prog(&progs->skb_verdict, NULL);
 }

-static inline int sk_psock_hooks_init(struct sk_psock_hooks *hooks,
-				       struct proto *ipv4_base)
-{
-	spin_lock_init(&hooks->ipv6_lock);
-	return hooks->rebuild_proto(hooks->ipv4, ipv4_base);
-}
-
 int sk_psock_hooks_install(struct sk_psock_hooks *hooks, struct sock *sk);

 #endif /* _LINUX_SKMSG_H */
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index fa7e474b981b..5cb9a0724cf6 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -570,13 +570,14 @@ static struct proto tcp_bpf_ipv6[TCP_BPF_NUM_CFGS];
 static struct sk_psock_hooks tcp_bpf_hooks __read_mostly = {
 	.ipv4 = &tcp_bpf_ipv4[0],
 	.ipv6 = &tcp_bpf_ipv6[0],
+	.ipv6_lock = __SPIN_LOCK_UNLOCKED(tcp_bpf_hooks.ipv6_lock),
 	.rebuild_proto = tcp_bpf_rebuild_proto,
 	.choose_proto = tcp_bpf_choose_proto,
 };

 static int __init tcp_bpf_init_psock_hooks(void)
 {
-	return sk_psock_hooks_init(&tcp_bpf_hooks, &tcp_prot);
+	return tcp_bpf_rebuild_proto(tcp_bpf_ipv4, &tcp_prot);
 }
 core_initcall(tcp_bpf_init_psock_hooks);

diff --git a/net/ipv4/udp_bpf.c b/net/ipv4/udp_bpf.c
index e085a0648a94..da5eb1d2265d 100644
--- a/net/ipv4/udp_bpf.c
+++ b/net/ipv4/udp_bpf.c
@@ -30,13 +30,14 @@ static struct proto udpv6_proto;
 static struct sk_psock_hooks udp_psock_proto __read_mostly = {
 	.ipv4 = &udpv4_proto,
 	.ipv6 = &udpv6_proto,
+	.ipv6_lock = __SPIN_LOCK_UNLOCKED(udp_psock_proto.ipv6_lock),
 	.rebuild_proto = udp_bpf_rebuild_protos,
 	.choose_proto = udp_bpf_choose_proto,
 };

 static int __init udp_bpf_init_psock_hooks(void)
 {
-	return sk_psock_hooks_init(&udp_psock_proto, &udp_prot);
+	return udp_bpf_rebuild_protos(&udpv4_proto, &udp_prot);
 }
 core_initcall(udp_bpf_init_psock_hooks);