[PATCH mptcp-next v11 00/11] BPF packet scheduler updates part 1

[PATCH mptcp-next v11 00/11] BPF packet scheduler updates part 1

[Geliang Tang]

v11:
 - use sk_storage instead of mptcp_storage.
 - drop mptcp_storage.

v10:
 - add mptcp_storage map type.
 - update scheduler API.

v9:
 - merge 'Squash to "BPF packet scheduler updates v8"' and 'mptcp:
 make the contexts array of sched_data invisible in BPF'

v8:
 address Paolo's comments in v7:
 - add sched_data into mptcp_sock as new field, not an pointer.
 - drop rename __mptcp_set_timeout, export mptcp_set_timeout into BPF
   context instead.
 - add a comment in bpf_burst to explan why not use
   "ssk = send_info[SSK_MODE_ACTIVE].ssk;" directly.
 - move mptcp_subflow_tcp_sock into bpf_tcp_helpers.h.

v7:
 - drop more last_snd, only set it in bpf_rr.
 - 1-10: save sched_data at mptcp_sock, for issues #342
 - 11-16: add bpf_burst scheduler
 - 17-19: add bpf_stale scheduler, for issues #349

v6:
 - Only update patches 14, 15. Add a helper bpf_mptcp_subflow_ctx, fix
   bug in bpf_burst_get_retrans().

v5:
 - 1-9: save sched_data at mptcp_sock, for issues #342
 - 10-15: add bpf_burst scheduler
 - 16-18: add bpf_stale scheduler, for issues #349

v4:
 - drop "mptcp: add three helpers for bpf_burst", implement the helpers
   in the bpf context directly.

v3:
 - address Florian's comments in v2.
 - split into three more patches.

v2:
 - fix this error reported by CI:
KASAN: slab-use-after-free in __mptcp_close_ssk (net/mptcp/protocol.c:2461)
 - add bpf burst scheduler.

This patchset adds sched_data pointer into mptcp_sock to to save some
data at MPTCP and subflows levels.

With these changes, the old patch "mptcp: register default scheduler" in
[1] now works.

https://patchwork.kernel.org/project/mptcp/cover/cover.1665753926.git.geliang.tang@suse.com/ [1]
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/342
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/349

Geliang Tang (11):
  Squash to "mptcp: drop last_snd and MPTCP_RESET_SCHEDULER"
  Squash to "mptcp: add struct mptcp_sched_ops"
  Squash to "mptcp: add sched_data_set_contexts helper"
  mptcp: register default scheduler
  Squash to "bpf: Add bpf_mptcp_sched_ops"
  Squash to "bpf: Add bpf_mptcp_sched_kfunc_set"
  Squash to "selftests/bpf: Add mptcp sched structs"
  Squash to "selftests/bpf: Add bpf_first scheduler"
  Squash to "selftests/bpf: Add bpf_bkup scheduler"
  Squash to "selftests/bpf: Add bpf_rr scheduler"
  Squash to "selftests/bpf: Add bpf_red scheduler"

 include/net/mptcp.h                           | 11 +++--
 net/mptcp/bpf.c                               | 17 ++++++-
 net/mptcp/protocol.c                          | 10 +---
 net/mptcp/protocol.h                          |  4 +-
 net/mptcp/sched.c                             | 46 ++++++++++++++++++-
 tools/testing/selftests/bpf/bpf_tcp_helpers.h | 20 +++++---
 .../selftests/bpf/progs/mptcp_bpf_bkup.c      | 21 +++++----
 .../selftests/bpf/progs/mptcp_bpf_first.c     | 12 ++---
 .../selftests/bpf/progs/mptcp_bpf_red.c       | 16 +++----
 .../selftests/bpf/progs/mptcp_bpf_rr.c        | 44 ++++++++++++++----
 10 files changed, 143 insertions(+), 58 deletions(-)

-- 
2.35.3

[PATCH mptcp-next v11 01/11] Squash to "mptcp: drop last_snd and MPTCP_RESET_SCHEDULER"

[Geliang Tang]

Drop all last_snd.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/protocol.c | 9 ---------
 net/mptcp/protocol.h | 1 -
 2 files changed, 10 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index aad21e0bf536..3ae0b94d25c0 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1573,7 +1573,6 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
 					continue;
 				}
 				do_check_data_fin = true;
-				msk->last_snd = ssk;
 			}
 		}
 	}
@@ -1614,7 +1613,6 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk, bool
 			if (ret <= 0)
 				break;
 			copied += ret;
-			msk->last_snd = ssk;
 			continue;
 		}
 
@@ -1627,7 +1625,6 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk, bool
 			if (ret <= 0)
 				keep_pushing = false;
 			copied += ret;
-			msk->last_snd = ssk;
 		}
 
 		mptcp_for_each_subflow(msk, subflow) {
@@ -2410,9 +2407,6 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
 		WRITE_ONCE(msk->first, NULL);
 
 out:
-	if (ssk == msk->last_snd)
-		msk->last_snd = NULL;
-
 	if (need_push)
 		__mptcp_push_pending(sk, 0);
 }
@@ -2592,8 +2586,6 @@ static void __mptcp_retrans(struct sock *sk)
 			}
 
 			release_sock(ssk);
-
-			msk->last_snd = ssk;
 		}
 	}
 
@@ -3104,7 +3096,6 @@ static int mptcp_disconnect(struct sock *sk, int flags)
 	 * subflow
 	 */
 	mptcp_destroy_common(msk, MPTCP_CF_FASTCLOSE);
-	msk->last_snd = NULL;
 	WRITE_ONCE(msk->flags, 0);
 	msk->cb_flags = 0;
 	msk->push_pending = 0;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index e9ca30189731..8c9b2a03dc94 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -268,7 +268,6 @@ struct mptcp_sock {
 	u64		rcv_data_fin_seq;
 	u64		bytes_retrans;
 	int		rmem_fwd_alloc;
-	struct sock	*last_snd;
 	int		snd_burst;
 	int		old_wspace;
 	u64		recovery_snd_nxt;	/* in recovery mode accept up to this seq;
-- 
2.35.3

[PATCH mptcp-next v11 02/11] Squash to "mptcp: add struct mptcp_sched_ops"

[Geliang Tang]

- Add subflows in mptcp_sched_data.

- Drop 'const' before msk in sched API.

- Add const before data in get_subflow().

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

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 828b10ddabee..ba0e1ee68a05 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -101,21 +101,22 @@ struct mptcp_out_options {
 
 struct mptcp_sched_data {
 	bool	reinject;
+	u8	subflows;
 	struct mptcp_subflow_context *contexts[MPTCP_SUBFLOWS_MAX];
 };
 
 struct mptcp_sched_ops {
-	void (*data_init)(const struct mptcp_sock *msk,
+	void (*data_init)(struct mptcp_sock *msk,
 			  struct mptcp_sched_data *data);
-	int (*get_subflow)(const struct mptcp_sock *msk,
-			   struct mptcp_sched_data *data);
+	int (*get_subflow)(struct mptcp_sock *msk,
+			   const struct mptcp_sched_data *data);
 
 	char			name[MPTCP_SCHED_NAME_MAX];
 	struct module		*owner;
 	struct list_head	list;
 
-	void (*init)(const struct mptcp_sock *msk);
-	void (*release)(const struct mptcp_sock *msk);
+	void (*init)(struct mptcp_sock *msk);
+	void (*release)(struct mptcp_sock *msk);
 } ____cacheline_aligned_in_smp;
 
 #ifdef CONFIG_MPTCP
-- 
2.35.3

[PATCH mptcp-next v11 03/11] Squash to "mptcp: add sched_data_set_contexts helper"

[Geliang Tang]

mptcp: add sched_data helpers

Add a new helper mptcp_sched_data_set_contexts() to set the subflow
pointers array in struct mptcp_sched_data. Add a new helper
mptcp_subflow_ctx_by_pos() to get the given pos subflow from the
contexts array in struct mptcp_sched_data. They will be invoked by
the BPF schedulers to export the subflow pointers to the BPF contexts.

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

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 8c9b2a03dc94..cb2f34c8ed09 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -664,6 +664,8 @@ void mptcp_subflow_set_scheduled(struct mptcp_subflow_context *subflow,
 				 bool scheduled);
 void mptcp_sched_data_set_contexts(const struct mptcp_sock *msk,
 				   struct mptcp_sched_data *data);
+struct mptcp_subflow_context *
+mptcp_subflow_ctx_by_pos(const struct mptcp_sched_data *data, unsigned int pos);
 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);
diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
index c7c167e48d72..82665263bb94 100644
--- a/net/mptcp/sched.c
+++ b/net/mptcp/sched.c
@@ -108,11 +108,20 @@ void mptcp_sched_data_set_contexts(const struct mptcp_sock *msk,
 		mptcp_subflow_set_scheduled(subflow, false);
 		data->contexts[i++] = subflow;
 	}
+	data->subflows = i;
 
 	for (; i < MPTCP_SUBFLOWS_MAX; i++)
 		data->contexts[i] = NULL;
 }
 
+struct mptcp_subflow_context *
+mptcp_subflow_ctx_by_pos(const struct mptcp_sched_data *data, unsigned int pos)
+{
+	if (pos >= MPTCP_SUBFLOWS_MAX)
+		return NULL;
+	return data->contexts[pos];
+}
+
 int mptcp_sched_get_send(struct mptcp_sock *msk)
 {
 	struct mptcp_subflow_context *subflow;
-- 
2.35.3

[PATCH mptcp-next v11 04/11] mptcp: register default scheduler

[Geliang Tang]

This patch defines the default packet scheduler mptcp_sched_default.
Register it in mptcp_sched_init(), which is invoked in mptcp_proto_init().
Skip deleting this default scheduler in mptcp_unregister_scheduler().

Set msk->sched to the default scheduler when the input parameter of
mptcp_init_sched() is NULL.

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

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 3ae0b94d25c0..5f9f046b2124 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3970,6 +3970,7 @@ void __init mptcp_proto_init(void)
 
 	mptcp_subflow_init();
 	mptcp_pm_init();
+	mptcp_sched_init();
 	mptcp_token_init();
 
 	if (proto_register(&mptcp_prot, 1) != 0)
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index cb2f34c8ed09..bb4d50c8c398 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -657,6 +657,7 @@ void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
 struct mptcp_sched_ops *mptcp_sched_find(const char *name);
 int mptcp_register_scheduler(struct mptcp_sched_ops *sched);
 void mptcp_unregister_scheduler(struct mptcp_sched_ops *sched);
+void mptcp_sched_init(void);
 int mptcp_init_sched(struct mptcp_sock *msk,
 		     struct mptcp_sched_ops *sched);
 void mptcp_release_sched(struct mptcp_sock *msk);
diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
index 82665263bb94..a80cf0481edf 100644
--- a/net/mptcp/sched.c
+++ b/net/mptcp/sched.c
@@ -16,6 +16,32 @@
 static DEFINE_SPINLOCK(mptcp_sched_list_lock);
 static LIST_HEAD(mptcp_sched_list);
 
+static void mptcp_sched_default_data_init(struct mptcp_sock *msk,
+					  struct mptcp_sched_data *data)
+{
+}
+
+static int mptcp_sched_default_get_subflow(struct mptcp_sock *msk,
+					   const struct mptcp_sched_data *data)
+{
+	struct sock *ssk;
+
+	ssk = data->reinject ? mptcp_subflow_get_retrans(msk) :
+			       mptcp_subflow_get_send(msk);
+	if (!ssk)
+		return -EINVAL;
+
+	mptcp_subflow_set_scheduled(mptcp_subflow_ctx(ssk), true);
+	return 0;
+}
+
+static struct mptcp_sched_ops mptcp_sched_default = {
+	.data_init	= mptcp_sched_default_data_init,
+	.get_subflow	= mptcp_sched_default_get_subflow,
+	.name		= "default",
+	.owner		= THIS_MODULE,
+};
+
 /* Must be called with rcu read lock held */
 struct mptcp_sched_ops *mptcp_sched_find(const char *name)
 {
@@ -50,16 +76,24 @@ int mptcp_register_scheduler(struct mptcp_sched_ops *sched)
 
 void mptcp_unregister_scheduler(struct mptcp_sched_ops *sched)
 {
+	if (sched == &mptcp_sched_default)
+		return;
+
 	spin_lock(&mptcp_sched_list_lock);
 	list_del_rcu(&sched->list);
 	spin_unlock(&mptcp_sched_list_lock);
 }
 
+void mptcp_sched_init(void)
+{
+	mptcp_register_scheduler(&mptcp_sched_default);
+}
+
 int mptcp_init_sched(struct mptcp_sock *msk,
 		     struct mptcp_sched_ops *sched)
 {
 	if (!sched)
-		goto out;
+		sched = &mptcp_sched_default;
 
 	if (!bpf_try_module_get(sched, sched->owner))
 		return -EBUSY;
@@ -70,7 +104,6 @@ int mptcp_init_sched(struct mptcp_sock *msk,
 
 	pr_debug("sched=%s", msk->sched->name);
 
-out:
 	return 0;
 }
 
-- 
2.35.3

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

[Geliang Tang]

Add accesses for bpf_sk_storage_get and bpf_sk_storage_delete.

Add write accesses for avg_pacing_rate of struct mptcp_subflow_context
in .btf_struct_access. They will be used in the bpf_burst selftests.

Add write accesses for all bit flags of struct mptcp_subflow_context
between map_csum_len and data_avail in .btf_struct_access. The stale
flag will be used in the bpf_stale selftests.

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

diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index dd1208670c54..204dc37e31cd 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -13,6 +13,7 @@
 #include <linux/bpf_verifier.h>
 #include <linux/btf.h>
 #include <linux/btf_ids.h>
+#include <net/bpf_sk_storage.h>
 #include "protocol.h"
 
 #ifdef CONFIG_BPF_JIT
@@ -30,7 +31,14 @@ static const struct bpf_func_proto *
 bpf_mptcp_sched_get_func_proto(enum bpf_func_id func_id,
 			       const struct bpf_prog *prog)
 {
-	return bpf_base_func_proto(func_id);
+	switch (func_id) {
+	case BPF_FUNC_sk_storage_get:
+		return &bpf_sk_storage_get_proto;
+	case BPF_FUNC_sk_storage_delete:
+		return &bpf_sk_storage_delete_proto;
+	default:
+		return bpf_base_func_proto(func_id);
+	}
 }
 
 static int bpf_mptcp_sched_btf_struct_access(struct bpf_verifier_log *log,
@@ -50,6 +58,12 @@ static int bpf_mptcp_sched_btf_struct_access(struct bpf_verifier_log *log,
 	case offsetof(struct mptcp_subflow_context, scheduled):
 		end = offsetofend(struct mptcp_subflow_context, scheduled);
 		break;
+	case offsetofend(struct mptcp_subflow_context, map_csum_len):
+		end = offsetof(struct mptcp_subflow_context, data_avail);
+		break;
+	case offsetof(struct mptcp_subflow_context, avg_pacing_rate):
+		end = offsetofend(struct mptcp_subflow_context, avg_pacing_rate);
+		break;
 	default:
 		bpf_log(log, "no write support to mptcp_subflow_context at off %d\n", off);
 		return -EACCES;
-- 
2.35.3

[PATCH mptcp-next v11 06/11] Squash to "bpf: Add bpf_mptcp_sched_kfunc_set"

[Geliang Tang]

Export mptcp_subflow_ctx_by_pos too.

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

diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index 204dc37e31cd..c580add9c7f1 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -174,6 +174,7 @@ struct bpf_struct_ops bpf_mptcp_sched_ops = {
 BTF_SET8_START(bpf_mptcp_sched_kfunc_ids)
 BTF_ID_FLAGS(func, mptcp_subflow_set_scheduled)
 BTF_ID_FLAGS(func, mptcp_sched_data_set_contexts)
+BTF_ID_FLAGS(func, mptcp_subflow_ctx_by_pos)
 BTF_SET8_END(bpf_mptcp_sched_kfunc_ids)
 
 static const struct btf_kfunc_id_set bpf_mptcp_sched_kfunc_set = {
-- 
2.35.3

[PATCH mptcp-next v11 07/11] Squash to "selftests/bpf: Add mptcp sched structs"

[Geliang Tang]

Update scheduler API.

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

diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index 72c618037386..b4b766c7a68f 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -240,19 +240,19 @@ struct mptcp_subflow_context {
 
 struct mptcp_sched_data {
 	bool	reinject;
-	struct mptcp_subflow_context *contexts[MPTCP_SUBFLOWS_MAX];
+	__u8	subflows;
 } __attribute__((preserve_access_index));
 
 struct mptcp_sched_ops {
 	char name[MPTCP_SCHED_NAME_MAX];
 
-	void (*init)(const struct mptcp_sock *msk);
-	void (*release)(const struct mptcp_sock *msk);
+	void (*init)(struct mptcp_sock *msk);
+	void (*release)(struct mptcp_sock *msk);
 
-	void (*data_init)(const struct mptcp_sock *msk,
+	void (*data_init)(struct mptcp_sock *msk,
 			  struct mptcp_sched_data *data);
-	int (*get_subflow)(const struct mptcp_sock *msk,
-			   struct mptcp_sched_data *data);
+	int (*get_subflow)(struct mptcp_sock *msk,
+			   const struct mptcp_sched_data *data);
 	void *owner;
 };
 
@@ -269,5 +269,7 @@ extern void mptcp_subflow_set_scheduled(struct mptcp_subflow_context *subflow,
 					bool scheduled) __ksym;
 extern void mptcp_sched_data_set_contexts(const struct mptcp_sock *msk,
 					  struct mptcp_sched_data *data) __ksym;
+extern struct mptcp_subflow_context *
+mptcp_subflow_ctx_by_pos(const struct mptcp_sched_data *data, unsigned int pos) __ksym;
 
 #endif
-- 
2.35.3

[PATCH mptcp-next v11 08/11] Squash to "selftests/bpf: Add bpf_first scheduler"

[Geliang Tang]

Update API.

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

diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_first.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_first.c
index e4caa2dd8c6f..3b6669d536df 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_bpf_first.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_first.c
@@ -7,25 +7,25 @@
 char _license[] SEC("license") = "GPL";
 
 SEC("struct_ops/mptcp_sched_first_init")
-void BPF_PROG(mptcp_sched_first_init, const struct mptcp_sock *msk)
+void BPF_PROG(mptcp_sched_first_init, struct mptcp_sock *msk)
 {
 }
 
 SEC("struct_ops/mptcp_sched_first_release")
-void BPF_PROG(mptcp_sched_first_release, const struct mptcp_sock *msk)
+void BPF_PROG(mptcp_sched_first_release, struct mptcp_sock *msk)
 {
 }
 
-void BPF_STRUCT_OPS(bpf_first_data_init, const struct mptcp_sock *msk,
+void BPF_STRUCT_OPS(bpf_first_data_init, struct mptcp_sock *msk,
 		    struct mptcp_sched_data *data)
 {
 	mptcp_sched_data_set_contexts(msk, data);
 }
 
-int BPF_STRUCT_OPS(bpf_first_get_subflow, const struct mptcp_sock *msk,
-		   struct mptcp_sched_data *data)
+int BPF_STRUCT_OPS(bpf_first_get_subflow, struct mptcp_sock *msk,
+		   const struct mptcp_sched_data *data)
 {
-	mptcp_subflow_set_scheduled(data->contexts[0], true);
+	mptcp_subflow_set_scheduled(mptcp_subflow_ctx_by_pos(data, 0), true);
 	return 0;
 }
 
-- 
2.35.3

[PATCH mptcp-next v11 09/11] Squash to "selftests/bpf: Add bpf_bkup scheduler"

[Geliang Tang]

Update API.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../selftests/bpf/progs/mptcp_bpf_bkup.c      | 21 +++++++++++--------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_bkup.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_bkup.c
index b2724426676e..e5ba97390e17 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_bpf_bkup.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_bkup.c
@@ -7,37 +7,40 @@
 char _license[] SEC("license") = "GPL";
 
 SEC("struct_ops/mptcp_sched_bkup_init")
-void BPF_PROG(mptcp_sched_bkup_init, const struct mptcp_sock *msk)
+void BPF_PROG(mptcp_sched_bkup_init, struct mptcp_sock *msk)
 {
 }
 
 SEC("struct_ops/mptcp_sched_bkup_release")
-void BPF_PROG(mptcp_sched_bkup_release, const struct mptcp_sock *msk)
+void BPF_PROG(mptcp_sched_bkup_release, struct mptcp_sock *msk)
 {
 }
 
-void BPF_STRUCT_OPS(bpf_bkup_data_init, const struct mptcp_sock *msk,
+void BPF_STRUCT_OPS(bpf_bkup_data_init, struct mptcp_sock *msk,
 		    struct mptcp_sched_data *data)
 {
 	mptcp_sched_data_set_contexts(msk, data);
 }
 
-int BPF_STRUCT_OPS(bpf_bkup_get_subflow, const struct mptcp_sock *msk,
-		   struct mptcp_sched_data *data)
+int BPF_STRUCT_OPS(bpf_bkup_get_subflow, struct mptcp_sock *msk,
+		   const struct mptcp_sched_data *data)
 {
 	int nr = 0;
 
-	for (int i = 0; i < MPTCP_SUBFLOWS_MAX; i++) {
-		if (!data->contexts[i])
+	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 (!BPF_CORE_READ_BITFIELD_PROBED(data->contexts[i], backup)) {
+		if (!BPF_CORE_READ_BITFIELD_PROBED(subflow, backup)) {
 			nr = i;
 			break;
 		}
 	}
 
-	mptcp_subflow_set_scheduled(data->contexts[nr], true);
+	mptcp_subflow_set_scheduled(mptcp_subflow_ctx_by_pos(data, nr), true);
 	return 0;
 }
 
-- 
2.35.3

[PATCH mptcp-next v11 10/11] Squash to "selftests/bpf: Add bpf_rr scheduler"

[Geliang Tang]

Use sk_storage to store last_snd, instead of using msk->last_snd.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/bpf/bpf_tcp_helpers.h |  6 ++-
 .../selftests/bpf/progs/mptcp_bpf_rr.c        | 44 ++++++++++++++-----
 2 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index b4b766c7a68f..945dd46c98c0 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -259,7 +259,6 @@ struct mptcp_sched_ops {
 struct mptcp_sock {
 	struct inet_connection_sock	sk;
 
-	struct sock	*last_snd;
 	__u32		token;
 	struct sock	*first;
 	char		ca_name[TCP_CA_NAME_MAX];
@@ -271,5 +270,10 @@ extern void mptcp_sched_data_set_contexts(const struct mptcp_sock *msk,
 					  struct mptcp_sched_data *data) __ksym;
 extern struct mptcp_subflow_context *
 mptcp_subflow_ctx_by_pos(const struct mptcp_sched_data *data, unsigned int pos) __ksym;
+static inline struct sock *
+mptcp_subflow_tcp_sock(const struct mptcp_subflow_context *subflow)
+{
+	return subflow->tcp_sock;
+}
 
 #endif
diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
index e101428e5906..21144e96ba56 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
@@ -6,33 +6,49 @@
 
 char _license[] SEC("license") = "GPL";
 
+struct mptcp_rr_storage {
+	struct sock *last_snd;
+};
+struct sock *last_snd;
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SK_STORAGE);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, int);
+	__type(value, struct mptcp_rr_storage);
+} mptcp_rr_map SEC(".maps");
+
 SEC("struct_ops/mptcp_sched_rr_init")
-void BPF_PROG(mptcp_sched_rr_init, const struct mptcp_sock *msk)
+void BPF_PROG(mptcp_sched_rr_init, struct mptcp_sock *msk)
 {
 }
 
 SEC("struct_ops/mptcp_sched_rr_release")
-void BPF_PROG(mptcp_sched_rr_release, const struct mptcp_sock *msk)
+void BPF_PROG(mptcp_sched_rr_release, struct mptcp_sock *msk)
 {
+	bpf_sk_storage_delete(&mptcp_rr_map, msk);
 }
 
-void BPF_STRUCT_OPS(bpf_rr_data_init, const struct mptcp_sock *msk,
+void BPF_STRUCT_OPS(bpf_rr_data_init, struct mptcp_sock *msk,
 		    struct mptcp_sched_data *data)
 {
 	mptcp_sched_data_set_contexts(msk, data);
 }
 
-int BPF_STRUCT_OPS(bpf_rr_get_subflow, const struct mptcp_sock *msk,
-		   struct mptcp_sched_data *data)
+int BPF_STRUCT_OPS(bpf_rr_get_subflow, struct mptcp_sock *msk,
+		   const struct mptcp_sched_data *data)
 {
+	struct mptcp_subflow_context *subflow;
+	struct mptcp_rr_storage *ptr;
 	int nr = 0;
 
-	for (int i = 0; i < MPTCP_SUBFLOWS_MAX; i++) {
-		if (!msk->last_snd || !data->contexts[i])
+	for (int i = 0; i < data->subflows && i < MPTCP_SUBFLOWS_MAX; i++) {
+		subflow = mptcp_subflow_ctx_by_pos(data, i);
+		if (!last_snd || !subflow)
 			break;
 
-		if (data->contexts[i]->tcp_sock == msk->last_snd) {
-			if (i + 1 == MPTCP_SUBFLOWS_MAX || !data->contexts[i + 1])
+		if (mptcp_subflow_tcp_sock(subflow) == last_snd) {
+			if (i + 1 == MPTCP_SUBFLOWS_MAX || !mptcp_subflow_ctx_by_pos(data, i + 1))
 				break;
 
 			nr = i + 1;
@@ -40,7 +56,15 @@ int BPF_STRUCT_OPS(bpf_rr_get_subflow, const struct mptcp_sock *msk,
 		}
 	}
 
-	mptcp_subflow_set_scheduled(data->contexts[nr], true);
+	subflow = mptcp_subflow_ctx_by_pos(data, nr);
+	if (!subflow)
+		return -1;
+	mptcp_subflow_set_scheduled(subflow, true);
+	last_snd = mptcp_subflow_tcp_sock(subflow);
+	ptr = bpf_sk_storage_get(&mptcp_rr_map, msk, 0,
+				 BPF_LOCAL_STORAGE_GET_F_CREATE);
+	if (ptr)
+		ptr->last_snd = last_snd;
 	return 0;
 }
 
-- 
2.35.3

[PATCH mptcp-next v11 11/11] Squash to "selftests/bpf: Add bpf_red scheduler"

[Geliang Tang]

Update API.

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

diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_red.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_red.c
index 30dd6f521b7f..576e4acd1fa0 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_bpf_red.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_red.c
@@ -7,29 +7,29 @@
 char _license[] SEC("license") = "GPL";
 
 SEC("struct_ops/mptcp_sched_red_init")
-void BPF_PROG(mptcp_sched_red_init, const struct mptcp_sock *msk)
+void BPF_PROG(mptcp_sched_red_init, struct mptcp_sock *msk)
 {
 }
 
 SEC("struct_ops/mptcp_sched_red_release")
-void BPF_PROG(mptcp_sched_red_release, const struct mptcp_sock *msk)
+void BPF_PROG(mptcp_sched_red_release, struct mptcp_sock *msk)
 {
 }
 
-void BPF_STRUCT_OPS(bpf_red_data_init, const struct mptcp_sock *msk,
+void BPF_STRUCT_OPS(bpf_red_data_init, struct mptcp_sock *msk,
 		    struct mptcp_sched_data *data)
 {
 	mptcp_sched_data_set_contexts(msk, data);
 }
 
-int BPF_STRUCT_OPS(bpf_red_get_subflow, const struct mptcp_sock *msk,
-		   struct mptcp_sched_data *data)
+int BPF_STRUCT_OPS(bpf_red_get_subflow, struct mptcp_sock *msk,
+		   const struct mptcp_sched_data *data)
 {
-	for (int i = 0; i < MPTCP_SUBFLOWS_MAX; i++) {
-		if (!data->contexts[i])
+	for (int i = 0; i < data->subflows && i < MPTCP_SUBFLOWS_MAX; i++) {
+		if (!mptcp_subflow_ctx_by_pos(data, i))
 			break;
 
-		mptcp_subflow_set_scheduled(data->contexts[i], true);
+		mptcp_subflow_set_scheduled(mptcp_subflow_ctx_by_pos(data, i), true);
 	}
 
 	return 0;
-- 
2.35.3

Re: Squash to "selftests/bpf: Add bpf_red scheduler": 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):
  - Script error! ❓:
  - Task: https://cirrus-ci.com/task/5556020068483072
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5556020068483072/summary/summary.txt

- KVM Validation: normal (only selftest_mptcp_join):
  - Script error! ❓:
  - Task: https://cirrus-ci.com/task/6681919975325696
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6681919975325696/summary/summary.txt

- KVM Validation: debug (except selftest_mptcp_join):
  - Script error! ❓:
  - Task: https://cirrus-ci.com/task/4641226394173440
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4641226394173440/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Script error! ❓:
  - Task: https://cirrus-ci.com/task/5767126301016064
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5767126301016064/summary/summary.txt

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


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: Squash to "selftests/bpf: Add bpf_red scheduler": 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):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5417011338018816
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5417011338018816/summary/summary.txt

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

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

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

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


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: [PATCH mptcp-next v11 10/11] Squash to "selftests/bpf: Add bpf_rr scheduler"

[Paolo Abeni]

On Tue, 2023-06-27 at 09:06 +0800, Geliang Tang wrote:
> Use sk_storage to store last_snd, instead of using msk->last_snd.
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  tools/testing/selftests/bpf/bpf_tcp_helpers.h |  6 ++-
>  .../selftests/bpf/progs/mptcp_bpf_rr.c        | 44 ++++++++++++++-----
>  2 files changed, 39 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> index b4b766c7a68f..945dd46c98c0 100644
> --- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> @@ -259,7 +259,6 @@ struct mptcp_sched_ops {
>  struct mptcp_sock {
>  	struct inet_connection_sock	sk;
>  
> -	struct sock	*last_snd;
>  	__u32		token;
>  	struct sock	*first;
>  	char		ca_name[TCP_CA_NAME_MAX];
> @@ -271,5 +270,10 @@ extern void mptcp_sched_data_set_contexts(const struct mptcp_sock *msk,
>  					  struct mptcp_sched_data *data) __ksym;
>  extern struct mptcp_subflow_context *
>  mptcp_subflow_ctx_by_pos(const struct mptcp_sched_data *data, unsigned int pos) __ksym;
> +static inline struct sock *
> +mptcp_subflow_tcp_sock(const struct mptcp_subflow_context *subflow)
> +{
> +	return subflow->tcp_sock;
> +}
>  
>  #endif
> diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
> index e101428e5906..21144e96ba56 100644
> --- a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
> +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
> @@ -6,33 +6,49 @@
>  
>  char _license[] SEC("license") = "GPL";
>  
> +struct mptcp_rr_storage {
> +	struct sock *last_snd;
> +};
> +struct sock *last_snd;
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_SK_STORAGE);
> +	__uint(map_flags, BPF_F_NO_PREALLOC);
> +	__type(key, int);
> +	__type(value, struct mptcp_rr_storage);
> +} mptcp_rr_map SEC(".maps");
> +
>  SEC("struct_ops/mptcp_sched_rr_init")
> -void BPF_PROG(mptcp_sched_rr_init, const struct mptcp_sock *msk)
> +void BPF_PROG(mptcp_sched_rr_init, struct mptcp_sock *msk)
>  {
>  }
>  
>  SEC("struct_ops/mptcp_sched_rr_release")
> -void BPF_PROG(mptcp_sched_rr_release, const struct mptcp_sock *msk)
> +void BPF_PROG(mptcp_sched_rr_release, struct mptcp_sock *msk)
>  {
> +	bpf_sk_storage_delete(&mptcp_rr_map, msk);
>  }
>  
> -void BPF_STRUCT_OPS(bpf_rr_data_init, const struct mptcp_sock *msk,
> +void BPF_STRUCT_OPS(bpf_rr_data_init, struct mptcp_sock *msk,
>  		    struct mptcp_sched_data *data)
>  {
>  	mptcp_sched_data_set_contexts(msk, data);
>  }
>  
> -int BPF_STRUCT_OPS(bpf_rr_get_subflow, const struct mptcp_sock *msk,
> -		   struct mptcp_sched_data *data)
> +int BPF_STRUCT_OPS(bpf_rr_get_subflow, struct mptcp_sock *msk,
> +		   const struct mptcp_sched_data *data)
>  {
> +	struct mptcp_subflow_context *subflow;
> +	struct mptcp_rr_storage *ptr;
>  	int nr = 0;
>  
> -	for (int i = 0; i < MPTCP_SUBFLOWS_MAX; i++) {
> -		if (!msk->last_snd || !data->contexts[i])
> +	for (int i = 0; i < data->subflows && i < MPTCP_SUBFLOWS_MAX; i++) {
> +		subflow = mptcp_subflow_ctx_by_pos(data, i);
> +		if (!last_snd || !subflow)

It looks like this is accessing a global variable ('last_snd'), which
is probably not very safe.

I think you could instead use inet_csk(msk)->icsk_ca_priv - currently
unused.

As this is the only comment I have, no need to send a whole v12. I
think we can merge the series as-is and then squash the needed change
to replace the global variable and map used here with inet_csk(msk)-
>icsk_ca_priv.

Thanks!

Paolo

Re: [PATCH mptcp-next v11 10/11] Squash to "selftests/bpf: Add bpf_rr scheduler"

[Geliang Tang]

On Mon, Jul 03, 2023 at 05:38:39PM +0200, Paolo Abeni wrote:
> On Tue, 2023-06-27 at 09:06 +0800, Geliang Tang wrote:
> > Use sk_storage to store last_snd, instead of using msk->last_snd.
> > 
> > Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> > ---
> >  tools/testing/selftests/bpf/bpf_tcp_helpers.h |  6 ++-
> >  .../selftests/bpf/progs/mptcp_bpf_rr.c        | 44 ++++++++++++++-----
> >  2 files changed, 39 insertions(+), 11 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> > index b4b766c7a68f..945dd46c98c0 100644
> > --- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> > +++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> > @@ -259,7 +259,6 @@ struct mptcp_sched_ops {
> >  struct mptcp_sock {
> >  	struct inet_connection_sock	sk;
> >  
> > -	struct sock	*last_snd;
> >  	__u32		token;
> >  	struct sock	*first;
> >  	char		ca_name[TCP_CA_NAME_MAX];
> > @@ -271,5 +270,10 @@ extern void mptcp_sched_data_set_contexts(const struct mptcp_sock *msk,
> >  					  struct mptcp_sched_data *data) __ksym;
> >  extern struct mptcp_subflow_context *
> >  mptcp_subflow_ctx_by_pos(const struct mptcp_sched_data *data, unsigned int pos) __ksym;
> > +static inline struct sock *
> > +mptcp_subflow_tcp_sock(const struct mptcp_subflow_context *subflow)
> > +{
> > +	return subflow->tcp_sock;
> > +}
> >  
> >  #endif
> > diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
> > index e101428e5906..21144e96ba56 100644
> > --- a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
> > +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
> > @@ -6,33 +6,49 @@
> >  
> >  char _license[] SEC("license") = "GPL";
> >  
> > +struct mptcp_rr_storage {
> > +	struct sock *last_snd;
> > +};
> > +struct sock *last_snd;
> > +
> > +struct {
> > +	__uint(type, BPF_MAP_TYPE_SK_STORAGE);
> > +	__uint(map_flags, BPF_F_NO_PREALLOC);
> > +	__type(key, int);
> > +	__type(value, struct mptcp_rr_storage);
> > +} mptcp_rr_map SEC(".maps");
> > +
> >  SEC("struct_ops/mptcp_sched_rr_init")
> > -void BPF_PROG(mptcp_sched_rr_init, const struct mptcp_sock *msk)
> > +void BPF_PROG(mptcp_sched_rr_init, struct mptcp_sock *msk)
> >  {
> >  }
> >  
> >  SEC("struct_ops/mptcp_sched_rr_release")
> > -void BPF_PROG(mptcp_sched_rr_release, const struct mptcp_sock *msk)
> > +void BPF_PROG(mptcp_sched_rr_release, struct mptcp_sock *msk)
> >  {
> > +	bpf_sk_storage_delete(&mptcp_rr_map, msk);
> >  }
> >  
> > -void BPF_STRUCT_OPS(bpf_rr_data_init, const struct mptcp_sock *msk,
> > +void BPF_STRUCT_OPS(bpf_rr_data_init, struct mptcp_sock *msk,
> >  		    struct mptcp_sched_data *data)
> >  {
> >  	mptcp_sched_data_set_contexts(msk, data);
> >  }
> >  
> > -int BPF_STRUCT_OPS(bpf_rr_get_subflow, const struct mptcp_sock *msk,
> > -		   struct mptcp_sched_data *data)
> > +int BPF_STRUCT_OPS(bpf_rr_get_subflow, struct mptcp_sock *msk,
> > +		   const struct mptcp_sched_data *data)
> >  {
> > +	struct mptcp_subflow_context *subflow;
> > +	struct mptcp_rr_storage *ptr;
> >  	int nr = 0;
> >  
> > -	for (int i = 0; i < MPTCP_SUBFLOWS_MAX; i++) {
> > -		if (!msk->last_snd || !data->contexts[i])
> > +	for (int i = 0; i < data->subflows && i < MPTCP_SUBFLOWS_MAX; i++) {
> > +		subflow = mptcp_subflow_ctx_by_pos(data, i);
> > +		if (!last_snd || !subflow)
> 
> It looks like this is accessing a global variable ('last_snd'), which
> is probably not very safe.

Yes, it's should be a local variable. I just sent a patch to ML to fix
this:

https://patchwork.kernel.org/project/mptcp/patch/0b57bf87dc30e15782ba1fe2e925a7ce6ef0fc90.1688434547.git.geliang.tang@suse.com/

> 
> I think you could instead use inet_csk(msk)->icsk_ca_priv - currently
> unused.
> 
> As this is the only comment I have, no need to send a whole v12. I
> think we can merge the series as-is and then squash the needed change
> to replace the global variable and map used here with inet_csk(msk)-
> >icsk_ca_priv.

One of the purposes of this test is to demonstrate an example of using a
bpf map to store persistent schedulers data, so I prefer to use the bpf
map to implement it here.

Using inet_csk(msk)->icsk_ca_priv is much more complex. We need to add
the special write accesses for icsk_ca_priv.

Thanks,
-Geliang

> 
> Thanks!
> 
> Paolo
> 

Re: [PATCH mptcp-next v11 00/11] BPF packet scheduler updates part 1

[Matthieu Baerts]

Hi Geliang, Paolo,

On 27/06/2023 03:06, Geliang Tang wrote:
> v11:
>  - use sk_storage instead of mptcp_storage.
>  - drop mptcp_storage.

Thank you for the patches and the reviews!
> https://patchwork.kernel.org/project/mptcp/cover/cover.1665753926.git.geliang.tang@suse.com/ [1]
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/342
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/349

Does this first part still close these two tickets?

Now in our tree (feat. for other trees) + one extra patch and a squash
of a squash. Please don't hesitate to check if everything has been done
properly because there were quite a few manipulations to do :)

- 8efabdbcaa2c: "squashed" patch 1/11 (with conflicts) in "mptcp: drop
last_snd and MPTCP_RESET_SCHEDULER"
- b0f6d5c5ae25: "squashed" patch 1/11 (with conflicts) in "mptcp: use
get_send wrapper"
- 9de04aa46c59: "squashed" patch 1/11 in "mptcp: use get_retrans wrapper"
- Results: e77e1cb5a372..3fcf166a9c7e (export)

- 6e930aa67c6e: "squashed" patch 2/11 in "mptcp: add struct mptcp_sched_ops"
- Results: 3fcf166a9c7e..437c8b684629 (export)

- f7e28c2443da: "squashed" patch 3/11 (with conflicts) in "mptcp: add
sched_data_set_contexts helper"
- 9dc7c0042a01: conflict in t/mptcp-add-scheduler-wrappers
- Results: 437c8b684629..504bc1f2d42d (export)

- Applied patch 4/11 with Paolo's ACK just before "bpf: Add
bpf_mptcp_sched_ops"
- 7f44744d88c6: mptcp: register default scheduler
- Results: 504bc1f2d42d..d45b69d4109b (export)

- 0c323c5cc341: "squashed" patch 5/11 in "bpf: Add bpf_mptcp_sched_ops"
- Results: d45b69d4109b..1b64ad359430 (export)

- 9ffacd26fad5: "squashed" patch 6/11 in "bpf: Add
bpf_mptcp_sched_kfunc_set"
- Results: 1b64ad359430..f95ddf57fd7e (export)

- 8f050b24bd4d: "squashed" patch 7/11 in "selftests/bpf: Add mptcp sched
structs"
- Results: f95ddf57fd7e..b1bfdcd53258 (export)

- a3eacd00bfb7: "squashed" patch 8/11 in "selftests/bpf: Add bpf_first
scheduler"
- Results: b1bfdcd53258..b80981f0543b (export)

- bc7512c33473: "squashed" patch 9/11 in "selftests/bpf: Add bpf_bkup
scheduler"
- Results: b80981f0543b..e1dbf174936b (export)

- 506feec3a419: "squashed" patch 10/11 in "selftests/bpf: Add bpf_rr
scheduler"
- Results: e1dbf174936b..c6edb7e5168c (export)

- 151a85fd02ab: "squashed" patch 11/11 in "selftests/bpf: Add bpf_red
scheduler"
- Results: c6edb7e5168c..679a4538b06c (export)

- 064c5746a0d9: "squashed" patch 12/11 (squash squash) in
"selftests/bpf: Add bpf_rr scheduler"
- Results: 679a4538b06c..255b06daaf0d (export)

- cebedb75ff65: "squashed" patch 6/7 from BPF 'force to MPTCP' v4 series
in "selftests/bpf: add two mptcp netns helpers"
- Results: 255b06daaf0d..cde4fa6a0a5e (export)


Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230705T090903

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

Re: [PATCH mptcp-next v11 00/11] BPF packet scheduler updates part 1

[Mat Martineau]

On Wed, 5 Jul 2023, Matthieu Baerts wrote:

> Hi Geliang, Paolo,
>
> On 27/06/2023 03:06, Geliang Tang wrote:
>> v11:
>>  - use sk_storage instead of mptcp_storage.
>>  - drop mptcp_storage.
>

sk_storage (and bpf_sk_storage_{get,delete}) are a very good solution for 
bpf scheduler data - thanks for finding that Geliang!

Glad that this is merged, appreciate the review by Paolo and Mattheiu 
applying the series.


- Mat

> Thank you for the patches and the reviews!
>> https://patchwork.kernel.org/project/mptcp/cover/cover.1665753926.git.geliang.tang@suse.com/ [1]
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/342
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/349
>
> Does this first part still close these two tickets?
>
> Now in our tree (feat. for other trees) + one extra patch and a squash
> of a squash. Please don't hesitate to check if everything has been done
> properly because there were quite a few manipulations to do :)
>
> - 8efabdbcaa2c: "squashed" patch 1/11 (with conflicts) in "mptcp: drop
> last_snd and MPTCP_RESET_SCHEDULER"
> - b0f6d5c5ae25: "squashed" patch 1/11 (with conflicts) in "mptcp: use
> get_send wrapper"
> - 9de04aa46c59: "squashed" patch 1/11 in "mptcp: use get_retrans wrapper"
> - Results: e77e1cb5a372..3fcf166a9c7e (export)
>
> - 6e930aa67c6e: "squashed" patch 2/11 in "mptcp: add struct mptcp_sched_ops"
> - Results: 3fcf166a9c7e..437c8b684629 (export)
>
> - f7e28c2443da: "squashed" patch 3/11 (with conflicts) in "mptcp: add
> sched_data_set_contexts helper"
> - 9dc7c0042a01: conflict in t/mptcp-add-scheduler-wrappers
> - Results: 437c8b684629..504bc1f2d42d (export)
>
> - Applied patch 4/11 with Paolo's ACK just before "bpf: Add
> bpf_mptcp_sched_ops"
> - 7f44744d88c6: mptcp: register default scheduler
> - Results: 504bc1f2d42d..d45b69d4109b (export)
>
> - 0c323c5cc341: "squashed" patch 5/11 in "bpf: Add bpf_mptcp_sched_ops"
> - Results: d45b69d4109b..1b64ad359430 (export)
>
> - 9ffacd26fad5: "squashed" patch 6/11 in "bpf: Add
> bpf_mptcp_sched_kfunc_set"
> - Results: 1b64ad359430..f95ddf57fd7e (export)
>
> - 8f050b24bd4d: "squashed" patch 7/11 in "selftests/bpf: Add mptcp sched
> structs"
> - Results: f95ddf57fd7e..b1bfdcd53258 (export)
>
> - a3eacd00bfb7: "squashed" patch 8/11 in "selftests/bpf: Add bpf_first
> scheduler"
> - Results: b1bfdcd53258..b80981f0543b (export)
>
> - bc7512c33473: "squashed" patch 9/11 in "selftests/bpf: Add bpf_bkup
> scheduler"
> - Results: b80981f0543b..e1dbf174936b (export)
>
> - 506feec3a419: "squashed" patch 10/11 in "selftests/bpf: Add bpf_rr
> scheduler"
> - Results: e1dbf174936b..c6edb7e5168c (export)
>
> - 151a85fd02ab: "squashed" patch 11/11 in "selftests/bpf: Add bpf_red
> scheduler"
> - Results: c6edb7e5168c..679a4538b06c (export)
>
> - 064c5746a0d9: "squashed" patch 12/11 (squash squash) in
> "selftests/bpf: Add bpf_rr scheduler"
> - Results: 679a4538b06c..255b06daaf0d (export)
>
> - cebedb75ff65: "squashed" patch 6/7 from BPF 'force to MPTCP' v4 series
> in "selftests/bpf: add two mptcp netns helpers"
> - Results: 255b06daaf0d..cde4fa6a0a5e (export)
>
>
> Tests are now in progress:
>
> https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230705T090903
>
> Cheers,
> Matt
> -- 
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
>
>

Re: [PATCH mptcp-next v11 00/11] BPF packet scheduler updates part 1

[Geliang Tang]

On Wed, Jul 05, 2023 at 11:11:24AM +0200, Matthieu Baerts wrote:
> Hi Geliang, Paolo,
> 
> On 27/06/2023 03:06, Geliang Tang wrote:
> > v11:
> >  - use sk_storage instead of mptcp_storage.
> >  - drop mptcp_storage.
> 
> Thank you for the patches and the reviews!
> > https://patchwork.kernel.org/project/mptcp/cover/cover.1665753926.git.geliang.tang@suse.com/ [1]
> > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/342
> > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/349
> 
> Does this first part still close these two tickets?

This first part can close the ticket #342 only. #349 will be closed by
part 2.

Thanks,
-Geliang

> 
> Now in our tree (feat. for other trees) + one extra patch and a squash
> of a squash. Please don't hesitate to check if everything has been done
> properly because there were quite a few manipulations to do :)
> 
> - 8efabdbcaa2c: "squashed" patch 1/11 (with conflicts) in "mptcp: drop
> last_snd and MPTCP_RESET_SCHEDULER"
> - b0f6d5c5ae25: "squashed" patch 1/11 (with conflicts) in "mptcp: use
> get_send wrapper"
> - 9de04aa46c59: "squashed" patch 1/11 in "mptcp: use get_retrans wrapper"
> - Results: e77e1cb5a372..3fcf166a9c7e (export)
> 
> - 6e930aa67c6e: "squashed" patch 2/11 in "mptcp: add struct mptcp_sched_ops"
> - Results: 3fcf166a9c7e..437c8b684629 (export)
> 
> - f7e28c2443da: "squashed" patch 3/11 (with conflicts) in "mptcp: add
> sched_data_set_contexts helper"
> - 9dc7c0042a01: conflict in t/mptcp-add-scheduler-wrappers
> - Results: 437c8b684629..504bc1f2d42d (export)
> 
> - Applied patch 4/11 with Paolo's ACK just before "bpf: Add
> bpf_mptcp_sched_ops"
> - 7f44744d88c6: mptcp: register default scheduler
> - Results: 504bc1f2d42d..d45b69d4109b (export)
> 
> - 0c323c5cc341: "squashed" patch 5/11 in "bpf: Add bpf_mptcp_sched_ops"
> - Results: d45b69d4109b..1b64ad359430 (export)
> 
> - 9ffacd26fad5: "squashed" patch 6/11 in "bpf: Add
> bpf_mptcp_sched_kfunc_set"
> - Results: 1b64ad359430..f95ddf57fd7e (export)
> 
> - 8f050b24bd4d: "squashed" patch 7/11 in "selftests/bpf: Add mptcp sched
> structs"
> - Results: f95ddf57fd7e..b1bfdcd53258 (export)
> 
> - a3eacd00bfb7: "squashed" patch 8/11 in "selftests/bpf: Add bpf_first
> scheduler"
> - Results: b1bfdcd53258..b80981f0543b (export)
> 
> - bc7512c33473: "squashed" patch 9/11 in "selftests/bpf: Add bpf_bkup
> scheduler"
> - Results: b80981f0543b..e1dbf174936b (export)
> 
> - 506feec3a419: "squashed" patch 10/11 in "selftests/bpf: Add bpf_rr
> scheduler"
> - Results: e1dbf174936b..c6edb7e5168c (export)
> 
> - 151a85fd02ab: "squashed" patch 11/11 in "selftests/bpf: Add bpf_red
> scheduler"
> - Results: c6edb7e5168c..679a4538b06c (export)
> 
> - 064c5746a0d9: "squashed" patch 12/11 (squash squash) in
> "selftests/bpf: Add bpf_rr scheduler"
> - Results: 679a4538b06c..255b06daaf0d (export)
> 
> - cebedb75ff65: "squashed" patch 6/7 from BPF 'force to MPTCP' v4 series
> in "selftests/bpf: add two mptcp netns helpers"
> - Results: 255b06daaf0d..cde4fa6a0a5e (export)
> 
> 
> Tests are now in progress:
> 
> https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230705T090903
> 
> Cheers,
> Matt
> -- 
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net

Re: [PATCH mptcp-next v11 03/11] Squash to "mptcp: add sched_data_set_contexts helper"

[Geliang Tang]

On Tue, Jun 27, 2023 at 09:06:46AM +0800, Geliang Tang wrote:
> mptcp: add sched_data helpers
> 
> Add a new helper mptcp_sched_data_set_contexts() to set the subflow
> pointers array in struct mptcp_sched_data. Add a new helper
> mptcp_subflow_ctx_by_pos() to get the given pos subflow from the
> contexts array in struct mptcp_sched_data. They will be invoked by
> the BPF schedulers to export the subflow pointers to the BPF contexts.

Hi Matt,

You forgot to help me update the subject and commit log :)

Thanks,
-Geliang

> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  net/mptcp/protocol.h | 2 ++
>  net/mptcp/sched.c    | 9 +++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 8c9b2a03dc94..cb2f34c8ed09 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -664,6 +664,8 @@ void mptcp_subflow_set_scheduled(struct mptcp_subflow_context *subflow,
>  				 bool scheduled);
>  void mptcp_sched_data_set_contexts(const struct mptcp_sock *msk,
>  				   struct mptcp_sched_data *data);
> +struct mptcp_subflow_context *
> +mptcp_subflow_ctx_by_pos(const struct mptcp_sched_data *data, unsigned int pos);
>  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);
> diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
> index c7c167e48d72..82665263bb94 100644
> --- a/net/mptcp/sched.c
> +++ b/net/mptcp/sched.c
> @@ -108,11 +108,20 @@ void mptcp_sched_data_set_contexts(const struct mptcp_sock *msk,
>  		mptcp_subflow_set_scheduled(subflow, false);
>  		data->contexts[i++] = subflow;
>  	}
> +	data->subflows = i;
>  
>  	for (; i < MPTCP_SUBFLOWS_MAX; i++)
>  		data->contexts[i] = NULL;
>  }
>  
> +struct mptcp_subflow_context *
> +mptcp_subflow_ctx_by_pos(const struct mptcp_sched_data *data, unsigned int pos)
> +{
> +	if (pos >= MPTCP_SUBFLOWS_MAX)
> +		return NULL;
> +	return data->contexts[pos];
> +}
> +
>  int mptcp_sched_get_send(struct mptcp_sock *msk)
>  {
>  	struct mptcp_subflow_context *subflow;
> -- 
> 2.35.3
> 

Re: [PATCH mptcp-next v11 03/11] Squash to "mptcp: add sched_data_set_contexts helper"

[Matthieu Baerts]

Hi Geliang,

On 13/07/2023 09:15, Geliang Tang wrote:
> On Tue, Jun 27, 2023 at 09:06:46AM +0800, Geliang Tang wrote:
>> mptcp: add sched_data helpers
>>
>> Add a new helper mptcp_sched_data_set_contexts() to set the subflow
>> pointers array in struct mptcp_sched_data. Add a new helper
>> mptcp_subflow_ctx_by_pos() to get the given pos subflow from the
>> contexts array in struct mptcp_sched_data. They will be invoked by
>> the BPF schedulers to export the subflow pointers to the BPF contexts.
> 
> Hi Matt,
> 
> You forgot to help me update the subject and commit log :)

Oops, I didn't see you were asking me to change the commit message.
Thank you for your message, now fixed:

New patches for t/upstream:
- 4194305e86f7: tg:msg: apply new commit message from Geliang
- Results: 2e4f8c818fa1..3ee02a7d38ae (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230713T084941

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