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 7064E5F35 for ; Thu, 4 Oct 2018 07:40:11 +0200 (CEST) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20181004054010euoutp021f273ca393e8b6e66f7c9e04bacbb5a1~aUUwcdR_41327113271euoutp02e for ; Thu, 4 Oct 2018 05:40:10 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20181004054010euoutp021f273ca393e8b6e66f7c9e04bacbb5a1~aUUwcdR_41327113271euoutp02e DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1538631610; bh=b113PxIsOnZpdcXe3outZo/RjwVVG+j6x5mky0h80h0=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=AwDXSo7eg5KuJUBvVq5We/m5m94VxUrd5IAQ8iPSmiv5jksnNDUZWSHPITr3GLBbc eVNJqwtj+WqJt0qPWXNW1rE0l2LgxMIu4PB5kAHH6WWxLia37SCbVtXDaXHjVKwoYu odAcH+SbNbZW6aYr+mSx6Y3G6Rk69Lfni6qW/GSk= Received: from eusmges2new.samsung.com (unknown [203.254.199.244]) by eucas1p2.samsung.com (KnoxPortal) with ESMTP id 20181004054009eucas1p2235c110d822949b6e77fa407f17da720~aUUv5v1uh1172811728eucas1p2h; Thu, 4 Oct 2018 05:40:09 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges2new.samsung.com (EUCPMTA) with SMTP id 6B.A1.04294.9B7A5BB5; Thu, 4 Oct 2018 06:40:09 +0100 (BST) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20181004054008eucas1p1ce42baf79937daf4c08cc127bbb92cbe~aUUu8tIPr2293922939eucas1p1S; Thu, 4 Oct 2018 05:40:08 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20181004054008eusmtrp21e8d7b4ffe36904d956a3e5b0932bf9e~aUUurok891660016600eusmtrp2f; Thu, 4 Oct 2018 05:40:08 +0000 (GMT) X-AuditID: cbfec7f4-84fff700000010c6-c8-5bb5a7b9ed79 Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 7F.69.04128.8B7A5BB5; Thu, 4 Oct 2018 06:40:08 +0100 (BST) Received: from [106.109.129.180] (unknown [106.109.129.180]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20181004054007eusmtip11cb5a0e8fffd9d30991608eb2c5e7fbb~aUUt-CfBJ2167921679eusmtip1J; Thu, 4 Oct 2018 05:40:07 +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 Cc: dgilbert@redhat.com, stable@dpdk.org, "Michael S. Tsirkin" From: Ilya Maximets Date: Thu, 4 Oct 2018 08:42:22 +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: Content-Language: en-GB Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA01SS0wTURT1daadoaFkKChXlBALG4nyC4tZGMDEaHVhwMSgdgFFho/QQloK woai/AlG6YJQGixBoFSCAUqlBBUaAiKmRoMIERABE4VQlI+RvwyDkd2559577jkvj8TEg3xv MkWZyaiU8jSJQIhbB9YdZ21NnbLg8k847Vhu5NFj5gjaufycR1dYpwh6pHidoB0tDTg9UNqD 07sb83x6uG4Up3dKtgi6U7vLozveV/IiXaUbxga+1KafJKT1PT940qryr5h06eVHgfSBxYyi BLeE5xKYtJQsRhUUHidMLiz6hmeU+9wtb2/CtUgPZYgkgQqD3xOxZUhIiikTgu05K84Vqwiq //QRXLGCwD7rRGXIZX/ju0mHcY0mBC3rPXyu+IXAUKwjWF0PKhwKp/1Y3pOaQDDS8EHA8hgV De11sayQgDoDb57274vilD9MtfViLD5K3YD+6bp9XkS5w1D1HM5ilz3J6sp8gsUY5QX3Vpv5 HPaF+501+4aAmiHA9LYB45azwProx4HrC7C5UIpx2APmBy0Eh0/Cru0xj8N58KWAnWeFShBU 2XcOGhFgWXAQXIDT8Kw7iKPPQ5dZi3Hv6AZji+6cHzeotFYd0CIoKRJz0/6w2dd04MAbxp0r xEMk0R9KqT+UTH8omf7/XSPCzciL0agVSYw6VMlkB6rlCrVGmRR4O13Rjva+1/DO4GoX6t6K tyOKRBJXUYnTIhPz5VnqHIUdAYlJPEUhxk6ZWJQgz8llVOmxKk0ao7ajEyQu8RI11rbJxFSS PJNJZZgMRvWvyyNdvLWoi7wsinR/5xG/bvTNhbrjy1fr7ZUDwXfmRlNNug5Hs5ffdrQzYzPH R2nIJuMKdVHKgSNX8oqirqnsMaKZtrCfNtl4XOilwJCE1gqLq8FzZqn1on3Wfe1zRG+YaWiy OFGY9WLq2CAZG64JWCzIT7z55PV4qWHt1PXaV74xho4aCa5OlocEYCq1/C8XBDoBWgMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrNIsWRmVeSWpSXmKPExsVy+t/xu7o7lm+NNri3j9Xi3KdlTBY3Vtlb vPu0ncmid9s9dosr7T/ZLc6tWcpicaxzD4vF/1+vWC1OL7zGYvGv4w+7xdaG/0wWmy9OYnLg 8fi1YCmrx85Zd9k9Fu95yeQxvfshs8f7fVfZPPq2rGIMYIvSsynKLy1JVcjILy6xVYo2tDDS M7S00DMysdQzNDaPtTIyVdK3s0lJzcksSy3St0vQy2hte8pS0C1b0b1pOUsD4yyJLkZODgkB E4kXKyYzdzFycQgJLGWU+P6vmx0iISXx49cFVghbWOLPtS42EFtI4D2jxNRp3l2MHBzCAnYS rQ+UQXpFBO4wSmyZdYYRpIZZIFBi//Zr7BBDZ7JJHDp4GWwom4COxKnVR8CKeIGaLzbuZgGx WQRUJO5tPMAMYosKREisXv6CFaJGUOLkzCdgNZxA9TMnNbJDLFCX+DPvEjOELS7R9GUlK4Qt L9G8dTbzBEahWUjaZyFpmYWkZRaSlgWMLKsYRVJLi3PTc4uN9IoTc4tL89L1kvNzNzECI3bb sZ9bdjB2vQs+xCjAwajEw9vxbku0EGtiWXFl7iFGCQ5mJRFewwVbo4V4UxIrq1KL8uOLSnNS iw8xmgI9N5FZSjQ5H5hM8kriDU0NzS0sDc2NzY3NLJTEec8bVEYJCaQnlqRmp6YWpBbB9DFx cEo1MM4Pag0QXPak4zh7/KFV3h7ClxezPc73WdHy/v/bHTwVkyfveZvmWCGjF9zqnb9Wu9c+ PFSHvzlK033WzUNZ+ReVFyQ92W7M7nP05a6aC4o3fn9cceXW6pRvmi5MHUFmsZncO7gWask+ bn+/OTM/6W/Ede2Nd3i8Qt4v+LrzkWXd4RcMSdt2zlRiKc5INNRiLipOBAAlRndl7gIAAA== Message-Id: <20181004054008eucas1p1ce42baf79937daf4c08cc127bbb92cbe~aUUu8tIPr2293922939eucas1p1S@eucas1p1.samsung.com> X-CMS-MailID: 20181004054008eucas1p1ce42baf79937daf4c08cc127bbb92cbe X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20181002093710epcas2p41edcdf02797df82d10457b464a72be77 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20181002093710epcas2p41edcdf02797df82d10457b464a72be77 References: <20181002093651.24795-1-maxime.coquelin@redhat.com> <20181002093651.24795-2-maxime.coquelin@redhat.com> <20181002141315eucas1p16c87759329eeb374528bcb70a2d71ee4~Z0CLLGpN92076620766eucas1p1R@eucas1p1.samsung.com> <2a57953d-67c6-26f3-f65f-4e5a1dcf1474@redhat.com> <20181003075530eucas1p1dd7191a728c1129fd5d9dbaed5fa1047~aChpN_6wn1404114041eucas1p1H@eucas1p1.samsung.com> <20181003083009eucas1p19f5d8234a3592b9e1753181d858a99aa~aC-5VXlJ51559815598eucas1p1k@eucas1p1.samsung.com> <20181003090454eucas1p180d79e1279ca4e1086b2447758686978~aDeOgna1t0120201202eucas1p1W@eucas1p1.samsung.com> Subject: Re: [dpdk-dev] [PATCH v2 01/17] vhost: fix messages error checks 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: Thu, 04 Oct 2018 05:40:11 -0000 On 03.10.2018 17:39, Maxime Coquelin wrote: > > > On 10/03/2018 11:07 AM, Ilya Maximets wrote: >> On 03.10.2018 11:32, Ilya Maximets wrote: >>> On 03.10.2018 11:02, Maxime Coquelin wrote: >>>> >>>> >>>> On 10/03/2018 09:57 AM, Ilya Maximets wrote: >>>>> On 03.10.2018 10:50, Maxime Coquelin wrote: >>>>>> >>>>>> >>>>>> On 10/02/2018 04:15 PM, Ilya Maximets wrote: >>>>>>> On 02.10.2018 12:36, Maxime Coquelin wrote: >>>>>>>> Return of message handling has now changed to an enum that can >>>>>>>> take non-negative value that is not zero in case a reply is >>>>>>>> needed. But the code checking the variable afterwards has not >>>>>>>> been updated, leading to success messages handling being >>>>>>>> treated as errors. >>>>>>>> >>>>>>>> Fixes: 4e601952cae6 ("vhost: message handling implemented as a callback array") >>>>>>>> >>>>>>>> Signed-off-by: Maxime Coquelin >>>>>>>> --- >>>>>>>>     lib/librte_vhost/vhost_user.c | 6 +++--- >>>>>>>>     1 file changed, 3 insertions(+), 3 deletions(-) >>>>>>>> >>>>>>>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c >>>>>>>> index 7ef3fb4a4..060b41893 100644 >>>>>>>> --- a/lib/librte_vhost/vhost_user.c >>>>>>>> +++ b/lib/librte_vhost/vhost_user.c >>>>>>>> @@ -1783,7 +1783,7 @@ vhost_user_msg_handler(int vid, int fd) >>>>>>>>         } >>>>>>>>       skip_to_post_handle: >>>>>>>> -    if (!ret && dev->extern_ops.post_msg_handle) { >>>>>>>> +    if (ret != VH_RESULT_ERR && dev->extern_ops.post_msg_handle) { >>>>>>>>             uint32_t need_reply; >>>>>>>>               ret = (*dev->extern_ops.post_msg_handle)( >>>>>>>> @@ -1800,10 +1800,10 @@ vhost_user_msg_handler(int vid, int fd) >>>>>>>>             vhost_user_unlock_all_queue_pairs(dev); >>>>>>>>           if (msg.flags & VHOST_USER_NEED_REPLY) { >>>>>>> >>>>>>> Maybe we need to reply here only if we didn't reply >>>>>>> already (not VH_RESULT_REPLY) ? Otherwise, we could >>>>>>> reply twice (with payload and with return code). >>>>>> >>>>>> Well, if the master sets this bit, it means it is waiting for >>>>>> a "reply-ack", so not sending it would cause the master to wait >>>>>> forever. >>>>>> >>>>>> It is the master responsibility to not set this bit for requests >>>>>> already expecting a non "reply-ack" reply (as you fixed it for >>>>>> postcopy's set mem table case). >>>>> >>>>> vhost-user docs in QEMU says: >>>>> " >>>>> For the message types that already solicit a reply from the client, the >>>>> presence of VHOST_USER_PROTOCOL_F_REPLY_ACK or need_reply bit being set brings >>>>> no behavioural change. >>>>> " >>>>> i.e. even if QEMU sets the need_reply flag, vhost should not reply twice. >>>>> Am I missing something? >>>> >>>> Oh, right. Thanks for pointing it out. >>>> >>>> So coming back to the DPDK implementation, I just had a look again, and it seems that we don't send a reply twice, as send_vhost_reply takes >>>> care of clearing the VHOST_USER_NEED_REPLY flag. >>>> Do you confirm my understanding is correct? >>> >>> Hmm. Yes, you're right. send_vhost_reply clears the VHOST_USER_NEED_REPLY >>> flag and vhost doesn't send replies twice. >>> Maybe some comment with clarifications needed here, or some more >>> refactoring to make this aspect more clear. >>> > > Agree. > I'm adding a comment, I don't think a refactoring is required, and I > would be reluctant to add one more refactoring so close to the > integration deadline. > > Does it work for you? Sure. Thanks. I agree that it's not the right time for refactoring now. > > Thanks, > Maxime > >