diff mbox series

[mptcp-next,v7,03/11] mptcp: sysctl: map pm_type to path_manager

Message ID 8840912119ce5a1180550745d496087ac7e81fa0.1740975633.git.tanggeliang@kylinos.cn (mailing list archive)
State Changes Requested
Delegated to: Matthieu Baerts
Headers show
Series BPF path manager, part 5 | expand

Checks

Context Check Description
matttbe/checkpatch success total: 0 errors, 0 warnings, 0 checks, 45 lines checked
matttbe/shellcheck success MPTCP selftests files have not been modified
matttbe/build success Build and static analysis OK
matttbe/KVM_Validation__normal warning Unstable: 1 failed test(s): selftest_mptcp_connect
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 3, 2025, 4:22 a.m. UTC 
From: Geliang Tang <tanggeliang@kylinos.cn>

This patch adds a new proc_handler "proc_pm_type" for "pm_type" to
map old path manager sysctl "pm_type" to the newly added "path_manager".

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

Comments

Matthieu Baerts (NGI0) March 3, 2025, 10:40 a.m. UTC | #1 
Hi Geliang,

On 03/03/2025 05:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch adds a new proc_handler "proc_pm_type" for "pm_type" to
> map old path manager sysctl "pm_type" to the newly added "path_manager".

Don't forget to add a selftest checking this new sysctl and the mapping
are correct, e.g. in userspace_pm.sh. See my previous comment:

https://lore.kernel.org/c49517d2-38e2-4848-9fb9-1c7748689cec@kernel.org

> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/ctrl.c | 33 ++++++++++++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> index d64e6b4f6d1d..32f13ab7db0a 100644
> --- a/net/mptcp/ctrl.c
> +++ b/net/mptcp/ctrl.c
> @@ -217,6 +217,35 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
>  	return ret;
>  }
>  
> +static int proc_pm_type(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,
> +						   pm_type);

Out of curiosity, do you plan to drop pernet->pm_type later on?

When all the mptcp_pm_is_{kernel,userspace}() will be dropped, I think
we can also drop it from the mptcp_pernet structure. At that point, we
could then read info from "path_manager" and write in ctl->data 0, 1, or
2. But only when we will no longer use pernet->pm_type.

> +	unsigned int val = READ_ONCE(*(u8 *)ctl->data);
> +	const struct ctl_table tbl = {
> +		.maxlen = sizeof(val),
> +		.data = &val,
> +	};
> +	int ret;
> +
> +	if (val > mptcp_pm_type_max)
> +		return -ERANGE;

You might not need this if ...

> +
> +	ret = proc_douintvec(&tbl, write, buffer, lenp, ppos);

... you use proc_dou8vec_minmax() here and ...

> +	if (write && ret == 0) {
> +		char *path_manager = "kernel";
> +
> +		if (val == MPTCP_PM_TYPE_USERSPACE)
> +			path_manager = "userspace";
> +		mptcp_set_path_manager(pernet->path_manager, path_manager);
> +		WRITE_ONCE(*(u8 *)ctl->data, val);
> +	}
> +
> +	return ret;
> +}
> +
>  static struct ctl_table mptcp_sysctl_table[] = {
>  	{
>  		.procname = "enabled",
> @@ -261,9 +290,7 @@ static struct ctl_table mptcp_sysctl_table[] = {
>  		.procname = "pm_type",
>  		.maxlen = sizeof(u8),
>  		.mode = 0644,
> -		.proc_handler = proc_dou8vec_minmax,
> -		.extra1       = SYSCTL_ZERO,
> -		.extra2       = &mptcp_pm_type_max

... you keep these two last lines.

> +		.proc_handler = proc_pm_type,
>  	},
>  	{
>  		.procname = "scheduler",
Cheers,
Matt
diff mbox series

Patch

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index d64e6b4f6d1d..32f13ab7db0a 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -217,6 +217,35 @@  static int proc_path_manager(const struct ctl_table *ctl, int write,
 	return ret;
 }
 
+static int proc_pm_type(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,
+						   pm_type);
+	unsigned int val = READ_ONCE(*(u8 *)ctl->data);
+	const struct ctl_table tbl = {
+		.maxlen = sizeof(val),
+		.data = &val,
+	};
+	int ret;
+
+	if (val > mptcp_pm_type_max)
+		return -ERANGE;
+
+	ret = proc_douintvec(&tbl, write, buffer, lenp, ppos);
+	if (write && ret == 0) {
+		char *path_manager = "kernel";
+
+		if (val == MPTCP_PM_TYPE_USERSPACE)
+			path_manager = "userspace";
+		mptcp_set_path_manager(pernet->path_manager, path_manager);
+		WRITE_ONCE(*(u8 *)ctl->data, val);
+	}
+
+	return ret;
+}
+
 static struct ctl_table mptcp_sysctl_table[] = {
 	{
 		.procname = "enabled",
@@ -261,9 +290,7 @@  static struct ctl_table mptcp_sysctl_table[] = {
 		.procname = "pm_type",
 		.maxlen = sizeof(u8),
 		.mode = 0644,
-		.proc_handler = proc_dou8vec_minmax,
-		.extra1       = SYSCTL_ZERO,
-		.extra2       = &mptcp_pm_type_max
+		.proc_handler = proc_pm_type,
 	},
 	{
 		.procname = "scheduler",