Re: [PATCH mptcp-net v2 3/3] mptcp: pm: change to fullmesh only for 'subflow'

[Mat Martineau <martineau@kernel.org>]

On Fri, 17 Jan 2025, Matthieu Baerts (NGI0) wrote:

Hi Matthieu -

> If an entrypoint

Did you mean "endpoint" here?

> has no type -- so not 'subflow', 'signal', 'implicit' --
> there are then no subflows to re-create from this local endpoint.
>
> In this case, there is then no need to iterate over all connections to
> do nothing. So stop early when this case is present.
>
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> net/mptcp/pm_netlink.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index ff1e5695dc1db5e32d5f45bef7cf22e43aea0ef1..b1fe2a74fcfe97896de8f9eaee9a1afa5378fabb 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1923,11 +1923,16 @@ static void mptcp_pm_nl_fullmesh(struct mptcp_sock *msk,
> }
>
> static void mptcp_nl_set_flags(struct net *net, struct mptcp_addr_info *addr,
> -			       u8 bkup, u8 changed)
> +			       u8 flags, u8 changed)
> {
> +	u8 is_subflow = !!(flags & MPTCP_PM_ADDR_FLAG_SUBFLOW);
> +	u8 bkup = !!(flags & MPTCP_PM_ADDR_FLAG_BACKUP);
> 	long s_slot = 0, s_num = 0;
> 	struct mptcp_sock *msk;
>
> +	if (changed == MPTCP_PM_ADDR_FLAG_FULLMESH && !is_subflow)
> +		return;
> +
> 	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
> 		struct sock *sk = (struct sock *)msk;
>
> @@ -1937,7 +1942,7 @@ static void mptcp_nl_set_flags(struct net *net, struct mptcp_addr_info *addr,
> 		lock_sock(sk);
> 		if (changed & MPTCP_PM_ADDR_FLAG_BACKUP)
> 			mptcp_pm_nl_mp_prio_send_ack(msk, addr, NULL, bkup);
> -		if (changed & MPTCP_PM_ADDR_FLAG_FULLMESH)
> +		if (is_subflow && (changed & MPTCP_PM_ADDR_FLAG_FULLMESH))

Could you add something to the beginning of the commit message explaining 
this part of the change? It matches the subject line but the body of the 
commit message only explains the loop optimization above.

Thanks,
Mat

> 			mptcp_pm_nl_fullmesh(msk, addr);
> 		release_sock(sk);
>
> @@ -1959,7 +1964,6 @@ int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
> 	struct mptcp_pm_addr_entry *entry;
> 	struct pm_nl_pernet *pernet;
> 	u8 lookup_by_id = 0;
> -	u8 bkup = 0;
>
> 	pernet = pm_nl_get_pernet(net);
>
> @@ -1972,9 +1976,6 @@ int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
> 		}
> 	}
>
> -	if (local->flags & MPTCP_PM_ADDR_FLAG_BACKUP)
> -		bkup = 1;
> -
> 	spin_lock_bh(&pernet->lock);
> 	entry = lookup_by_id ? __lookup_addr_by_id(pernet, local->addr.id) :
> 			       __lookup_addr(pernet, &local->addr);
> @@ -1996,7 +1997,7 @@ int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
> 	*local = *entry;
> 	spin_unlock_bh(&pernet->lock);
>
> -	mptcp_nl_set_flags(net, &local->addr, bkup, changed);
> +	mptcp_nl_set_flags(net, &local->addr, entry->flags, changed);
> 	return 0;
> }
>
>
> -- 
> 2.47.1
>
>
>