From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6A810E80A9A for ; Wed, 27 Sep 2023 07:41:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230042AbjI0Hl1 (ORCPT ); Wed, 27 Sep 2023 03:41:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50708 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230020AbjI0HlX (ORCPT ); Wed, 27 Sep 2023 03:41:23 -0400 Received: from mx1.sberdevices.ru (mx1.sberdevices.ru [37.18.73.165]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 969D911D; Wed, 27 Sep 2023 00:41:20 -0700 (PDT) Received: from p-infra-ksmg-sc-msk01 (localhost [127.0.0.1]) by mx1.sberdevices.ru (Postfix) with ESMTP id 7E736100007; Wed, 27 Sep 2023 10:41:17 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.sberdevices.ru 7E736100007 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=salutedevices.com; s=mail; t=1695800477; bh=Za3rP9C9xGzI8+6GmtBw4W+jfaaaUYjD9zH0XW5gO14=; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type:From; b=u7WJT9mKPLXTeU4AxM51IMjFJcGPEIULzhy4BiiiMWETAcmBCHZzy+mCK0mgIzTrI 6jZPD9CqRFPIN2oWsp05jDilZFcvpbiSBS14IDjTnFsFb9RPcytrDLkcB/XNSI0wYE 0I4DsJju4wblRlSR6IaiiKllBonFIajU6BPZZayebNPk0uF3C7RQcAhVuuSFDJPl7Y AInP1+40Vhmf8cWwWR+6VNhWtIbLQ8oxW1636InWzOQ6OwBsSb0KMPg0cKZRoG9pbx K1PzIFwsfjqmt4TotuR0RPJdFAPRbnexWNMWWJHq8XFxXqktqMx0FxxSqLO0pj8h+l 3L+Epx46sHBMQ== 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; Wed, 27 Sep 2023 10:41:17 +0300 (MSK) Received: from [192.168.0.106] (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; Wed, 27 Sep 2023 10:41:16 +0300 Message-ID: <83498975-8947-4863-be11-a889b15a25a7@salutedevices.com> Date: Wed, 27 Sep 2023 10:34:15 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH net-next v1 02/12] vsock: read from socket's error queue Content-Language: en-US To: Stefano Garzarella CC: Stefan Hajnoczi , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , "Michael S. Tsirkin" , Jason Wang , Bobby Eshleman , , , , , , References: <20230922052428.4005676-1-avkrasnov@salutedevices.com> <20230922052428.4005676-3-avkrasnov@salutedevices.com> <3oys2ouhlkitsjx7q7utp7wkitnnl4kisl2r54wwa2addd644p@jzyu7ubfrcog> From: Arseniy Krasnov In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [100.64.160.123] X-ClientProxiedBy: p-i-exch-sc-m01.sberdevices.ru (172.16.192.107) 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: 180155 [Sep 27 2023] X-KSMG-AntiSpam-Version: 5.9.59.0 X-KSMG-AntiSpam-Envelope-From: avkrasnov@salutedevices.com 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: 534 534 808c2ea49f7195c68d40844e073217da4fa0d1e3, {Tracking_from_domain_doesnt_match_to}, 127.0.0.199:7.1.2;p-i-exch-sc-m01.sberdevices.ru:7.1.1,5.0.1;100.64.160.123:7.1.2;d41d8cd98f00b204e9800998ecf8427e.com:7.1.1;salutedevices.com:7.1.1, 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/09/27 04:23:00 #21991401 X-KSMG-AntiVirus-Status: Clean, skipped Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 27.09.2023 10:34, Stefano Garzarella wrote: > On Tue, Sep 26, 2023 at 10:36:58PM +0300, Arseniy Krasnov wrote: >> >> >> On 26.09.2023 15:55, Stefano Garzarella wrote: >>> On Fri, Sep 22, 2023 at 08:24:18AM +0300, Arseniy Krasnov wrote: >>>> This adds handling of MSG_ERRQUEUE input flag in receive call. This flag >>>> is used to read socket's error queue instead of data queue. Possible >>>> scenario of error queue usage is receiving completions for transmission >>>> with MSG_ZEROCOPY flag. This patch also adds new defines: 'SOL_VSOCK' >>>> and 'VSOCK_RECVERR'. >>>> >>>> Signed-off-by: Arseniy Krasnov >>>> --- >>>> Changelog: >>>> v5(big patchset) -> v1: >>>>  * R-b tag removed, due to added defines to 'include/uapi/linux/vsock.h'. >>>>    Both 'SOL_VSOCK' and 'VSOCK_RECVERR' are needed by userspace, so >>>>    they were placed to 'include/uapi/linux/vsock.h'. At the same time, >>>>    the same define for 'SOL_VSOCK' was placed to 'include/linux/socket.h'. >>>>    This is needed because this file contains SOL_XXX defines for different >>>>    types of socket, so it prevents situation when another new SOL_XXX >>>>    will use constant 287. >>>> >>>> include/linux/socket.h     | 1 + >>>> include/uapi/linux/vsock.h | 9 +++++++++ >>>> net/vmw_vsock/af_vsock.c   | 6 ++++++ >>>> 3 files changed, 16 insertions(+) >>>> create mode 100644 include/uapi/linux/vsock.h >>>> >>>> diff --git a/include/linux/socket.h b/include/linux/socket.h >>>> index 39b74d83c7c4..cfcb7e2c3813 100644 >>>> --- a/include/linux/socket.h >>>> +++ b/include/linux/socket.h >>>> @@ -383,6 +383,7 @@ struct ucred { >>>> #define SOL_MPTCP    284 >>>> #define SOL_MCTP    285 >>>> #define SOL_SMC        286 >>>> +#define SOL_VSOCK    287 >>>> >>>> /* IPX options */ >>>> #define IPX_TYPE    1 >>>> diff --git a/include/uapi/linux/vsock.h b/include/uapi/linux/vsock.h >>>> new file mode 100644 >>>> index 000000000000..b25c1347a3b8 >>>> --- /dev/null >>>> +++ b/include/uapi/linux/vsock.h >>> >>> We already have include/uapi/linux/vm_sockets.h >>> >>> Should we include these changes there instead of creating a new header? >>> >>>> @@ -0,0 +1,9 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ >>>> +#ifndef _UAPI_LINUX_VSOCK_H >>>> +#define _UAPI_LINUX_VSOCK_H >>>> + >>>> +#define SOL_VSOCK    287 >>> >>> Why we need to re-define this also here? >> >> Reason of this re-define is that SOL_VSOCK must be exported to userspace, so >> i place it to include/uapi/XXX. At the same time include/linux/socket.h contains >> constants for SOL_XXX and they goes sequentially in this file (e.g. one by one, >> each new value is +1 to the previous). So if I add SOL_VSOCK to include/uapi/XXX >> only, it is possible that someone will add new SOL_VERY_NEW_SOCKET == 287 to >> include/linux/socket.h in future. I think it is not good that two SOL_XXX will >> have same value. >> >> For example SOL_RDS and SOL_TIPS uses the same approach - there are two same defines: >> one in include/uapi/ and another is in include/linux/socket.h > > Okay, I was confused, I though socket.h was the uapi one. > If others do the same, it's fine. > > But why adding a new vsock.h instead of reusing vm_sockets.h? Yes, that's my mistake, I'll use vm_sockets.h. Seems I just forget about vm_sockets.h... > >> >>> >>> In that case, should we protect with some guards to avoid double >>> defines? >> >> May be: >> >> in include/linux/socket.h >> >> #ifndef SOL_VSOCK >> #define SOL_VSOCK 287 >> #endif >> >> But not sure... > > Nope, let's follow others definition. > > Sorry for the confusion ;-) No problem! Thanks, Arseniy > >> >>> >>>> + >>>> +#define VSOCK_RECVERR    1 >>>> + >>>> +#endif /* _UAPI_LINUX_VSOCK_H */ >>>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >>>> index d841f4de33b0..4fd11bf34bc7 100644 >>>> --- a/net/vmw_vsock/af_vsock.c >>>> +++ b/net/vmw_vsock/af_vsock.c >>>> @@ -110,6 +110,8 @@ >>>> #include >>>> #include >>>> #include >>>> +#include >>>> +#include >>>> >>>> static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr); >>>> static void vsock_sk_destruct(struct sock *sk); >>>> @@ -2137,6 +2139,10 @@ vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, >>>>     int err; >>>> >>>>     sk = sock->sk; >>>> + >>>> +    if (unlikely(flags & MSG_ERRQUEUE)) >>>> +        return sock_recv_errqueue(sk, msg, len, SOL_VSOCK, VSOCK_RECVERR); >>>> + >>>>     vsk = vsock_sk(sk); >>>>     err = 0; >>>> >>>> --  >>>> 2.25.1 >>>> >>> >> >