[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <802a2629b135d7f69f42f7bac5a5350374ed2570.camel@kernel.org>
Date: Sat, 06 Sep 2025 09:29:01 +0800
From: Geliang Tang <geliang@...nel.org>
To: Krister Johansen <kjlx@...pleofstupid.com>, Matthieu Baerts
<matttbe@...nel.org>, Mat Martineau <martineau@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
<pabeni@...hat.com>, Simon Horman <horms@...nel.org>, Florian Westphal
<fw@...len.de>, netdev@...r.kernel.org, mptcp@...ts.linux.dev,
linux-kernel@...r.kernel.org, David Reaver <me@...idreaver.com>
Subject: Re: [PATCH mptcp] mptcp: sockopt: make sync_socket_options
propagate SOCK_KEEPOPEN
Hi Krister,
On Fri, 2025-09-05 at 17:43 -0700, Krister Johansen wrote:
> Users reported a scenario where MPTCP connections that were
> configured
> with SO_KEEPALIVE prior to connect would fail to enable their
> keepalives
> if MTPCP fell back to TCP mode.
>
> After investigating, this affects keepalives for any connection where
> sync_socket_options is called on a socket that is in the closed or
> listening state. Joins are handled properly. For connects,
> sync_socket_options is called when the socket is still in the closed
> state. The tcp_set_keepalive() function does not act on sockets that
> are closed or listening, hence keepalive is not immediately enabled.
> Since the SO_KEEPOPEN flag is absent, it is not enabled later in the
> connect sequence via tcp_finish_connect. Setting the keepalive via
> sockopt after connect does work, but would not address any
> subsequently
> created flows.
>
> Fortunately, the fix here is straight-forward: set SOCK_KEEPOPEN on
> the
> subflow when calling sync_socket_options.
>
> The fix was valdidated both by using tcpdump to observe keeplaive
> packets not being sent before the fix, and being sent after the fix.
> It
> was also possible to observe via ss that the keepalive timer was not
> enabled on these sockets before the fix, but was enabled afterwards.
>
> Fixes: 1b3e7ede1365 ("mptcp: setsockopt: handle SO_KEEPALIVE and
> SO_PRIORITY")
> Cc: stable@...r.kernel.org
> Signed-off-by: Krister Johansen <kjlx@...pleofstupid.com>
Thanks for this fix. Good catch!
Reviewed-by: Geliang Tang <geliang@...nel.org>
-Geliang
> ---
> net/mptcp/sockopt.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index 2c267aff95be..13108e9f982b 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -1532,13 +1532,11 @@ static void sync_socket_options(struct
> mptcp_sock *msk, struct sock *ssk)
> {
> static const unsigned int tx_rx_locks = SOCK_RCVBUF_LOCK |
> SOCK_SNDBUF_LOCK;
> struct sock *sk = (struct sock *)msk;
> + int kaval = !!sock_flag(sk, SOCK_KEEPOPEN);
>
> - if (ssk->sk_prot->keepalive) {
> - if (sock_flag(sk, SOCK_KEEPOPEN))
> - ssk->sk_prot->keepalive(ssk, 1);
> - else
> - ssk->sk_prot->keepalive(ssk, 0);
> - }
> + if (ssk->sk_prot->keepalive)
> + ssk->sk_prot->keepalive(ssk, kaval);
> + sock_valbool_flag(ssk, SOCK_KEEPOPEN, kaval);
>
> ssk->sk_priority = sk->sk_priority;
> ssk->sk_bound_dev_if = sk->sk_bound_dev_if;
>
> base-commit: 319f7385f22c85618235ab0169b80092fa3c7696
Powered by blists - more mailing lists