* [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops
2022-03-24 14:09 [PATCH RESEND mptcp-next v5 0/8] BPF packet scheduler Geliang Tang
@ 2022-03-24 14:09 ` Geliang Tang
2022-03-24 14:27 ` Florian Westphal
2022-03-24 22:14 ` Mat Martineau
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 2/8] mptcp: register default scheduler Geliang Tang
` (7 subsequent siblings)
8 siblings, 2 replies; 20+ messages in thread
From: Geliang Tang @ 2022-03-24 14:09 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops Geliang Tang
@ 2022-03-24 14:27 ` Florian Westphal
2022-03-24 14:41 ` Geliang Tang
2022-03-24 22:14 ` Mat Martineau
1 sibling, 1 reply; 20+ messages in thread
From: Florian Westphal @ 2022-03-24 14:27 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
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?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops
2022-03-24 14:27 ` Florian Westphal
@ 2022-03-24 14:41 ` Geliang Tang
2022-03-24 15:03 ` Florian Westphal
0 siblings, 1 reply; 20+ messages in thread
From: Geliang Tang @ 2022-03-24 14:41 UTC (permalink / raw)
To: Florian Westphal; +Cc: mptcp
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?
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops
2022-03-24 14:41 ` Geliang Tang
@ 2022-03-24 15:03 ` Florian Westphal
2022-03-24 16:06 ` Geliang Tang
0 siblings, 1 reply; 20+ messages in thread
From: Florian Westphal @ 2022-03-24 15:03 UTC (permalink / raw)
To: Geliang Tang; +Cc: Florian Westphal, mptcp
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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops
2022-03-24 15:03 ` Florian Westphal
@ 2022-03-24 16:06 ` Geliang Tang
2022-03-24 17:08 ` Florian Westphal
0 siblings, 1 reply; 20+ messages in thread
From: Geliang Tang @ 2022-03-24 16:06 UTC (permalink / raw)
To: Florian Westphal; +Cc: mptcp
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.
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops
2022-03-24 16:06 ` Geliang Tang
@ 2022-03-24 17:08 ` Florian Westphal
0 siblings, 0 replies; 20+ messages in thread
From: Florian Westphal @ 2022-03-24 17:08 UTC (permalink / raw)
To: Geliang Tang; +Cc: Florian Westphal, mptcp
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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops Geliang Tang
2022-03-24 14:27 ` Florian Westphal
@ 2022-03-24 22:14 ` Mat Martineau
2022-03-24 23:30 ` Mat Martineau
1 sibling, 1 reply; 20+ messages in thread
From: Mat Martineau @ 2022-03-24 22:14 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops
2022-03-24 22:14 ` Mat Martineau
@ 2022-03-24 23:30 ` Mat Martineau
2022-03-24 23:36 ` Mat Martineau
0 siblings, 1 reply; 20+ messages in thread
From: Mat Martineau @ 2022-03-24 23:30 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops
2022-03-24 23:30 ` Mat Martineau
@ 2022-03-24 23:36 ` Mat Martineau
2022-03-25 16:30 ` Geliang Tang
0 siblings, 1 reply; 20+ messages in thread
From: Mat Martineau @ 2022-03-24 23:36 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops
2022-03-24 23:36 ` Mat Martineau
@ 2022-03-25 16:30 ` Geliang Tang
0 siblings, 0 replies; 20+ messages in thread
From: Geliang Tang @ 2022-03-25 16:30 UTC (permalink / raw)
To: Mat Martineau; +Cc: mptcp
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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH RESEND mptcp-next v5 2/8] mptcp: register default scheduler
2022-03-24 14:09 [PATCH RESEND mptcp-next v5 0/8] BPF packet scheduler Geliang Tang
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops Geliang Tang
@ 2022-03-24 14:09 ` Geliang Tang
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 3/8] mptcp: add a new sysctl scheduler Geliang Tang
` (6 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Geliang Tang @ 2022-03-24 14:09 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch defined the default packet scheduler mptcp_sched_default,
registered it in mptcp_sched_init().
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 2 +-
net/mptcp/protocol.h | 1 +
net/mptcp/sched.c | 10 ++++++++++
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index b1d7c8b0c112..81b77ade9511 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1424,7 +1424,7 @@ bool mptcp_subflow_active(struct mptcp_subflow_context *subflow)
* returns the subflow that will transmit the next DSS
* additionally updates the rtx timeout
*/
-static struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
+struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
{
struct subflow_send_info send_info[SSK_MODE_MAX];
struct mptcp_subflow_context *subflow;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3258b740c8ee..46ac5b4528d5 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -614,6 +614,7 @@ 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);
+struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk);
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
index 3798a5cefeb6..f6ace8cfe865 100644
--- a/net/mptcp/sched.c
+++ b/net/mptcp/sched.c
@@ -23,6 +23,12 @@ struct sched_pernet {
struct list_head sched_list;
};
+static struct mptcp_sched_ops mptcp_sched_default = {
+ .get_subflow = mptcp_subflow_get_send,
+ .name = "default",
+ .owner = THIS_MODULE,
+};
+
static struct sched_pernet *sched_get_pernet(const struct net *net)
{
return net_generic(net, sched_pernet_id);
@@ -70,6 +76,9 @@ void mptcp_unregister_scheduler(const struct net *net,
{
struct sched_pernet *pernet = sched_get_pernet(net);
+ if (sched == &mptcp_sched_default)
+ return;
+
spin_lock(&pernet->lock);
list_del_rcu(&sched->list);
spin_unlock(&pernet->lock);
@@ -111,4 +120,5 @@ void mptcp_sched_init(void)
{
if (register_pernet_subsys(&mptcp_sched_pernet_ops) < 0)
panic("Failed to register MPTCP sched pernet subsystem.\n");
+ mptcp_register_scheduler(&init_net, &mptcp_sched_default);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RESEND mptcp-next v5 3/8] mptcp: add a new sysctl scheduler
2022-03-24 14:09 [PATCH RESEND mptcp-next v5 0/8] BPF packet scheduler Geliang Tang
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 1/8] mptcp: add struct mptcp_sched_ops Geliang Tang
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 2/8] mptcp: register default scheduler Geliang Tang
@ 2022-03-24 14:09 ` Geliang Tang
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 4/8] mptcp: add sched in mptcp_sock Geliang Tang
` (5 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Geliang Tang @ 2022-03-24 14:09 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch added a new sysctl, named scheduler, to support for selection
of different schedulers.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
Documentation/networking/mptcp-sysctl.rst | 8 ++++++++
net/mptcp/ctrl.c | 14 ++++++++++++++
net/mptcp/protocol.h | 1 +
3 files changed, 23 insertions(+)
diff --git a/Documentation/networking/mptcp-sysctl.rst b/Documentation/networking/mptcp-sysctl.rst
index e263dfcc4b40..d9e69fdc7ea3 100644
--- a/Documentation/networking/mptcp-sysctl.rst
+++ b/Documentation/networking/mptcp-sysctl.rst
@@ -75,3 +75,11 @@ stale_loss_cnt - INTEGER
This is a per-namespace sysctl.
Default: 4
+
+scheduler - STRING
+ Select the scheduler of your choice.
+
+ Support for selection of different schedulers. This is a per-namespace
+ sysctl.
+
+ Default: "default"
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index ae20b7d92e28..c46c22a84d23 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -32,6 +32,7 @@ struct mptcp_pernet {
u8 checksum_enabled;
u8 allow_join_initial_addr_port;
u8 pm_type;
+ char scheduler[MPTCP_SCHED_NAME_MAX];
};
static struct mptcp_pernet *mptcp_get_pernet(const struct net *net)
@@ -69,6 +70,11 @@ int mptcp_get_pm_type(const struct net *net)
return mptcp_get_pernet(net)->pm_type;
}
+const char *mptcp_get_scheduler(const struct net *net)
+{
+ return mptcp_get_pernet(net)->scheduler;
+}
+
static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
{
pernet->mptcp_enabled = 1;
@@ -77,6 +83,7 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
pernet->allow_join_initial_addr_port = 1;
pernet->stale_loss_cnt = 4;
pernet->pm_type = MPTCP_PM_TYPE_KERNEL;
+ strcpy(pernet->scheduler, "default");
}
#ifdef CONFIG_SYSCTL
@@ -128,6 +135,12 @@ static struct ctl_table mptcp_sysctl_table[] = {
.extra1 = SYSCTL_ZERO,
.extra2 = &mptcp_pm_type_max
},
+ {
+ .procname = "scheduler",
+ .maxlen = MPTCP_SCHED_NAME_MAX,
+ .mode = 0644,
+ .proc_handler = proc_dostring,
+ },
{}
};
@@ -149,6 +162,7 @@ static int mptcp_pernet_new_table(struct net *net, struct mptcp_pernet *pernet)
table[3].data = &pernet->allow_join_initial_addr_port;
table[4].data = &pernet->stale_loss_cnt;
table[5].data = &pernet->pm_type;
+ table[6].data = &pernet->scheduler;
hdr = register_net_sysctl(net, MPTCP_SYSCTL_PATH, table);
if (!hdr)
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 46ac5b4528d5..3e69c1e5600a 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -586,6 +586,7 @@ int mptcp_is_checksum_enabled(const struct net *net);
int mptcp_allow_join_id0(const struct net *net);
unsigned int mptcp_stale_loss_cnt(const struct net *net);
int mptcp_get_pm_type(const struct net *net);
+const char *mptcp_get_scheduler(const struct net *net);
void mptcp_subflow_fully_established(struct mptcp_subflow_context *subflow,
struct mptcp_options_received *mp_opt);
bool __mptcp_retransmit_pending_data(struct sock *sk);
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RESEND mptcp-next v5 4/8] mptcp: add sched in mptcp_sock
2022-03-24 14:09 [PATCH RESEND mptcp-next v5 0/8] BPF packet scheduler Geliang Tang
` (2 preceding siblings ...)
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 3/8] mptcp: add a new sysctl scheduler Geliang Tang
@ 2022-03-24 14:09 ` Geliang Tang
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 5/8] mptcp: add get_subflow wrapper Geliang Tang
` (4 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Geliang Tang @ 2022-03-24 14:09 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch added a new struct member sched in struct mptcp_sock.
And two helpers mptcp_init_sched() and mptcp_release_sched() to
init and release it.
Init it with the sysctl scheduler in mptcp_init_sock(), copy the
scheduler from the parent in mptcp_sk_clone(), and release it in
__mptcp_destroy_sock().
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 4 ++++
net/mptcp/protocol.h | 4 ++++
net/mptcp/sched.c | 21 +++++++++++++++++++++
3 files changed, 29 insertions(+)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 81b77ade9511..c7969b9b567e 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2661,6 +2661,8 @@ static int mptcp_init_sock(struct sock *sk)
* propagate the correct value
*/
mptcp_ca_reset(sk);
+ mptcp_init_sched(mptcp_sk(sk),
+ mptcp_sched_find(net, mptcp_get_scheduler(net)));
sk_sockets_allocated_inc(sk);
sk->sk_rcvbuf = sock_net(sk)->ipv4.sysctl_tcp_rmem[1];
@@ -2816,6 +2818,7 @@ static void __mptcp_destroy_sock(struct sock *sk)
sk_stop_timer(sk, &sk->sk_timer);
mptcp_data_unlock(sk);
msk->pm.status = 0;
+ mptcp_release_sched(msk);
/* clears msk->subflow, allowing the following loop to close
* even the initial subflow
@@ -2993,6 +2996,7 @@ struct sock *mptcp_sk_clone(const struct sock *sk,
msk->snd_una = msk->write_seq;
msk->wnd_end = msk->snd_nxt + req->rsk_rcv_wnd;
msk->setsockopt_seq = mptcp_sk(sk)->setsockopt_seq;
+ mptcp_init_sched(msk, mptcp_sk(sk)->sched);
if (mp_opt->suboptions & OPTIONS_MPTCP_MPC) {
msk->can_ack = true;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3e69c1e5600a..ad45281e8562 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -288,6 +288,7 @@ struct mptcp_sock {
struct socket *subflow; /* outgoing connect/listener/!mp_capable */
struct sock *first;
struct mptcp_pm_data pm;
+ struct mptcp_sched_ops *sched;
struct {
u32 space; /* bytes copied in last measurement window */
u32 copied; /* bytes copied in this measurement window */
@@ -617,6 +618,9 @@ void mptcp_unregister_scheduler(const struct net *net,
struct mptcp_sched_ops *sched);
struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk);
void mptcp_sched_init(void);
+void mptcp_init_sched(struct mptcp_sock *msk,
+ struct mptcp_sched_ops *sched);
+void mptcp_release_sched(struct mptcp_sock *msk);
static inline bool __mptcp_subflow_active(struct mptcp_subflow_context *subflow)
{
diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
index f6ace8cfe865..e431b1eec5e1 100644
--- a/net/mptcp/sched.c
+++ b/net/mptcp/sched.c
@@ -122,3 +122,24 @@ void mptcp_sched_init(void)
panic("Failed to register MPTCP sched pernet subsystem.\n");
mptcp_register_scheduler(&init_net, &mptcp_sched_default);
}
+
+void mptcp_init_sched(struct mptcp_sock *msk,
+ struct mptcp_sched_ops *sched)
+{
+ if (!sched)
+ msk->sched = &mptcp_sched_default;
+ else
+ msk->sched = sched;
+
+ if (msk->sched->init)
+ msk->sched->init(msk);
+
+ pr_debug("sched=%s", msk->sched->name);
+}
+
+void mptcp_release_sched(struct mptcp_sock *msk)
+{
+ if (msk->sched && msk->sched->release)
+ msk->sched->release(msk);
+ msk->sched = NULL;
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RESEND mptcp-next v5 5/8] mptcp: add get_subflow wrapper
2022-03-24 14:09 [PATCH RESEND mptcp-next v5 0/8] BPF packet scheduler Geliang Tang
` (3 preceding siblings ...)
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 4/8] mptcp: add sched in mptcp_sock Geliang Tang
@ 2022-03-24 14:09 ` Geliang Tang
2022-03-24 23:23 ` Mat Martineau
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 6/8] mptcp: add bpf_mptcp_sched_ops Geliang Tang
` (3 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Geliang Tang @ 2022-03-24 14:09 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch defined a new wrapper mptcp_sched_get_subflow(), invoke
get_subflow of msk->sched in it. Use the wrapper instead of using
mptcp_subflow_get_send() directly.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 6 +++---
net/mptcp/protocol.h | 1 +
net/mptcp/sched.c | 7 +++++++
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c7969b9b567e..241cfda822c0 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1567,7 +1567,7 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
int ret = 0;
prev_ssk = ssk;
- ssk = mptcp_subflow_get_send(msk);
+ ssk = mptcp_sched_get_subflow(msk);
/* First check. If the ssk has changed since
* the last round, release prev_ssk
@@ -1636,7 +1636,7 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk)
* check for a different subflow usage only after
* spooling the first chunk of data
*/
- xmit_ssk = first ? ssk : mptcp_subflow_get_send(mptcp_sk(sk));
+ xmit_ssk = first ? ssk : mptcp_sched_get_subflow(mptcp_sk(sk));
if (!xmit_ssk)
goto out;
if (xmit_ssk != ssk) {
@@ -3134,7 +3134,7 @@ void __mptcp_check_push(struct sock *sk, struct sock *ssk)
return;
if (!sock_owned_by_user(sk)) {
- struct sock *xmit_ssk = mptcp_subflow_get_send(mptcp_sk(sk));
+ struct sock *xmit_ssk = mptcp_sched_get_subflow(mptcp_sk(sk));
if (xmit_ssk == ssk)
__mptcp_subflow_push_pending(sk, ssk);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index ad45281e8562..7781eb527b9a 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -621,6 +621,7 @@ void mptcp_sched_init(void);
void mptcp_init_sched(struct mptcp_sock *msk,
struct mptcp_sched_ops *sched);
void mptcp_release_sched(struct mptcp_sock *msk);
+struct sock *mptcp_sched_get_subflow(struct mptcp_sock *msk);
static inline bool __mptcp_subflow_active(struct mptcp_subflow_context *subflow)
{
diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
index e431b1eec5e1..2fd2a034ba49 100644
--- a/net/mptcp/sched.c
+++ b/net/mptcp/sched.c
@@ -11,6 +11,7 @@
#include <linux/list.h>
#include <linux/rculist.h>
#include <linux/spinlock.h>
+#include <linux/indirect_call_wrapper.h>
#include <net/tcp.h>
#include <net/netns/generic.h>
#include "protocol.h"
@@ -143,3 +144,9 @@ void mptcp_release_sched(struct mptcp_sock *msk)
msk->sched->release(msk);
msk->sched = NULL;
}
+
+struct sock *mptcp_sched_get_subflow(struct mptcp_sock *msk)
+{
+ return INDIRECT_CALL_INET_1(msk->sched->get_subflow,
+ mptcp_subflow_get_send, msk);
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND mptcp-next v5 5/8] mptcp: add get_subflow wrapper
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 5/8] mptcp: add get_subflow wrapper Geliang Tang
@ 2022-03-24 23:23 ` Mat Martineau
0 siblings, 0 replies; 20+ messages in thread
From: Mat Martineau @ 2022-03-24 23:23 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
On Thu, 24 Mar 2022, Geliang Tang wrote:
> This patch defined a new wrapper mptcp_sched_get_subflow(), invoke
> get_subflow of msk->sched in it. Use the wrapper instead of using
> mptcp_subflow_get_send() directly.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> net/mptcp/protocol.c | 6 +++---
> net/mptcp/protocol.h | 1 +
> net/mptcp/sched.c | 7 +++++++
> 3 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index c7969b9b567e..241cfda822c0 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1567,7 +1567,7 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
> int ret = 0;
>
> prev_ssk = ssk;
> - ssk = mptcp_subflow_get_send(msk);
> + ssk = mptcp_sched_get_subflow(msk);
>
> /* First check. If the ssk has changed since
> * the last round, release prev_ssk
> @@ -1636,7 +1636,7 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk)
> * check for a different subflow usage only after
> * spooling the first chunk of data
> */
> - xmit_ssk = first ? ssk : mptcp_subflow_get_send(mptcp_sk(sk));
> + xmit_ssk = first ? ssk : mptcp_sched_get_subflow(mptcp_sk(sk));
> if (!xmit_ssk)
> goto out;
> if (xmit_ssk != ssk) {
> @@ -3134,7 +3134,7 @@ void __mptcp_check_push(struct sock *sk, struct sock *ssk)
> return;
>
> if (!sock_owned_by_user(sk)) {
> - struct sock *xmit_ssk = mptcp_subflow_get_send(mptcp_sk(sk));
> + struct sock *xmit_ssk = mptcp_sched_get_subflow(mptcp_sk(sk));
>
> if (xmit_ssk == ssk)
> __mptcp_subflow_push_pending(sk, ssk);
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index ad45281e8562..7781eb527b9a 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -621,6 +621,7 @@ void mptcp_sched_init(void);
> void mptcp_init_sched(struct mptcp_sock *msk,
> struct mptcp_sched_ops *sched);
> void mptcp_release_sched(struct mptcp_sock *msk);
> +struct sock *mptcp_sched_get_subflow(struct mptcp_sock *msk);
>
> static inline bool __mptcp_subflow_active(struct mptcp_subflow_context *subflow)
> {
> diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
> index e431b1eec5e1..2fd2a034ba49 100644
> --- a/net/mptcp/sched.c
> +++ b/net/mptcp/sched.c
> @@ -11,6 +11,7 @@
> #include <linux/list.h>
> #include <linux/rculist.h>
> #include <linux/spinlock.h>
> +#include <linux/indirect_call_wrapper.h>
> #include <net/tcp.h>
> #include <net/netns/generic.h>
> #include "protocol.h"
> @@ -143,3 +144,9 @@ void mptcp_release_sched(struct mptcp_sock *msk)
> msk->sched->release(msk);
> msk->sched = NULL;
> }
> +
> +struct sock *mptcp_sched_get_subflow(struct mptcp_sock *msk)
> +{
> + return INDIRECT_CALL_INET_1(msk->sched->get_subflow,
> + mptcp_subflow_get_send, msk);
> +}
Since the purpose of this is to limit function call overhead, this could
be implemented as a static inline function in protocol.h
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH RESEND mptcp-next v5 6/8] mptcp: add bpf_mptcp_sched_ops
2022-03-24 14:09 [PATCH RESEND mptcp-next v5 0/8] BPF packet scheduler Geliang Tang
` (4 preceding siblings ...)
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 5/8] mptcp: add get_subflow wrapper Geliang Tang
@ 2022-03-24 14:09 ` Geliang Tang
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 7/8] selftests: bpf: add bpf_first scheduler Geliang Tang
` (2 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Geliang Tang @ 2022-03-24 14:09 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch implemented a new struct bpf_struct_ops, bpf_mptcp_sched_ops.
Register and unregister the mptcp scheduler in .reg and .unreg.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
kernel/bpf/bpf_struct_ops_types.h | 4 ++
net/mptcp/bpf.c | 102 ++++++++++++++++++++++++++++++
2 files changed, 106 insertions(+)
diff --git a/kernel/bpf/bpf_struct_ops_types.h b/kernel/bpf/bpf_struct_ops_types.h
index 5678a9ddf817..5a6b0c0d8d3d 100644
--- a/kernel/bpf/bpf_struct_ops_types.h
+++ b/kernel/bpf/bpf_struct_ops_types.h
@@ -8,5 +8,9 @@ BPF_STRUCT_OPS_TYPE(bpf_dummy_ops)
#ifdef CONFIG_INET
#include <net/tcp.h>
BPF_STRUCT_OPS_TYPE(tcp_congestion_ops)
+#ifdef CONFIG_MPTCP
+#include <net/mptcp.h>
+BPF_STRUCT_OPS_TYPE(mptcp_sched_ops)
+#endif
#endif
#endif
diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index 535602ba2582..14a42cae89cb 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -10,8 +10,110 @@
#define pr_fmt(fmt) "MPTCP: " fmt
#include <linux/bpf.h>
+#include <linux/bpf_verifier.h>
+#include <linux/btf.h>
+#include <linux/btf_ids.h>
#include "protocol.h"
+extern struct bpf_struct_ops bpf_mptcp_sched_ops;
+extern struct btf *btf_vmlinux;
+
+static u32 optional_ops[] = {
+ offsetof(struct mptcp_sched_ops, init),
+ offsetof(struct mptcp_sched_ops, release),
+ offsetof(struct mptcp_sched_ops, get_subflow),
+};
+
+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);
+}
+
+static const struct bpf_verifier_ops bpf_mptcp_sched_verifier_ops = {
+ .get_func_proto = bpf_mptcp_sched_get_func_proto,
+ .is_valid_access = btf_ctx_access,
+ .btf_struct_access = btf_struct_access,
+};
+
+static int bpf_mptcp_sched_reg(void *kdata)
+{
+ return mptcp_register_scheduler(&init_net, kdata);
+}
+
+static void bpf_mptcp_sched_unreg(void *kdata)
+{
+ mptcp_unregister_scheduler(&init_net, kdata);
+}
+
+static int bpf_mptcp_sched_check_member(const struct btf_type *t,
+ const struct btf_member *member)
+{
+ return 0;
+}
+
+static bool is_optional(u32 member_offset)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(optional_ops); i++) {
+ if (member_offset == optional_ops[i])
+ return true;
+ }
+
+ return false;
+}
+
+static int bpf_mptcp_sched_init_member(const struct btf_type *t,
+ const struct btf_member *member,
+ void *kdata, const void *udata)
+{
+ const struct mptcp_sched_ops *usched;
+ struct mptcp_sched_ops *sched;
+ int prog_fd;
+ u32 moff;
+
+ usched = (const struct mptcp_sched_ops *)udata;
+ sched = (struct mptcp_sched_ops *)kdata;
+
+ moff = __btf_member_bit_offset(t, member) / 8;
+ switch (moff) {
+ case offsetof(struct mptcp_sched_ops, name):
+ if (bpf_obj_name_cpy(sched->name, usched->name,
+ sizeof(sched->name)) <= 0)
+ return -EINVAL;
+ if (mptcp_sched_find(&init_net, usched->name))
+ return -EEXIST;
+ return 1;
+ }
+
+ if (!btf_type_resolve_func_ptr(btf_vmlinux, member->type, NULL))
+ return 0;
+
+ /* Ensure bpf_prog is provided for compulsory func ptr */
+ prog_fd = (int)(*(unsigned long *)(udata + moff));
+ if (!prog_fd && !is_optional(moff))
+ return -EINVAL;
+
+ return 0;
+}
+
+static int bpf_mptcp_sched_init(struct btf *btf)
+{
+ return 0;
+}
+
+struct bpf_struct_ops bpf_mptcp_sched_ops = {
+ .verifier_ops = &bpf_mptcp_sched_verifier_ops,
+ .reg = bpf_mptcp_sched_reg,
+ .unreg = bpf_mptcp_sched_unreg,
+ .check_member = bpf_mptcp_sched_check_member,
+ .init_member = bpf_mptcp_sched_init_member,
+ .init = bpf_mptcp_sched_init,
+ .name = "mptcp_sched_ops",
+};
+
struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
{
if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RESEND mptcp-next v5 7/8] selftests: bpf: add bpf_first scheduler
2022-03-24 14:09 [PATCH RESEND mptcp-next v5 0/8] BPF packet scheduler Geliang Tang
` (5 preceding siblings ...)
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 6/8] mptcp: add bpf_mptcp_sched_ops Geliang Tang
@ 2022-03-24 14:09 ` Geliang Tang
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 8/8] selftests: bpf: add bpf_first test Geliang Tang
2022-03-24 23:15 ` [PATCH RESEND mptcp-next v5 0/8] BPF packet scheduler Mat Martineau
8 siblings, 0 replies; 20+ messages in thread
From: Geliang Tang @ 2022-03-24 14:09 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch implemented the simplest MPTCP scheduler, named bpf_first,
which always picks the first subflow to send data.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
tools/testing/selftests/bpf/bpf_tcp_helpers.h | 12 ++++++++
tools/testing/selftests/bpf/progs/bpf_first.c | 30 +++++++++++++++++++
2 files changed, 42 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/bpf_first.c
diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index f92357597e63..5bb21b24231f 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -197,6 +197,18 @@ struct tcp_congestion_ops {
void *owner;
};
+#define MPTCP_SCHED_NAME_MAX 16
+
+struct mptcp_sched_ops {
+ char name[MPTCP_SCHED_NAME_MAX];
+
+ void (*init)(struct mptcp_sock *msk);
+ void (*release)(struct mptcp_sock *msk);
+
+ struct sock * (*get_subflow)(struct mptcp_sock *msk);
+ void *owner;
+};
+
#define min(a, b) ((a) < (b) ? (a) : (b))
#define max(a, b) ((a) > (b) ? (a) : (b))
#define min_not_zero(x, y) ({ \
diff --git a/tools/testing/selftests/bpf/progs/bpf_first.c b/tools/testing/selftests/bpf/progs/bpf_first.c
new file mode 100644
index 000000000000..3ae5c1eab5f5
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/bpf_first.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2022, SUSE. */
+
+#include <linux/bpf.h>
+#include "bpf_tcp_helpers.h"
+
+char _license[] SEC("license") = "GPL";
+
+SEC("struct_ops/mptcp_sched_first_init")
+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, struct mptcp_sock *msk)
+{
+}
+
+struct sock *BPF_STRUCT_OPS(bpf_first_get_subflow, struct mptcp_sock *msk)
+{
+ return msk->first;
+}
+
+SEC(".struct_ops")
+struct mptcp_sched_ops first = {
+ .init = (void *)mptcp_sched_first_init,
+ .release = (void *)mptcp_sched_first_release,
+ .get_subflow = (void *)bpf_first_get_subflow,
+ .name = "bpf_first",
+};
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RESEND mptcp-next v5 8/8] selftests: bpf: add bpf_first test
2022-03-24 14:09 [PATCH RESEND mptcp-next v5 0/8] BPF packet scheduler Geliang Tang
` (6 preceding siblings ...)
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 7/8] selftests: bpf: add bpf_first scheduler Geliang Tang
@ 2022-03-24 14:09 ` Geliang Tang
2022-03-24 23:15 ` [PATCH RESEND mptcp-next v5 0/8] BPF packet scheduler Mat Martineau
8 siblings, 0 replies; 20+ messages in thread
From: Geliang Tang @ 2022-03-24 14:09 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch expended the MPTCP test base to support MPTCP packet
scheduler tests. Add the bpf_first MPTCP sched test in it. Use
sysctl to set net.mptcp.scheduler to use this sched.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
.../testing/selftests/bpf/prog_tests/mptcp.c | 42 +++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index b7c230c4efef..151350202fa6 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -4,6 +4,7 @@
#include <test_progs.h>
#include "cgroup_helpers.h"
#include "network_helpers.h"
+#include "bpf_first.skel.h"
#ifndef TCP_CA_NAME_MAX
#define TCP_CA_NAME_MAX 16
@@ -19,6 +20,15 @@ struct mptcp_storage {
};
static char monitor_log_path[64];
+static int duration;
+
+static void send_byte(int fd)
+{
+ char b = 0x55;
+
+ if (CHECK_FAIL(write(fd, &b, sizeof(b)) != 1))
+ perror("Failed to send single byte");
+}
static int verify_tsk(int map_fd, int client_fd)
{
@@ -202,6 +212,8 @@ static int run_test(int cgroup_fd, int server_fd, bool is_mptcp)
goto close_client_fd;
}
+ send_byte(client_fd);
+
err += is_mptcp ? verify_msk(map_fd, client_fd) :
verify_tsk(map_fd, client_fd);
@@ -254,8 +266,38 @@ void test_base(void)
close(cgroup_fd);
}
+static void test_first(void)
+{
+ struct bpf_first *first_skel;
+ int server_fd, client_fd;
+ struct bpf_link *link;
+
+ first_skel = bpf_first__open_and_load();
+ if (CHECK(!first_skel, "bpf_first__open_and_load", "failed\n"))
+ return;
+
+ link = bpf_map__attach_struct_ops(first_skel->maps.first);
+ if (!ASSERT_OK_PTR(link, "bpf_map__attach_struct_ops")) {
+ bpf_first__destroy(first_skel);
+ return;
+ }
+
+ system("sysctl -q net.mptcp.scheduler=bpf_first");
+ server_fd = start_mptcp_server(AF_INET, NULL, 0, 0);
+ client_fd = connect_to_mptcp_fd(server_fd, 0);
+
+ send_byte(client_fd);
+
+ close(client_fd);
+ close(server_fd);
+ bpf_link__destroy(link);
+ bpf_first__destroy(first_skel);
+}
+
void test_mptcp(void)
{
if (test__start_subtest("base"))
test_base();
+ if (test__start_subtest("first"))
+ test_first();
}
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH RESEND mptcp-next v5 0/8] BPF packet scheduler
2022-03-24 14:09 [PATCH RESEND mptcp-next v5 0/8] BPF packet scheduler Geliang Tang
` (7 preceding siblings ...)
2022-03-24 14:09 ` [PATCH RESEND mptcp-next v5 8/8] selftests: bpf: add bpf_first test Geliang Tang
@ 2022-03-24 23:15 ` Mat Martineau
8 siblings, 0 replies; 20+ messages in thread
From: Mat Martineau @ 2022-03-24 23:15 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
On Thu, 24 Mar 2022, Geliang Tang wrote:
> RESEND:
> - rebased to export/20220324T054815.
> - depends on: "add skc_to_mptcp_sock" v12.
>
> v5:
> - patch 1: define per-namespace sched_list (but only used init_net
> namespace. It is difficult to get 'net' in bpf_mptcp_sched_reg and
> bpf_mptcp_sched_unreg. I need some suggestions here.)
> - patch 2: skip mptcp_sched_default in mptcp_unregister_scheduler.
> - patch 8: add tests into mptcp.c, instead of bpf_tcp_ca.c.
>
I ran in to a build problem because CONFIG_BPF_SYSCALL was not set in my
.config and net/mptcp/Kconfig did not capture any BPF dependencies.
I think it would be best to allow MPTCP use without BPF enabled, so a new
CONFIG_MPTCP_BPF_SCHEDULER config option (with dependencies on MPTCP and
BPF_SYSCALL) would be good.
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 20+ messages in thread