Hi Geliang,
On 03/03/2025 05:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Now pm->pm_type can be replaced by pm->ops->name, then "pm_type" filed
> of struct mptcp_pm_data can be dropped.
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> net/mptcp/pm.c | 4 +---
> net/mptcp/protocol.h | 5 ++---
> 2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index e8b34f2ecb35..1ce58d16370a 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -979,7 +979,6 @@ void mptcp_pm_destroy(struct mptcp_sock *msk)
> void mptcp_pm_data_reset(struct mptcp_sock *msk)
> {
> const char *path_manager = mptcp_get_path_manager(sock_net((struct sock *)msk));
> - u8 pm_type = mptcp_get_pm_type(sock_net((struct sock *)msk));
> struct mptcp_pm_data *pm = &msk->pm;
> int ret;
>
> @@ -989,7 +988,6 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
> pm->subflows = 0;
> pm->rm_list_tx.nr = 0;
> pm->rm_list_rx.nr = 0;
> - WRITE_ONCE(pm->pm_type, pm_type);
>
> rcu_read_lock();
> ret = mptcp_pm_initialize(msk, mptcp_pm_find(path_manager));
> @@ -997,7 +995,7 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
> if (ret)
> return;
>
> - if (pm_type == MPTCP_PM_TYPE_KERNEL) {
> + if (mptcp_pm_is_kernel(msk)) {
The code here could be done in the new init() callback maybe? So what
you introduced in the previous patch 7/11.
> bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk);
>
> /* pm->work_pending must be only be set to 'true' when
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 172450455c2a..56eeee1cbccc 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -233,7 +233,6 @@ struct mptcp_pm_data {
> u8 add_addr_signaled;
> u8 add_addr_accepted;
> u8 local_addr_used;
> - u8 pm_type;
> u8 subflows;
> u8 status;
> DECLARE_BITMAP(id_avail_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
> @@ -1101,12 +1100,12 @@ static inline bool mptcp_pm_should_rm_signal(struct mptcp_sock *msk)
>
> static inline bool mptcp_pm_is_userspace(const struct mptcp_sock *msk)
> {
> - return READ_ONCE(msk->pm.pm_type) == MPTCP_PM_TYPE_USERSPACE;
> + return !strncmp(msk->pm.ops->name, "userspace", MPTCP_PM_NAME_MAX);
Please don't do a string comparison here.
I think it is better to drop msk->pm.pm_type when mptcp_pm_is_userspace
and mptcp_pm_is_kernel are both dropped, no?
If you want to drop pm_type before, then you could compare &msk->pm.ops,
but I think that's something that should be done later. I guess
mptcp_pm_is_userspace() will still be needed, but only from
pm_userspace.c with mptcp_userspace_pm_get_sock(). No?
Same for mptcp_pm_is_kernel(), only from pm_kernel.c when iterating over
all connections. Except if you use introduce a new macro like
mptcp_pm_for_each_msk() taking in argument "&mptcp_pm_kernel"?
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.