[PATCH RESEND mptcp-next v5 2/3] bpf: implement bpf_mptcp_sock()

Geliang Tang posted 3 patches 3 years, 11 months ago
Maintainers: Jakub Kicinski <kuba@kernel.org>, Matthieu Baerts <matthieu.baerts@tessares.net>, Song Liu <songliubraving@fb.com>, John Fastabend <john.fastabend@gmail.com>, Shuah Khan <shuah@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, "David S. Miller" <davem@davemloft.net>, Yonghong Song <yhs@fb.com>, Mat Martineau <mathew.j.martineau@linux.intel.com>, Andrii Nakryiko <andrii@kernel.org>, Martin KaFai Lau <kafai@fb.com>, KP Singh <kpsingh@kernel.org>, Alexei Starovoitov <ast@kernel.org>
[PATCH RESEND mptcp-next v5 2/3] bpf: implement bpf_mptcp_sock()
Posted by Geliang Tang 3 years, 11 months ago
This patch implemented the function bpf_mptcp_sock(). Defined a new bpf_id
BTF_SOCK_TYPE_MPTCP, and added a new helper bpf_mptcp_sock_from_subflow()
to get struct bpf_mptcp_sock from a given subflow socket.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 include/linux/bpf.h     |  6 ++++++
 include/linux/btf_ids.h |  3 ++-
 net/core/filter.c       | 17 +++++++++++++++++
 net/mptcp/bpf.c         | 15 +++++++++++++++
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 0ae57df2f226..38290b249d5f 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2359,6 +2359,7 @@ bool bpf_mptcp_sock_is_valid_access(int off, int size,
 				    enum bpf_access_type type,
 				    struct bpf_insn_access_aux *info);
 
+struct bpf_mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk);
 #else /* CONFIG_MPTCP */
 static inline bool bpf_mptcp_sock_is_valid_access(int off, int size,
 						  enum bpf_access_type type,
@@ -2366,6 +2367,11 @@ static inline bool bpf_mptcp_sock_is_valid_access(int off, int size,
 {
 	return false;
 }
+
+static inline struct bpf_mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
+{
+	return NULL;
+}
 #endif /* CONFIG_MPTCP */
 
 enum bpf_text_poke_type {
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index bc5d9cc34e4c..335a19092368 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -178,7 +178,8 @@ extern struct btf_id_set name;
 	BTF_SOCK_TYPE(BTF_SOCK_TYPE_TCP6, tcp6_sock)			\
 	BTF_SOCK_TYPE(BTF_SOCK_TYPE_UDP, udp_sock)			\
 	BTF_SOCK_TYPE(BTF_SOCK_TYPE_UDP6, udp6_sock)			\
-	BTF_SOCK_TYPE(BTF_SOCK_TYPE_UNIX, unix_sock)
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_UNIX, unix_sock)			\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_MPTCP, mptcp_sock)
 
 enum {
 #define BTF_SOCK_TYPE(name, str) name,
diff --git a/net/core/filter.c b/net/core/filter.c
index f64454722a7e..83ed8db32c79 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7395,6 +7395,19 @@ static const struct bpf_func_proto bpf_sock_ops_reserve_hdr_opt_proto = {
 	.arg3_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_1(bpf_mptcp_sock, struct sock *, sk)
+{
+	return (unsigned long)bpf_mptcp_sock_from_subflow(sk);
+}
+
+static const struct bpf_func_proto bpf_mptcp_sock_proto = {
+	.func		= bpf_mptcp_sock,
+	.gpl_only	= false,
+	.ret_type	= RET_PTR_TO_BTF_ID_OR_NULL,
+	.arg1_type	= ARG_PTR_TO_SOCK_COMMON,
+	.ret_btf_id	= &btf_sock_ids[BTF_SOCK_TYPE_MPTCP],
+};
+
 #endif /* CONFIG_INET */
 
 bool bpf_helper_changes_pkt_data(void *func)
@@ -7850,6 +7863,10 @@ sock_ops_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	case BPF_FUNC_tcp_sock:
 		return &bpf_tcp_sock_proto;
 #endif /* CONFIG_INET */
+#ifdef CONFIG_MPTCP
+	case BPF_FUNC_mptcp_sock:
+		return &bpf_mptcp_sock_proto;
+#endif /* CONFIG_MPTCP */
 	default:
 		return bpf_sk_base_func_proto(func_id);
 	}
diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index 2e42a95d1206..e3213818c45b 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -25,3 +25,18 @@ bool bpf_mptcp_sock_is_valid_access(int off, int size, enum bpf_access_type type
 		return size == sizeof(__u32);
 	}
 }
+
+struct bpf_mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
+{
+	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) {
+		struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
+		struct mptcp_sock *msk = mptcp_sk(subflow->conn);
+		struct bpf_mptcp_sock *bpf_msk;
+
+		bpf_msk = (struct bpf_mptcp_sock *)subflow->conn;
+		bpf_msk->token = msk->token;
+		return bpf_msk;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL(bpf_mptcp_sock_from_subflow);
-- 
2.34.1


Re: [PATCH RESEND mptcp-next v5 2/3] bpf: implement bpf_mptcp_sock()
Posted by Matthieu Baerts 3 years, 11 months ago
Hi Geliang,

On 03/03/2022 16:22, Geliang Tang wrote:
> This patch implemented the function bpf_mptcp_sock(). Defined a new bpf_id
> BTF_SOCK_TYPE_MPTCP, and added a new helper bpf_mptcp_sock_from_subflow()
> to get struct bpf_mptcp_sock from a given subflow socket.

(...)

> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index 2e42a95d1206..e3213818c45b 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -25,3 +25,18 @@ bool bpf_mptcp_sock_is_valid_access(int off, int size, enum bpf_access_type type
>  		return size == sizeof(__u32);
>  	}
>  }
> +
> +struct bpf_mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
> +{
> +	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) {
> +		struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
> +		struct mptcp_sock *msk = mptcp_sk(subflow->conn);
> +		struct bpf_mptcp_sock *bpf_msk;
> +
> +		bpf_msk = (struct bpf_mptcp_sock *)subflow->conn;
> +		bpf_msk->token = msk->token;
> +		return bpf_msk;

If I'm not mistaken, you are still using this definition of the
bpf_mptcp_sock structure:

  struct bpf_mptcp_sock {
      __u32 token;
  };

Is that correct?

If yes, what you are doing above doesn't seem correct: you cannot cast
mptcp_sock into this structure. Or did I miss something?


In other words, I don't think you should return a bpf_mptcp_sock
structure here but an mptcp_sock one: not a specific one for BPF. By
doing that, we will be able to access all items of the MPTCP socket, not
just the token you copy here in a new structure.
If we can access the mptcp_sock structure, we no longer need to convert
anything and copy only some selected fields from what I understand.

It would then be similar to bpf_skc_to_tcp_sock() returning a "struct
tcp_sock" pointer where it is possible to access all fields from it, not
only what has been copied.

Do you see what I mean? :)

BTW it might be better to have a squash-to patch for "bpf: add
'bpf_mptcp_sock' structure and helper" which would also modify the
commit title and message! From what I understood from Alexei,
'bpf_mptcp_sock' structure is no longer needed.

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net