Re: [mptcp-net] mptcp: pm: Defer freeing of MPTCP userspace path manager entries

[Mat Martineau <martineau@kernel.org>]

On Thu, 10 Apr 2025, Matthieu Baerts wrote:

> Hi Mat,
>
> On 10/04/2025 04:59, Mat Martineau wrote:
>> When path manager entries are deleted from the local address list, they
>> are first unlinked from the address list using list_del_rcu(). The
>> entries must not be freed until after the RCU grace period, but the
>> existing code immediately frees the entry.
>>
>> Use kfree_rcu_mightsleep() and adjust sk_omem_alloc in open code instead
>> of using the sock_kfree_s() helper. This code path is only called in a
>> netlink handler, so the "might sleep" function is preferable to adding
>> a rarely-used rcu_head member to struct mptcp_pm_addr_entry.
>
> Thank you for having looked at this!
>
> Should we add a Fixes tag? The following one or was it already there before?
>
> Fixes: 88d097316371 ("mptcp: drop free_list for deleting entries")

Hi Matthieu -

Yes, for -net I should have included a Fixes tag.

>
>
> Also, should we not also modify mptcp_userspace_pm_delete_local_addr()
> doing the following?
>
>  list_del_rcu(&entry->list);
>  sock_kfree_s(sk, entry, sizeof(*entry));
>

Geliang's series deletes this code, so I skipped this change intentionally 
(as explained in my note after the signoff).

> If yes, and while at it, do you think it might be good to add a short
> comment above "atomic_sub()" in the code explaining this is doing the
> same as sock_kfree_s(), but with RCU support? In case sock_kfree_s is
> modified later, and to understand what's being done here without using
> blame.

Good point, I can add that comment. It's a small change, but I'll send a 
v2 with that and the Fixes tag, and get the [PATCH mptcp-net] subject 
prefix correct this time :)


- Mat