From nobody Fri May 17 02:41:27 2024 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A99BDA939 for ; Wed, 23 Aug 2023 08:28:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1692779334; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=pEcTAIgQimIjgxx8oEynxFmf0wRPobmL9wC7HpqOL7w=; b=SWq8FSWR/GuafwQdUrAs3wJfyR1cGl45qXCr6u4RSA/VcrOdejzUjuqKI/IyZxgWhr0acM VwE8rcIG9qb9dvbVnM8Gy2jIIGrSnT7w7WpOwUeZ5pQlk7Dh2qv4jJuoVEoQcXrlJIiPg3 gKsN+RM7dpXrKxzISCj51VEKnciqlbo= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-606-MymwerveNS-BtJmI2yyxAA-1; Wed, 23 Aug 2023 04:28:52 -0400 X-MC-Unique: MymwerveNS-BtJmI2yyxAA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 09FB78015A8 for ; Wed, 23 Aug 2023 08:28:52 +0000 (UTC) Received: from gerbillo.redhat.com (unknown [10.45.224.138]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8E9A2492C14 for ; Wed, 23 Aug 2023 08:28:51 +0000 (UTC) From: Paolo Abeni To: mptcp@lists.linux.dev Subject: [PATCH v2 mptcp-net 1/2] mptcp: move __mptcp_error_report in protocol.c Date: Wed, 23 Aug 2023 10:28:40 +0200 Message-ID: In-Reply-To: References: Precedence: bulk X-Mailing-List: mptcp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8"; x-default="true" This will simplify the next patch. No functional change intended. Signed-off-by: Paolo Abeni Reviewed-by: Mat Martineau --- net/mptcp/protocol.c | 36 ++++++++++++++++++++++++++++++++++++ net/mptcp/subflow.c | 36 ------------------------------------ 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index ab3aaf6cacc5..12d3b575ceba 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -764,6 +764,42 @@ static bool __mptcp_ofo_queue(struct mptcp_sock *msk) return moved; } =20 +void __mptcp_error_report(struct sock *sk) +{ + struct mptcp_subflow_context *subflow; + struct mptcp_sock *msk =3D mptcp_sk(sk); + + mptcp_for_each_subflow(msk, subflow) { + struct sock *ssk =3D mptcp_subflow_tcp_sock(subflow); + int err =3D sock_error(ssk); + int ssk_state; + + if (!err) + continue; + + /* only propagate errors on fallen-back sockets or + * on MPC connect + */ + if (sk->sk_state !=3D TCP_SYN_SENT && !__mptcp_check_fallback(msk)) + continue; + + /* We need to propagate only transition to CLOSE state. + * Orphaned socket will see such state change via + * subflow_sched_work_if_closed() and that path will properly + * destroy the msk as needed. + */ + ssk_state =3D inet_sk_state_load(ssk); + if (ssk_state =3D=3D TCP_CLOSE && !sock_flag(sk, SOCK_DEAD)) + inet_sk_state_store(sk, ssk_state); + WRITE_ONCE(sk->sk_err, -err); + + /* This barrier is coupled with smp_rmb() in mptcp_poll() */ + smp_wmb(); + sk_error_report(sk); + break; + } +} + /* In most cases we will be able to lock the mptcp socket. If its already * owned, we need to defer to the work queue to avoid ABBA deadlock. */ diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 9bf3c7bc1762..2f40c23fdb0d 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -1362,42 +1362,6 @@ void mptcp_space(const struct sock *ssk, int *space,= int *full_space) *full_space =3D mptcp_win_from_space(sk, READ_ONCE(sk->sk_rcvbuf)); } =20 -void __mptcp_error_report(struct sock *sk) -{ - struct mptcp_subflow_context *subflow; - struct mptcp_sock *msk =3D mptcp_sk(sk); - - mptcp_for_each_subflow(msk, subflow) { - struct sock *ssk =3D mptcp_subflow_tcp_sock(subflow); - int err =3D sock_error(ssk); - int ssk_state; - - if (!err) - continue; - - /* only propagate errors on fallen-back sockets or - * on MPC connect - */ - if (sk->sk_state !=3D TCP_SYN_SENT && !__mptcp_check_fallback(msk)) - continue; - - /* We need to propagate only transition to CLOSE state. - * Orphaned socket will see such state change via - * subflow_sched_work_if_closed() and that path will properly - * destroy the msk as needed. - */ - ssk_state =3D inet_sk_state_load(ssk); - if (ssk_state =3D=3D TCP_CLOSE && !sock_flag(sk, SOCK_DEAD)) - inet_sk_state_store(sk, ssk_state); - WRITE_ONCE(sk->sk_err, -err); - - /* This barrier is coupled with smp_rmb() in mptcp_poll() */ - smp_wmb(); - sk_error_report(sk); - break; - } -} - static void subflow_error_report(struct sock *ssk) { struct sock *sk =3D mptcp_subflow_ctx(ssk)->conn; --=20 2.41.0 From nobody Fri May 17 02:41:27 2024 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A25F39461 for ; Wed, 23 Aug 2023 08:28:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1692779334; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=1cHa1MSyZ3x3xK0M8HiklDz1YxiHgu4Fahe4XTDx5Lg=; b=RU1h8Dib8q/16LqTlMq2IZuCa5945rrrLWau9MV9rmntTrSsys+1kf+zrTrinf8W2mlJn7 R1S71c1Xke/iSUAAQcukr0JdWgSqDWOhNmXADV7katF/g1VesS0+BuSM6YyzZk1Gp+pb7O 9oWMGs+gxhW7Nueto6TrTVoo6C3xelg= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-395-IwFm26h6NomemYbMdH73cA-1; Wed, 23 Aug 2023 04:28:53 -0400 X-MC-Unique: IwFm26h6NomemYbMdH73cA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id BF24D858EED for ; Wed, 23 Aug 2023 08:28:52 +0000 (UTC) Received: from gerbillo.redhat.com (unknown [10.45.224.138]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4F22E492C14 for ; Wed, 23 Aug 2023 08:28:52 +0000 (UTC) From: Paolo Abeni To: mptcp@lists.linux.dev Subject: [PATCH v2 mptcp-net 2/2] mptcp: process pending subflow error on close Date: Wed, 23 Aug 2023 10:28:41 +0200 Message-ID: In-Reply-To: References: Precedence: bulk X-Mailing-List: mptcp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8"; x-default="true" On incoming TCP reset, subflow closing could happen before error propagation. That in turn could cause the socket error being ignored, and a missing socket state transition, as reported by Daire-Byrne. Address the issues explicitly checking for subflow socket error at close time. To avoid code duplication, factor-out of __mptcp_error_report() a new helper implementing the relevant bits. Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/429 Fixes: 15cc10453398 ("mptcp: deliver ssk errors to msk") Signed-off-by: Paolo Abeni Reviewed-by: Mat Martineau --- v1 -> v2: - fix compile warning --- net/mptcp/protocol.c | 63 ++++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 29 deletions(-) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 12d3b575ceba..357030f40cef 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -764,40 +764,44 @@ static bool __mptcp_ofo_queue(struct mptcp_sock *msk) return moved; } =20 -void __mptcp_error_report(struct sock *sk) +static bool __mptcp_subflow_error_report(struct sock *sk, struct sock *ssk) { - struct mptcp_subflow_context *subflow; - struct mptcp_sock *msk =3D mptcp_sk(sk); + int err =3D sock_error(ssk); + int ssk_state; =20 - mptcp_for_each_subflow(msk, subflow) { - struct sock *ssk =3D mptcp_subflow_tcp_sock(subflow); - int err =3D sock_error(ssk); - int ssk_state; + if (!err) + return false; =20 - if (!err) - continue; + /* only propagate errors on fallen-back sockets or + * on MPC connect + */ + if (sk->sk_state !=3D TCP_SYN_SENT && !__mptcp_check_fallback(mptcp_sk(sk= ))) + return false; =20 - /* only propagate errors on fallen-back sockets or - * on MPC connect - */ - if (sk->sk_state !=3D TCP_SYN_SENT && !__mptcp_check_fallback(msk)) - continue; + /* We need to propagate only transition to CLOSE state. + * Orphaned socket will see such state change via + * subflow_sched_work_if_closed() and that path will properly + * destroy the msk as needed. + */ + ssk_state =3D inet_sk_state_load(ssk); + if (ssk_state =3D=3D TCP_CLOSE && !sock_flag(sk, SOCK_DEAD)) + inet_sk_state_store(sk, ssk_state); + WRITE_ONCE(sk->sk_err, -err); =20 - /* We need to propagate only transition to CLOSE state. - * Orphaned socket will see such state change via - * subflow_sched_work_if_closed() and that path will properly - * destroy the msk as needed. - */ - ssk_state =3D inet_sk_state_load(ssk); - if (ssk_state =3D=3D TCP_CLOSE && !sock_flag(sk, SOCK_DEAD)) - inet_sk_state_store(sk, ssk_state); - WRITE_ONCE(sk->sk_err, -err); - - /* This barrier is coupled with smp_rmb() in mptcp_poll() */ - smp_wmb(); - sk_error_report(sk); - break; - } + /* This barrier is coupled with smp_rmb() in mptcp_poll() */ + smp_wmb(); + sk_error_report(sk); + return true; +} + +void __mptcp_error_report(struct sock *sk) +{ + struct mptcp_subflow_context *subflow; + struct mptcp_sock *msk =3D mptcp_sk(sk); + + mptcp_for_each_subflow(msk, subflow) + if (__mptcp_subflow_error_report(sk, mptcp_subflow_tcp_sock(subflow))) + break; } =20 /* In most cases we will be able to lock the mptcp socket. If its already @@ -2422,6 +2426,7 @@ static void __mptcp_close_ssk(struct sock *sk, struct= sock *ssk, } =20 out_release: + __mptcp_subflow_error_report(sk, ssk); release_sock(ssk); =20 sock_put(ssk); --=20 2.41.0