[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aL8RoSniweGJgm3h@templeofstupid.com>
Date: Mon, 8 Sep 2025 10:25:53 -0700
From: Krister Johansen <kjlx@...pleofstupid.com>
To: Matthieu Baerts <matttbe@...nel.org>
Cc: Geliang Tang <geliang@...nel.org>, Mat Martineau <martineau@...nel.org>,
"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
On Mon, Sep 08, 2025 at 07:13:12PM +0200, Matthieu Baerts wrote:
> Hi Geliang,
>
> On 07/09/2025 02:51, Geliang Tang wrote:
> > 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.
>
> Sure, I can do the modification and send it with other fixes we have.
Thanks for the reviews, Geliang and Matt. If you'd like me to fix the
formatting up and send a v2, I'm happy to do that as well. Just let me
know.
-K
Powered by blists - more mailing lists