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();