[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <83191d507b7bc9b0693568c2848319932e6b974e.camel@kernel.org>
Date: Sun, 07 Sep 2025 08:51:31 +0800
From: Geliang Tang <geliang@...nel.org>
To: Matthieu Baerts <matttbe@...nel.org>, Krister Johansen
<kjlx@...pleofstupid.com>, 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 Matt,
On Sat, 2025-09-06 at 15:26 +0200, Matthieu Baerts wrote:
> Hi Krister,
>
> On 06/09/2025 02:43, 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.
>
>
> Thank you for the fix! Indeed, the SOCK_KEEPOPEN flag was missing!
> This
> patch looks good to me as well:
>
> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@...nel.org>
>
>
> @Netdev Maintainers: please apply this patch in 'net' directly. But I
> can always re-send it later if preferred.
nit:
I just noticed his patch breaks 'Reverse X-Mas Tree' order in
sync_socket_options(). If you think any changes are needed, please
update this when you re-send it.
Thanks,
-Geliang
>
> Cheers,
> Matt
Powered by blists - more mailing lists