This patch added struct mptcp_sched_ops. And define the scheduler
register, unregister and find functions.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 include/net/mptcp.h  |  13 +++++
 net/mptcp/Makefile   |   2 +-
 net/mptcp/protocol.c |   1 +
 net/mptcp/protocol.h |   7 +++
 net/mptcp/sched.c    | 114 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 136 insertions(+), 1 deletion(-)
 create mode 100644 net/mptcp/sched.c

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 8b1afd6f5cc4..e3a0baa8dbd7 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -95,6 +95,19 @@ struct mptcp_out_options {
 #endif
 };
 
+#define MPTCP_SCHED_NAME_MAX 16
+
+struct mptcp_sched_ops {
+	struct sock *	(*get_subflow)(struct mptcp_sock *msk);
+
+	char			name[MPTCP_SCHED_NAME_MAX];
+	struct module		*owner;
+	struct list_head	list;
+
+	void (*init)(struct mptcp_sock *msk);
+	void (*release)(struct mptcp_sock *msk);
+} ____cacheline_aligned_in_smp;
+
 #ifdef CONFIG_MPTCP
 extern struct request_sock_ops mptcp_subflow_request_sock_ops;
 
diff --git a/net/mptcp/Makefile b/net/mptcp/Makefile
index 0a0608b6b4b4..aa5c10d1b80a 100644
--- a/net/mptcp/Makefile
+++ b/net/mptcp/Makefile
@@ -3,7 +3,7 @@ obj-$(CONFIG_MPTCP) += mptcp.o
 ccflags-y += -DDEBUG
 
 mptcp-y := protocol.o subflow.o options.o token.o crypto.o ctrl.o pm.o diag.o \
-	   mib.o pm_netlink.o sockopt.o
+	   mib.o pm_netlink.o sockopt.o sched.o
 
 obj-$(CONFIG_SYN_COOKIES) += syncookies.o
 obj-$(CONFIG_INET_MPTCP_DIAG) += mptcp_diag.o
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index d3887f628b54..b1d7c8b0c112 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3807,6 +3807,7 @@ void __init mptcp_proto_init(void)
 
 	mptcp_subflow_init();
 	mptcp_pm_init();
+	mptcp_sched_init();
 	mptcp_token_init();
 
 	if (proto_register(&mptcp_prot, MPTCP_USE_SLAB) != 0)
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index fd82fd113113..3258b740c8ee 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -608,6 +608,13 @@ int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock);
 void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
 			 struct sockaddr_storage *addr,
 			 unsigned short family);
+struct mptcp_sched_ops *mptcp_sched_find(const struct net *net,
+					 const char *name);
+int mptcp_register_scheduler(const struct net *net,
+			     struct mptcp_sched_ops *sched);
+void mptcp_unregister_scheduler(const struct net *net,
+				struct mptcp_sched_ops *sched);
+void mptcp_sched_init(void);
 
 static inline bool __mptcp_subflow_active(struct mptcp_subflow_context *subflow)
 {
diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
new file mode 100644
index 000000000000..3798a5cefeb6
--- /dev/null
+++ b/net/mptcp/sched.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Multipath TCP
+ *
+ * Copyright (c) 2022, SUSE.
+ */
+
+#define pr_fmt(fmt) "MPTCP: " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/list.h>
+#include <linux/rculist.h>
+#include <linux/spinlock.h>
+#include <net/tcp.h>
+#include <net/netns/generic.h>
+#include "protocol.h"
+
+static int sched_pernet_id;
+
+struct sched_pernet {
+	/* protects pernet updates */
+	spinlock_t		lock;
+	struct list_head	sched_list;
+};
+
+static struct sched_pernet *sched_get_pernet(const struct net *net)
+{
+	return net_generic(net, sched_pernet_id);
+}
+
+struct mptcp_sched_ops *mptcp_sched_find(const struct net *net,
+					 const char *name)
+{
+	struct sched_pernet *pernet = sched_get_pernet(net);
+	struct mptcp_sched_ops *sched, *ret = NULL;
+
+	spin_lock(&pernet->lock);
+	list_for_each_entry_rcu(sched, &pernet->sched_list, list) {
+		if (!strcmp(sched->name, name)) {
+			ret = sched;
+			break;
+		}
+	}
+	spin_unlock(&pernet->lock);
+
+	return ret;
+}
+
+int mptcp_register_scheduler(const struct net *net,
+			     struct mptcp_sched_ops *sched)
+{
+	struct sched_pernet *pernet = sched_get_pernet(net);
+
+	if (!sched->get_subflow)
+		return -EINVAL;
+
+	if (mptcp_sched_find(net, sched->name))
+		return -EEXIST;
+
+	spin_lock(&pernet->lock);
+	list_add_tail_rcu(&sched->list, &pernet->sched_list);
+	spin_unlock(&pernet->lock);
+
+	pr_debug("%s registered", sched->name);
+	return 0;
+}
+
+void mptcp_unregister_scheduler(const struct net *net,
+				struct mptcp_sched_ops *sched)
+{
+	struct sched_pernet *pernet = sched_get_pernet(net);
+
+	spin_lock(&pernet->lock);
+	list_del_rcu(&sched->list);
+	spin_unlock(&pernet->lock);
+
+	synchronize_rcu();
+}
+
+static int __net_init sched_init_net(struct net *net)
+{
+	struct sched_pernet *pernet = sched_get_pernet(net);
+
+	INIT_LIST_HEAD_RCU(&pernet->sched_list);
+	spin_lock_init(&pernet->lock);
+
+	return 0;
+}
+
+static void __net_exit sched_exit_net(struct net *net)
+{
+	struct sched_pernet *pernet = sched_get_pernet(net);
+	struct mptcp_sched_ops *sched;
+
+	spin_lock(&pernet->lock);
+	list_for_each_entry_rcu(sched, &pernet->sched_list, list)
+		list_del_rcu(&sched->list);
+	spin_unlock(&pernet->lock);
+
+	synchronize_rcu();
+}
+
+static struct pernet_operations mptcp_sched_pernet_ops = {
+	.init = sched_init_net,
+	.exit = sched_exit_net,
+	.id = &sched_pernet_id,
+	.size = sizeof(struct sched_pernet),
+};
+
+void mptcp_sched_init(void)
+{
+	if (register_pernet_subsys(&mptcp_sched_pernet_ops) < 0)
+		panic("Failed to register MPTCP sched pernet subsystem.\n");
+}
-- 
2.34.1

Geliang Tang <geliang.tang@suse.com> wrote:
> This patch added struct mptcp_sched_ops. And define the scheduler
> register, unregister and find functions.

... but why are they pernet?  Makes no sense to me, so an
explanation would help.  Or, remove the pernet ops.

All callers pass &init_net, so I don't think there is any reason
for pernet data structures here.

> +struct mptcp_sched_ops *mptcp_sched_find(const struct net *net,
> +					 const char *name)
> +{
> +	struct sched_pernet *pernet = sched_get_pernet(net);
> +	struct mptcp_sched_ops *sched, *ret = NULL;
> +
> +	spin_lock(&pernet->lock);
> +	list_for_each_entry_rcu(sched, &pernet->sched_list, list) {
> +		if (!strcmp(sched->name, name)) {
> +			ret = sched;
> +			break;
> +		}
> +	}
> +	spin_unlock(&pernet->lock);
> +

Locking isn't needed here if caller holds rcu read lock.

> +int mptcp_register_scheduler(const struct net *net,
> +			     struct mptcp_sched_ops *sched)
> +{
> +	struct sched_pernet *pernet = sched_get_pernet(net);
> +
> +	if (!sched->get_subflow)
> +		return -EINVAL;
> +
> +	if (mptcp_sched_find(net, sched->name))
> +		return -EEXIST;
> +
> +	spin_lock(&pernet->lock);
> +	list_add_tail_rcu(&sched->list, &pernet->sched_list);
> +	spin_unlock(&pernet->lock);

I would use global list/lock, not pernet registration.
I don't see a reason why that is needed, its just duplicate info.

> +void mptcp_unregister_scheduler(const struct net *net,
> +				struct mptcp_sched_ops *sched)
> +{
> +	struct sched_pernet *pernet = sched_get_pernet(net);
> +
> +	spin_lock(&pernet->lock);
> +	list_del_rcu(&sched->list);
> +	spin_unlock(&pernet->lock);
> +
> +	synchronize_rcu();

Why synchronize_rcu()?  Comment please why that is needed, no freeing
happens?

> +	struct sched_pernet *pernet = sched_get_pernet(net);
> +	struct mptcp_sched_ops *sched;
> +
> +	spin_lock(&pernet->lock);
> +	list_for_each_entry_rcu(sched, &pernet->sched_list, list)
> +		list_del_rcu(&sched->list);
> +	spin_unlock(&pernet->lock);
> +
> +	synchronize_rcu();

Same, why is it needed?

On Thu, Mar 24, 2022 at 03:27:53PM +0100, Florian Westphal wrote:
> Geliang Tang <geliang.tang@suse.com> wrote:
> > This patch added struct mptcp_sched_ops. And define the scheduler
> > register, unregister and find functions.
> 
> ... but why are they pernet?  Makes no sense to me, so an
> explanation would help.  Or, remove the pernet ops.
> 
> All callers pass &init_net, so I don't think there is any reason
> for pernet data structures here.

v4 used global list instead of pernet list:

https://patchwork.kernel.org/project/mptcp/patch/02c1d2d2eee134713737fe5c5f73127e8c741589.1647942374.git.geliang.tang@suse.com/

Should I go back to v4?

> 
> > +struct mptcp_sched_ops *mptcp_sched_find(const struct net *net,
> > +					 const char *name)
> > +{
> > +	struct sched_pernet *pernet = sched_get_pernet(net);
> > +	struct mptcp_sched_ops *sched, *ret = NULL;
> > +
> > +	spin_lock(&pernet->lock);
> > +	list_for_each_entry_rcu(sched, &pernet->sched_list, list) {
> > +		if (!strcmp(sched->name, name)) {
> > +			ret = sched;
> > +			break;
> > +		}
> > +	}
> > +	spin_unlock(&pernet->lock);
> > +
> 
> Locking isn't needed here if caller holds rcu read lock.
> 
> > +int mptcp_register_scheduler(const struct net *net,
> > +			     struct mptcp_sched_ops *sched)
> > +{
> > +	struct sched_pernet *pernet = sched_get_pernet(net);
> > +
> > +	if (!sched->get_subflow)
> > +		return -EINVAL;
> > +
> > +	if (mptcp_sched_find(net, sched->name))
> > +		return -EEXIST;
> > +
> > +	spin_lock(&pernet->lock);
> > +	list_add_tail_rcu(&sched->list, &pernet->sched_list);
> > +	spin_unlock(&pernet->lock);
> 
> I would use global list/lock, not pernet registration.
> I don't see a reason why that is needed, its just duplicate info.
> 
> > +void mptcp_unregister_scheduler(const struct net *net,
> > +				struct mptcp_sched_ops *sched)
> > +{
> > +	struct sched_pernet *pernet = sched_get_pernet(net);
> > +
> > +	spin_lock(&pernet->lock);
> > +	list_del_rcu(&sched->list);
> > +	spin_unlock(&pernet->lock);
> > +
> > +	synchronize_rcu();
> 
> Why synchronize_rcu()?  Comment please why that is needed, no freeing
> happens?
> 
> > +	struct sched_pernet *pernet = sched_get_pernet(net);
> > +	struct mptcp_sched_ops *sched;
> > +
> > +	spin_lock(&pernet->lock);
> > +	list_for_each_entry_rcu(sched, &pernet->sched_list, list)
> > +		list_del_rcu(&sched->list);
> > +	spin_unlock(&pernet->lock);
> > +
> > +	synchronize_rcu();
> 
> Same, why is it needed?
> 

Geliang Tang <geliang.tang@suse.com> wrote:
> On Thu, Mar 24, 2022 at 03:27:53PM +0100, Florian Westphal wrote:
> > Geliang Tang <geliang.tang@suse.com> wrote:
> > > This patch added struct mptcp_sched_ops. And define the scheduler
> > > register, unregister and find functions.
> > 
> > ... but why are they pernet?  Makes no sense to me, so an
> > explanation would help.  Or, remove the pernet ops.
> > 
> > All callers pass &init_net, so I don't think there is any reason
> > for pernet data structures here.
> 
> v4 used global list instead of pernet list:
> 
> https://patchwork.kernel.org/project/mptcp/patch/02c1d2d2eee134713737fe5c5f73127e8c741589.1647942374.git.geliang.tang@suse.com/
> 
> Should I go back to v4?

Mhh.  Mat, why the pernet thing? AFAICS all net namespaces would contain the
same (albeit we'd have to kmemdup first to get unique list_heads....) info,
so I don't understand why thats a good idea?

A single/global lock is fine if its not taken in the normal (per packet) case.

On Thu, Mar 24, 2022 at 04:03:27PM +0100, Florian Westphal wrote:
> Geliang Tang <geliang.tang@suse.com> wrote:
> > On Thu, Mar 24, 2022 at 03:27:53PM +0100, Florian Westphal wrote:
> > > Geliang Tang <geliang.tang@suse.com> wrote:
> > > > This patch added struct mptcp_sched_ops. And define the scheduler
> > > > register, unregister and find functions.
> > > 
> > > ... but why are they pernet?  Makes no sense to me, so an
> > > explanation would help.  Or, remove the pernet ops.
> > > 
> > > All callers pass &init_net, so I don't think there is any reason
> > > for pernet data structures here.
> > 
> > v4 used global list instead of pernet list:
> > 
> > https://patchwork.kernel.org/project/mptcp/patch/02c1d2d2eee134713737fe5c5f73127e8c741589.1647942374.git.geliang.tang@suse.com/

Florian, should I remove synchronize_rcu() in v4 too?

> > 
> > Should I go back to v4?
> 
> Mhh.  Mat, why the pernet thing? AFAICS all net namespaces would contain the
> same (albeit we'd have to kmemdup first to get unique list_heads....) info,
> so I don't understand why thats a good idea?
> 
> A single/global lock is fine if its not taken in the normal (per packet) case.
> 

Geliang Tang <geliang.tang@suse.com> wrote:
> On Thu, Mar 24, 2022 at 04:03:27PM +0100, Florian Westphal wrote:
> > Geliang Tang <geliang.tang@suse.com> wrote:
> > > On Thu, Mar 24, 2022 at 03:27:53PM +0100, Florian Westphal wrote:
> > > > Geliang Tang <geliang.tang@suse.com> wrote:
> > > > > This patch added struct mptcp_sched_ops. And define the scheduler
> > > > > register, unregister and find functions.
> > > > 
> > > > ... but why are they pernet?  Makes no sense to me, so an
> > > > explanation would help.  Or, remove the pernet ops.
> > > > 
> > > > All callers pass &init_net, so I don't think there is any reason
> > > > for pernet data structures here.
> > > 
> > > v4 used global list instead of pernet list:
> > > 
> > > https://patchwork.kernel.org/project/mptcp/patch/02c1d2d2eee134713737fe5c5f73127e8c741589.1647942374.git.geliang.tang@suse.com/
> 
> Florian, should I remove synchronize_rcu() in v4 too?

I think so.  Else you need to add a comment as to why its needed.

I suspect full 'rmmod mptcp_sched_example' sequence would be:

1. mptcp_unregister_scheduler(&example_sched);
2. walk all active mptcp sockets to re-set their scheduler
in case something still uses 'example_sched'
3. complete the rmmod

In that case, we might need a synchronize_rcu after 2), but
not 1.

On Thu, 24 Mar 2022, Geliang Tang wrote:

> This patch added struct mptcp_sched_ops. And define the scheduler
> register, unregister and find functions.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> include/net/mptcp.h  |  13 +++++
> net/mptcp/Makefile   |   2 +-
> net/mptcp/protocol.c |   1 +
> net/mptcp/protocol.h |   7 +++
> net/mptcp/sched.c    | 114 +++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 136 insertions(+), 1 deletion(-)
> create mode 100644 net/mptcp/sched.c
>
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index 8b1afd6f5cc4..e3a0baa8dbd7 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -95,6 +95,19 @@ struct mptcp_out_options {
> #endif
> };
>
> +#define MPTCP_SCHED_NAME_MAX 16
> +
> +struct mptcp_sched_ops {
> +	struct sock *	(*get_subflow)(struct mptcp_sock *msk);
> +
> +	char			name[MPTCP_SCHED_NAME_MAX];
> +	struct module		*owner;
> +	struct list_head	list;
> +
> +	void (*init)(struct mptcp_sock *msk);
> +	void (*release)(struct mptcp_sock *msk);
> +} ____cacheline_aligned_in_smp;
> +
> #ifdef CONFIG_MPTCP
> extern struct request_sock_ops mptcp_subflow_request_sock_ops;
>
> diff --git a/net/mptcp/Makefile b/net/mptcp/Makefile
> index 0a0608b6b4b4..aa5c10d1b80a 100644
> --- a/net/mptcp/Makefile
> +++ b/net/mptcp/Makefile
> @@ -3,7 +3,7 @@ obj-$(CONFIG_MPTCP) += mptcp.o
> ccflags-y += -DDEBUG

This line of context keeps making 'git am' fail - looks like you have a 
debug-enabling commit in your private tree?

This is probably why patchew is having trouble with this series too.

- Mat

>
> mptcp-y := protocol.o subflow.o options.o token.o crypto.o ctrl.o pm.o diag.o \
> -	   mib.o pm_netlink.o sockopt.o
> +	   mib.o pm_netlink.o sockopt.o sched.o
>
> obj-$(CONFIG_SYN_COOKIES) += syncookies.o
> obj-$(CONFIG_INET_MPTCP_DIAG) += mptcp_diag.o
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index d3887f628b54..b1d7c8b0c112 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -3807,6 +3807,7 @@ void __init mptcp_proto_init(void)
>
> 	mptcp_subflow_init();
> 	mptcp_pm_init();
> +	mptcp_sched_init();
> 	mptcp_token_init();
>
> 	if (proto_register(&mptcp_prot, MPTCP_USE_SLAB) != 0)
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index fd82fd113113..3258b740c8ee 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -608,6 +608,13 @@ int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock);
> void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
> 			 struct sockaddr_storage *addr,
> 			 unsigned short family);
> +struct mptcp_sched_ops *mptcp_sched_find(const struct net *net,
> +					 const char *name);
> +int mptcp_register_scheduler(const struct net *net,
> +			     struct mptcp_sched_ops *sched);
> +void mptcp_unregister_scheduler(const struct net *net,
> +				struct mptcp_sched_ops *sched);
> +void mptcp_sched_init(void);
>
> static inline bool __mptcp_subflow_active(struct mptcp_subflow_context *subflow)
> {
> diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
> new file mode 100644
> index 000000000000..3798a5cefeb6
> --- /dev/null
> +++ b/net/mptcp/sched.c
> @@ -0,0 +1,114 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Multipath TCP
> + *
> + * Copyright (c) 2022, SUSE.
> + */
> +
> +#define pr_fmt(fmt) "MPTCP: " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/list.h>
> +#include <linux/rculist.h>
> +#include <linux/spinlock.h>
> +#include <net/tcp.h>
> +#include <net/netns/generic.h>
> +#include "protocol.h"
> +
> +static int sched_pernet_id;
> +
> +struct sched_pernet {
> +	/* protects pernet updates */
> +	spinlock_t		lock;
> +	struct list_head	sched_list;
> +};
> +
> +static struct sched_pernet *sched_get_pernet(const struct net *net)
> +{
> +	return net_generic(net, sched_pernet_id);
> +}
> +
> +struct mptcp_sched_ops *mptcp_sched_find(const struct net *net,
> +					 const char *name)
> +{
> +	struct sched_pernet *pernet = sched_get_pernet(net);
> +	struct mptcp_sched_ops *sched, *ret = NULL;
> +
> +	spin_lock(&pernet->lock);
> +	list_for_each_entry_rcu(sched, &pernet->sched_list, list) {
> +		if (!strcmp(sched->name, name)) {
> +			ret = sched;
> +			break;
> +		}
> +	}
> +	spin_unlock(&pernet->lock);
> +
> +	return ret;
> +}
> +
> +int mptcp_register_scheduler(const struct net *net,
> +			     struct mptcp_sched_ops *sched)
> +{
> +	struct sched_pernet *pernet = sched_get_pernet(net);
> +
> +	if (!sched->get_subflow)
> +		return -EINVAL;
> +
> +	if (mptcp_sched_find(net, sched->name))
> +		return -EEXIST;
> +
> +	spin_lock(&pernet->lock);
> +	list_add_tail_rcu(&sched->list, &pernet->sched_list);
> +	spin_unlock(&pernet->lock);
> +
> +	pr_debug("%s registered", sched->name);
> +	return 0;
> +}
> +
> +void mptcp_unregister_scheduler(const struct net *net,
> +				struct mptcp_sched_ops *sched)
> +{
> +	struct sched_pernet *pernet = sched_get_pernet(net);
> +
> +	spin_lock(&pernet->lock);
> +	list_del_rcu(&sched->list);
> +	spin_unlock(&pernet->lock);
> +
> +	synchronize_rcu();
> +}
> +
> +static int __net_init sched_init_net(struct net *net)
> +{
> +	struct sched_pernet *pernet = sched_get_pernet(net);
> +
> +	INIT_LIST_HEAD_RCU(&pernet->sched_list);
> +	spin_lock_init(&pernet->lock);
> +
> +	return 0;
> +}
> +
> +static void __net_exit sched_exit_net(struct net *net)
> +{
> +	struct sched_pernet *pernet = sched_get_pernet(net);
> +	struct mptcp_sched_ops *sched;
> +
> +	spin_lock(&pernet->lock);
> +	list_for_each_entry_rcu(sched, &pernet->sched_list, list)
> +		list_del_rcu(&sched->list);
> +	spin_unlock(&pernet->lock);
> +
> +	synchronize_rcu();
> +}
> +
> +static struct pernet_operations mptcp_sched_pernet_ops = {
> +	.init = sched_init_net,
> +	.exit = sched_exit_net,
> +	.id = &sched_pernet_id,
> +	.size = sizeof(struct sched_pernet),
> +};
> +
> +void mptcp_sched_init(void)
> +{
> +	if (register_pernet_subsys(&mptcp_sched_pernet_ops) < 0)
> +		panic("Failed to register MPTCP sched pernet subsystem.\n");
> +}
> -- 
> 2.34.1
>
>
>

--
Mat Martineau
Intel

On Thu, 24 Mar 2022, Mat Martineau wrote:

> On Thu, 24 Mar 2022, Geliang Tang wrote:
>
>> This patch added struct mptcp_sched_ops. And define the scheduler
>> register, unregister and find functions.
>> 
>> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
>> ---
>> include/net/mptcp.h  |  13 +++++
>> net/mptcp/Makefile   |   2 +-
>> net/mptcp/protocol.c |   1 +
>> net/mptcp/protocol.h |   7 +++
>> net/mptcp/sched.c    | 114 +++++++++++++++++++++++++++++++++++++++++++
>> 5 files changed, 136 insertions(+), 1 deletion(-)
>> create mode 100644 net/mptcp/sched.c
>> 
>> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
>> index 8b1afd6f5cc4..e3a0baa8dbd7 100644
>> --- a/include/net/mptcp.h
>> +++ b/include/net/mptcp.h
>> @@ -95,6 +95,19 @@ struct mptcp_out_options {
>> #endif
>> };
>> 
>> +#define MPTCP_SCHED_NAME_MAX 16
>> +
>> +struct mptcp_sched_ops {
>> +	struct sock *	(*get_subflow)(struct mptcp_sock *msk);
>> +
>> +	char			name[MPTCP_SCHED_NAME_MAX];
>> +	struct module		*owner;
>> +	struct list_head	list;
>> +
>> +	void (*init)(struct mptcp_sock *msk);
>> +	void (*release)(struct mptcp_sock *msk);
>> +} ____cacheline_aligned_in_smp;
>> +
>> #ifdef CONFIG_MPTCP
>> extern struct request_sock_ops mptcp_subflow_request_sock_ops;
>> 
>> diff --git a/net/mptcp/Makefile b/net/mptcp/Makefile
>> index 0a0608b6b4b4..aa5c10d1b80a 100644
>> --- a/net/mptcp/Makefile
>> +++ b/net/mptcp/Makefile
>> @@ -3,7 +3,7 @@ obj-$(CONFIG_MPTCP) += mptcp.o
>> ccflags-y += -DDEBUG
>
> This line of context keeps making 'git am' fail - looks like you have a 
> debug-enabling commit in your private tree?
>
> This is probably why patchew is having trouble with this series too.
>
> - Mat
>
>> 
>> mptcp-y := protocol.o subflow.o options.o token.o crypto.o ctrl.o pm.o 
>> diag.o \
>> -	   mib.o pm_netlink.o sockopt.o
>> +	   mib.o pm_netlink.o sockopt.o sched.o
>> 
>> obj-$(CONFIG_SYN_COOKIES) += syncookies.o
>> obj-$(CONFIG_INET_MPTCP_DIAG) += mptcp_diag.o
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index d3887f628b54..b1d7c8b0c112 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -3807,6 +3807,7 @@ void __init mptcp_proto_init(void)
>>
>> 	mptcp_subflow_init();
>> 	mptcp_pm_init();
>> +	mptcp_sched_init();
>> 	mptcp_token_init();
>>
>> 	if (proto_register(&mptcp_prot, MPTCP_USE_SLAB) != 0)
>> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
>> index fd82fd113113..3258b740c8ee 100644
>> --- a/net/mptcp/protocol.h
>> +++ b/net/mptcp/protocol.h
>> @@ -608,6 +608,13 @@ int mptcp_subflow_create_socket(struct sock *sk, 
>> struct socket **new_sock);
>> void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
>> 			 struct sockaddr_storage *addr,
>> 			 unsigned short family);
>> +struct mptcp_sched_ops *mptcp_sched_find(const struct net *net,
>> +					 const char *name);
>> +int mptcp_register_scheduler(const struct net *net,
>> +			     struct mptcp_sched_ops *sched);
>> +void mptcp_unregister_scheduler(const struct net *net,
>> +				struct mptcp_sched_ops *sched);
>> +void mptcp_sched_init(void);
>> 
>> static inline bool __mptcp_subflow_active(struct mptcp_subflow_context 
>> *subflow)
>> {
>> diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
>> new file mode 100644
>> index 000000000000..3798a5cefeb6
>> --- /dev/null
>> +++ b/net/mptcp/sched.c
>> @@ -0,0 +1,114 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Multipath TCP
>> + *
>> + * Copyright (c) 2022, SUSE.
>> + */
>> +
>> +#define pr_fmt(fmt) "MPTCP: " fmt
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/list.h>
>> +#include <linux/rculist.h>
>> +#include <linux/spinlock.h>
>> +#include <net/tcp.h>
>> +#include <net/netns/generic.h>
>> +#include "protocol.h"
>> +
>> +static int sched_pernet_id;
>> +
>> +struct sched_pernet {
>> +	/* protects pernet updates */
>> +	spinlock_t		lock;
>> +	struct list_head	sched_list;
>> +};
>> +
>> +static struct sched_pernet *sched_get_pernet(const struct net *net)
>> +{
>> +	return net_generic(net, sched_pernet_id);
>> +}
>> +
>> +struct mptcp_sched_ops *mptcp_sched_find(const struct net *net,
>> +					 const char *name)
>> +{
>> +	struct sched_pernet *pernet = sched_get_pernet(net);
>> +	struct mptcp_sched_ops *sched, *ret = NULL;
>> +
>> +	spin_lock(&pernet->lock);
>> +	list_for_each_entry_rcu(sched, &pernet->sched_list, list) {
>> +		if (!strcmp(sched->name, name)) {
>> +			ret = sched;
>> +			break;
>> +		}
>> +	}
>> +	spin_unlock(&pernet->lock);
>> +
>> +	return ret;
>> +}
>> +
>> +int mptcp_register_scheduler(const struct net *net,
>> +			     struct mptcp_sched_ops *sched)
>> +{
>> +	struct sched_pernet *pernet = sched_get_pernet(net);
>> +
>> +	if (!sched->get_subflow)
>> +		return -EINVAL;

Should also validate sched->name, don't want to allow a NULL pointer 
there.

>> +
>> +	if (mptcp_sched_find(net, sched->name))
>> +		return -EEXIST;
>> +
>> +	spin_lock(&pernet->lock);
>> +	list_add_tail_rcu(&sched->list, &pernet->sched_list);
>> +	spin_unlock(&pernet->lock);
>> +
>> +	pr_debug("%s registered", sched->name);
>> +	return 0;
>> +}
>> +
>> +void mptcp_unregister_scheduler(const struct net *net,
>> +				struct mptcp_sched_ops *sched)
>> +{
>> +	struct sched_pernet *pernet = sched_get_pernet(net);
>> +
>> +	spin_lock(&pernet->lock);
>> +	list_del_rcu(&sched->list);
>> +	spin_unlock(&pernet->lock);
>> +
>> +	synchronize_rcu();
>> +}
>> +
>> +static int __net_init sched_init_net(struct net *net)
>> +{
>> +	struct sched_pernet *pernet = sched_get_pernet(net);
>> +
>> +	INIT_LIST_HEAD_RCU(&pernet->sched_list);
>> +	spin_lock_init(&pernet->lock);
>> +
>> +	return 0;
>> +}
>> +
>> +static void __net_exit sched_exit_net(struct net *net)
>> +{
>> +	struct sched_pernet *pernet = sched_get_pernet(net);
>> +	struct mptcp_sched_ops *sched;
>> +
>> +	spin_lock(&pernet->lock);
>> +	list_for_each_entry_rcu(sched, &pernet->sched_list, list)
>> +		list_del_rcu(&sched->list);
>> +	spin_unlock(&pernet->lock);
>> +
>> +	synchronize_rcu();
>> +}
>> +
>> +static struct pernet_operations mptcp_sched_pernet_ops = {
>> +	.init = sched_init_net,
>> +	.exit = sched_exit_net,
>> +	.id = &sched_pernet_id,
>> +	.size = sizeof(struct sched_pernet),
>> +};
>> +
>> +void mptcp_sched_init(void)
>> +{
>> +	if (register_pernet_subsys(&mptcp_sched_pernet_ops) < 0)
>> +		panic("Failed to register MPTCP sched pernet subsystem.\n");
>> +}

--
Mat Martineau
Intel

On Thu, 24 Mar 2022, Mat Martineau wrote:

> On Thu, 24 Mar 2022, Mat Martineau wrote:
>
>> On Thu, 24 Mar 2022, Geliang Tang wrote:
>> 
>>> This patch added struct mptcp_sched_ops. And define the scheduler
>>> register, unregister and find functions.
>>> 
>>> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
>>> ---
>>> include/net/mptcp.h  |  13 +++++
>>> net/mptcp/Makefile   |   2 +-
>>> net/mptcp/protocol.c |   1 +
>>> net/mptcp/protocol.h |   7 +++
>>> net/mptcp/sched.c    | 114 +++++++++++++++++++++++++++++++++++++++++++
>>> 5 files changed, 136 insertions(+), 1 deletion(-)
>>> create mode 100644 net/mptcp/sched.c
>>> 
>>> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
>>> index 8b1afd6f5cc4..e3a0baa8dbd7 100644
>>> --- a/include/net/mptcp.h
>>> +++ b/include/net/mptcp.h
>>> @@ -95,6 +95,19 @@ struct mptcp_out_options {
>>> #endif
>>> };
>>> 
>>> +#define MPTCP_SCHED_NAME_MAX 16
>>> +
>>> +struct mptcp_sched_ops {
>>> +	struct sock *	(*get_subflow)(struct mptcp_sock *msk);
>>> +
>>> +	char			name[MPTCP_SCHED_NAME_MAX];
>>> +	struct module		*owner;
>>> +	struct list_head	list;
>>> +
>>> +	void (*init)(struct mptcp_sock *msk);
>>> +	void (*release)(struct mptcp_sock *msk);
>>> +} ____cacheline_aligned_in_smp;
>>> +
>>> #ifdef CONFIG_MPTCP
>>> extern struct request_sock_ops mptcp_subflow_request_sock_ops;
>>> 
>>> diff --git a/net/mptcp/Makefile b/net/mptcp/Makefile
>>> index 0a0608b6b4b4..aa5c10d1b80a 100644
>>> --- a/net/mptcp/Makefile
>>> +++ b/net/mptcp/Makefile
>>> @@ -3,7 +3,7 @@ obj-$(CONFIG_MPTCP) += mptcp.o
>>> ccflags-y += -DDEBUG
>> 
>> This line of context keeps making 'git am' fail - looks like you have a 
>> debug-enabling commit in your private tree?
>> 
>> This is probably why patchew is having trouble with this series too.
>> 
>> - Mat
>> 
>>> 
>>> mptcp-y := protocol.o subflow.o options.o token.o crypto.o ctrl.o pm.o 
>>> diag.o \
>>> -	   mib.o pm_netlink.o sockopt.o
>>> +	   mib.o pm_netlink.o sockopt.o sched.o
>>> 
>>> obj-$(CONFIG_SYN_COOKIES) += syncookies.o
>>> obj-$(CONFIG_INET_MPTCP_DIAG) += mptcp_diag.o
>>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>>> index d3887f628b54..b1d7c8b0c112 100644
>>> --- a/net/mptcp/protocol.c
>>> +++ b/net/mptcp/protocol.c
>>> @@ -3807,6 +3807,7 @@ void __init mptcp_proto_init(void)
>>>
>>> 	mptcp_subflow_init();
>>> 	mptcp_pm_init();
>>> +	mptcp_sched_init();
>>> 	mptcp_token_init();
>>>
>>> 	if (proto_register(&mptcp_prot, MPTCP_USE_SLAB) != 0)
>>> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
>>> index fd82fd113113..3258b740c8ee 100644
>>> --- a/net/mptcp/protocol.h
>>> +++ b/net/mptcp/protocol.h
>>> @@ -608,6 +608,13 @@ int mptcp_subflow_create_socket(struct sock *sk, 
>>> struct socket **new_sock);
>>> void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
>>> 			 struct sockaddr_storage *addr,
>>> 			 unsigned short family);
>>> +struct mptcp_sched_ops *mptcp_sched_find(const struct net *net,
>>> +					 const char *name);
>>> +int mptcp_register_scheduler(const struct net *net,
>>> +			     struct mptcp_sched_ops *sched);
>>> +void mptcp_unregister_scheduler(const struct net *net,
>>> +				struct mptcp_sched_ops *sched);
>>> +void mptcp_sched_init(void);
>>> 
>>> static inline bool __mptcp_subflow_active(struct mptcp_subflow_context 
>>> *subflow)
>>> {
>>> diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
>>> new file mode 100644
>>> index 000000000000..3798a5cefeb6
>>> --- /dev/null
>>> +++ b/net/mptcp/sched.c
>>> @@ -0,0 +1,114 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/* Multipath TCP
>>> + *
>>> + * Copyright (c) 2022, SUSE.
>>> + */
>>> +
>>> +#define pr_fmt(fmt) "MPTCP: " fmt
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/list.h>
>>> +#include <linux/rculist.h>
>>> +#include <linux/spinlock.h>
>>> +#include <net/tcp.h>
>>> +#include <net/netns/generic.h>
>>> +#include "protocol.h"
>>> +
>>> +static int sched_pernet_id;
>>> +
>>> +struct sched_pernet {
>>> +	/* protects pernet updates */
>>> +	spinlock_t		lock;
>>> +	struct list_head	sched_list;
>>> +};
>>> +
>>> +static struct sched_pernet *sched_get_pernet(const struct net *net)
>>> +{
>>> +	return net_generic(net, sched_pernet_id);
>>> +}
>>> +
>>> +struct mptcp_sched_ops *mptcp_sched_find(const struct net *net,
>>> +					 const char *name)
>>> +{
>>> +	struct sched_pernet *pernet = sched_get_pernet(net);
>>> +	struct mptcp_sched_ops *sched, *ret = NULL;
>>> +
>>> +	spin_lock(&pernet->lock);
>>> +	list_for_each_entry_rcu(sched, &pernet->sched_list, list) {
>>> +		if (!strcmp(sched->name, name)) {
>>> +			ret = sched;
>>> +			break;
>>> +		}
>>> +	}
>>> +	spin_unlock(&pernet->lock);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +int mptcp_register_scheduler(const struct net *net,
>>> +			     struct mptcp_sched_ops *sched)
>>> +{
>>> +	struct sched_pernet *pernet = sched_get_pernet(net);
>>> +
>>> +	if (!sched->get_subflow)
>>> +		return -EINVAL;
>
> Should also validate sched->name, don't want to allow a NULL pointer there.

Oops, never mind - name is an array, not a pointer!

>>> +
>>> +	if (mptcp_sched_find(net, sched->name))
>>> +		return -EEXIST;
>>> +
>>> +	spin_lock(&pernet->lock);
>>> +	list_add_tail_rcu(&sched->list, &pernet->sched_list);
>>> +	spin_unlock(&pernet->lock);
>>> +
>>> +	pr_debug("%s registered", sched->name);
>>> +	return 0;
>>> +}
>>> +
>>> +void mptcp_unregister_scheduler(const struct net *net,
>>> +				struct mptcp_sched_ops *sched)
>>> +{
>>> +	struct sched_pernet *pernet = sched_get_pernet(net);
>>> +
>>> +	spin_lock(&pernet->lock);
>>> +	list_del_rcu(&sched->list);
>>> +	spin_unlock(&pernet->lock);
>>> +
>>> +	synchronize_rcu();
>>> +}
>>> +
>>> +static int __net_init sched_init_net(struct net *net)
>>> +{
>>> +	struct sched_pernet *pernet = sched_get_pernet(net);
>>> +
>>> +	INIT_LIST_HEAD_RCU(&pernet->sched_list);
>>> +	spin_lock_init(&pernet->lock);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void __net_exit sched_exit_net(struct net *net)
>>> +{
>>> +	struct sched_pernet *pernet = sched_get_pernet(net);
>>> +	struct mptcp_sched_ops *sched;
>>> +
>>> +	spin_lock(&pernet->lock);
>>> +	list_for_each_entry_rcu(sched, &pernet->sched_list, list)
>>> +		list_del_rcu(&sched->list);
>>> +	spin_unlock(&pernet->lock);
>>> +
>>> +	synchronize_rcu();
>>> +}
>>> +
>>> +static struct pernet_operations mptcp_sched_pernet_ops = {
>>> +	.init = sched_init_net,
>>> +	.exit = sched_exit_net,
>>> +	.id = &sched_pernet_id,
>>> +	.size = sizeof(struct sched_pernet),
>>> +};
>>> +
>>> +void mptcp_sched_init(void)
>>> +{
>>> +	if (register_pernet_subsys(&mptcp_sched_pernet_ops) < 0)
>>> +		panic("Failed to register MPTCP sched pernet subsystem.\n");
>>> +}
>
> --
> Mat Martineau
> Intel
>
>

--
Mat Martineau
Intel

Sorry Mat, I gave you the wrong feedback at yesterday's weekly meeting.
BPF does work in different namespaces, see kernel/bpf/net_namespace.c
and include/linux/bpf-netns.h. I think we should continue to use the
pernet sched_list. v6 just sent out.

Thanks,
-Geliang

On Thu, Mar 24, 2022 at 04:36:51PM -0700, Mat Martineau wrote:
> On Thu, 24 Mar 2022, Mat Martineau wrote:
> 
> > On Thu, 24 Mar 2022, Mat Martineau wrote:
> > 
> > > On Thu, 24 Mar 2022, Geliang Tang wrote:
> > > 
> > > > This patch added struct mptcp_sched_ops. And define the scheduler
> > > > register, unregister and find functions.
> > > > 
> > > > Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> > > > ---
> > > > include/net/mptcp.h  |  13 +++++
> > > > net/mptcp/Makefile   |   2 +-
> > > > net/mptcp/protocol.c |   1 +
> > > > net/mptcp/protocol.h |   7 +++
> > > > net/mptcp/sched.c    | 114 +++++++++++++++++++++++++++++++++++++++++++
> > > > 5 files changed, 136 insertions(+), 1 deletion(-)
> > > > create mode 100644 net/mptcp/sched.c
> > > > 
> > > > diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> > > > index 8b1afd6f5cc4..e3a0baa8dbd7 100644
> > > > --- a/include/net/mptcp.h
> > > > +++ b/include/net/mptcp.h
> > > > @@ -95,6 +95,19 @@ struct mptcp_out_options {
> > > > #endif
> > > > };
> > > > 
> > > > +#define MPTCP_SCHED_NAME_MAX 16
> > > > +
> > > > +struct mptcp_sched_ops {
> > > > +	struct sock *	(*get_subflow)(struct mptcp_sock *msk);
> > > > +
> > > > +	char			name[MPTCP_SCHED_NAME_MAX];
> > > > +	struct module		*owner;
> > > > +	struct list_head	list;
> > > > +
> > > > +	void (*init)(struct mptcp_sock *msk);
> > > > +	void (*release)(struct mptcp_sock *msk);
> > > > +} ____cacheline_aligned_in_smp;
> > > > +
> > > > #ifdef CONFIG_MPTCP
> > > > extern struct request_sock_ops mptcp_subflow_request_sock_ops;
> > > > 
> > > > diff --git a/net/mptcp/Makefile b/net/mptcp/Makefile
> > > > index 0a0608b6b4b4..aa5c10d1b80a 100644
> > > > --- a/net/mptcp/Makefile
> > > > +++ b/net/mptcp/Makefile
> > > > @@ -3,7 +3,7 @@ obj-$(CONFIG_MPTCP) += mptcp.o
> > > > ccflags-y += -DDEBUG
> > > 
> > > This line of context keeps making 'git am' fail - looks like you
> > > have a debug-enabling commit in your private tree?
> > > 
> > > This is probably why patchew is having trouble with this series too.
> > > 
> > > - Mat
> > > 
> > > > 
> > > > mptcp-y := protocol.o subflow.o options.o token.o crypto.o
> > > > ctrl.o pm.o diag.o \
> > > > -	   mib.o pm_netlink.o sockopt.o
> > > > +	   mib.o pm_netlink.o sockopt.o sched.o
> > > > 
> > > > obj-$(CONFIG_SYN_COOKIES) += syncookies.o
> > > > obj-$(CONFIG_INET_MPTCP_DIAG) += mptcp_diag.o
> > > > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > > > index d3887f628b54..b1d7c8b0c112 100644
> > > > --- a/net/mptcp/protocol.c
> > > > +++ b/net/mptcp/protocol.c
> > > > @@ -3807,6 +3807,7 @@ void __init mptcp_proto_init(void)
> > > > 
> > > > 	mptcp_subflow_init();
> > > > 	mptcp_pm_init();
> > > > +	mptcp_sched_init();
> > > > 	mptcp_token_init();
> > > > 
> > > > 	if (proto_register(&mptcp_prot, MPTCP_USE_SLAB) != 0)
> > > > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > > > index fd82fd113113..3258b740c8ee 100644
> > > > --- a/net/mptcp/protocol.h
> > > > +++ b/net/mptcp/protocol.h
> > > > @@ -608,6 +608,13 @@ int mptcp_subflow_create_socket(struct sock
> > > > *sk, struct socket **new_sock);
> > > > void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
> > > > 			 struct sockaddr_storage *addr,
> > > > 			 unsigned short family);
> > > > +struct mptcp_sched_ops *mptcp_sched_find(const struct net *net,
> > > > +					 const char *name);
> > > > +int mptcp_register_scheduler(const struct net *net,
> > > > +			     struct mptcp_sched_ops *sched);
> > > > +void mptcp_unregister_scheduler(const struct net *net,
> > > > +				struct mptcp_sched_ops *sched);
> > > > +void mptcp_sched_init(void);
> > > > 
> > > > static inline bool __mptcp_subflow_active(struct
> > > > mptcp_subflow_context *subflow)
> > > > {
> > > > diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
> > > > new file mode 100644
> > > > index 000000000000..3798a5cefeb6
> > > > --- /dev/null
> > > > +++ b/net/mptcp/sched.c
> > > > @@ -0,0 +1,114 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/* Multipath TCP
> > > > + *
> > > > + * Copyright (c) 2022, SUSE.
> > > > + */
> > > > +
> > > > +#define pr_fmt(fmt) "MPTCP: " fmt
> > > > +
> > > > +#include <linux/kernel.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/list.h>
> > > > +#include <linux/rculist.h>
> > > > +#include <linux/spinlock.h>
> > > > +#include <net/tcp.h>
> > > > +#include <net/netns/generic.h>
> > > > +#include "protocol.h"
> > > > +
> > > > +static int sched_pernet_id;
> > > > +
> > > > +struct sched_pernet {
> > > > +	/* protects pernet updates */
> > > > +	spinlock_t		lock;
> > > > +	struct list_head	sched_list;
> > > > +};
> > > > +
> > > > +static struct sched_pernet *sched_get_pernet(const struct net *net)
> > > > +{
> > > > +	return net_generic(net, sched_pernet_id);
> > > > +}
> > > > +
> > > > +struct mptcp_sched_ops *mptcp_sched_find(const struct net *net,
> > > > +					 const char *name)
> > > > +{
> > > > +	struct sched_pernet *pernet = sched_get_pernet(net);
> > > > +	struct mptcp_sched_ops *sched, *ret = NULL;
> > > > +
> > > > +	spin_lock(&pernet->lock);
> > > > +	list_for_each_entry_rcu(sched, &pernet->sched_list, list) {
> > > > +		if (!strcmp(sched->name, name)) {
> > > > +			ret = sched;
> > > > +			break;
> > > > +		}
> > > > +	}
> > > > +	spin_unlock(&pernet->lock);
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +int mptcp_register_scheduler(const struct net *net,
> > > > +			     struct mptcp_sched_ops *sched)
> > > > +{
> > > > +	struct sched_pernet *pernet = sched_get_pernet(net);
> > > > +
> > > > +	if (!sched->get_subflow)
> > > > +		return -EINVAL;
> > 
> > Should also validate sched->name, don't want to allow a NULL pointer there.
> 
> Oops, never mind - name is an array, not a pointer!
> 
> > > > +
> > > > +	if (mptcp_sched_find(net, sched->name))
> > > > +		return -EEXIST;
> > > > +
> > > > +	spin_lock(&pernet->lock);
> > > > +	list_add_tail_rcu(&sched->list, &pernet->sched_list);
> > > > +	spin_unlock(&pernet->lock);
> > > > +
> > > > +	pr_debug("%s registered", sched->name);
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +void mptcp_unregister_scheduler(const struct net *net,
> > > > +				struct mptcp_sched_ops *sched)
> > > > +{
> > > > +	struct sched_pernet *pernet = sched_get_pernet(net);
> > > > +
> > > > +	spin_lock(&pernet->lock);
> > > > +	list_del_rcu(&sched->list);
> > > > +	spin_unlock(&pernet->lock);
> > > > +
> > > > +	synchronize_rcu();
> > > > +}
> > > > +
> > > > +static int __net_init sched_init_net(struct net *net)
> > > > +{
> > > > +	struct sched_pernet *pernet = sched_get_pernet(net);
> > > > +
> > > > +	INIT_LIST_HEAD_RCU(&pernet->sched_list);
> > > > +	spin_lock_init(&pernet->lock);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static void __net_exit sched_exit_net(struct net *net)
> > > > +{
> > > > +	struct sched_pernet *pernet = sched_get_pernet(net);
> > > > +	struct mptcp_sched_ops *sched;
> > > > +
> > > > +	spin_lock(&pernet->lock);
> > > > +	list_for_each_entry_rcu(sched, &pernet->sched_list, list)
> > > > +		list_del_rcu(&sched->list);
> > > > +	spin_unlock(&pernet->lock);
> > > > +
> > > > +	synchronize_rcu();
> > > > +}
> > > > +
> > > > +static struct pernet_operations mptcp_sched_pernet_ops = {
> > > > +	.init = sched_init_net,
> > > > +	.exit = sched_exit_net,
> > > > +	.id = &sched_pernet_id,
> > > > +	.size = sizeof(struct sched_pernet),
> > > > +};
> > > > +
> > > > +void mptcp_sched_init(void)
> > > > +{
> > > > +	if (register_pernet_subsys(&mptcp_sched_pernet_ops) < 0)
> > > > +		panic("Failed to register MPTCP sched pernet subsystem.\n");
> > > > +}
> > 
> > --
> > Mat Martineau
> > Intel
> > 
> > 
> 
> --
> Mat Martineau
> Intel
>