If an endpoint doesn't have the 'subflow' endpoint -- in fact, has no
type, so not 'subflow', 'signal', nor 'implicit' -- there are then no
subflows created from this local endpoint to at least the initial
destination address. In this case, no need to call mptcp_pm_nl_fullmesh()
which is there to recreate the subflows to reflect the new value of the
fullmesh attribute.
Similarly, there is then no need to iterate over all connections to do
nothing, if only the 'fullmesh' flag has been changed, and the endpoint
doesn't have the 'subflow' one. So stop early when dealing with this
specific case.
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Notes:
v2:
- clarify the modifications: new comment and updated description (Mat)
---
net/mptcp/pm_netlink.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index ff1e5695dc1db5e32d5f45bef7cf22e43aea0ef1..1a0695e087af02347678b9b6914d303554bcf1f3 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,8 @@ 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)
+ /* Subflows will only be recreated if the SUBFLOW flag is set */
+ if (is_subflow && (changed & MPTCP_PM_ADDR_FLAG_FULLMESH))
mptcp_pm_nl_fullmesh(msk, addr);
release_sock(sk);
@@ -1959,7 +1965,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 +1977,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 +1998,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.1On Wed, 22 Jan 2025, Matthieu Baerts (NGI0) wrote:
> If an endpoint doesn't have the 'subflow' endpoint
'flag'?
> -- in fact, has no
> type, so not 'subflow', 'signal', nor 'implicit' -- there are then no
> subflows created from this local endpoint to at least the initial
> destination address. In this case, no need to call mptcp_pm_nl_fullmesh()
> which is there to recreate the subflows to reflect the new value of the
> fullmesh attribute.
>
> Similarly, there is then no need to iterate over all connections to do
> nothing, if only the 'fullmesh' flag has been changed, and the endpoint
> doesn't have the 'subflow' one. So stop early when dealing with this
> specific case.
>
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> Notes:
> v2:
> - clarify the modifications: new comment and updated description (Mat)
The new comment (with the one fix above) does make it clearer, thank you!
Reviewed-by: Mat Martineau <martineau@kernel.org>
> ---
> net/mptcp/pm_netlink.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index ff1e5695dc1db5e32d5f45bef7cf22e43aea0ef1..1a0695e087af02347678b9b6914d303554bcf1f3 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,8 @@ 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)
> + /* Subflows will only be recreated if the SUBFLOW flag is set */
> + if (is_subflow && (changed & MPTCP_PM_ADDR_FLAG_FULLMESH))
> mptcp_pm_nl_fullmesh(msk, addr);
> release_sock(sk);
>
> @@ -1959,7 +1965,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 +1977,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 +1998,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
>
>
>
Hi Mat, On 24/01/2025 02:53, Mat Martineau wrote: > On Wed, 22 Jan 2025, Matthieu Baerts (NGI0) wrote: > >> If an endpoint doesn't have the 'subflow' endpoint > > 'flag'? Oops, good catch! (fixed) >> -- in fact, has no >> type, so not 'subflow', 'signal', nor 'implicit' -- there are then no >> subflows created from this local endpoint to at least the initial >> destination address. In this case, no need to call mptcp_pm_nl_fullmesh() >> which is there to recreate the subflows to reflect the new value of the >> fullmesh attribute. >> >> Similarly, there is then no need to iterate over all connections to do >> nothing, if only the 'fullmesh' flag has been changed, and the endpoint >> doesn't have the 'subflow' one. So stop early when dealing with this >> specific case. >> >> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> >> --- >> Notes: >> v2: >> - clarify the modifications: new comment and updated description (Mat) > > The new comment (with the one fix above) does make it clearer, thank you! > > Reviewed-by: Mat Martineau <martineau@kernel.org> Thank you for the review! Now in our tree (feat. for net-next): New patches for t/upstream: - 6a6c787311fd: mptcp: pm: remove unused ret value to set flags - d7fe0f41b38e: mptcp: pm: change to fullmesh only for 'subflow' - Results: 9ddf0cb9d180..3b679399ffaf (export) Tests are now in progress: - export: https://github.com/multipath-tcp/mptcp_net-next/commit/848d0e332ae8b7807e4afe73b7d7ceb6e689f4e3/checks Cheers, Matt -- Sponsored by the NGI0 Core fund.
© 2016 - 2025 Red Hat, Inc.