Re: [PATCH mptcp-next v8 06/12] mptcp: pm: in-kernel: register mptcp_pm_kernel

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 05/03/2025 02:35, Geliang Tang wrote:
> Hi Matt,
> 
> On Tue, 2025-03-04 at 19:40 +0800, 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_pm_kernel", and register it in
>> mptcp_pm_kernel_register().
>>
>> This mptcp_pm_ops will be skipped in mptcp_pm_unregister().
>>
>> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
>> ---
>>  net/mptcp/pm.c        |  4 ++++
>>  net/mptcp/pm_kernel.c | 26 ++++++++++++++++++++++++++
>>  net/mptcp/protocol.h  |  3 +++
>>  3 files changed, 33 insertions(+)
>>
>> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
>> index a2b210873b23..28ea8bdaa8b0 100644
>> --- a/net/mptcp/pm.c
>> +++ b/net/mptcp/pm.c
>> @@ -1076,6 +1076,10 @@ int mptcp_pm_register(struct mptcp_pm_ops *pm)
>>  
>>  void mptcp_pm_unregister(struct mptcp_pm_ops *pm)
>>  {
>> +	/* skip unregistering the default path manager */
>> +	if (pm == &mptcp_pm_kernel)
>> +		return;
>> +
>>  	spin_lock(&mptcp_pm_list_lock);
>>  	list_del_rcu(&pm->list);
>>  	spin_unlock(&mptcp_pm_list_lock);
>> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
>> index 806a9b5b3c07..e6a1aef738a8 100644
>> --- a/net/mptcp/pm_kernel.c
>> +++ b/net/mptcp/pm_kernel.c
>> @@ -1398,8 +1398,34 @@ static struct pernet_operations
>> mptcp_pm_pernet_ops = {
>>  	.size = sizeof(struct pm_nl_pernet),
>>  };
>>  
>> +static void mptcp_pm_nl_initialize(struct mptcp_sock *msk)
>> +{
>> +	bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk);
>> +	struct mptcp_pm_data *pm = &msk->pm;
>> +
>> +	/* pm->work_pending must be only be set to 'true' when
>> +	 * pm is the default path manager
>> +	 */
>> +	WRITE_ONCE(pm->work_pending,
>> +		   (!!mptcp_pm_get_local_addr_max(msk) &&
>> +		    subflows_allowed) ||
>> +		   !!mptcp_pm_get_add_addr_signal_max(msk));
>> +	WRITE_ONCE(pm->accept_addr,
>> +		   !!mptcp_pm_get_add_addr_accept_max(msk) &&
>> +		   subflows_allowed);
>> +	WRITE_ONCE(pm->accept_subflow, subflows_allowed);
>> +}
>> +
>> +struct mptcp_pm_ops mptcp_pm_kernel = {
>> +	.init			= mptcp_pm_nl_initialize,
> 
> A better name would be mptcp_pm_nl_init, but this is already used in
> pm_netlink.c. I would like to rename mptcp_pm_nl_init in pm_netlink.c
> to mptcp_pm_genl_init, do you think this is a good idea?
> 
> If so, is it better to squash this renaming into "mptcp: pm: split
> netlink and in-kernel init" or make it a separate patch?

What about calling the one here mptcp_pm_kernel_init()?

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