[mptcp-next,v2] mptcp: pm: exit early with ADD_ADDR echo if possible

Commit Message

Matthieu Baerts Feb. 26, 2025, 2:21 p.m. UTC

When the userspace PM is used, or when the in-kernel limits are reached,
there will be no need to schedule the PM worker to signal new addresses.
That corresponds to pm->work_pending set to 0.

In this case, an early exit can be done in mptcp_pm_add_addr_echoed()
not to hold the PM lock, and iterate over the announced addresses list,
not to schedule the worker anyway in this case. This is similar to what
is done when a connection or a subflow has been established.

Signed-off-by: Matthieu Baerts (NGI0) <[e-mail]>
---
Changes in v2:
- Link to v1: https://lore.kernel.org/r/[e-mail]
---
Notes:
- v2:
  - Only modify mptcp_pm_add_addr_echoed().
  - The rest doesn't need to or shouldn't be modified (Geliang).
Cc: Geliang Tang <[e-mail]>
---
 net/mptcp/pm.c | 3 +++
 1 file changed, 3 insertions(+)


---
base-commit: 1238896935ea03f333a183a32fab666cc0c20e3b
change-id: 20250224-mptcp-userspace-avoid-worker-93f367e39ca6

Best regards,

Comments

MPTCP CI Feb. 26, 2025, 3:26 p.m. UTC | #1

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/13546321767

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/b505c426b765
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=938079


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Geliang Tang Feb. 27, 2025, 1:55 a.m. UTC | #2

Hi Matt,

On Wed, 2025-02-26 at 15:21 +0100, Matthieu Baerts (NGI0) wrote:
> When the userspace PM is used, or when the in-kernel limits are
> reached,
> there will be no need to schedule the PM worker to signal new
> addresses.
> That corresponds to pm->work_pending set to 0.
> 
> In this case, an early exit can be done in mptcp_pm_add_addr_echoed()
> not to hold the PM lock, and iterate over the announced addresses
> list,
> not to schedule the worker anyway in this case. This is similar to
> what
> is done when a connection or a subflow has been established.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <[e-mail]>

This v2 looks good to me, thanks.

Reviewed-by: Geliang Tang <[e-mail]>

> ---
> Changes in v2:
> - Link to v1:
> https://lore.kernel.org/r/[e-mail]
> ---
> Notes:
> - v2:
>   - Only modify mptcp_pm_add_addr_echoed().
>   - The rest doesn't need to or shouldn't be modified (Geliang).
> Cc: Geliang Tang <[e-mail]>
> ---
>  net/mptcp/pm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index
> 16cacce6c10fe86467aa7ef8e588f9f535b586fb..6c8cadf84f31f4c7dcc38b787be
> da048d5362dc8 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -251,6 +251,9 @@ void mptcp_pm_add_addr_echoed(struct mptcp_sock
> *msk,
>  
>  	pr_debug("msk=%p\n", msk);
>  
> +	if (!READ_ONCE(pm->work_pending))
> +		return;
> +
>  	spin_lock_bh(&pm->lock);
>  
>  	if (mptcp_lookup_anno_list_by_saddr(msk, addr) &&
> READ_ONCE(pm->work_pending))
> 
> ---
> base-commit: 1238896935ea03f333a183a32fab666cc0c20e3b
> change-id: 20250224-mptcp-userspace-avoid-worker-93f367e39ca6
> 
> Best regards,

Matthieu Baerts Feb. 27, 2025, 8:59 a.m. UTC | #3

Hi Geliang,

On 27/02/2025 02:55, Geliang Tang wrote:
> Hi Matt,
> 
> On Wed, 2025-02-26 at 15:21 +0100, Matthieu Baerts (NGI0) wrote:
>> When the userspace PM is used, or when the in-kernel limits are
>> reached,
>> there will be no need to schedule the PM worker to signal new
>> addresses.
>> That corresponds to pm->work_pending set to 0.
>>
>> In this case, an early exit can be done in mptcp_pm_add_addr_echoed()
>> not to hold the PM lock, and iterate over the announced addresses
>> list,
>> not to schedule the worker anyway in this case. This is similar to
>> what
>> is done when a connection or a subflow has been established.
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <[e-mail]>
> 
> This v2 looks good to me, thanks.

Thank you for the review!

Now in our tree (feat. for net-next):

New patches for t/upstream:
- 6bb3d64aade7: mptcp: pm: exit early with ADD_ADDR echo if possible
- Results: d8bb22f4550f..6b32949274eb (export)

Tests are now in progress:

- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/e404c290681fe80af3a4a943a6e2df1a1f9a6bb6/checks

Cheers,
Matt

Patch

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 16cacce6c10fe86467aa7ef8e588f9f535b586fb..6c8cadf84f31f4c7dcc38b787beda048d5362dc8 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -251,6 +251,9 @@  void mptcp_pm_add_addr_echoed(struct mptcp_sock *msk,
 
 	pr_debug("msk=%p\n", msk);
 
+	if (!READ_ONCE(pm->work_pending))
+		return;
+
 	spin_lock_bh(&pm->lock);
 
 	if (mptcp_lookup_anno_list_by_saddr(msk, addr) && READ_ONCE(pm->work_pending))