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 76FC82BF4 for ; Wed, 3 Oct 2018 09:55:33 +0200 (CEST) Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20181003075532euoutp02b4cfc6ec797d29aafc9be57d4195b4ee~aChqqKB7j0999209992euoutp02J for ; Wed, 3 Oct 2018 07:55:32 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20181003075532euoutp02b4cfc6ec797d29aafc9be57d4195b4ee~aChqqKB7j0999209992euoutp02J DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1538553332; bh=uuHkT9NTQ7h0kSlu9ceVLJRJlaG9xRHzJosSha95c5o=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=PefUijjhJUdDJSIlbvI4HEUESbXThp0OLZHyzqz7AvkVFMrmE6FM+Ci+5arGycV8O 0dFKR7ukMvG7LRjCwTW8CKSIsZSkGV+pmFqWUDGhqP3ZroxFa3EQRsKTima4Txg3NM 10K7WTX4wHxTs2ZnHr/6xXsxFWepD8FzOTv90jJs= Received: from eusmges1new.samsung.com (unknown [203.254.199.242]) by eucas1p2.samsung.com (KnoxPortal) with ESMTP id 20181003075531eucas1p27a965ecbe52c04e70b27f20c0ca5d404~aChqAw2m00240702407eucas1p26; Wed, 3 Oct 2018 07:55:31 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges1new.samsung.com (EUCPMTA) with SMTP id 8C.3A.04441.3F574BB5; Wed, 3 Oct 2018 08:55:31 +0100 (BST) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20181003075530eucas1p1dd7191a728c1129fd5d9dbaed5fa1047~aChpN_6wn1404114041eucas1p1H; Wed, 3 Oct 2018 07:55:30 +0000 (GMT) Received: from eusmgms1.samsung.com (unknown [182.198.249.179]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20181003075530eusmtrp278babc3fd1a2dae2ebfee91d4a9288e9~aCho8XHpM1144611446eusmtrp2Y; Wed, 3 Oct 2018 07:55:30 +0000 (GMT) X-AuditID: cbfec7f2-5e3ff70000001159-df-5bb475f3fa89 Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms1.samsung.com (EUCPMTA) with SMTP id 92.7D.04284.2F574BB5; Wed, 3 Oct 2018 08:55:30 +0100 (BST) Received: from [106.109.129.180] (unknown [106.109.129.180]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20181003075529eusmtip15e0bb3ecab2137959da10936c5c65a49~aChoOpwqM1349213492eusmtip1K; Wed, 3 Oct 2018 07:55:29 +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: Wed, 3 Oct 2018 10:57:48 +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: <2a57953d-67c6-26f3-f65f-4e5a1dcf1474@redhat.com> Content-Language: en-GB Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrKKsWRmVeSWpSXmKPExsWy7djP87qfS7dEGzxfL2px7tMyJosbq+wt 3n3azmTRu+0eu8WV9p/sFufWLGWxONa5h8Xi/69XrBanF15jsfjX8YfdYmvDfyaLzRcnMTnw ePxasJTVY+esu+wei/e8ZPKY3v2Q2eP9vqtsHn1bVjEGsEVx2aSk5mSWpRbp2yVwZVzqeshe cFC0YuOc6AbG74JdjJwcEgImEr++TGbtYuTiEBJYwSjR/u8AE0hCSOALo8TxE3UQic+MEpf+ zGaG6bj66SkjRGI5o8SWvnVsEM5HRol7L7cCVXFwCAvYSbQ+UAaJiwjcYZS4svQSG0icWSBQ YtPCeJBBbAI6EqdWH2EEsVkEVCSWPF4CtllUIELiyIOFYHFeAUGJkzOfsIDYnEAjV55sZAex mQXEJZq+rGSFsOUlmreCHMcFdNwtdonOaevBdvEKlEm8P5UKcbSLxLYLG1kgbGGJV8e3sEPY MhL/d85ngrDrJe63vGSEmNPBKDH90D+ohL3Eltfn2CHu15RYv0sfIuwosWNVA9i7EgJ8Ejfe CkKcwycxadt0qDCvREebEES1isTvg8uhQSglcfPdZ/YJjEqzkDw5C8ljs5A8Ngth7wJGllWM 4qmlxbnpqcWGeanlesWJucWleel6yfm5mxiBiev0v+OfdjB+vZR0iFGAg1GJh3fHws3RQqyJ ZcWVuYcYJTiYlUR4+xKBQrwpiZVVqUX58UWlOanFhxilOViUxHmXzdsYLSSQnliSmp2aWpBa BJNl4uCUamAMb14trXAr/Ob3xyILL+jF/+2ZemBFy6dkNq1maa4akbCAVcJibp1Fb79E910N s6nsXatv9H+C4KGPTm+cXaLuTWZ/ZBLanPKmMbqntNn6b6ve412h+0LLBBy1pU3XPr9v+jdx +3Ne3wbPnUY9kRwRp1PTUkRudK6c1liXxJ1w75djAW9wqBJLcUaioRZzUXEiADx6VDNYAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrNIsWRmVeSWpSXmKPExsVy+t/xu7qfSrdEGxzsl7I492kZk8WNVfYW 7z5tZ7Lo3XaP3eJK+092i3NrlrJYHOvcw2Lx/9crVovTC6+xWPzr+MNusbXhP5PF5ouTmBx4 PH4tWMrqsXPWXXaPxXteMnlM737I7PF+31U2j74tqxgD2KL0bIryS0tSFTLyi0tslaINLYz0 DC0t9IxMLPUMjc1jrYxMlfTtbFJSczLLUov07RL0Mi51PWQvOChasXFOdAPjd8EuRk4OCQET iaufnjJ2MXJxCAksZZT4/mgCC0RCSuLHrwusELawxJ9rXWwQRe8ZJdas72LvYuTgEBawk2h9 oAwSFxG4wyixZdYZRpAGZoFAif3br7FDNLxikvi85BXYJDYBHYlTq4+AFfECNR/a+xlsG4uA isSSx0uYQGxRgQiJ1ctfsELUCEqcnPkErIYTqH7lyUZ2iAXqEn/mXWKGsMUlmr6sZIWw5SWa t85mnsAoNAtJ+ywkLbOQtMxC0rKAkWUVo0hqaXFuem6xoV5xYm5xaV66XnJ+7iZGYMRuO/Zz 8w7GSxuDDzEKcDAq8fDuWLg5Wog1say4MvcQowQHs5IIb18iUIg3JbGyKrUoP76oNCe1+BCj KdBzE5mlRJPzgckkryTe0NTQ3MLS0NzY3NjMQkmc97xBZZSQQHpiSWp2ampBahFMHxMHp1QD Y7mE7+Xth76ouLjsn+Fw43GtfsqqVMk5couavM6s7vkjvkTvhDgjA6+1fNFCcx0ti60m76e2 /HCd4FGpkfvryxmZ8sMmJtYXJZOetFl8T4v4s8aGN0Z5myhzc7fw43iZKf9uKDp7pDdyrBN9 bpvx/USEz5a/Ye2RVSIWp16wbJr78Ezl6d9/lFiKMxINtZiLihMBwTPcyu4CAAA= Message-Id: <20181003075530eucas1p1dd7191a728c1129fd5d9dbaed5fa1047~aChpN_6wn1404114041eucas1p1H@eucas1p1.samsung.com> X-CMS-MailID: 20181003075530eucas1p1dd7191a728c1129fd5d9dbaed5fa1047 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> 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: Wed, 03 Oct 2018 07:55:33 -0000 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? > >>> -        msg.payload.u64 = !!ret; >>> +        msg.payload.u64 = ret == VH_RESULT_ERR; >>>           msg.size = sizeof(msg.payload.u64); >>>           send_vhost_reply(fd, &msg); >>> -    } else if (ret) { >>> +    } else if (ret == VH_RESULT_ERR) { >>>           RTE_LOG(ERR, VHOST_CONFIG, >>>               "vhost message handling failed.\n"); >>>           return -1; >>> > >