Re: [PATCH mptcp] mptcp: sockopt: make sync_socket_options propagate SOCK_KEEPOPEN

[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:
> >>>> 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