From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id 6484D1B450 for ; Fri, 12 Oct 2018 11:50:42 +0200 (CEST) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20181012095040euoutp027669770c5d9ea103e6b00714f696f62d~c05xGNVEe0734107341euoutp024 for ; Fri, 12 Oct 2018 09:50:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20181012095040euoutp027669770c5d9ea103e6b00714f696f62d~c05xGNVEe0734107341euoutp024 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1539337840; bh=3nkTACp7+GSl0QNuEtxH96MZPXTmEo/fAuQXF4YuFb8=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=E6c2cqfl7LtEquSJJizMBDFJUIS5O8Pm6ESpHgK+f8dIWaV0DLudMxaKZkXedRcyl AuJRRt0iWz6paeVmn+QAgNpS/mSX1sKIxtlrEb9X+FH6WKHiNdJe7rI33ypFXQuS8u pJaUXKMFF1bTsxuwlHbDGCF5ft4h36s9rx/IFEnE= Received: from eusmges1new.samsung.com (unknown [203.254.199.242]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20181012095040eucas1p1454b8df5815f72e79f77396a66fd59a2~c05wi8kvr3029530295eucas1p1x; Fri, 12 Oct 2018 09:50:40 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges1new.samsung.com (EUCPMTA) with SMTP id 06.5D.04441.F6E60CB5; Fri, 12 Oct 2018 10:50:39 +0100 (BST) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20181012095039eucas1p12ad20f88162a3e0eed0464d05acc8a08~c05vvgLoP1463314633eucas1p1l; Fri, 12 Oct 2018 09:50:39 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20181012095039eusmtrp29ff5e6566133fdb3e897edc010fcf79b~c05vgTV192969929699eusmtrp2a; Fri, 12 Oct 2018 09:50:39 +0000 (GMT) X-AuditID: cbfec7f2-a1ae89c000001159-ec-5bc06e6f01ae Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 99.E3.04128.E6E60CB5; Fri, 12 Oct 2018 10:50:39 +0100 (BST) Received: from [106.109.129.180] (unknown [106.109.129.180]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20181012095038eusmtip152db95d2351c5272ed448142654ca8b7~c05uye9cG2189021890eusmtip1t; Fri, 12 Oct 2018 09:50:38 +0000 (GMT) To: Maxime Coquelin , dev@dpdk.org, tiwei.bie@intel.com, zhihong.wang@intel.com, jfreimann@redhat.com, nicknickolaev@gmail.com, bruce.richardson@intel.com, alejandro.lucero@netronome.com, dgilbert@redhat.com Cc: stable@dpdk.org From: Ilya Maximets Date: Fri, 12 Oct 2018 12:53:06 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <72d63d93-ba13-d228-d67a-0bbdbebeceaf@redhat.com> Content-Language: en-GB Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA01Se0hTcRjld+/ddn1MrtPww4JoFJTkCxQuEWplOSIookAapEuvj9Ipm1pG 0kpNW/OBf6TOMh84y0zJ9wtT0WmKK01NDR89EObIaDrU8pHbneR/5zvnfL/zHfiRuKCP40pG SxMYmVQSI+TaEk3adZ17nLRL7LW56kvrjBqMnqzyp5eMzRid1TTLo8cy1nm0rrqCoLWPOwh6 qHSCoLcyN3h0o2Ibo+tH8rAAO9GfkgqOqFU9wxOVd+gxUf6Tr7joV+c4V5TdUIUuca/Zngxn YqKTGJmnX6htVEGqlhuv97xTvj3IVaCRo0pkQwLlA70bhRwlsiUF1EsEfYZPBDusIFBlpVuH ZQQGbQG2uzJVtsllhUoE35+qrK7fCMbS53Gzy4liIG1EjZkFZ7PQNmBEZgGnnCCjd4IwYy51 HAZf91p4gjoCzYouS8Q+Khh650stPJ9yhPeFPyx+G8oP1lQanH3HBR6uvOKw+CCkNhbh5jCg RnmQlbOOs8tJoMxTEezdgTBZNoOz2AkW+xt4LD4A260vrN3uw1yaHrEPZSLI79myCv7QYNDt LJA7acegts2TpU/Bm+4xzEwD5QCTPx3ZexwgrykfZ2k+ZD4SsO7D8Le70nqBK0wtLfNykVC9 p6V6TzP1nmbq/7kliKhCLkyiPDaSkXtLmdseckmsPFEa6REWF1uHdj7X0Fa/sQWZRm/0IIpE Qnt+Sc47sYAjSZInx/YgIHGhM7/kZpdYwA+XJN9lZHEhssQYRt6D9pOE0IWvKX4rFlCRkgTm FsPEM7JdFSNtXBWow27V4DXpg0KUTaB2NUUoxWvaheGkGYW9z+K96x+mvC6HDaiFpzVXz7We OWGo0Vfn8k0Jz4pNcymhV3KnLy6cPxTe/jGqSDZkV64Kms3rnHX8cuHbcEra2UDNgtvnlroA 0DLN097Z7o2+znZdBcEBxkXjg+f1NRG1Fe5B7eMOQkIeJfF2w2VyyT+cYkp/WAMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrBIsWRmVeSWpSXmKPExsVy+t/xu7r5eQeiDVZNZrQ492kZk8WNVfYW 7z5tZ7Lo3XaP3eJK+092i3NrlrJYHOvcw2JxeuE1Fot/HX/YLbY2/Gey2HxxEpMDt8evBUtZ PXbOusvusXjPSyaP6d0PmT3e77vK5tG3ZRVjAFuUnk1RfmlJqkJGfnGJrVK0oYWRnqGlhZ6R iaWeobF5rJWRqZK+nU1Kak5mWWqRvl2CXsaM5mNsBS/1Kxb/P8XWwHhRo4uRk0NCwETi5qK/ bF2MXBxCAksZJfomfGaHSEhJ/Ph1gRXCFpb4c60Lqug9o8SSPceYQBLCAqkSLRdnMYEkRAQ+ Mkq8PLSQGSTBDNTRfuQaC0THJWaJbfuOgSXYBHQkTq0+wghi8wrYSfzufwNmswioSmxvOAA2 VVQgQmL18hesEDWCEidnPmEBsTmB6n/0LINaoC7xZ94lKFtcounLSlYIW16ieets5gmMQrOQ tM9C0jILScssJC0LGFlWMYqklhbnpucWG+kVJ+YWl+al6yXn525iBMbqtmM/t+xg7HoXfIhR gINRiYf3x8T90UKsiWXFlbmHGCU4mJVEeBdkHYgW4k1JrKxKLcqPLyrNSS0+xGgK9NxEZinR 5HxgGskriTc0NTS3sDQ0NzY3NrNQEuc9b1AZJSSQnliSmp2aWpBaBNPHxMEp1cDIw6X/y5Xj wLOPgROdLjeHOKieNjkusPD3tXS5Zj6RmzUHonUcj1RNOL74gL+Mw+525wvv+7YGrVAvm3Vn v3pTWNTUBQv/KC75fHnVrMPn57MmORn/FKln353Xt+1UVu2pKbvY7U9I9UXt1+nbw7jv8dUT 0rPEVxlMurGuPY+zuPX8wde3UnW6lFiKMxINtZiLihMBQNF1WusCAAA= Message-Id: <20181012095039eucas1p12ad20f88162a3e0eed0464d05acc8a08~c05vvgLoP1463314633eucas1p1l@eucas1p1.samsung.com> X-CMS-MailID: 20181012095039eucas1p12ad20f88162a3e0eed0464d05acc8a08 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20181011092527epcas3p4fd03af04db4d68c5c094521cf509e26c X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20181011092527epcas3p4fd03af04db4d68c5c094521cf509e26c References: <20181011092432.22275-1-maxime.coquelin@redhat.com> <20181011092432.22275-8-maxime.coquelin@redhat.com> <20181011155709eucas1p20da3d82e3f74a0f8b8bd06eae4baa0c5~cmQddRjzg2527125271eucas1p2_@eucas1p2.samsung.com> <377e0ee1-562d-e5cb-e411-e041f8c60ba5@redhat.com> <7df5b8ea-6570-e2ab-1983-e74d316e530d@redhat.com> <72d63d93-ba13-d228-d67a-0bbdbebeceaf@redhat.com> Subject: Re: [dpdk-dev] [PATCH v6 07/19] vhost: add number of fds to vhost-user messages and use it X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Oct 2018 09:50:42 -0000 On 12.10.2018 11:57, Maxime Coquelin wrote: > > > On 10/12/2018 10:45 AM, Maxime Coquelin wrote: >> >> >> On 10/12/2018 10:43 AM, Maxime Coquelin wrote: >>> >>> >>> On 10/11/2018 05:59 PM, Ilya Maximets wrote: >>>> On 11.10.2018 12:24, Maxime Coquelin wrote: >>>>> As soon as some ancillary data (fds) are received, it is copied >>>>> without checking its length. >>>>> >>>>> This patch adds the number of fds received to the message, >>>>> which is set in read_vhost_message(). >>>>> >>>>> This is preliminary work to support sending fds to Qemu. >>>>> >>>>> Signed-off-by: Dr. David Alan Gilbert >>>>> Signed-off-by: Maxime Coquelin >>>>> --- >>>>>   lib/librte_vhost/socket.c     | 25 ++++++++++++++++++++----- >>>>>   lib/librte_vhost/vhost_user.c |  2 +- >>>>>   lib/librte_vhost/vhost_user.h |  4 +++- >>>>>   3 files changed, 24 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c >>>>> index d63031747..3b0287a26 100644 >>>>> --- a/lib/librte_vhost/socket.c >>>>> +++ b/lib/librte_vhost/socket.c >>>>> @@ -94,18 +94,24 @@ static struct vhost_user vhost_user = { >>>>>       .mutex = PTHREAD_MUTEX_INITIALIZER, >>>>>   }; >>>>> -/* return bytes# of read on success or negative val on failure. */ >>>>> +/* >>>>> + * return bytes# of read on success or negative val on failure. Update fdnum >>>>> + * with number of fds read. >>>>> + */ >>>>>   int >>>>> -read_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num) >>>>> +read_fd_message(int sockfd, char *buf, int buflen, int *fds, int max_fds, >>>>> +        int *fd_num) >>>>>   { >>>>>       struct iovec iov; >>>>>       struct msghdr msgh; >>>>> -    size_t fdsize = fd_num * sizeof(int); >>>>> -    char control[CMSG_SPACE(fdsize)]; >>>>> +    char control[CMSG_SPACE(max_fds * sizeof(int))]; >>>>>       struct cmsghdr *cmsg; >>>>>       int got_fds = 0; >>>>> +    int *tmp_fds; >>>>>       int ret; >>>>> +    *fd_num = 0; >>>>> + >>>>>       memset(&msgh, 0, sizeof(msgh)); >>>>>       iov.iov_base = buf; >>>>>       iov.iov_len  = buflen; >>>>> @@ -131,13 +137,22 @@ read_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num) >>>>>           if ((cmsg->cmsg_level == SOL_SOCKET) && >>>>>               (cmsg->cmsg_type == SCM_RIGHTS)) { >>>>>               got_fds = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int); >>>>> +            if (got_fds > max_fds) { >>>> >>>> Hmm. I just noticed that 'msg_controllen' is set to receive >>>> not more than max_fds descriptors. So, this case should not >>>> be possible. We will receive MSG_CTRUNC and return before >>>> the loop. >>> >>> Maybe it is better to remove check for MSG_CTRUNC. >> s/remove/rework/ >> >>> IIUC, if MSG_CTRUNC happens, we may have to close anyway the ones >>> received. >>> >>> Do you agree? > > So it seems that other use of MSG_CTRUNC in DPDK and QEMU does care > to close the ones that would have been received and just return an > error. Did you mean 'does not care'? 'read_msg()' in lib/librte_eal/common/eal_common_proc.c just returns -1 and 'slave_read()' in hw/virtio/vhost-user.c does not close fds, because 'fdsize' is not set at the time of checking the flag. > > I propose to do the same for now, an remove the got_fds > max_fds part. > >>>> +                RTE_LOG(ERR, VHOST_CONFIG, >>>>> +                    "Received msg contains more fds than supported\n"); >>>>> +                tmp_fds = (int *)CMSG_DATA(cmsg); >>>>> +                while (got_fds--) >>>>> +                    close(tmp_fds[got_fds]); >>>>> +                return -1; >>>>> +            } >>>>> +            *fd_num = got_fds; >>>>>               memcpy(fds, CMSG_DATA(cmsg), got_fds * sizeof(int)); >>>>>               break; >>>>>           } >>>>>       } >>>>>       /* Clear out unused file descriptors */ >>>>> -    while (got_fds < fd_num) >>>>> +    while (got_fds < max_fds) >>>>>           fds[got_fds++] = -1; >>>>>       return ret; >>>>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c >>>>> index 83d3e6321..c1c5f35ff 100644 >>>>> --- a/lib/librte_vhost/vhost_user.c >>>>> +++ b/lib/librte_vhost/vhost_user.c >>>>> @@ -1509,7 +1509,7 @@ read_vhost_message(int sockfd, struct VhostUserMsg *msg) >>>>>       int ret; >>>>>       ret = read_fd_message(sockfd, (char *)msg, VHOST_USER_HDR_SIZE, >>>>> -        msg->fds, VHOST_MEMORY_MAX_NREGIONS); >>>>> +        msg->fds, VHOST_MEMORY_MAX_NREGIONS, &msg->fd_num); >>>>>       if (ret <= 0) >>>>>           return ret; >>>>> diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h >>>>> index 62654f736..9a91d496b 100644 >>>>> --- a/lib/librte_vhost/vhost_user.h >>>>> +++ b/lib/librte_vhost/vhost_user.h >>>>> @@ -132,6 +132,7 @@ typedef struct VhostUserMsg { >>>>>           VhostUserVringArea area; >>>>>       } payload; >>>>>       int fds[VHOST_MEMORY_MAX_NREGIONS]; >>>>> +    int fd_num; >>>>>   } __attribute((packed)) VhostUserMsg; >>>>>   #define VHOST_USER_HDR_SIZE offsetof(VhostUserMsg, payload.u64) >>>>> @@ -155,7 +156,8 @@ int vhost_user_iotlb_miss(struct virtio_net *dev, uint64_t iova, uint8_t perm); >>>>>   int vhost_user_host_notifier_ctrl(int vid, bool enable); >>>>>   /* socket.c */ >>>>> -int read_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num); >>>>> +int read_fd_message(int sockfd, char *buf, int buflen, int *fds, int max_fds, >>>>> +        int *fd_num); >>>>>   int send_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num); >>>>>   #endif >>>>> > >