[v6] BPF path manager, part 4

[PATCH mptcp-next v6 05/13] mptcp: pm: in-kernel: register mptcp_in_kernel_pm

Posted by Geliang Tang 11 months, 3 weeks ago

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch defines the original in-kernel netlink path manager as a new
struct mptcp_pm_ops named "mptcp_in_kernel_pm", and register it in
mptcp_pm_nl_init().

This mptcp_pm_ops will be skipped in mptcp_pm_unregister().

Only get_local_id() and get_priority() interfaces are implemented here.
mptcp_pm_nl_is_backup() becomes a wrapper of get_priority().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c         |  3 +++
 net/mptcp/pm_netlink.c | 18 +++++++++++++++++-
 net/mptcp/protocol.h   |  2 ++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 97fccd930cd0..07789526eecc 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -698,6 +698,9 @@ int mptcp_pm_register(struct mptcp_pm_ops *pm)
 
 void mptcp_pm_unregister(struct mptcp_pm_ops *pm)
 {
+	if (pm == &mptcp_in_kernel_pm)
+		return;
+
 	spin_lock(&mptcp_pm_list_lock);
 	list_del_rcu(&pm->list);
 	spin_unlock(&mptcp_pm_list_lock);
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 69a2f7aa1825..aa9be671293d 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1169,7 +1169,8 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk,
 	return ret;
 }
 
-bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
+static bool mptcp_pm_nl_get_priority(struct mptcp_sock *msk,
+				     struct mptcp_addr_info *skc)
 {
 	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
 	struct mptcp_pm_addr_entry *entry;
@@ -1183,6 +1184,11 @@ bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
 	return backup;
 }
 
+bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
+{
+	return mptcp_pm_nl_get_priority(msk, skc);
+}
+
 #define MPTCP_PM_CMD_GRP_OFFSET       0
 #define MPTCP_PM_EV_GRP_OFFSET        1
 
@@ -2370,6 +2376,14 @@ static struct pernet_operations mptcp_pm_pernet_ops = {
 	.size = sizeof(struct pm_nl_pernet),
 };
 
+struct mptcp_pm_ops mptcp_in_kernel_pm = {
+	.get_local_id		= mptcp_pm_nl_get_local_id,
+	.get_priority		= mptcp_pm_nl_get_priority,
+	.type			= MPTCP_PM_TYPE_KERNEL,
+	.name			= "in-kernel",
+	.owner			= THIS_MODULE,
+};
+
 void __init mptcp_pm_nl_init(void)
 {
 	if (register_pernet_subsys(&mptcp_pm_pernet_ops) < 0)
@@ -2377,4 +2391,6 @@ void __init mptcp_pm_nl_init(void)
 
 	if (genl_register_family(&mptcp_genl_family))
 		panic("Failed to register MPTCP PM netlink family\n");
+
+	mptcp_pm_register(&mptcp_in_kernel_pm);
 }
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3c24e8e1bc13..2b1fd10b4b7e 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1048,6 +1048,8 @@ int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_
 void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
 				struct mptcp_pm_addr_entry *entry);
 
+extern struct mptcp_pm_ops mptcp_in_kernel_pm;
+
 struct mptcp_pm_ops *mptcp_pm_find(const char *name);
 int mptcp_pm_validate(struct mptcp_pm_ops *pm);
 int mptcp_pm_register(struct mptcp_pm_ops *pm);
-- 
2.43.0

Re: [PATCH mptcp-next v6 05/13] mptcp: pm: in-kernel: register mptcp_in_kernel_pm

Posted by Matthieu Baerts 11 months, 3 weeks ago

On 23/02/2025 15:19, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch defines the original in-kernel netlink path manager as a new
> struct mptcp_pm_ops named "mptcp_in_kernel_pm", and register it in
> mptcp_pm_nl_init().
> 
> This mptcp_pm_ops will be skipped in mptcp_pm_unregister().
> 
> Only get_local_id() and get_priority() interfaces are implemented here.
> mptcp_pm_nl_is_backup() becomes a wrapper of get_priority().
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm.c         |  3 +++
>  net/mptcp/pm_netlink.c | 18 +++++++++++++++++-
>  net/mptcp/protocol.h   |  2 ++
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 97fccd930cd0..07789526eecc 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -698,6 +698,9 @@ int mptcp_pm_register(struct mptcp_pm_ops *pm)
>  
>  void mptcp_pm_unregister(struct mptcp_pm_ops *pm)
>  {
> +	if (pm == &mptcp_in_kernel_pm)
> +		return;
> +
>  	spin_lock(&mptcp_pm_list_lock);
>  	list_del_rcu(&pm->list);
>  	spin_unlock(&mptcp_pm_list_lock);
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 69a2f7aa1825..aa9be671293d 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1169,7 +1169,8 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk,
>  	return ret;
>  }
>  
> -bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
> +static bool mptcp_pm_nl_get_priority(struct mptcp_sock *msk,
> +				     struct mptcp_addr_info *skc)
>  {
>  	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
>  	struct mptcp_pm_addr_entry *entry;
> @@ -1183,6 +1184,11 @@ bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
>  	return backup;
>  }
>  
> +bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
> +{
> +	return mptcp_pm_nl_get_priority(msk, skc);
> +}
> +
>  #define MPTCP_PM_CMD_GRP_OFFSET       0
>  #define MPTCP_PM_EV_GRP_OFFSET        1
>  
> @@ -2370,6 +2376,14 @@ static struct pernet_operations mptcp_pm_pernet_ops = {
>  	.size = sizeof(struct pm_nl_pernet),
>  };
>  
> +struct mptcp_pm_ops mptcp_in_kernel_pm = {
> +	.get_local_id		= mptcp_pm_nl_get_local_id,
> +	.get_priority		= mptcp_pm_nl_get_priority,
> +	.type			= MPTCP_PM_TYPE_KERNEL,
> +	.name			= "in-kernel",

Small detail: should we call it "kernel" because it is shorter and still
meaningful?

We would see:

  net.mptcp.path_manager = kernel
  net.mptcp.path_manager = userspace

Seems OK, no?

> +	.owner			= THIS_MODULE,
> +};
> +
>  void __init mptcp_pm_nl_init(void)
>  {
>  	if (register_pernet_subsys(&mptcp_pm_pernet_ops) < 0)
> @@ -2377,4 +2391,6 @@ void __init mptcp_pm_nl_init(void)
>  
>  	if (genl_register_family(&mptcp_genl_family))
>  		panic("Failed to register MPTCP PM netlink family\n");
> +
> +	mptcp_pm_register(&mptcp_in_kernel_pm);
>  }
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 3c24e8e1bc13..2b1fd10b4b7e 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -1048,6 +1048,8 @@ int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_
>  void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
>  				struct mptcp_pm_addr_entry *entry);
>  
> +extern struct mptcp_pm_ops mptcp_in_kernel_pm;
> +
>  struct mptcp_pm_ops *mptcp_pm_find(const char *name);
>  int mptcp_pm_validate(struct mptcp_pm_ops *pm);
>  int mptcp_pm_register(struct mptcp_pm_ops *pm);

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next v6 05/13] mptcp: pm: in-kernel: register mptcp_in_kernel_pm

Posted by Matthieu Baerts 11 months, 3 weeks ago

On 23/02/2025 15:19, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch defines the original in-kernel netlink path manager as a new
> struct mptcp_pm_ops named "mptcp_in_kernel_pm", and register it in
> mptcp_pm_nl_init().
> 
> This mptcp_pm_ops will be skipped in mptcp_pm_unregister().
> 
> Only get_local_id() and get_priority() interfaces are implemented here.
> mptcp_pm_nl_is_backup() becomes a wrapper of get_priority().
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm.c         |  3 +++
>  net/mptcp/pm_netlink.c | 18 +++++++++++++++++-
>  net/mptcp/protocol.h   |  2 ++
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 97fccd930cd0..07789526eecc 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -698,6 +698,9 @@ int mptcp_pm_register(struct mptcp_pm_ops *pm)
>  
>  void mptcp_pm_unregister(struct mptcp_pm_ops *pm)
>  {
> +	if (pm == &mptcp_in_kernel_pm)
> +		return;
> +
>  	spin_lock(&mptcp_pm_list_lock);
>  	list_del_rcu(&pm->list);
>  	spin_unlock(&mptcp_pm_list_lock);
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 69a2f7aa1825..aa9be671293d 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1169,7 +1169,8 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk,
>  	return ret;
>  }
>  
> -bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
> +static bool mptcp_pm_nl_get_priority(struct mptcp_sock *msk,
> +				     struct mptcp_addr_info *skc)
>  {
>  	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
>  	struct mptcp_pm_addr_entry *entry;
> @@ -1183,6 +1184,11 @@ bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
>  	return backup;
>  }
>  
> +bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
> +{
> +	return mptcp_pm_nl_get_priority(msk, skc);
> +}

Why do you need this helper simply calling mptcp_pm_nl_is_backup()? Why
not using mptcp_pm_nl_is_backup() directly?

Same for the userspace PM. No need to add an extra level.

> +
>  #define MPTCP_PM_CMD_GRP_OFFSET       0
>  #define MPTCP_PM_EV_GRP_OFFSET        1
>  
> @@ -2370,6 +2376,14 @@ static struct pernet_operations mptcp_pm_pernet_ops = {
>  	.size = sizeof(struct pm_nl_pernet),
>  };
>  
> +struct mptcp_pm_ops mptcp_in_kernel_pm = {
> +	.get_local_id		= mptcp_pm_nl_get_local_id,
> +	.get_priority		= mptcp_pm_nl_get_priority,
> +	.type			= MPTCP_PM_TYPE_KERNEL,

Do you need the type?

> +	.name			= "in-kernel",
> +	.owner			= THIS_MODULE,
> +};
> +
>  void __init mptcp_pm_nl_init(void)
>  {
>  	if (register_pernet_subsys(&mptcp_pm_pernet_ops) < 0)
> @@ -2377,4 +2391,6 @@ void __init mptcp_pm_nl_init(void)
>  
>  	if (genl_register_family(&mptcp_genl_family))
>  		panic("Failed to register MPTCP PM netlink family\n");
> +
> +	mptcp_pm_register(&mptcp_in_kernel_pm);
>  }
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 3c24e8e1bc13..2b1fd10b4b7e 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -1048,6 +1048,8 @@ int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_
>  void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
>  				struct mptcp_pm_addr_entry *entry);
>  
> +extern struct mptcp_pm_ops mptcp_in_kernel_pm;
> +
>  struct mptcp_pm_ops *mptcp_pm_find(const char *name);
>  int mptcp_pm_validate(struct mptcp_pm_ops *pm);
>  int mptcp_pm_register(struct mptcp_pm_ops *pm);

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.