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@vger.kernel.org
Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
---
 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
-- 
2.43.0

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.

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

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.

Thanks,
-Geliang

> 
> Cheers,
> Matt

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.

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

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

Hi Krister,

On 08/09/2025 19:25, Krister Johansen wrote:
> 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@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.
> 
> 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.

I was going to apply this diff:

> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index 13108e9f982b..2abe6f1e9940 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -1532,11 +1532,12 @@ 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);
> +       bool keep_open;
>  
> +       keep_open = sock_flag(sk, SOCK_KEEPOPEN);
>         if (ssk->sk_prot->keepalive)
> -               ssk->sk_prot->keepalive(ssk, kaval);
> -       sock_valbool_flag(ssk, SOCK_KEEPOPEN, kaval);
> +               ssk->sk_prot->keepalive(ssk, keep_open);
> +       sock_valbool_flag(ssk, SOCK_KEEPOPEN, keep_open);
>  
>         ssk->sk_priority = sk->sk_priority;
>         ssk->sk_bound_dev_if = sk->sk_bound_dev_if;

(sock_flag() returns a bool, and 'keep_open' is maybe clearer)

But up to you, I really don't mind if you prefer to send the v2 by
yourself, just let me know.

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

On Mon, Sep 08, 2025 at 07:31:43PM +0200, Matthieu Baerts wrote:
> Hi Krister,
> 
> On 08/09/2025 19:25, Krister Johansen wrote:
> > 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@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.
> > 
> > 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.
> 
> I was going to apply this diff:
> 
> > diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> > index 13108e9f982b..2abe6f1e9940 100644
> > --- a/net/mptcp/sockopt.c
> > +++ b/net/mptcp/sockopt.c
> > @@ -1532,11 +1532,12 @@ 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);
> > +       bool keep_open;
> >  
> > +       keep_open = sock_flag(sk, SOCK_KEEPOPEN);
> >         if (ssk->sk_prot->keepalive)
> > -               ssk->sk_prot->keepalive(ssk, kaval);
> > -       sock_valbool_flag(ssk, SOCK_KEEPOPEN, kaval);
> > +               ssk->sk_prot->keepalive(ssk, keep_open);
> > +       sock_valbool_flag(ssk, SOCK_KEEPOPEN, keep_open);
> >  
> >         ssk->sk_priority = sk->sk_priority;
> >         ssk->sk_bound_dev_if = sk->sk_bound_dev_if;
> 
> (sock_flag() returns a bool, and 'keep_open' is maybe clearer)
> 
> But up to you, I really don't mind if you prefer to send the v2 by
> yourself, just let me know.

Thanks, I'll go ahead and amend as you suggest and then send a v2.

-K

8 Sept 2025 19:45:32 Krister Johansen <kjlx@templeofstupid.com>:

> On Mon, Sep 08, 2025 at 07:31:43PM +0200, Matthieu Baerts wrote:
>> Hi Krister,
>>
>> On 08/09/2025 19:25, Krister Johansen wrote:
>>> 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:
>>>>>> …
>>>>>
>>>>> 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.
>>
>> I was going to apply this diff:
>>
>>> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
>>> index 13108e9f982b..2abe6f1e9940 100644
>>> --- a/net/mptcp/sockopt.c
>>> +++ b/net/mptcp/sockopt.c
>>> @@ -1532,11 +1532,12 @@ 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);
>>> +       bool keep_open;
>>>
>>> +       keep_open = sock_flag(sk, SOCK_KEEPOPEN);
>>>         if (ssk->sk_prot->keepalive)
>>> -               ssk->sk_prot->keepalive(ssk, kaval);
>>> -       sock_valbool_flag(ssk, SOCK_KEEPOPEN, kaval);
>>> +               ssk->sk_prot->keepalive(ssk, keep_open);
>>> +       sock_valbool_flag(ssk, SOCK_KEEPOPEN, keep_open);
>>>
>>>         ssk->sk_priority = sk->sk_priority;
>>>         ssk->sk_bound_dev_if = sk->sk_bound_dev_if;
>>
>> (sock_flag() returns a bool, and 'keep_open' is maybe clearer)
>>
>> But up to you, I really don't mind if you prefer to send the v2 by
>> yourself, just let me know.
>
> Thanks, I'll go ahead and amend as you suggest and then send a v2.

Great, thanks.

While at it, please use [PATCH net] as prefix.

Cheers,
Matt

On Mon, Sep 08, 2025 at 07:51:10PM +0200, Matthieu Baerts wrote:
> 8 Sept 2025 19:45:32 Krister Johansen <kjlx@templeofstupid.com>:
> 
> > On Mon, Sep 08, 2025 at 07:31:43PM +0200, Matthieu Baerts wrote:
> >> Hi Krister,
> >>
> >> On 08/09/2025 19:25, Krister Johansen wrote:
> >>> 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:
> >>>>>> …
> >>>>>
> >>>>> 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.
> >>
> >> I was going to apply this diff:
> >>
> >>> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> >>> index 13108e9f982b..2abe6f1e9940 100644
> >>> --- a/net/mptcp/sockopt.c
> >>> +++ b/net/mptcp/sockopt.c
> >>> @@ -1532,11 +1532,12 @@ 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);
> >>> +       bool keep_open;
> >>>
> >>> +       keep_open = sock_flag(sk, SOCK_KEEPOPEN);
> >>>         if (ssk->sk_prot->keepalive)
> >>> -               ssk->sk_prot->keepalive(ssk, kaval);
> >>> -       sock_valbool_flag(ssk, SOCK_KEEPOPEN, kaval);
> >>> +               ssk->sk_prot->keepalive(ssk, keep_open);
> >>> +       sock_valbool_flag(ssk, SOCK_KEEPOPEN, keep_open);
> >>>
> >>>         ssk->sk_priority = sk->sk_priority;
> >>>         ssk->sk_bound_dev_if = sk->sk_bound_dev_if;
> >>
> >> (sock_flag() returns a bool, and 'keep_open' is maybe clearer)
> >>
> >> But up to you, I really don't mind if you prefer to send the v2 by
> >> yourself, just let me know.
> >
> > Thanks, I'll go ahead and amend as you suggest and then send a v2.
> 
> Great, thanks.
> 
> While at it, please use [PATCH net] as prefix.

Thanks, will do.  May I preserve the Reveiwed-By tags from the v1, or
would you like to review again?

-K

8 Sept 2025 19:56:23 Krister Johansen <kjlx@templeofstupid.com>:

> On Mon, Sep 08, 2025 at 07:51:10PM +0200, Matthieu Baerts wrote:
>> 8 Sept 2025 19:45:32 Krister Johansen <kjlx@templeofstupid.com>:
>>
>>> On Mon, Sep 08, 2025 at 07:31:43PM +0200, Matthieu Baerts wrote:
>>>> Hi Krister,
>>>>
>>>> On 08/09/2025 19:25, Krister Johansen wrote:
>>>>> On Mon, Sep 08, 2025 at 07:13:12PM +0200, Matthieu Baerts wrote:
>>>>>> …
>>>>>
>>>>> 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.
>>>>
>>>> I was going to apply this diff:
>>>>
>>>>> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
>>>>> index 13108e9f982b..2abe6f1e9940 100644
>>>>> --- a/net/mptcp/sockopt.c
>>>>> +++ b/net/mptcp/sockopt.c
>>>>> @@ -1532,11 +1532,12 @@ 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);
>>>>> +       bool keep_open;
>>>>>
>>>>> +       keep_open = sock_flag(sk, SOCK_KEEPOPEN);
>>>>>         if (ssk->sk_prot->keepalive)
>>>>> -               ssk->sk_prot->keepalive(ssk, kaval);
>>>>> -       sock_valbool_flag(ssk, SOCK_KEEPOPEN, kaval);
>>>>> +               ssk->sk_prot->keepalive(ssk, keep_open);
>>>>> +       sock_valbool_flag(ssk, SOCK_KEEPOPEN, keep_open);
>>>>>
>>>>>         ssk->sk_priority = sk->sk_priority;
>>>>>         ssk->sk_bound_dev_if = sk->sk_bound_dev_if;
>>>>
>>>> (sock_flag() returns a bool, and 'keep_open' is maybe clearer)
>>>>
>>>> But up to you, I really don't mind if you prefer to send the v2 by
>>>> yourself, just let me know.
>>>
>>> Thanks, I'll go ahead and amend as you suggest and then send a v2.
>>
>> Great, thanks.
>>
>> While at it, please use [PATCH net] as prefix.
>
> Thanks, will do.  May I preserve the Reveiwed-By tags from the v1, or
> would you like to review again?

You can preserve it.

Cheers,
Matt

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@vger.kernel.org
> Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>

Thanks for this fix. Good catch!

Reviewed-by: Geliang Tang <geliang@kernel.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