diff mbox series

[mptcp-next,v10,09/12] mptcp: sysctl: map path_manager to pm_type

Message ID a38a4f3b6cc2ac7082401a22bc33eb0d844f0b37.1741258415.git.tanggeliang@kylinos.cn (mailing list archive)
State Superseded
Commit 59f4fd0d9b0b2739f63d516b13e57be5c12f28b4
Headers show
Series BPF path manager, part 5 | expand

Checks

Context Check Description
matttbe/checkpatch success total: 0 errors, 0 warnings, 0 checks, 26 lines checked
matttbe/shellcheck success MPTCP selftests files have not been modified
matttbe/build success Build and static analysis OK
matttbe/KVM_Validation__normal success Success! ✅
matttbe/KVM_Validation__debug success Success! ✅
matttbe/KVM_Validation__btf-normal__only_bpftest_all_ success Success! ✅
matttbe/KVM_Validation__btf-debug__only_bpftest_all_ success Success! ✅

Commit Message

Geliang Tang March 6, 2025, 11:01 a.m. UTC 
From: Geliang Tang <tanggeliang@kylinos.cn>

This patch maps the newly added path manager sysctl "path_manager"
to the old one "pm_type".

	path_manager   pm_type

	"kernel"    -> MPTCP_PM_TYPE_KERNEL
	"userspace" -> MPTCP_PM_TYPE_USERSPACE
	others      -> __MPTCP_PM_TYPE_NR

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/ctrl.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Matthieu Baerts (NGI0) March 10, 2025, 11:19 p.m. UTC | #1 
Hi Geliang,

On 06/03/2025 12:01, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch maps the newly added path manager sysctl "path_manager"
> to the old one "pm_type".
> 
> 	path_manager   pm_type
> 
> 	"kernel"    -> MPTCP_PM_TYPE_KERNEL
> 	"userspace" -> MPTCP_PM_TYPE_USERSPACE
> 	others      -> __MPTCP_PM_TYPE_NR
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/ctrl.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> index 1f405be6bc00..6a35e6bb0a63 100644
> --- a/net/mptcp/ctrl.c
> +++ b/net/mptcp/ctrl.c
> @@ -200,6 +200,9 @@ static int mptcp_set_path_manager(char *path_manager, const char *name)
>  static int proc_path_manager(const struct ctl_table *ctl, int write,
>  			     void *buffer, size_t *lenp, loff_t *ppos)
>  {
> +	struct mptcp_pernet *pernet = container_of(ctl->data,
> +						   struct mptcp_pernet,
> +						   path_manager);
>  	char (*path_manager)[MPTCP_PM_NAME_MAX] = ctl->data;
>  	char pm_name[MPTCP_PM_NAME_MAX];
>  	const struct ctl_table tbl = {
> @@ -211,8 +214,16 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
>  	strscpy(pm_name, *path_manager, MPTCP_PM_NAME_MAX);
>  
>  	ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
> -	if (write && ret == 0)
> +	if (write && ret == 0) {
> +		u8 pm_type = __MPTCP_PM_TYPE_NR;
> +
> +		if (strncmp(pm_name, "kernel", MPTCP_PM_NAME_MAX) == 0)
> +			pm_type = MPTCP_PM_TYPE_KERNEL;
> +		else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0)
> +			pm_type = MPTCP_PM_TYPE_USERSPACE;
> +		pernet->pm_type = pm_type;

This should be done after mptcp_set_path_manager(), and if there were no
errors (ret == 0), because if the name is not valid, the path-manager
will not be modified, so pm_type should not be modified as well.

BTW, did you check that setting "pm_type = __MPTCP_PM_TYPE_NR" is OK?
Can we display 2, but not allow to set 2 for pm_type here? In other
words, please check that this is OK with your modifications:

  # sysctl net.mptcp.pm_type=2
  sysctl: setting key "net.mptcp.pm_type": Invalid argument
  # sysctl -q net.mptcp.path_manager = "$BPF_PM"
  # sysctl -n net.mptcp.pm_type
  2

>  		ret = mptcp_set_path_manager(*path_manager, pm_name);
> +	}
>  
>  	return ret;
>  }
Cheers,
Matt
Geliang Tang March 11, 2025, 4:29 a.m. UTC | #2 
On Tue, 2025-03-11 at 00:19 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 06/03/2025 12:01, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > This patch maps the newly added path manager sysctl "path_manager"
> > to the old one "pm_type".
> > 
> > 	path_manager   pm_type
> > 
> > 	"kernel"    -> MPTCP_PM_TYPE_KERNEL
> > 	"userspace" -> MPTCP_PM_TYPE_USERSPACE
> > 	others      -> __MPTCP_PM_TYPE_NR
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >  net/mptcp/ctrl.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> > index 1f405be6bc00..6a35e6bb0a63 100644
> > --- a/net/mptcp/ctrl.c
> > +++ b/net/mptcp/ctrl.c
> > @@ -200,6 +200,9 @@ static int mptcp_set_path_manager(char
> > *path_manager, const char *name)
> >  static int proc_path_manager(const struct ctl_table *ctl, int
> > write,
> >  			     void *buffer, size_t *lenp, loff_t
> > *ppos)
> >  {
> > +	struct mptcp_pernet *pernet = container_of(ctl->data,
> > +						   struct
> > mptcp_pernet,
> > +						   path_manager);
> >  	char (*path_manager)[MPTCP_PM_NAME_MAX] = ctl->data;
> >  	char pm_name[MPTCP_PM_NAME_MAX];
> >  	const struct ctl_table tbl = {
> > @@ -211,8 +214,16 @@ static int proc_path_manager(const struct
> > ctl_table *ctl, int write,
> >  	strscpy(pm_name, *path_manager, MPTCP_PM_NAME_MAX);
> >  
> >  	ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
> > -	if (write && ret == 0)
> > +	if (write && ret == 0) {
> > +		u8 pm_type = __MPTCP_PM_TYPE_NR;
> > +
> > +		if (strncmp(pm_name, "kernel", MPTCP_PM_NAME_MAX)
> > == 0)
> > +			pm_type = MPTCP_PM_TYPE_KERNEL;
> > +		else if (strncmp(pm_name, "userspace",
> > MPTCP_PM_NAME_MAX) == 0)
> > +			pm_type = MPTCP_PM_TYPE_USERSPACE;
> > +		pernet->pm_type = pm_type;
> 
> This should be done after mptcp_set_path_manager(), and if there were
> no
> errors (ret == 0), because if the name is not valid, the path-manager
> will not be modified, so pm_type should not be modified as well.

Good catch! Done in v11.

> 
> BTW, did you check that setting "pm_type = __MPTCP_PM_TYPE_NR" is OK?
> Can we display 2, but not allow to set 2 for pm_type here? In other
> words, please check that this is OK with your modifications:
> 
>   # sysctl net.mptcp.pm_type=2
>   sysctl: setting key "net.mptcp.pm_type": Invalid argument
>   # sysctl -q net.mptcp.path_manager = "$BPF_PM"
>   # sysctl -n net.mptcp.pm_type
>   2

I did check this and it matches the expectation.

Thanks,
-Geliang

> 
> >  		ret = mptcp_set_path_manager(*path_manager,
> > pm_name);
> > +	}
> >  
> >  	return ret;
> >  }
> Cheers,
> Matt
diff mbox series

Patch

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 1f405be6bc00..6a35e6bb0a63 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -200,6 +200,9 @@  static int mptcp_set_path_manager(char *path_manager, const char *name)
 static int proc_path_manager(const struct ctl_table *ctl, int write,
 			     void *buffer, size_t *lenp, loff_t *ppos)
 {
+	struct mptcp_pernet *pernet = container_of(ctl->data,
+						   struct mptcp_pernet,
+						   path_manager);
 	char (*path_manager)[MPTCP_PM_NAME_MAX] = ctl->data;
 	char pm_name[MPTCP_PM_NAME_MAX];
 	const struct ctl_table tbl = {
@@ -211,8 +214,16 @@  static int proc_path_manager(const struct ctl_table *ctl, int write,
 	strscpy(pm_name, *path_manager, MPTCP_PM_NAME_MAX);
 
 	ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
-	if (write && ret == 0)
+	if (write && ret == 0) {
+		u8 pm_type = __MPTCP_PM_TYPE_NR;
+
+		if (strncmp(pm_name, "kernel", MPTCP_PM_NAME_MAX) == 0)
+			pm_type = MPTCP_PM_TYPE_KERNEL;
+		else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0)
+			pm_type = MPTCP_PM_TYPE_USERSPACE;
+		pernet->pm_type = pm_type;
 		ret = mptcp_set_path_manager(*path_manager, pm_name);
+	}
 
 	return ret;
 }