Message ID | 20230801141727.481156-2-AVKrasnov@sberdevices.ru |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:918b:0:b0:3e4:2afc:c1 with SMTP id s11csp2828360vqg; Tue, 1 Aug 2023 10:38:37 -0700 (PDT) X-Google-Smtp-Source: APBJJlEXHD43zwlnzBu7MDAMRgVhpuK8zKfsLbCw1r2/c8MygjmnSJN/yUpKTRPd+hVYsUa5gNTb X-Received: by 2002:a05:651c:1022:b0:2b9:cf90:aba1 with SMTP id w2-20020a05651c102200b002b9cf90aba1mr2818201ljm.14.1690911517289; Tue, 01 Aug 2023 10:38:37 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690911517; cv=none; d=google.com; s=arc-20160816; b=atLwencmaUmB0Eu06kajzDZhn+eH6GtCBvFj/pbuPdeEd09I4lwZMpidTF53eSxRc1 EDCoCFsKLQjmDJr6pMSdoKogcmU4ZXwn+3ExxNoc8pztXn3Q7S1RybLvXaf1RhBl0BV4 19P3gwms2RC6kkUkaHLS5OCd9wHUzGkIGuZeHAMxTobnCgobULNQt6fS6ZaXV+s/uLiK XQJoiw9xM0p86q+BVa2Ezti5bnVKToeKzFD2ZzrTLFILDwSzaug+eSLhvIdw44sXJgDG zUn+Qnx8xfqSiPYhnaRxxnHnY9smDiX1e2EWCvo7SNOObNO6LiR77hEAAceUQGTm4mCy n96w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature:dkim-filter; bh=xqPpn0dnX0/HAUZejDb3RPtDZduneTW/LI1q4+pg6sk=; fh=IiLlUAYZQ4hWuQbi3V9KE00VdjFsw4tg4Y6JCUFRLn0=; b=QXPJd3CRdBdQ0ogtZ8OzEH4P6Odc76J64RQ79QECEda4y4tcZzqerK7gEDBlQ5Kb3I hBB+O7EsrX4nqh/K/HdFeQN61z/wQNeBiMSGGM5VwvY8mj+iB5Pt5+7iZ8y0mZ3Pr9jW lCG/tdLJlFb3pBLyTalLRtaebp2AeoARFTZXr1k4BqP+QXOTt/OnPmJsXcRAyH+U5GKp WmQNKb8F+XpvfVBLtySNgw8mLnGYrM9jtwbfwiCvLkt0WSYUPjSLsmV5IIyrUdiHnclL Tp6mDtQNDqaoNGXs33tjhj/FPk88hIeFsI3P0erlbGjfmG5q4PvMrhVHY2MxMMqH6ZqS mbdA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sberdevices.ru header.s=mail header.b=Ljp4qYVR; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=sberdevices.ru Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id gq25-20020a170906e25900b0099bcf2371d3si8712721ejb.926.2023.08.01.10.38.13; Tue, 01 Aug 2023 10:38:37 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@sberdevices.ru header.s=mail header.b=Ljp4qYVR; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=sberdevices.ru Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233913AbjHAOXq (ORCPT <rfc822;maxi.paulin@gmail.com> + 99 others); Tue, 1 Aug 2023 10:23:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45758 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232408AbjHAOXe (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 1 Aug 2023 10:23:34 -0400 Received: from mx1.sberdevices.ru (mx1.sberdevices.ru [37.18.73.165]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9D422125; Tue, 1 Aug 2023 07:23:32 -0700 (PDT) Received: from p-infra-ksmg-sc-msk01 (localhost [127.0.0.1]) by mx1.sberdevices.ru (Postfix) with ESMTP id 1558C10001D; Tue, 1 Aug 2023 17:23:31 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.sberdevices.ru 1558C10001D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sberdevices.ru; s=mail; t=1690899811; bh=xqPpn0dnX0/HAUZejDb3RPtDZduneTW/LI1q4+pg6sk=; h=From:To:Subject:Date:Message-ID:MIME-Version:Content-Type:From; b=Ljp4qYVR+jUoJ96zP19ctHH+kORhoGppE+THWG/ieHOHMwt/ml84TOV8FQDs4A/zn 0CjhUHIzdrC2BITusHo+W6a6EaVCLwURg/hJy2SSXGkw7CEppEaWACz5shhvtrXjle 1tF7oWxG74LZXt2OjBkpfrusf+aPXjNoZM0rdSrSQ0P9IvlO+y1W60ioDd4kjD7SwH R2YxZrhVpUL0AQYPeO7OI98W3PsAwrJYDRKWjIxS1UVVXGPBAwN1HcDJ9z1YNQWdSq Inwwydr4VnRjgegdBFaDgpTM/AxbTA+rGISHcExEXm2b3FOmQ0BG5IoO5p4vgl0z7y MwuwSVn5AYooQ== Received: from p-i-exch-sc-m01.sberdevices.ru (p-i-exch-sc-m01.sberdevices.ru [172.16.192.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.sberdevices.ru (Postfix) with ESMTPS; Tue, 1 Aug 2023 17:23:30 +0300 (MSK) Received: from localhost.localdomain (100.64.160.123) by p-i-exch-sc-m01.sberdevices.ru (172.16.192.107) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.30; Tue, 1 Aug 2023 17:23:27 +0300 From: Arseniy Krasnov <AVKrasnov@sberdevices.ru> To: Stefan Hajnoczi <stefanha@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>, Bobby Eshleman <bobby.eshleman@bytedance.com> CC: <kvm@vger.kernel.org>, <virtualization@lists.linux-foundation.org>, <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <kernel@sberdevices.ru>, <oxffffaa@gmail.com>, <avkrasnov@sberdevices.ru>, Arseniy Krasnov <AVKrasnov@sberdevices.ru> Subject: [RFC PATCH v1 1/2] vsock: send SIGPIPE on write to shutdowned socket Date: Tue, 1 Aug 2023 17:17:26 +0300 Message-ID: <20230801141727.481156-2-AVKrasnov@sberdevices.ru> X-Mailer: git-send-email 2.35.0 In-Reply-To: <20230801141727.481156-1-AVKrasnov@sberdevices.ru> References: <20230801141727.481156-1-AVKrasnov@sberdevices.ru> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [100.64.160.123] X-ClientProxiedBy: p-i-exch-sc-m02.sberdevices.ru (172.16.192.103) To p-i-exch-sc-m01.sberdevices.ru (172.16.192.107) X-KSMG-Rule-ID: 10 X-KSMG-Message-Action: clean X-KSMG-AntiSpam-Lua-Profiles: 178796 [Jul 22 2023] X-KSMG-AntiSpam-Version: 5.9.59.0 X-KSMG-AntiSpam-Envelope-From: AVKrasnov@sberdevices.ru X-KSMG-AntiSpam-Rate: 0 X-KSMG-AntiSpam-Status: not_detected X-KSMG-AntiSpam-Method: none X-KSMG-AntiSpam-Auth: dkim=none X-KSMG-AntiSpam-Info: LuaCore: 525 525 723604743bfbdb7e16728748c3fa45e9eba05f7d, {Tracking_from_domain_doesnt_match_to}, FromAlignment: s, ApMailHostAddress: 100.64.160.123 X-MS-Exchange-Organization-SCL: -1 X-KSMG-AntiSpam-Interceptor-Info: scan successful X-KSMG-AntiPhishing: Clean X-KSMG-LinksScanning: Clean X-KSMG-AntiVirus: Kaspersky Secure Mail Gateway, version 2.0.1.6960, bases: 2023/07/23 08:49:00 #21663637 X-KSMG-AntiVirus-Status: Clean, skipped X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773049235226055266 X-GMAIL-MSGID: 1773049235226055266 |
Series |
vsock: handle writes to shutdowned socket
|
|
Commit Message
Arseniy Krasnov
Aug. 1, 2023, 2:17 p.m. UTC
POSIX requires to send SIGPIPE on write to SOCK_STREAM socket which was
shutdowned with SHUT_WR flag or its peer was shutdowned with SHUT_RD
flag. Also we must not send SIGPIPE if MSG_NOSIGNAL flag is set.
Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
---
net/vmw_vsock/af_vsock.c | 3 +++
1 file changed, 3 insertions(+)
Comments
On Tue, Aug 01, 2023 at 05:17:26PM +0300, Arseniy Krasnov wrote: >POSIX requires to send SIGPIPE on write to SOCK_STREAM socket which was >shutdowned with SHUT_WR flag or its peer was shutdowned with SHUT_RD >flag. Also we must not send SIGPIPE if MSG_NOSIGNAL flag is set. > >Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> >--- > net/vmw_vsock/af_vsock.c | 3 +++ > 1 file changed, 3 insertions(+) > >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >index 020cf17ab7e4..013b65241b65 100644 >--- a/net/vmw_vsock/af_vsock.c >+++ b/net/vmw_vsock/af_vsock.c >@@ -1921,6 +1921,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg, > err = total_written; > } > out: >+ if (sk->sk_type == SOCK_STREAM) >+ err = sk_stream_error(sk, msg->msg_flags, err); Do you know why we don't need this for SOCK_SEQPACKET and SOCK_DGRAM? Thanks, Stefano >+ > release_sock(sk); > return err; > } >-- >2.25.1 >
Hi Stefano, On 02.08.2023 10:46, Stefano Garzarella wrote: > On Tue, Aug 01, 2023 at 05:17:26PM +0300, Arseniy Krasnov wrote: >> POSIX requires to send SIGPIPE on write to SOCK_STREAM socket which was >> shutdowned with SHUT_WR flag or its peer was shutdowned with SHUT_RD >> flag. Also we must not send SIGPIPE if MSG_NOSIGNAL flag is set. >> >> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> >> --- >> net/vmw_vsock/af_vsock.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >> index 020cf17ab7e4..013b65241b65 100644 >> --- a/net/vmw_vsock/af_vsock.c >> +++ b/net/vmw_vsock/af_vsock.c >> @@ -1921,6 +1921,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg, >> err = total_written; >> } >> out: >> + if (sk->sk_type == SOCK_STREAM) >> + err = sk_stream_error(sk, msg->msg_flags, err); > > Do you know why we don't need this for SOCK_SEQPACKET and SOCK_DGRAM? Yes, here is my explanation: This function checks that input error is SIGPIPE, and if so it sends SIGPIPE to the 'current' thread (except case when MSG_NOSIGNAL flag is set). This behaviour is described in POSIX: Page 367 (description of defines from sys/socket.h): MSG_NOSIGNAL: No SIGPIPE generated when an attempt to send is made on a stream- oriented socket that is no longer connected. Page 497 (description of SOCK_STREAM): A SIGPIPE signal is raised if a thread sends on a broken stream (one that is no longer connected). Page 1802 (description of 'send()' call): MSG_NOSIGNAL Requests not to send the SIGPIPE signal if an attempt to send is made on a stream-oriented socket that is no longer connected. The [EPIPE] error shall still be returned And the same for 'sendto()' and 'sendmsg()' Link to the POSIX document: https://www.open-std.org/jtc1/sc22/open/n4217.pdf TCP (I think we must rely on it), KCM, SMC sockets (all of them are stream) work in the same way by calling this function. AF_UNIX also works in the same way, but it implements SIGPIPE handling without this function. The only thing that confused me a little bit, that sockets above returns EPIPE when we have only SEND_SHUTDOWN set, but for AF_VSOCK EPIPE is returned for RCV_SHUTDOWN also, but I think it is related to this patchset. Thanks, Arseniy > > Thanks, > Stefano > >> + >> release_sock(sk); >> return err; >> } >> -- >> 2.25.1 >> >
On Fri, Aug 04, 2023 at 03:46:47PM +0300, Arseniy Krasnov wrote: >Hi Stefano, > >On 02.08.2023 10:46, Stefano Garzarella wrote: >> On Tue, Aug 01, 2023 at 05:17:26PM +0300, Arseniy Krasnov wrote: >>> POSIX requires to send SIGPIPE on write to SOCK_STREAM socket which was >>> shutdowned with SHUT_WR flag or its peer was shutdowned with SHUT_RD >>> flag. Also we must not send SIGPIPE if MSG_NOSIGNAL flag is set. >>> >>> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> >>> --- >>> net/vmw_vsock/af_vsock.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >>> index 020cf17ab7e4..013b65241b65 100644 >>> --- a/net/vmw_vsock/af_vsock.c >>> +++ b/net/vmw_vsock/af_vsock.c >>> @@ -1921,6 +1921,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg, >>> err = total_written; >>> } >>> out: >>> + if (sk->sk_type == SOCK_STREAM) >>> + err = sk_stream_error(sk, msg->msg_flags, err); >> >> Do you know why we don't need this for SOCK_SEQPACKET and SOCK_DGRAM? > >Yes, here is my explanation: > >This function checks that input error is SIGPIPE, and if so it sends SIGPIPE to the 'current' thread >(except case when MSG_NOSIGNAL flag is set). This behaviour is described in POSIX: > >Page 367 (description of defines from sys/socket.h): >MSG_NOSIGNAL: No SIGPIPE generated when an attempt to send is made on a stream- >oriented socket that is no longer connected. > >Page 497 (description of SOCK_STREAM): >A SIGPIPE signal is raised if a thread sends on a broken stream (one that is >no longer connected). Okay, but I think we should do also for SEQPACKET: https://pubs.opengroup.org/onlinepubs/009696699/functions/xsh_chap02_10.html In 2.10.6 Socket Types: "The SOCK_SEQPACKET socket type is similar to the SOCK_STREAM type, and is also connection-oriented. The only difference between these types is that record boundaries ..." Then in 2.10.14 Signals: "The SIGPIPE signal shall be sent to a thread that attempts to send data on a socket that is no longer able to send. In addition, the send operation fails with the error [EPIPE]." It's honestly not super clear, but I assume the problem is similar with seqpacket since it's connection-oriented, or did I miss something? For example in sctp_sendmsg() IIUC we raise a SIGPIPE regardless of whether the socket is STREAM or SEQPACKET. > >Page 1802 (description of 'send()' call): >MSG_NOSIGNAL > >Requests not to send the SIGPIPE signal if an attempt to >send is made on a stream-oriented socket that is no >longer connected. The [EPIPE] error shall still be >returned > >And the same for 'sendto()' and 'sendmsg()' > >Link to the POSIX document: >https://www.open-std.org/jtc1/sc22/open/n4217.pdf > >TCP (I think we must rely on it), KCM, SMC sockets (all of them are stream) work in the same >way by calling this function. AF_UNIX also works in the same way, but it implements SIGPIPE handling >without this function. I'm okay calling this function. > >The only thing that confused me a little bit, that sockets above returns EPIPE when >we have only SEND_SHUTDOWN set, but for AF_VSOCK EPIPE is returned for RCV_SHUTDOWN >also, but I think it is related to this patchset. Do you mean that it is NOT related to this patchset? Thanks, Stefano
On 04.08.2023 17:28, Stefano Garzarella wrote: > On Fri, Aug 04, 2023 at 03:46:47PM +0300, Arseniy Krasnov wrote: >> Hi Stefano, >> >> On 02.08.2023 10:46, Stefano Garzarella wrote: >>> On Tue, Aug 01, 2023 at 05:17:26PM +0300, Arseniy Krasnov wrote: >>>> POSIX requires to send SIGPIPE on write to SOCK_STREAM socket which was >>>> shutdowned with SHUT_WR flag or its peer was shutdowned with SHUT_RD >>>> flag. Also we must not send SIGPIPE if MSG_NOSIGNAL flag is set. >>>> >>>> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> >>>> --- >>>> net/vmw_vsock/af_vsock.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >>>> index 020cf17ab7e4..013b65241b65 100644 >>>> --- a/net/vmw_vsock/af_vsock.c >>>> +++ b/net/vmw_vsock/af_vsock.c >>>> @@ -1921,6 +1921,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg, >>>> err = total_written; >>>> } >>>> out: >>>> + if (sk->sk_type == SOCK_STREAM) >>>> + err = sk_stream_error(sk, msg->msg_flags, err); >>> >>> Do you know why we don't need this for SOCK_SEQPACKET and SOCK_DGRAM? >> >> Yes, here is my explanation: >> >> This function checks that input error is SIGPIPE, and if so it sends SIGPIPE to the 'current' thread >> (except case when MSG_NOSIGNAL flag is set). This behaviour is described in POSIX: >> >> Page 367 (description of defines from sys/socket.h): >> MSG_NOSIGNAL: No SIGPIPE generated when an attempt to send is made on a stream- >> oriented socket that is no longer connected. >> >> Page 497 (description of SOCK_STREAM): >> A SIGPIPE signal is raised if a thread sends on a broken stream (one that is >> no longer connected). > > Okay, but I think we should do also for SEQPACKET: > > https://pubs.opengroup.org/onlinepubs/009696699/functions/xsh_chap02_10.html > > In 2.10.6 Socket Types: > > "The SOCK_SEQPACKET socket type is similar to the SOCK_STREAM type, and > is also connection-oriented. The only difference between these types is > that record boundaries ..." > > Then in 2.10.14 Signals: > > "The SIGPIPE signal shall be sent to a thread that attempts to send data > on a socket that is no longer able to send. In addition, the send > operation fails with the error [EPIPE]." > > It's honestly not super clear, but I assume the problem is similar with > seqpacket since it's connection-oriented, or did I miss something? > > For example in sctp_sendmsg() IIUC we raise a SIGPIPE regardless of > whether the socket is STREAM or SEQPACKET. Hm, yes, you're right. Seems check for socket type is not needed in this case, as this function is only for connection oriented sockets. > >> >> Page 1802 (description of 'send()' call): >> MSG_NOSIGNAL >> >> Requests not to send the SIGPIPE signal if an attempt to >> send is made on a stream-oriented socket that is no >> longer connected. The [EPIPE] error shall still be >> returned >> >> And the same for 'sendto()' and 'sendmsg()' >> >> Link to the POSIX document: >> https://www.open-std.org/jtc1/sc22/open/n4217.pdf >> >> TCP (I think we must rely on it), KCM, SMC sockets (all of them are stream) work in the same >> way by calling this function. AF_UNIX also works in the same way, but it implements SIGPIPE handling >> without this function. > > I'm okay calling this function. > >> >> The only thing that confused me a little bit, that sockets above returns EPIPE when >> we have only SEND_SHUTDOWN set, but for AF_VSOCK EPIPE is returned for RCV_SHUTDOWN >> also, but I think it is related to this patchset. > > Do you mean that it is NOT related to this patchset? Yes, **NOT** > > Thanks, > Stefano > Thanks, Arseniy
On Fri, Aug 04, 2023 at 05:34:20PM +0300, Arseniy Krasnov wrote: > > >On 04.08.2023 17:28, Stefano Garzarella wrote: >> On Fri, Aug 04, 2023 at 03:46:47PM +0300, Arseniy Krasnov wrote: >>> Hi Stefano, >>> >>> On 02.08.2023 10:46, Stefano Garzarella wrote: >>>> On Tue, Aug 01, 2023 at 05:17:26PM +0300, Arseniy Krasnov wrote: >>>>> POSIX requires to send SIGPIPE on write to SOCK_STREAM socket which was >>>>> shutdowned with SHUT_WR flag or its peer was shutdowned with SHUT_RD >>>>> flag. Also we must not send SIGPIPE if MSG_NOSIGNAL flag is set. >>>>> >>>>> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> >>>>> --- >>>>> net/vmw_vsock/af_vsock.c | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >>>>> index 020cf17ab7e4..013b65241b65 100644 >>>>> --- a/net/vmw_vsock/af_vsock.c >>>>> +++ b/net/vmw_vsock/af_vsock.c >>>>> @@ -1921,6 +1921,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg, >>>>> err = total_written; >>>>> } >>>>> out: >>>>> + if (sk->sk_type == SOCK_STREAM) >>>>> + err = sk_stream_error(sk, msg->msg_flags, err); >>>> >>>> Do you know why we don't need this for SOCK_SEQPACKET and SOCK_DGRAM? >>> >>> Yes, here is my explanation: >>> >>> This function checks that input error is SIGPIPE, and if so it sends SIGPIPE to the 'current' thread >>> (except case when MSG_NOSIGNAL flag is set). This behaviour is described in POSIX: >>> >>> Page 367 (description of defines from sys/socket.h): >>> MSG_NOSIGNAL: No SIGPIPE generated when an attempt to send is made on a stream- >>> oriented socket that is no longer connected. >>> >>> Page 497 (description of SOCK_STREAM): >>> A SIGPIPE signal is raised if a thread sends on a broken stream (one that is >>> no longer connected). >> >> Okay, but I think we should do also for SEQPACKET: >> >> https://pubs.opengroup.org/onlinepubs/009696699/functions/xsh_chap02_10.html >> >> In 2.10.6 Socket Types: >> >> "The SOCK_SEQPACKET socket type is similar to the SOCK_STREAM type, and >> is also connection-oriented. The only difference between these types is >> that record boundaries ..." >> >> Then in 2.10.14 Signals: >> >> "The SIGPIPE signal shall be sent to a thread that attempts to send data >> on a socket that is no longer able to send. In addition, the send >> operation fails with the error [EPIPE]." >> >> It's honestly not super clear, but I assume the problem is similar with >> seqpacket since it's connection-oriented, or did I miss something? >> >> For example in sctp_sendmsg() IIUC we raise a SIGPIPE regardless of >> whether the socket is STREAM or SEQPACKET. > >Hm, yes, you're right. Seems check for socket type is not needed in this case, >as this function is only for connection oriented sockets. Ack! > >> >>> >>> Page 1802 (description of 'send()' call): >>> MSG_NOSIGNAL >>> >>> Requests not to send the SIGPIPE signal if an attempt to >>> send is made on a stream-oriented socket that is no >>> longer connected. The [EPIPE] error shall still be >>> returned >>> >>> And the same for 'sendto()' and 'sendmsg()' >>> >>> Link to the POSIX document: >>> https://www.open-std.org/jtc1/sc22/open/n4217.pdf >>> >>> TCP (I think we must rely on it), KCM, SMC sockets (all of them are stream) work in the same >>> way by calling this function. AF_UNIX also works in the same way, but it implements SIGPIPE handling >>> without this function. >> >> I'm okay calling this function. >> >>> >>> The only thing that confused me a little bit, that sockets above returns EPIPE when >>> we have only SEND_SHUTDOWN set, but for AF_VSOCK EPIPE is returned for RCV_SHUTDOWN >>> also, but I think it is related to this patchset. >> >> Do you mean that it is NOT related to this patchset? > >Yes, **NOT** Got it, so if you have time when you're back, let's check also that (not for this series as you mentioned). Thanks, Stefano
On 04.08.2023 18:02, Stefano Garzarella wrote: > On Fri, Aug 04, 2023 at 05:34:20PM +0300, Arseniy Krasnov wrote: >> >> >> On 04.08.2023 17:28, Stefano Garzarella wrote: >>> On Fri, Aug 04, 2023 at 03:46:47PM +0300, Arseniy Krasnov wrote: >>>> Hi Stefano, >>>> >>>> On 02.08.2023 10:46, Stefano Garzarella wrote: >>>>> On Tue, Aug 01, 2023 at 05:17:26PM +0300, Arseniy Krasnov wrote: >>>>>> POSIX requires to send SIGPIPE on write to SOCK_STREAM socket which was >>>>>> shutdowned with SHUT_WR flag or its peer was shutdowned with SHUT_RD >>>>>> flag. Also we must not send SIGPIPE if MSG_NOSIGNAL flag is set. >>>>>> >>>>>> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> >>>>>> --- >>>>>> net/vmw_vsock/af_vsock.c | 3 +++ >>>>>> 1 file changed, 3 insertions(+) >>>>>> >>>>>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >>>>>> index 020cf17ab7e4..013b65241b65 100644 >>>>>> --- a/net/vmw_vsock/af_vsock.c >>>>>> +++ b/net/vmw_vsock/af_vsock.c >>>>>> @@ -1921,6 +1921,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg, >>>>>> err = total_written; >>>>>> } >>>>>> out: >>>>>> + if (sk->sk_type == SOCK_STREAM) >>>>>> + err = sk_stream_error(sk, msg->msg_flags, err); >>>>> >>>>> Do you know why we don't need this for SOCK_SEQPACKET and SOCK_DGRAM? >>>> >>>> Yes, here is my explanation: >>>> >>>> This function checks that input error is SIGPIPE, and if so it sends SIGPIPE to the 'current' thread >>>> (except case when MSG_NOSIGNAL flag is set). This behaviour is described in POSIX: >>>> >>>> Page 367 (description of defines from sys/socket.h): >>>> MSG_NOSIGNAL: No SIGPIPE generated when an attempt to send is made on a stream- >>>> oriented socket that is no longer connected. >>>> >>>> Page 497 (description of SOCK_STREAM): >>>> A SIGPIPE signal is raised if a thread sends on a broken stream (one that is >>>> no longer connected). >>> >>> Okay, but I think we should do also for SEQPACKET: >>> >>> https://pubs.opengroup.org/onlinepubs/009696699/functions/xsh_chap02_10.html >>> >>> In 2.10.6 Socket Types: >>> >>> "The SOCK_SEQPACKET socket type is similar to the SOCK_STREAM type, and >>> is also connection-oriented. The only difference between these types is >>> that record boundaries ..." >>> >>> Then in 2.10.14 Signals: >>> >>> "The SIGPIPE signal shall be sent to a thread that attempts to send data >>> on a socket that is no longer able to send. In addition, the send >>> operation fails with the error [EPIPE]." >>> >>> It's honestly not super clear, but I assume the problem is similar with >>> seqpacket since it's connection-oriented, or did I miss something? >>> >>> For example in sctp_sendmsg() IIUC we raise a SIGPIPE regardless of >>> whether the socket is STREAM or SEQPACKET. >> >> Hm, yes, you're right. Seems check for socket type is not needed in this case, >> as this function is only for connection oriented sockets. > > Ack! > >> >>> >>>> >>>> Page 1802 (description of 'send()' call): >>>> MSG_NOSIGNAL >>>> >>>> Requests not to send the SIGPIPE signal if an attempt to >>>> send is made on a stream-oriented socket that is no >>>> longer connected. The [EPIPE] error shall still be >>>> returned >>>> >>>> And the same for 'sendto()' and 'sendmsg()' >>>> >>>> Link to the POSIX document: >>>> https://www.open-std.org/jtc1/sc22/open/n4217.pdf >>>> >>>> TCP (I think we must rely on it), KCM, SMC sockets (all of them are stream) work in the same >>>> way by calling this function. AF_UNIX also works in the same way, but it implements SIGPIPE handling >>>> without this function. >>> >>> I'm okay calling this function. >>> >>>> >>>> The only thing that confused me a little bit, that sockets above returns EPIPE when >>>> we have only SEND_SHUTDOWN set, but for AF_VSOCK EPIPE is returned for RCV_SHUTDOWN >>>> also, but I think it is related to this patchset. >>> >>> Do you mean that it is NOT related to this patchset? >> >> Yes, **NOT** > > Got it, so if you have time when you're back, let's check also that > (not for this series as you mentioned). Sure! Thanks, Arseniy > > Thanks, > Stefano >
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index 020cf17ab7e4..013b65241b65 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -1921,6 +1921,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg, err = total_written; } out: + if (sk->sk_type == SOCK_STREAM) + err = sk_stream_error(sk, msg->msg_flags, err); + release_sock(sk); return err; }