netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthieu Baerts <matttbe@kernel.org>
To: Geliang Tang <geliang@kernel.org>,
	Krister Johansen <kjlx@templeofstupid.com>,
	Mat Martineau <martineau@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>, Florian Westphal <fw@strlen.de>,
	netdev@vger.kernel.org, mptcp@lists.linux.dev,
	linux-kernel@vger.kernel.org, David Reaver <me@davidreaver.com>
Subject: Re: [PATCH mptcp] mptcp: sockopt: make sync_socket_options propagate SOCK_KEEPOPEN
Date: Mon, 8 Sep 2025 19:13:12 +0200	[thread overview]
Message-ID: <78d4a7b8-8025-493a-805c-a4c5d26836a8@kernel.org> (raw)
In-Reply-To: <83191d507b7bc9b0693568c2848319932e6b974e.camel@kernel.org>

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@kernel.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.

pw-bot: cr

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


  reply	other threads:[~2025-09-08 17:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-06  0:43 [PATCH mptcp] mptcp: sockopt: make sync_socket_options propagate SOCK_KEEPOPEN Krister Johansen
2025-09-06  1:29 ` Geliang Tang
2025-09-06 13:26 ` Matthieu Baerts
2025-09-07  0:51   ` Geliang Tang
2025-09-08 17:13     ` Matthieu Baerts [this message]
2025-09-08 17:25       ` Krister Johansen
2025-09-08 17:31         ` Matthieu Baerts
2025-09-08 17:45           ` Krister Johansen
2025-09-08 17:51             ` Matthieu Baerts
2025-09-08 17:56               ` Krister Johansen
2025-09-08 18:12                 ` Matthieu Baerts

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=78d4a7b8-8025-493a-805c-a4c5d26836a8@kernel.org \
    --to=matttbe@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fw@strlen.de \
    --cc=geliang@kernel.org \
    --cc=horms@kernel.org \
    --cc=kjlx@templeofstupid.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martineau@kernel.org \
    --cc=me@davidreaver.com \
    --cc=mptcp@lists.linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).