From: Geliang Tang <tanggeliang@kylinos.cn>

Define mptcp_subflow_ctx() using bpf_core_cast as Martin suggested.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/progs/mptcp_bpf.h       | 10 ++++++++++
 tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf.h b/tools/testing/selftests/bpf/progs/mptcp_bpf.h
index 4e901941d5dd..815bbf865ee4 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_bpf.h
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf.h
@@ -39,6 +39,16 @@ mptcp_subflow_tcp_sock(const struct mptcp_subflow_context *subflow)
 	return subflow->tcp_sock;
 }
 
+static __always_inline struct mptcp_subflow_context *
+mptcp_subflow_ctx(const struct sock *sk)
+{
+	const struct inet_connection_sock *
+		icsk = bpf_core_cast(sk, struct inet_connection_sock);
+
+	/* Use RCU on icsk_ulp_data only for sock diag code */
+	return bpf_core_cast(icsk->icsk_ulp_data, struct mptcp_subflow_context);
+}
+
 /* ksym */
 extern struct mptcp_subflow_context *
 bpf_mptcp_subflow_ctx(const struct sock *sk) __ksym;
diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c
index a1d8f9b20259..88a3100dfdc2 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c
@@ -48,7 +48,7 @@ int iters_subflow(struct bpf_sockopt *ctx)
 		goto out;
 
 	/* only to check the following kfunc works */
-	subflow = bpf_mptcp_subflow_ctx(ssk);
+	subflow = mptcp_subflow_ctx(ssk);
 	if (!subflow || subflow->token != msk->token)
 		goto out;
 
-- 
2.43.0

Hi Geliang,

Thank you for this follow-up!

On 20/05/2025 10:48, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Define mptcp_subflow_ctx() using bpf_core_cast as Martin suggested.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  tools/testing/selftests/bpf/progs/mptcp_bpf.h       | 10 ++++++++++
>  tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c |  2 +-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf.h b/tools/testing/selftests/bpf/progs/mptcp_bpf.h
> index 4e901941d5dd..815bbf865ee4 100644
> --- a/tools/testing/selftests/bpf/progs/mptcp_bpf.h
> +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf.h
> @@ -39,6 +39,16 @@ mptcp_subflow_tcp_sock(const struct mptcp_subflow_context *subflow)
>  	return subflow->tcp_sock;
>  }
>  
> +static __always_inline struct mptcp_subflow_context *
> +mptcp_subflow_ctx(const struct sock *sk)
> +{
> +	const struct inet_connection_sock *
> +		icsk = bpf_core_cast(sk, struct inet_connection_sock);
> +
> +	/* Use RCU on icsk_ulp_data only for sock diag code */
> +	return bpf_core_cast(icsk->icsk_ulp_data, struct mptcp_subflow_context);
> +}
> +
>  /* ksym */
>  extern struct mptcp_subflow_context *
>  bpf_mptcp_subflow_ctx(const struct sock *sk) __ksym;
> diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c
> index a1d8f9b20259..88a3100dfdc2 100644
> --- a/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c
> +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c
> @@ -48,7 +48,7 @@ int iters_subflow(struct bpf_sockopt *ctx)
>  		goto out;
>  
>  	/* only to check the following kfunc works */

Then we can remove this comment.

> -	subflow = bpf_mptcp_subflow_ctx(ssk);
> +	subflow = mptcp_subflow_ctx(ssk);
>  	if (!subflow || subflow->token != msk->token)
>  		goto out;
>  

Note that when reading Martin's review again, it looks like this
selftest is not really useful. Maybe we should drop it? WDYT?

If we do, the "common" kfunc should no longer be registered for
BPF_PROG_TYPE_CGROUP_SOCKOPT, right? I think it makes sense anyway. I
don't think we need that, at least not for the moment. Then, better to
restrict to struct_ops to help with the maintenance ; except if you see
useful use-cases for this CGroup SockOpt program type?

What we did in the selftest could be done without the kfunc, and we
didn't find anything really useful to do with the kfunc, right?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.