Re: [PATCH mptcp-next 03/11] mptcp: only reset subflow errors when propagated

[Geliang Tang <geliang@kernel.org>]

On Sun, 2026-01-04 at 11:30 +0800, Geliang Tang wrote:
> Hi Matt,
> 
> On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> > Some subflow socket errors need to be reported to the MPTCP socket:
> > the
> > initial subflow connect (MP_CAPABLE), and the ones from the
> > fallback
> > sockets. The others are not propagated.
> > 
> > The issue is that sock_error() was used to retrieve the error,
> > which
> > was
> > also resetting the sk_err field. Because of that, when notifying
> > the
> > userspace about subflow close events later on from the MPTCP
> > worker,
> > the
> > ssk->sk_err field was always 0.
> > 
> > Now, the error (sk_err) is only reset when propagating it to the
> > msk.
> > 
> > Fixes: 15cc10453398 ("mptcp: deliver ssk errors to msk")
> > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> > ---
> > Note: I guess we could also duplicate the error in
> > mptcp_subflow_context
> > struct before scheduling the worker, and use this field in
> > mptcp_event_put_token_and_ssk(), but I don't see why we should
> > always
> > reset ssk->sk_err in __mptcp_subflow_error_report() in all cases.
> > ---
> >  net/mptcp/protocol.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > index 1d68648b5194..5d6c987089c6 100644
> > --- a/net/mptcp/protocol.c
> > +++ b/net/mptcp/protocol.c
> > @@ -817,10 +817,9 @@ static bool __mptcp_ofo_queue(struct
> > mptcp_sock
> > *msk)
> >  
> >  static bool __mptcp_subflow_error_report(struct sock *sk, struct
> > sock *ssk)
> >  {
> > -	int err = sock_error(ssk);
> >  	int ssk_state;
> >  
> > -	if (!err)
> > +	if (!READ_ONCE(ssk->sk_err))
> >  		return false;
> 
> Your fix works. I'm wondering if swapping the order of 'if (sk-
> > sk_state != TCP_SYN_SENT && !__mptcp_check_fallback(mptcp_sk(sk)))'
> and 'if (!err)' would be a bit simpler, i.e.:
> 
> int err = sock_error(ssk);
> int ssk_state;
> 
> if (sk->sk_state != TCP_SYN_SENT &&
>     !__mptcp_check_fallback(mptcp_sk(sk)))
>         return false;
> 
> if (!err)
>         return false;

Sorry, I was mistaken. It should be like this:

int ssk_state;
int err;

if (sk->sk_state != TCP_SYN_SENT &&
    !__mptcp_check_fallback(mptcp_sk(sk)))
        return false;

err = sock_error(ssk);
if (!err)
        return false;

> 
> WDYT?
> 
> Thanks,
> -Geliang
> 
> >  
> >  	/* only propagate errors on fallen-back sockets or
> > @@ -837,7 +836,7 @@ static bool __mptcp_subflow_error_report(struct
> > sock *sk, struct sock *ssk)
> >  	ssk_state = inet_sk_state_load(ssk);
> >  	if (ssk_state == TCP_CLOSE && !sock_flag(sk, SOCK_DEAD))
> >  		mptcp_set_state(sk, ssk_state);
> > -	WRITE_ONCE(sk->sk_err, -err);
> > +	WRITE_ONCE(sk->sk_err, -sock_error(ssk));
> >  
> >  	/* This barrier is coupled with smp_rmb() in mptcp_poll()
> > */
> >  	smp_wmb();