Squash to "bpf: Add bpf_mptcp_sched_ops" II

As is reported in #417 by sferlin, we can't access tcp_sock fields in
BPF schedulers.

I gave two suggestions in the comments of #417 to solve it:

1. Move the helpers which needs to access tcp_sock fields like
tcp_packets_in_flight() into the kernel space, call the helpers in BPF
context.
2. Add more helpers in the kernel space to access more tcp_sock fields,
call the helpers in BPF context.

Although these can solve this issue to some extent, but they aren't the
best solutions.

A better one is using bpf_skc_to_tcp_sock() to get the tcp_sock instead
of using tcp_sk():

    tp = bpf_skc_to_tcp_sock(sk);

Then we can access every fields of tp and keep sferlin's patch as is.

But this needs to add accesses to call bpf_skc_to_tcp_sock() in BPF
schedulers.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/417
Cc: sferlinr@redhat.com
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/bpf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index 5c82cc35bfe0..f17a947635bf 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -30,6 +30,10 @@ bpf_mptcp_sched_get_func_proto(enum bpf_func_id func_id,
 		return &bpf_sk_storage_get_proto;
 	case BPF_FUNC_sk_storage_delete:
 		return &bpf_sk_storage_delete_proto;
+	case BPF_FUNC_skc_to_tcp6_sock:
+		return &bpf_skc_to_tcp6_sock_proto;
+	case BPF_FUNC_skc_to_tcp_sock:
+		return &bpf_skc_to_tcp_sock_proto;
 	default:
 		return bpf_base_func_proto(func_id);
 	}
-- 
2.35.3

Hi Geliang, Mat,

On 02/08/2023 10:07, Geliang Tang wrote:
> Squash to "bpf: Add bpf_mptcp_sched_ops" II
> 
> As is reported in #417 by sferlin, we can't access tcp_sock fields in
> BPF schedulers.
> 
> I gave two suggestions in the comments of #417 to solve it:
> 
> 1. Move the helpers which needs to access tcp_sock fields like
> tcp_packets_in_flight() into the kernel space, call the helpers in BPF
> context.
> 2. Add more helpers in the kernel space to access more tcp_sock fields,
> call the helpers in BPF context.
> 
> Although these can solve this issue to some extent, but they aren't the
> best solutions.
> 
> A better one is using bpf_skc_to_tcp_sock() to get the tcp_sock instead
> of using tcp_sk():
> 
>     tp = bpf_skc_to_tcp_sock(sk);
> 
> Then we can access every fields of tp and keep sferlin's patch as is.

Good idea!

> But this needs to add accesses to call bpf_skc_to_tcp_sock() in BPF
> schedulers.

Thank you for the patch and the review!

Now in our tree:

New patches for t/upstream:
- db817ce80c6d: "squashed" in "bpf: Add bpf_mptcp_sched_ops"
- Results: 49d27f1d2549..7875a2426749 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230804T123239

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

On Wed, 2 Aug 2023, Geliang Tang wrote:

> Squash to "bpf: Add bpf_mptcp_sched_ops" II
>
> As is reported in #417 by sferlin, we can't access tcp_sock fields in
> BPF schedulers.
>
> I gave two suggestions in the comments of #417 to solve it:
>
> 1. Move the helpers which needs to access tcp_sock fields like
> tcp_packets_in_flight() into the kernel space, call the helpers in BPF
> context.
> 2. Add more helpers in the kernel space to access more tcp_sock fields,
> call the helpers in BPF context.
>
> Although these can solve this issue to some extent, but they aren't the
> best solutions.
>
> A better one is using bpf_skc_to_tcp_sock() to get the tcp_sock instead
> of using tcp_sk():
>
>    tp = bpf_skc_to_tcp_sock(sk);
>
> Then we can access every fields of tp and keep sferlin's patch as is.
>
> But this needs to add accesses to call bpf_skc_to_tcp_sock() in BPF
> schedulers.
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/417
> Cc: sferlinr@redhat.com
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> net/mptcp/bpf.c | 4 ++++
> 1 file changed, 4 insertions(+)

Looks good to squash, thank you Geliang.

- Mat

>
> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index 5c82cc35bfe0..f17a947635bf 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -30,6 +30,10 @@ bpf_mptcp_sched_get_func_proto(enum bpf_func_id func_id,
> 		return &bpf_sk_storage_get_proto;
> 	case BPF_FUNC_sk_storage_delete:
> 		return &bpf_sk_storage_delete_proto;
> +	case BPF_FUNC_skc_to_tcp6_sock:
> +		return &bpf_skc_to_tcp6_sock_proto;
> +	case BPF_FUNC_skc_to_tcp_sock:
> +		return &bpf_skc_to_tcp_sock_proto;
> 	default:
> 		return bpf_base_func_proto(func_id);
> 	}
> -- 
> 2.35.3
>
>
>