[RFC mptcp-next 00/11] BPF packet scheduler updates part 4

[RFC mptcp-next 00/11] BPF packet scheduler updates part 4

[Geliang Tang]

There's a bug in bpf_burst. snd_burst stored in mptcp_burst_storage in
BPF context is not used. msk->snd_burst is still used in kernel space.
To fix this, add two new interfaces in mptcp_sched_ops to get and set
scheduler's paramters from BPF context to kernel space.

Geliang Tang (11):
  Squash to "mptcp: add struct mptcp_sched_ops"
  Squash to "mptcp: add scheduler wrappers"
  mptcp: use snd_burst wrappers
  Squash to "mptcp: register default scheduler"
  Squash to "bpf: Add bpf_mptcp_sched_ops"
  Squash to "selftests/bpf: Add mptcp sched structs"
  Squash to "selftests/bpf: Add bpf_bkup scheduler"
  Squash to "selftests/bpf: Add bpf_rr scheduler"
  Squash to "selftests/bpf: Add bpf_burst scheduler"
  selftests/bpf: Add bpf_stale scheduler
  selftests/bpf: Add bpf_stale test

 include/net/mptcp.h                           |  10 ++
 net/mptcp/bpf.c                               |  27 ++-
 net/mptcp/protocol.c                          |   8 +-
 net/mptcp/protocol.h                          |   2 +
 net/mptcp/sched.c                             |  49 ++++++
 tools/testing/selftests/bpf/bpf_tcp_helpers.h |  11 ++
 .../testing/selftests/bpf/prog_tests/mptcp.c  |  38 ++++
 .../selftests/bpf/progs/mptcp_bpf_bkup.c      |   4 +-
 .../selftests/bpf/progs/mptcp_bpf_burst.c     |  43 ++++-
 .../selftests/bpf/progs/mptcp_bpf_rr.c        |   2 +
 .../selftests/bpf/progs/mptcp_bpf_stale.c     | 163 ++++++++++++++++++
 11 files changed, 339 insertions(+), 18 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/mptcp_bpf_stale.c

-- 
2.35.3

[RFC mptcp-next 01/11] Squash to "mptcp: add struct mptcp_sched_ops"

[Geliang Tang]

Add get_params and set_params interfaces.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 include/net/mptcp.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index ba0e1ee68a05..84ca8380acb1 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -105,11 +105,21 @@ struct mptcp_sched_data {
 	struct mptcp_subflow_context *contexts[MPTCP_SUBFLOWS_MAX];
 };
 
+struct mptcp_sched_params {
+	union {
+		int	snd_burst;
+	};
+};
+
 struct mptcp_sched_ops {
 	void (*data_init)(struct mptcp_sock *msk,
 			  struct mptcp_sched_data *data);
 	int (*get_subflow)(struct mptcp_sock *msk,
 			   const struct mptcp_sched_data *data);
+	int (*get_params)(struct mptcp_sock *msk,
+			  struct mptcp_sched_params *params);
+	int (*set_params)(struct mptcp_sock *msk,
+			  struct mptcp_sched_params *params);
 
 	char			name[MPTCP_SCHED_NAME_MAX];
 	struct module		*owner;
-- 
2.35.3

[RFC mptcp-next 02/11] Squash to "mptcp: add scheduler wrappers"

[Geliang Tang]

Add mptcp_get_snd_burst and mptcp_set_snd_burst.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/protocol.h |  2 ++
 net/mptcp/sched.c    | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 4572f01b978d..8c382384090b 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -673,6 +673,8 @@ struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk);
 struct sock *mptcp_subflow_get_retrans(struct mptcp_sock *msk);
 int mptcp_sched_get_send(struct mptcp_sock *msk);
 int mptcp_sched_get_retrans(struct mptcp_sock *msk);
+int mptcp_get_snd_burst(struct mptcp_sock *msk);
+int mptcp_set_snd_burst(struct mptcp_sock *msk, int burst);
 
 static inline bool __tcp_can_send(const struct sock *ssk)
 {
diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
index 5d78efc9c96c..e96f69762071 100644
--- a/net/mptcp/sched.c
+++ b/net/mptcp/sched.c
@@ -229,3 +229,35 @@ int mptcp_sched_get_retrans(struct mptcp_sock *msk)
 	msk->sched->data_init(msk, &data);
 	return msk->sched->get_subflow(msk, &data);
 }
+
+int mptcp_get_snd_burst(struct mptcp_sock *msk)
+{
+	if (!msk->sched)
+		return msk->snd_burst;
+
+	if (msk->sched->get_params) {
+		struct mptcp_sched_params params;
+
+		msk->sched->get_params(msk, &params);
+		return params.snd_burst;
+	}
+	return 0;
+}
+
+
+int mptcp_set_snd_burst(struct mptcp_sock *msk, int burst)
+{
+	if (!msk->sched) {
+		msk->snd_burst = burst;
+		return 0;
+	}
+
+	if (msk->sched->set_params) {
+		struct mptcp_sched_params params;
+
+		params.snd_burst = burst;
+		return msk->sched->set_params(msk, &params);
+	}
+	return 0;
+
+}
-- 
2.35.3

[RFC mptcp-next 03/11] mptcp: use snd_burst wrappers

[Geliang Tang]

Use mptcp_get_snd_burst() and mptcp_set_snd_burst() wrappers, instead of
read and set msk->snd_burst directly.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/protocol.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 40ca9f5d1360..77aa05631add 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1436,7 +1436,7 @@ struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
 	subflow->avg_pacing_rate = div_u64((u64)subflow->avg_pacing_rate * wmem +
 					   READ_ONCE(ssk->sk_pacing_rate) * burst,
 					   burst + wmem);
-	msk->snd_burst = burst;
+	mptcp_set_snd_burst(msk, burst);
 	return ssk;
 }
 
@@ -1454,7 +1454,7 @@ static void mptcp_update_post_push(struct mptcp_sock *msk,
 
 	dfrag->already_sent += sent;
 
-	msk->snd_burst -= sent;
+	mptcp_set_snd_burst(msk, mptcp_get_snd_burst(msk) - sent);
 
 	snd_nxt_new += dfrag->already_sent;
 
@@ -1507,7 +1507,7 @@ static int __subflow_push_pending(struct sock *sk, struct sock *ssk,
 		}
 		WRITE_ONCE(msk->first_pending, mptcp_send_next(sk));
 
-		if (msk->snd_burst <= 0 ||
+		if (mptcp_get_snd_burst(msk) <= 0 ||
 		    !sk_stream_memory_free(ssk) ||
 		    !mptcp_subflow_active(mptcp_subflow_ctx(ssk))) {
 			err = copied;
@@ -2290,7 +2290,7 @@ bool __mptcp_retransmit_pending_data(struct sock *sk)
 	mptcp_data_unlock(sk);
 
 	msk->first_pending = rtx_head;
-	msk->snd_burst = 0;
+	mptcp_set_snd_burst(msk, 0);
 
 	/* be sure to clear the "sent status" on all re-injected fragments */
 	list_for_each_entry(cur, &msk->rtx_queue, list) {
-- 
2.35.3

[RFC mptcp-next 04/11] Squash to "mptcp: register default scheduler"

[Geliang Tang]

Add .get_params and .set_params.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/sched.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
index e96f69762071..6e07708b80d1 100644
--- a/net/mptcp/sched.c
+++ b/net/mptcp/sched.c
@@ -35,9 +35,26 @@ static int mptcp_sched_default_get_subflow(struct mptcp_sock *msk,
 	return 0;
 }
 
+static int mptcp_sched_default_get_params(struct mptcp_sock *msk,
+					  struct mptcp_sched_params *params)
+{
+	params->snd_burst = msk->snd_burst;
+	return 0;
+}
+
+
+static int mptcp_sched_default_set_params(struct mptcp_sock *msk,
+					  struct mptcp_sched_params *params)
+{
+	msk->snd_burst = params->snd_burst;
+	return 0;
+}
+
 static struct mptcp_sched_ops mptcp_sched_default = {
 	.data_init	= mptcp_sched_default_data_init,
 	.get_subflow	= mptcp_sched_default_get_subflow,
+	.get_params	= mptcp_sched_default_get_params,
+	.set_params	= mptcp_sched_default_set_params,
 	.name		= "default",
 	.owner		= THIS_MODULE,
 };
-- 
2.35.3

[RFC mptcp-next 05/11] Squash to "bpf: Add bpf_mptcp_sched_ops"

[Geliang Tang]

Add write access to params->snd_burst.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/bpf.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index a48ca9ea55cd..1c8cfb305529 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -18,8 +18,9 @@
 
 #ifdef CONFIG_BPF_JIT
 extern struct bpf_struct_ops bpf_mptcp_sched_ops;
-static const struct btf_type *mptcp_sched_type __read_mostly;
-static u32 mptcp_sched_id;
+static const struct btf_type *mptcp_subflow_type __read_mostly;
+static const struct btf_type *mptcp_params_type __read_mostly;
+static u32 mptcp_subflow_id, mptcp_params_id;
 
 static const struct bpf_func_proto *
 bpf_mptcp_sched_get_func_proto(enum bpf_func_id func_id,
@@ -47,8 +48,8 @@ static int bpf_mptcp_sched_btf_struct_access(struct bpf_verifier_log *log,
 	size_t end;
 
 	t = btf_type_by_id(reg->btf, reg->btf_id);
-	if (t != mptcp_sched_type) {
-		bpf_log(log, "only access to mptcp_subflow_context is supported\n");
+	if (t != mptcp_subflow_type && t != mptcp_params_type) {
+		bpf_log(log, "only access to mptcp_sched is supported\n");
 		return -EACCES;
 	}
 
@@ -59,13 +60,16 @@ static int bpf_mptcp_sched_btf_struct_access(struct bpf_verifier_log *log,
 	case offsetof(struct mptcp_subflow_context, avg_pacing_rate):
 		end = offsetofend(struct mptcp_subflow_context, avg_pacing_rate);
 		break;
+	case offsetof(struct mptcp_sched_params, snd_burst):
+		end = offsetofend(struct mptcp_sched_params, snd_burst);
+		break;
 	default:
-		bpf_log(log, "no write support to mptcp_subflow_context at off %d\n", off);
+		bpf_log(log, "no write support to mptcp_sched at off %d\n", off);
 		return -EACCES;
 	}
 
 	if (off + size > end) {
-		bpf_log(log, "access beyond mptcp_subflow_context at off %u size %u ended at %zu",
+		bpf_log(log, "access beyond mptcp_sched at off %u size %u ended at %zu",
 			off, size, end);
 		return -EACCES;
 	}
@@ -129,8 +133,15 @@ static int bpf_mptcp_sched_init(struct btf *btf)
 					BTF_KIND_STRUCT);
 	if (type_id < 0)
 		return -EINVAL;
-	mptcp_sched_id = type_id;
-	mptcp_sched_type = btf_type_by_id(btf, mptcp_sched_id);
+	mptcp_subflow_id = type_id;
+	mptcp_subflow_type = btf_type_by_id(btf, mptcp_subflow_id);
+
+	type_id = btf_find_by_name_kind(btf, "mptcp_sched_params",
+					BTF_KIND_STRUCT);
+	if (type_id < 0)
+		return -EINVAL;
+	mptcp_params_id = type_id;
+	mptcp_params_type = btf_type_by_id(btf, mptcp_params_id);
 
 	return 0;
 }
-- 
2.35.3

[RFC mptcp-next 06/11] Squash to "selftests/bpf: Add mptcp sched structs"

[Geliang Tang]

Add get_params and set_params.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/bpf/bpf_tcp_helpers.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index 776c54948a4a..6360593f64a4 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -247,6 +247,12 @@ struct mptcp_sched_data {
 	__u8	subflows;
 } __attribute__((preserve_access_index));
 
+struct mptcp_sched_params {
+	union {
+		int	snd_burst;
+	};
+};
+
 struct mptcp_sched_ops {
 	char name[MPTCP_SCHED_NAME_MAX];
 
@@ -257,6 +263,10 @@ struct mptcp_sched_ops {
 			  struct mptcp_sched_data *data);
 	int (*get_subflow)(struct mptcp_sock *msk,
 			   const struct mptcp_sched_data *data);
+	int (*get_params)(struct mptcp_sock *msk,
+			  struct mptcp_sched_params *params);
+	int (*set_params)(struct mptcp_sock *msk,
+			  struct mptcp_sched_params *params);
 	void *owner;
 };
 
-- 
2.35.3

[RFC mptcp-next 07/11] Squash to "selftests/bpf: Add bpf_bkup scheduler"

[Geliang Tang]

Return -1 when no subflow is selected.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/bpf/progs/mptcp_bpf_bkup.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_bkup.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_bkup.c
index 21493a3affdb..331c8e7e10be 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_bpf_bkup.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_bkup.c
@@ -40,8 +40,10 @@ int BPF_STRUCT_OPS(bpf_bkup_get_subflow, struct mptcp_sock *msk,
 		}
 	}
 
-	if (nr != -1)
+	if (nr != -1) {
 		mptcp_subflow_set_scheduled(mptcp_subflow_ctx_by_pos(data, nr), true);
+		return -1;
+	}
 	return 0;
 }
 
-- 
2.35.3

[RFC mptcp-next 08/11] Squash to "selftests/bpf: Add bpf_rr scheduler"

[Geliang Tang]

Alloc and init mptcp_rr_map first in mptcp_sched_rr_init().

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
index 8d9442996e0f..5842b47d9c5e 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
@@ -20,6 +20,8 @@ struct {
 SEC("struct_ops/mptcp_sched_rr_init")
 void BPF_PROG(mptcp_sched_rr_init, struct mptcp_sock *msk)
 {
+	bpf_sk_storage_get(&mptcp_rr_map, msk, 0,
+			   BPF_LOCAL_STORAGE_GET_F_CREATE);
 }
 
 SEC("struct_ops/mptcp_sched_rr_release")
-- 
2.35.3

[RFC mptcp-next 09/11] Squash to "selftests/bpf: Add bpf_burst scheduler"

[Geliang Tang]

Add .get_params and .set_params.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../selftests/bpf/progs/mptcp_bpf_burst.c     | 43 ++++++++++++++++---
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_burst.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_burst.c
index 8cf2ab32c159..a7bf37005c5d 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_bpf_burst.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_burst.c
@@ -69,6 +69,8 @@ static __always_inline bool sk_stream_memory_free(const struct sock *sk)
 SEC("struct_ops/mptcp_sched_burst_init")
 void BPF_PROG(mptcp_sched_burst_init, struct mptcp_sock *msk)
 {
+	bpf_sk_storage_get(&mptcp_burst_map, msk, 0,
+			   BPF_LOCAL_STORAGE_GET_F_CREATE);
 }
 
 SEC("struct_ops/mptcp_sched_burst_release")
@@ -83,13 +85,25 @@ void BPF_STRUCT_OPS(bpf_burst_data_init, struct mptcp_sock *msk,
 	mptcp_sched_data_set_contexts(msk, data);
 }
 
+static int set_snd_burst(struct mptcp_sock *msk, int burst)
+{
+	struct mptcp_burst_storage *ptr;
+
+	ptr = bpf_sk_storage_get(&mptcp_burst_map, msk, 0,
+				 BPF_LOCAL_STORAGE_GET_F_CREATE);
+	if (!ptr)
+		return -1;
+
+	ptr->snd_burst = burst;
+	return 0;
+}
+
 static int bpf_burst_get_send(struct mptcp_sock *msk,
 			      const struct mptcp_sched_data *data)
 {
 	struct subflow_send_info send_info[SSK_MODE_MAX];
 	struct mptcp_subflow_context *subflow;
 	struct sock *sk = (struct sock *)msk;
-	struct mptcp_burst_storage *ptr;
 	__u32 pace, burst, wmem;
 	__u64 linger_time;
 	struct sock *ssk;
@@ -146,10 +160,7 @@ static int bpf_burst_get_send(struct mptcp_sock *msk,
 	subflow->avg_pacing_rate = div_u64((__u64)subflow->avg_pacing_rate * wmem +
 					   ssk->sk_pacing_rate * burst,
 					   burst + wmem);
-	ptr = bpf_sk_storage_get(&mptcp_burst_map, msk, 0,
-				 BPF_LOCAL_STORAGE_GET_F_CREATE);
-	if (ptr)
-		ptr->snd_burst = burst;
+	set_snd_burst(msk, burst);
 
 out:
 	mptcp_subflow_set_scheduled(subflow, true);
@@ -212,11 +223,33 @@ int BPF_STRUCT_OPS(bpf_burst_get_subflow, struct mptcp_sock *msk,
 	return bpf_burst_get_send(msk, data);
 }
 
+int BPF_STRUCT_OPS(bpf_burst_get_params, struct mptcp_sock *msk,
+		   struct mptcp_sched_params *params)
+{
+	struct mptcp_burst_storage *ptr;
+
+	ptr = bpf_sk_storage_get(&mptcp_burst_map, msk, 0,
+			BPF_LOCAL_STORAGE_GET_F_CREATE);
+	if (!ptr)
+		return -1;
+
+	params->snd_burst = ptr->snd_burst;
+	return 0;
+}
+
+int BPF_STRUCT_OPS(bpf_burst_set_params, struct mptcp_sock *msk,
+		   struct mptcp_sched_params *params)
+{
+	return set_snd_burst(msk, params->snd_burst);
+}
+
 SEC(".struct_ops")
 struct mptcp_sched_ops burst = {
 	.init		= (void *)mptcp_sched_burst_init,
 	.release	= (void *)mptcp_sched_burst_release,
 	.data_init	= (void *)bpf_burst_data_init,
 	.get_subflow	= (void *)bpf_burst_get_subflow,
+	.get_params	= (void *)bpf_burst_get_params,
+	.set_params	= (void *)bpf_burst_set_params,
 	.name		= "bpf_burst",
 };
-- 
2.35.3

[RFC mptcp-next 10/11] selftests/bpf: Add bpf_stale scheduler

[Geliang Tang]

This patch implements the setting a subflow as stale/unstale in BPF MPTCP
scheduler, named bpf_stale. The staled subflow id will be added into a
map in sk_storage.

Two helper mptcp_subflow_set_stale() and mptcp_subflow_clear_stale() are
added.

In this test, subflow 1 is set as stale in bpf_stale_data_init(). Each
subflow is checked whether it's a stale one in bpf_stale_get_subflow() to
select a unstale subflow to send data.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/bpf/bpf_tcp_helpers.h |   1 +
 .../selftests/bpf/progs/mptcp_bpf_stale.c     | 163 ++++++++++++++++++
 2 files changed, 164 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/mptcp_bpf_stale.c

diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index 6360593f64a4..4f7a41c8a47a 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -239,6 +239,7 @@ struct mptcp_subflow_context {
 	unsigned long avg_pacing_rate;
 	__u32	backup : 1;
 	__u8	stale_count;
+	__u32	subflow_id;
 	struct	sock *tcp_sock;	    /* tcp sk backpointer */
 } __attribute__((preserve_access_index));
 
diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_stale.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_stale.c
new file mode 100644
index 000000000000..10e56fa07818
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_stale.c
@@ -0,0 +1,163 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023, SUSE. */
+
+#include <linux/bpf.h>
+#include "bpf_tcp_helpers.h"
+
+char _license[] SEC("license") = "GPL";
+
+struct mptcp_stale_storage {
+	__u8 nr;
+	__u32 ids[MPTCP_SUBFLOWS_MAX];
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SK_STORAGE);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, int);
+	__type(value, struct mptcp_stale_storage);
+} mptcp_stale_map SEC(".maps");
+
+static void mptcp_subflow_set_stale(struct mptcp_stale_storage *storage,
+				    __u32 subflow_id)
+{
+	if (!subflow_id)
+		return;
+
+	for (int i = 0; i < storage->nr && i < MPTCP_SUBFLOWS_MAX; i++) {
+		if (storage->ids[i] == subflow_id)
+			return;
+	}
+
+	if (storage->nr < MPTCP_SUBFLOWS_MAX - 1)
+		storage->ids[storage->nr++] = subflow_id;
+}
+
+static void mptcp_subflow_clear_stale(struct mptcp_stale_storage *storage,
+				      __u32 subflow_id)
+{
+	if (!subflow_id)
+		return;
+
+	for (int i = 0; i < storage->nr && i < MPTCP_SUBFLOWS_MAX; i++) {
+		if (storage->ids[i] == subflow_id) {
+			for (int j = i; j < MPTCP_SUBFLOWS_MAX - 1; j++) {
+				if (!storage->ids[j + 1])
+					break;
+				storage->ids[j] = storage->ids[j + 1];
+				storage->ids[j + 1] = 0;
+			}
+			storage->nr--;
+			return;
+		}
+	}
+}
+
+static bool mptcp_subflow_is_stale(struct mptcp_stale_storage *storage,
+				   __u32 subflow_id)
+{
+	for (int i = 0; i < storage->nr && i < MPTCP_SUBFLOWS_MAX; i++) {
+		if (storage->ids[i] == subflow_id)
+			return true;
+	}
+
+	return false;
+}
+
+static bool mptcp_subflow_is_active(struct mptcp_sched_data *data,
+				    __u32 subflow_id)
+{
+	for (int i = 0; i < data->subflows && i < MPTCP_SUBFLOWS_MAX; i++) {
+		struct mptcp_subflow_context *subflow;
+
+		subflow = mptcp_subflow_ctx_by_pos(data, i);
+		if (!subflow)
+			break;
+		if (subflow->subflow_id == subflow_id)
+			return true;
+	}
+
+	return false;
+}
+
+SEC("struct_ops/mptcp_sched_stale_init")
+void BPF_PROG(mptcp_sched_stale_init, struct mptcp_sock *msk)
+{
+	struct mptcp_stale_storage *storage;
+
+	storage = bpf_sk_storage_get(&mptcp_stale_map, msk, 0,
+				     BPF_LOCAL_STORAGE_GET_F_CREATE);
+	if (!storage)
+		return;
+
+	storage->nr = 0;
+}
+
+SEC("struct_ops/mptcp_sched_stale_release")
+void BPF_PROG(mptcp_sched_stale_release, struct mptcp_sock *msk)
+{
+	bpf_sk_storage_delete(&mptcp_stale_map, msk);
+}
+
+void BPF_STRUCT_OPS(bpf_stale_data_init, struct mptcp_sock *msk,
+		    struct mptcp_sched_data *data)
+{
+	struct mptcp_subflow_context *subflow;
+	struct mptcp_stale_storage *storage;
+
+	mptcp_sched_data_set_contexts(msk, data);
+
+	storage = bpf_sk_storage_get(&mptcp_stale_map, msk, 0,
+				     BPF_LOCAL_STORAGE_GET_F_CREATE);
+	if (!storage)
+		return;
+
+	for (int i = 0; i < storage->nr && i < MPTCP_SUBFLOWS_MAX; i++) {
+		if (!mptcp_subflow_is_active(data, storage->ids[i]))
+			mptcp_subflow_clear_stale(storage, storage->ids[i]);
+	}
+
+	subflow = mptcp_subflow_ctx_by_pos(data, 1);
+	if (subflow)
+		mptcp_subflow_set_stale(storage, subflow->subflow_id);
+}
+
+int BPF_STRUCT_OPS(bpf_stale_get_subflow, struct mptcp_sock *msk,
+		   const struct mptcp_sched_data *data)
+{
+	struct mptcp_stale_storage *storage;
+	int nr = -1;
+
+	storage = bpf_sk_storage_get(&mptcp_stale_map, msk, 0,
+				     BPF_LOCAL_STORAGE_GET_F_CREATE);
+	if (!storage)
+		return -1;
+
+	for (int i = 0; i < data->subflows && i < MPTCP_SUBFLOWS_MAX; i++) {
+		struct mptcp_subflow_context *subflow;
+
+		subflow = mptcp_subflow_ctx_by_pos(data, i);
+		if (!subflow)
+			break;
+
+		if (mptcp_subflow_is_stale(storage, subflow->subflow_id))
+			continue;
+
+		nr = i;
+	}
+
+	if (nr != -1) {
+		mptcp_subflow_set_scheduled(mptcp_subflow_ctx_by_pos(data, nr), true);
+		return -1;
+	}
+	return 0;
+}
+
+SEC(".struct_ops")
+struct mptcp_sched_ops stale = {
+	.init		= (void *)mptcp_sched_stale_init,
+	.release	= (void *)mptcp_sched_stale_release,
+	.data_init	= (void *)bpf_stale_data_init,
+	.get_subflow	= (void *)bpf_stale_get_subflow,
+	.name		= "bpf_stale",
+};
-- 
2.35.3

[RFC mptcp-next 11/11] selftests/bpf: Add bpf_stale test

[Geliang Tang]

This patch adds the bpf_stale scheduler test: test_stale(). Use sysctl to
set net.mptcp.scheduler to use this sched. Add two veth net devices to
simulate the multiple addresses case. Use 'ip mptcp endpoint' command to
add the new endpoint ADDR_2 to PM netlink. Send data and check bytes_sent
of 'ss' output after it to make sure the data has been only sent on ADDR_1
since ADDR_2 is set as stale.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 09770aa31f4a..5accc34d35e5 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -12,6 +12,7 @@
 #include "mptcp_bpf_rr.skel.h"
 #include "mptcp_bpf_red.skel.h"
 #include "mptcp_bpf_burst.skel.h"
+#include "mptcp_bpf_stale.skel.h"
 
 char NS_TEST[32];
 
@@ -524,6 +525,41 @@ static void test_burst(void)
 	mptcp_bpf_burst__destroy(burst_skel);
 }
 
+static void test_stale(void)
+{
+	struct mptcp_bpf_stale *stale_skel;
+	int server_fd, client_fd;
+	struct nstoken *nstoken;
+	struct bpf_link *link;
+
+	stale_skel = mptcp_bpf_stale__open_and_load();
+	if (!ASSERT_OK_PTR(stale_skel, "bpf_stale__open_and_load"))
+		return;
+
+	link = bpf_map__attach_struct_ops(stale_skel->maps.stale);
+	if (!ASSERT_OK_PTR(link, "bpf_map__attach_struct_ops")) {
+		mptcp_bpf_stale__destroy(stale_skel);
+		return;
+	}
+
+	nstoken = sched_init("subflow", "bpf_stale");
+	if (!ASSERT_OK_PTR(nstoken, "sched_init:bpf_stale"))
+		goto fail;
+	server_fd = start_mptcp_server(AF_INET, ADDR_1, PORT_1, 0);
+	client_fd = connect_to_fd(server_fd, 0);
+
+	send_data(server_fd, client_fd, "bpf_stale");
+	ASSERT_OK(has_bytes_sent(ADDR_1), "has_bytes_sent addr_1");
+	ASSERT_GT(has_bytes_sent(ADDR_2), 0, "has_bytes_sent addr_2");
+
+	close(client_fd);
+	close(server_fd);
+fail:
+	cleanup_netns(nstoken);
+	bpf_link__destroy(link);
+	mptcp_bpf_stale__destroy(stale_skel);
+}
+
 void test_mptcp(void)
 {
 	if (test__start_subtest("base"))
@@ -540,4 +576,6 @@ void test_mptcp(void)
 		test_red();
 	if (test__start_subtest("burst"))
 		test_burst();
+	if (test__start_subtest("stale"))
+		test_stale();
 }
-- 
2.35.3

Re: selftests/bpf: Add bpf_stale test: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join):
  - Unstable: 1 failed test(s): selftest_simult_flows 🔴:
  - Task: https://cirrus-ci.com/task/6331847710343168
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6331847710343168/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5487422780211200
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5487422780211200/summary/summary.txt

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6050372733632512
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6050372733632512/summary/summary.txt

- KVM Validation: normal (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/4924472826789888
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4924472826789888/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/95a1a1a4b69b


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)

Re: [RFC mptcp-next 00/11] BPF packet scheduler updates part 4

[Matthieu Baerts]

Hi Geliang,

Thank you for these patches!

On 10/08/2023 16:09, Geliang Tang wrote:
> There's a bug in bpf_burst. snd_burst stored in mptcp_burst_storage in
> BPF context is not used. msk->snd_burst is still used in kernel space.
> To fix this, add two new interfaces in mptcp_sched_ops to get and set
> scheduler's paramters from BPF context to kernel space.

I didn't fully look at this series in details (and I don't have the full
picture in mind) but I'm a bit worry about these parameters. I see two
ways to extend the BPF packet schedulers:

(1) either we keep the API as it is and we parametrise actions like here

(2) or the new schedulers have more control on what is being done
replacing what the core is doing to select what can be sent and where

I think that if we take the first option, we would limit the extensions:
when a new scheduler will want to change what the core is doing, a new
parameter will need to be added with more indirections, complexity, etc.

The second option requires some "big" changes in the scheduler API but
it is probably the right time to do that and probably worth it. (but
before modifying the code, we should probably discuss high level
architecture). With this new API, BPF scheduler should be able to only
modify the parts they want, leaving the rest like it is when the default
scheduler is running (or use helpers to keep the behaviour of the core).

This is also somehow linked to issues #343 and #344 on GitHub.

> Geliang Tang (11):
>   Squash to "mptcp: add struct mptcp_sched_ops"
>   Squash to "mptcp: add scheduler wrappers"
>   mptcp: use snd_burst wrappers
>   Squash to "mptcp: register default scheduler"
In other words, I think we should not modify the patches above and send
them as their are to netdev. Then think about the strategy we want  to
allow "any kind" of packet schedulers implemented in BPF.

WDYT?

>   Squash to "bpf: Add bpf_mptcp_sched_ops"
>   Squash to "selftests/bpf: Add mptcp sched structs"
>   Squash to "selftests/bpf: Add bpf_bkup scheduler"
>   Squash to "selftests/bpf: Add bpf_rr scheduler"
>   Squash to "selftests/bpf: Add bpf_burst scheduler"
>   selftests/bpf: Add bpf_stale scheduler
>   selftests/bpf: Add bpf_stale test

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

Re: [RFC mptcp-next 00/11] BPF packet scheduler updates part 4

[Geliang Tang]

On Thu, Aug 10, 2023 at 05:50:05PM +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> Thank you for these patches!
> 
> On 10/08/2023 16:09, Geliang Tang wrote:
> > There's a bug in bpf_burst. snd_burst stored in mptcp_burst_storage in
> > BPF context is not used. msk->snd_burst is still used in kernel space.
> > To fix this, add two new interfaces in mptcp_sched_ops to get and set
> > scheduler's paramters from BPF context to kernel space.
> 
> I didn't fully look at this series in details (and I don't have the full
> picture in mind) but I'm a bit worry about these parameters. I see two
> ways to extend the BPF packet schedulers:
> 
> (1) either we keep the API as it is and we parametrise actions like here
> 
> (2) or the new schedulers have more control on what is being done
> replacing what the core is doing to select what can be sent and where
> 
> I think that if we take the first option, we would limit the extensions:
> when a new scheduler will want to change what the core is doing, a new
> parameter will need to be added with more indirections, complexity, etc.
> 
> The second option requires some "big" changes in the scheduler API but
> it is probably the right time to do that and probably worth it. (but
> before modifying the code, we should probably discuss high level
> architecture). With this new API, BPF scheduler should be able to only
> modify the parts they want, leaving the rest like it is when the default
> scheduler is running (or use helpers to keep the behaviour of the core).
> 
> This is also somehow linked to issues #343 and #344 on GitHub.
> 
> > Geliang Tang (11):
> >   Squash to "mptcp: add struct mptcp_sched_ops"
> >   Squash to "mptcp: add scheduler wrappers"
> >   mptcp: use snd_burst wrappers
> >   Squash to "mptcp: register default scheduler"
> In other words, I think we should not modify the patches above and send
> them as their are to netdev. Then think about the strategy we want  to
> allow "any kind" of packet schedulers implemented in BPF.
> 
> WDYT?

I agree, let's deal with them after the scheduler refactor patches are
merged. Some patches below are still valid. They address to Mat's
comments in the series "add bpf_stale scheduler" v2. I'll repost them
as a v3 this week.

Thanks,
-Geliang

> 
> >   Squash to "bpf: Add bpf_mptcp_sched_ops"
> >   Squash to "selftests/bpf: Add mptcp sched structs"
> >   Squash to "selftests/bpf: Add bpf_bkup scheduler"
> >   Squash to "selftests/bpf: Add bpf_rr scheduler"
> >   Squash to "selftests/bpf: Add bpf_burst scheduler"
> >   selftests/bpf: Add bpf_stale scheduler
> >   selftests/bpf: Add bpf_stale test
> 
> Cheers,
> Matt
> -- 
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net