1. Patchew
  2. MPTCP
  3. [PATCH mptcp-next v7 0/5] use bpf_iter in bpf schedulers
  4. View patch

[PATCH mptcp-next v7 2/5] Squash to "selftests/bpf: Add bpf_rr scheduler & test"

Geliang Tang posted 5 patches 7 months ago
There is a newer version of this series
[PATCH mptcp-next v7 2/5] Squash to "selftests/bpf: Add bpf_rr scheduler & test"
Posted by Geliang Tang 7 months ago 
From: Geliang Tang <tanggeliang@kylinos.cn>

Use the newly added bpf_for_each() helper to walk the conn_list.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../selftests/bpf/progs/mptcp_bpf_rr.c        | 26 ++++++++-----------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
index 638ea6aa63b7..e02b8cfb262b 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
@@ -34,10 +34,9 @@ SEC("struct_ops")
 int BPF_PROG(bpf_rr_get_subflow, struct mptcp_sock *msk,
 	     struct mptcp_sched_data *data)
 {
-	struct mptcp_subflow_context *subflow;
+	struct mptcp_subflow_context *subflow, *next;
 	struct mptcp_rr_storage *ptr;
 	struct sock *last_snd = NULL;
-	int nr = 0;
 
 	ptr = bpf_sk_storage_get(&mptcp_rr_map, msk, 0,
 				 BPF_LOCAL_STORAGE_GET_F_CREATE);
@@ -45,31 +44,28 @@ int BPF_PROG(bpf_rr_get_subflow, struct mptcp_sock *msk,
 		return -1;
 
 	last_snd = ptr->last_snd;
+	next = bpf_mptcp_subflow_ctx(msk->first);
 
-	for (int i = 0; i < data->subflows && i < MPTCP_SUBFLOWS_MAX; i++) {
-		subflow = bpf_mptcp_subflow_ctx_by_pos(data, i);
-		if (!last_snd || !subflow)
+	bpf_for_each(mptcp_subflow, subflow, msk) {
+		if (!last_snd)
 			break;
 
-		if (mptcp_subflow_tcp_sock(subflow) == last_snd) {
-			if (i + 1 == MPTCP_SUBFLOWS_MAX ||
-			    !bpf_mptcp_subflow_ctx_by_pos(data, i + 1))
+		if (bpf_mptcp_subflow_tcp_sock(subflow) == last_snd) {
+			subflow = bpf_iter_mptcp_subflow_next(&___it);
+			if (!subflow)
 				break;
 
-			nr = i + 1;
+			next = subflow;
 			break;
 		}
 	}
 
-	subflow = bpf_mptcp_subflow_ctx_by_pos(data, nr);
-	if (!subflow)
-		return -1;
-	mptcp_subflow_set_scheduled(subflow, true);
-	ptr->last_snd = mptcp_subflow_tcp_sock(subflow);
+	mptcp_subflow_set_scheduled(next, true);
+	ptr->last_snd = bpf_mptcp_subflow_tcp_sock(next);
 	return 0;
 }
 
-SEC(".struct_ops")
+SEC(".struct_ops.link")
 struct mptcp_sched_ops rr = {
 	.init		= (void *)mptcp_sched_rr_init,
 	.release	= (void *)mptcp_sched_rr_release,
-- 
2.45.2
Re: [PATCH mptcp-next v7 2/5] Squash to "selftests/bpf: Add bpf_rr scheduler & test"
Posted by Mat Martineau 7 months ago 
On Tue, 22 Oct 2024, Geliang Tang wrote:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Use the newly added bpf_for_each() helper to walk the conn_list.
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> .../selftests/bpf/progs/mptcp_bpf_rr.c        | 26 ++++++++-----------
> 1 file changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
> index 638ea6aa63b7..e02b8cfb262b 100644
> --- a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
> +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
> @@ -34,10 +34,9 @@ SEC("struct_ops")
> int BPF_PROG(bpf_rr_get_subflow, struct mptcp_sock *msk,
> 	     struct mptcp_sched_data *data)
> {
> -	struct mptcp_subflow_context *subflow;
> +	struct mptcp_subflow_context *subflow, *next;
> 	struct mptcp_rr_storage *ptr;
> 	struct sock *last_snd = NULL;
> -	int nr = 0;
>
> 	ptr = bpf_sk_storage_get(&mptcp_rr_map, msk, 0,
> 				 BPF_LOCAL_STORAGE_GET_F_CREATE);

Hi Geliang -

Thanks for the series. The bpf_iter code simplifies these scheduler 
patches significantly!

> @@ -45,31 +44,28 @@ int BPF_PROG(bpf_rr_get_subflow, struct mptcp_sock *msk,
> 		return -1;
>
> 	last_snd = ptr->last_snd;
> +	next = bpf_mptcp_subflow_ctx(msk->first);
>
> -	for (int i = 0; i < data->subflows && i < MPTCP_SUBFLOWS_MAX; i++) {
> -		subflow = bpf_mptcp_subflow_ctx_by_pos(data, i);
> -		if (!last_snd || !subflow)
> +	bpf_for_each(mptcp_subflow, subflow, msk) {
> +		if (!last_snd)

last_snd is never changed inside the loop, it's more efficient to check it 
before bpf_for_each().

- Mat

> 			break;
>
> -		if (mptcp_subflow_tcp_sock(subflow) == last_snd) {
> -			if (i + 1 == MPTCP_SUBFLOWS_MAX ||
> -			    !bpf_mptcp_subflow_ctx_by_pos(data, i + 1))
> +		if (bpf_mptcp_subflow_tcp_sock(subflow) == last_snd) {
> +			subflow = bpf_iter_mptcp_subflow_next(&___it);
> +			if (!subflow)
> 				break;
>
> -			nr = i + 1;
> +			next = subflow;
> 			break;
> 		}
> 	}
>
> -	subflow = bpf_mptcp_subflow_ctx_by_pos(data, nr);
> -	if (!subflow)
> -		return -1;
> -	mptcp_subflow_set_scheduled(subflow, true);
> -	ptr->last_snd = mptcp_subflow_tcp_sock(subflow);
> +	mptcp_subflow_set_scheduled(next, true);
> +	ptr->last_snd = bpf_mptcp_subflow_tcp_sock(next);
> 	return 0;
> }
>
> -SEC(".struct_ops")
> +SEC(".struct_ops.link")
> struct mptcp_sched_ops rr = {
> 	.init		= (void *)mptcp_sched_rr_init,
> 	.release	= (void *)mptcp_sched_rr_release,
> -- 
> 2.45.2
>
>
>
Re: [PATCH mptcp-next v7 2/5] Squash to "selftests/bpf: Add bpf_rr scheduler & test"
Posted by Geliang Tang 7 months ago 
Hi Mat,

Thanks for your review.

On Tue, 2024-10-22 at 16:52 -0700, Mat Martineau wrote:
> On Tue, 22 Oct 2024, Geliang Tang wrote:
> 
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > Use the newly added bpf_for_each() helper to walk the conn_list.
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> > .../selftests/bpf/progs/mptcp_bpf_rr.c        | 26 ++++++++--------
> > ---
> > 1 file changed, 11 insertions(+), 15 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
> > b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
> > index 638ea6aa63b7..e02b8cfb262b 100644
> > --- a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
> > +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
> > @@ -34,10 +34,9 @@ SEC("struct_ops")
> > int BPF_PROG(bpf_rr_get_subflow, struct mptcp_sock *msk,
> > 	     struct mptcp_sched_data *data)
> > {
> > -	struct mptcp_subflow_context *subflow;
> > +	struct mptcp_subflow_context *subflow, *next;
> > 	struct mptcp_rr_storage *ptr;
> > 	struct sock *last_snd = NULL;
> > -	int nr = 0;
> > 
> > 	ptr = bpf_sk_storage_get(&mptcp_rr_map, msk, 0,
> > 				 BPF_LOCAL_STORAGE_GET_F_CREATE);
> 
> Hi Geliang -
> 
> Thanks for the series. The bpf_iter code simplifies these scheduler 
> patches significantly!
> 
> > @@ -45,31 +44,28 @@ int BPF_PROG(bpf_rr_get_subflow, struct
> > mptcp_sock *msk,
> > 		return -1;
> > 
> > 	last_snd = ptr->last_snd;
> > +	next = bpf_mptcp_subflow_ctx(msk->first);
> > 
> > -	for (int i = 0; i < data->subflows && i <
> > MPTCP_SUBFLOWS_MAX; i++) {
> > -		subflow = bpf_mptcp_subflow_ctx_by_pos(data, i);
> > -		if (!last_snd || !subflow)
> > +	bpf_for_each(mptcp_subflow, subflow, msk) {
> > +		if (!last_snd)
> 
> last_snd is never changed inside the loop, it's more efficient to
> check it 
> before bpf_for_each().

This is indeed better, updated in v8.

> 
> - Mat
> 
> > 			break;
> > 
> > -		if (mptcp_subflow_tcp_sock(subflow) == last_snd) {
> > -			if (i + 1 == MPTCP_SUBFLOWS_MAX ||
> > -			    !bpf_mptcp_subflow_ctx_by_pos(data, i
> > + 1))
> > +		if (bpf_mptcp_subflow_tcp_sock(subflow) ==
> > last_snd) {
> > +			subflow =
> > bpf_iter_mptcp_subflow_next(&___it);
> > +			if (!subflow)
> > 				break;
> > 
> > -			nr = i + 1;
> > +			next = subflow;
> > 			break;
> > 		}
> > 	}
> > 
> > -	subflow = bpf_mptcp_subflow_ctx_by_pos(data, nr);
> > -	if (!subflow)
> > -		return -1;
> > -	mptcp_subflow_set_scheduled(subflow, true);
> > -	ptr->last_snd = mptcp_subflow_tcp_sock(subflow);
> > +	mptcp_subflow_set_scheduled(next, true);
> > +	ptr->last_snd = bpf_mptcp_subflow_tcp_sock(next);
> > 	return 0;
> > }
> > 
> > -SEC(".struct_ops")
> > +SEC(".struct_ops.link")
> > struct mptcp_sched_ops rr = {
> > 	.init		= (void *)mptcp_sched_rr_init,
> > 	.release	= (void *)mptcp_sched_rr_release,
> > -- 
> > 2.45.2
> > 
> > 
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.