[v7] dump for userspace pm

[PATCH mptcp-next v7 04/24] mptcp: check addrs list in userspace_pm_get_local_id

Posted by Geliang Tang 8 months, 1 week ago

Before adding a new entry in mptcp_userspace_pm_get_local_id(), it's
better to check whether this address is already in userspace pm local
address list. If it's in the list, no need to add a new entry, just
return it's address ID and use this address.

Fixes: 8b20137012d9 ("mptcp: read attributes of addr entries managed by userspace PMs")
Signed-off-by: Geliang Tang <geliang.tang@linux.dev>
---
 net/mptcp/pm_userspace.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index ce8ae7aa5aad..c592536b9889 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -135,10 +135,21 @@ int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
 int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
 				    struct mptcp_addr_info *skc)
 {
-	struct mptcp_pm_addr_entry new_entry;
+	struct mptcp_pm_addr_entry *entry = NULL, *e, new_entry;
 	__be16 msk_sport =  ((struct inet_sock *)
 			     inet_sk((struct sock *)msk))->inet_sport;
 
+	spin_lock_bh(&msk->pm.lock);
+	list_for_each_entry(e, &msk->pm.userspace_pm_local_addr_list, list) {
+		if (mptcp_addresses_equal(&e->addr, skc, false)) {
+			entry = e;
+			break;
+		}
+	}
+	spin_unlock_bh(&msk->pm.lock);
+	if (entry)
+		return entry->addr.id;
+
 	memset(&new_entry, 0, sizeof(struct mptcp_pm_addr_entry));
 	new_entry.addr = *skc;
 	new_entry.addr.id = 0;
-- 
2.39.2

Re: [PATCH mptcp-next v7 04/24] mptcp: check addrs list in userspace_pm_get_local_id

Posted by Mat Martineau 8 months ago

On Sat, 30 Dec 2023, Geliang Tang wrote:

> Before adding a new entry in mptcp_userspace_pm_get_local_id(), it's
> better to check whether this address is already in userspace pm local
> address list. If it's in the list, no need to add a new entry, just
> return it's address ID and use this address.
>

Good idea!

Reviewed-by: Mat Martineau <martineau@kernel.org>

> Fixes: 8b20137012d9 ("mptcp: read attributes of addr entries managed by userspace PMs")
> Signed-off-by: Geliang Tang <geliang.tang@linux.dev>
> ---
> net/mptcp/pm_userspace.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index ce8ae7aa5aad..c592536b9889 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -135,10 +135,21 @@ int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
> int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
> 				    struct mptcp_addr_info *skc)
> {
> -	struct mptcp_pm_addr_entry new_entry;
> +	struct mptcp_pm_addr_entry *entry = NULL, *e, new_entry;
> 	__be16 msk_sport =  ((struct inet_sock *)
> 			     inet_sk((struct sock *)msk))->inet_sport;
>
> +	spin_lock_bh(&msk->pm.lock);
> +	list_for_each_entry(e, &msk->pm.userspace_pm_local_addr_list, list) {
> +		if (mptcp_addresses_equal(&e->addr, skc, false)) {
> +			entry = e;
> +			break;
> +		}
> +	}
> +	spin_unlock_bh(&msk->pm.lock);
> +	if (entry)
> +		return entry->addr.id;
> +
> 	memset(&new_entry, 0, sizeof(struct mptcp_pm_addr_entry));
> 	new_entry.addr = *skc;
> 	new_entry.addr.id = 0;
> -- 
> 2.39.2
>
>
>

Re: [PATCH mptcp-next v7 04/24] mptcp: check addrs list in userspace_pm_get_local_id

Posted by Mat Martineau 8 months ago

On Fri, 5 Jan 2024, Mat Martineau wrote:

> On Sat, 30 Dec 2023, Geliang Tang wrote:
>
>> Before adding a new entry in mptcp_userspace_pm_get_local_id(), it's
>> better to check whether this address is already in userspace pm local
>> address list. If it's in the list, no need to add a new entry, just
>> return it's address ID and use this address.
>> 
>
> Good idea!
>
> Reviewed-by: Mat Martineau <martineau@kernel.org>
>

Matthieu -

To clarify (based on your discussion in 
https://lore.kernel.org/mptcp/c22d8a90-8b52-4518-8e46-d270f9988c7a@kernel.org/#t 
)

this patch is for mptcp-net.

- Mat

Re: [PATCH mptcp-next v7 04/24] mptcp: check addrs list in userspace_pm_get_local_id

Posted by Matthieu Baerts 8 months ago

Hi Geliang, Mat,

On 06/01/2024 01:55, Mat Martineau wrote:
> On Fri, 5 Jan 2024, Mat Martineau wrote:
> 
>> On Sat, 30 Dec 2023, Geliang Tang wrote:
>>
>>> Before adding a new entry in mptcp_userspace_pm_get_local_id(), it's
>>> better to check whether this address is already in userspace pm local
>>> address list. If it's in the list, no need to add a new entry, just
>>> return it's address ID and use this address.
>>>
>>
>> Good idea!
>>
>> Reviewed-by: Mat Martineau <martineau@kernel.org>
>>
> 
> Matthieu -
> 
> To clarify (based on your discussion in
> https://lore.kernel.org/mptcp/c22d8a90-8b52-4518-8e46-d270f9988c7a@kernel.org/#t )
> 
> this patch is for mptcp-net.

Thank you for the patch, the review and the note!

I just applied it because it doesn't look like it depends on the
previous patches. Now in our tree (fix for -net):

New patches for t/upstream-net and t/upstream:
- 6c8806a2bf91: mptcp: check addrs list in userspace_pm_get_local_id
- Results: 92194b020fd4..9cb02effbb76 (export-net)
- Results: 9610e17f4010..ca5de126f347 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20240108T110134
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20240108T110134

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