| Message ID | 20240809-mptcp-pm-avail-v7-3-3d0916ba39b4@kernel.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | cee0b5912a36699482d6b6b61cb76bf35eb4cdd4 |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | mptcp: pm: more fixes around the ID 0 | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/build | success | Build and static analysis OK |
| matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 28 lines checked |
| matttbe/shellcheck | success | MPTCP selftests files have not been modified |
| matttbe/KVM_Validation__normal | success | Success! ✅ |
| matttbe/KVM_Validation__debug | success | Success! ✅ |
| matttbe/KVM_Validation__btf__only_bpftest_all_ | success | Success! ✅ |
On Fri, 9 Aug 2024, Matthieu Baerts (NGI0) wrote: > If the subflow is considered as "staled", it is better to avoid it to > send an ACK carrying an ADD_ADDR or RM_ADDR. Another subflow, if any, > will then be selected. > > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > --- > Notes: > - It sounds safer to do this modification in -next. I also wonder if > we should not add more check, e.g. not on backup flow except if > there are no other non-backup ones, or still pick a staled one if > there are no others, etc. But maybe we don't need to care for an > ACK? And we can always improve that later! +1 for the -next tree. I don't think fancier logic is called for, if all subflows are stale a single ACK on a backup subflow (if it happens to be first in the list) does not seem like a problem. If all were stale, for reliability it might make sense to send the ACK on all subflows! For this corner case, I think the current patch is sufficient. - Mat
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index d3b1b459e6f3..3bdb0219188f 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -762,7 +762,7 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) void mptcp_pm_nl_addr_send_ack(struct mptcp_sock *msk) { - struct mptcp_subflow_context *subflow; + struct mptcp_subflow_context *subflow, *alt = NULL; msk_owned_by_me(msk); lockdep_assert_held(&msk->pm.lock); @@ -773,10 +773,18 @@ void mptcp_pm_nl_addr_send_ack(struct mptcp_sock *msk) mptcp_for_each_subflow(msk, subflow) { if (__mptcp_subflow_active(subflow)) { - mptcp_pm_send_ack(msk, subflow, false, false); - break; + if (!subflow->stale) { + mptcp_pm_send_ack(msk, subflow, false, false); + return; + } + + if (!alt) + alt = subflow; } } + + if (alt) + mptcp_pm_send_ack(msk, alt, false, false); } int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk,
If the subflow is considered as "staled", it is better to avoid it to send an ACK carrying an ADD_ADDR or RM_ADDR. Another subflow, if any, will then be selected. Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> --- Notes: - It sounds safer to do this modification in -next. I also wonder if we should not add more check, e.g. not on backup flow except if there are no other non-backup ones, or still pick a staled one if there are no others, etc. But maybe we don't need to care for an ACK? And we can always improve that later! - v7: - Include the Squash-to patch. (Mat) --- net/mptcp/pm_netlink.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)