| Message ID | 4edb40bf773e65c00d4447f4795cec26f9a047a3.1741088339.git.tanggeliang@kylinos.cn (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | BPF path manager, part 5 | expand |
Hi Geliang, On 04/03/2025 12:40, 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". > > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> > --- > net/mptcp/ctrl.c | 28 +++++++++++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c > index d64e6b4f6d1d..d425fcbd036a 100644 > --- a/net/mptcp/ctrl.c > +++ b/net/mptcp/ctrl.c > @@ -217,6 +217,32 @@ 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); > + u8 pm_type = READ_ONCE(*(u8 *)ctl->data); > + const struct ctl_table tbl = { > + .maxlen = sizeof(pm_type), > + .data = &pm_type, > + }; > + int ret; > + > + ret = proc_dou8vec_minmax(&tbl, write, buffer, lenp, ppos); I missed that in my previous review. Do you need tbl here? Can you not use "ctl" here instead? If you need tbl, you will need to move the "extra[12]" fields there I suppose, otherwise the limits will not be checked, no? In the proc_handler, a new ctl_table structure is needed when it is required to have a way to prevent the writing of the data after having called proc_do(...). Here, that's not the case: if the new value is between 0 and mptcp_pm_type_max, we will always write ctl->data. In other words, I think you can remove tlb, use ctl instead, and remove the 'WRITE_ONCE(*(u8 *)ctl->data, pm_type)' here below. > + if (write && ret == 0) { > + char *path_manager = "kernel"; > + > + if (pm_type == MPTCP_PM_TYPE_USERSPACE) pm_type should be read here in the 'if' statement, after having potentially be modified in proc_dou8vec_minmax(). Or use "buffer" directly. > + path_manager = "userspace"; > + mptcp_set_path_manager(pernet->path_manager, path_manager); > + WRITE_ONCE(*(u8 *)ctl->data, pm_type); > + } > + > + return ret; > +} > + > static struct ctl_table mptcp_sysctl_table[] = { > { > .procname = "enabled", Cheers, Matt
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c index d64e6b4f6d1d..d425fcbd036a 100644 --- a/net/mptcp/ctrl.c +++ b/net/mptcp/ctrl.c @@ -217,6 +217,32 @@ 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); + u8 pm_type = READ_ONCE(*(u8 *)ctl->data); + const struct ctl_table tbl = { + .maxlen = sizeof(pm_type), + .data = &pm_type, + }; + int ret; + + ret = proc_dou8vec_minmax(&tbl, write, buffer, lenp, ppos); + if (write && ret == 0) { + char *path_manager = "kernel"; + + if (pm_type == MPTCP_PM_TYPE_USERSPACE) + path_manager = "userspace"; + mptcp_set_path_manager(pernet->path_manager, path_manager); + WRITE_ONCE(*(u8 *)ctl->data, pm_type); + } + + return ret; +} + static struct ctl_table mptcp_sysctl_table[] = { { .procname = "enabled", @@ -261,7 +287,7 @@ static struct ctl_table mptcp_sysctl_table[] = { .procname = "pm_type", .maxlen = sizeof(u8), .mode = 0644, - .proc_handler = proc_dou8vec_minmax, + .proc_handler = proc_pm_type, .extra1 = SYSCTL_ZERO, .extra2 = &mptcp_pm_type_max },