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 156884C8E for ; Tue, 12 Mar 2019 17:14:44 +0100 (CET) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20190312161444euoutp026970bd4ace309c4d02af4052da4230ae~LQjMnGg-c0804008040euoutp02c for ; Tue, 12 Mar 2019 16:14:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20190312161444euoutp026970bd4ace309c4d02af4052da4230ae~LQjMnGg-c0804008040euoutp02c DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1552407284; bh=ab9TtYQUYUSCIlwGaRCQYRPn+JZUbRcwX4uCWSG3YDo=; h=Subject:To:From:Date:In-Reply-To:References:From; b=BkhalLd8nyZiTRSUAOjO9hxLuzx00FeG+3lnwNxZ9Xy80U9PbwblHMswYPE3KRi+o gZSRxMKL3ERggg1DbNOKem+UzdtUiV0C1koUOYA2Kae2/M5RjXhoWe2I96LwX9DAhZ Llj95zJoMuQ7ztwmlJ1Zs+N/SuXC2pXEroJq6EbY= Received: from eusmges1new.samsung.com (unknown [203.254.199.242]) by eucas1p2.samsung.com (KnoxPortal) with ESMTP id 20190312161443eucas1p270fc13c46c46b48cad428c46ef22942b~LQjMGtLHR0414704147eucas1p23; Tue, 12 Mar 2019 16:14:43 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges1new.samsung.com (EUCPMTA) with SMTP id 5A.35.04441.3FAD78C5; Tue, 12 Mar 2019 16:14:43 +0000 (GMT) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20190312161442eucas1p24c6c317b8183eb81af8389adaf0c395c~LQjLaLVjU0850508505eucas1p2R; Tue, 12 Mar 2019 16:14:42 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20190312161442eusmtrp21e221327896bb9af0a532a36fadfa9bd~LQjLMJ-vn0199701997eusmtrp2W; Tue, 12 Mar 2019 16:14:42 +0000 (GMT) X-AuditID: cbfec7f2-5c9ff70000001159-cb-5c87daf3ec7a Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 65.35.04128.2FAD78C5; Tue, 12 Mar 2019 16:14:42 +0000 (GMT) Received: from [106.109.129.180] (unknown [106.109.129.180]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20190312161441eusmtip1a1a22b8a9ef4016360f8eaf18a38c68c~LQjKuSE2u2803628036eusmtip1G; Tue, 12 Mar 2019 16:14:41 +0000 (GMT) To: Maxime Coquelin , dev@dpdk.org, changpeng.liu@intel.com, tiwei.bie@intel.com, dariusz.stojaczyk@intel.com From: Ilya Maximets Message-ID: Date: Tue, 12 Mar 2019 19:14:41 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: <20190312145410.570-3-maxime.coquelin@redhat.com> Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrAKsWRmVeSWpSXmKPExsWy7djPc7qfb7XHGBw8zmtx8edXdov3v9cw Wrz7tJ3J4kr7T3aLY517WCy2NvxncmDz+LVgKavH4j0vmTze77vK5tG3ZRVjAEsUl01Kak5m WWqRvl0CV8biOYkFszwr3h38wtjA+MGyi5GTQ0LARKLn8SrGLkYuDiGBFYwSVxvmsEA4Xxgl nq26CpX5zCixdMIWJpiWxVtWskEkljNKPJ93hB3C+cgocfLkdrAqYYFgiftf/oBViQh0M0rM /rmRHSTBJqAjcWr1EUYQm1fATuLr7l6wOIuAqsSfE5PBmkUFIiTeP93NAlEjKHFy5hMgm4OD E6j+6Il6kDCzgLhE05eVrBC2vMT2t3OYQXZJCExml3gy8RzUqS4SN7ZMZIawhSVeHd/CDmHL SPzfOR+qpl7ifstLRojmDkaJ6Yf+QSXsJba8PscOsphZQFNi/S59iLCjxOu+t2BhCQE+iRtv BSFu4JOYtG06M0SYV6KjTQiiWkXi98HlUBdISdx89xmq00Pi1SKZCYyKs5D8OAvJY7OQPDYL 4YQFjCyrGMVTS4tz01OLDfNSy/WKE3OLS/PS9ZLzczcxAtPN6X/HP+1g/Hop6RCjAAejEg9v wu72GCHWxLLiytxDjBIczEoivBY5QCHelMTKqtSi/Pii0pzU4kOM0hwsSuK81QwPooUE0hNL UrNTUwtSi2CyTBycUg2MHBaBufUtff4R4VXf/cPNv1/3sz375PyumA978n4v1fua5T5Z69vO rHUCpfWzRFxk7k2yv7z58YcZW2y2/jWVj5s9296iRP+A6orvLYnRITleLmoL7W/+attsMOUQ t/J1xpoNWrtzuAS3Xa+QPXYzZG1SXt/qyMzNjFtn/G/VNQnw4Jv8neuaEktxRqKhFnNRcSIA 5p0SADMDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrOIsWRmVeSWpSXmKPExsVy+t/xu7qfbrXHGJx9KmFx8edXdov3v9cw Wrz7tJ3J4kr7T3aLY517WCy2NvxncmDz+LVgKavH4j0vmTze77vK5tG3ZRVjAEuUnk1RfmlJ qkJGfnGJrVK0oYWRnqGlhZ6RiaWeobF5rJWRqZK+nU1Kak5mWWqRvl2CXsbiOYkFszwr3h38 wtjA+MGyi5GTQ0LARGLxlpVsXYxcHEICSxklutt2sUAkpCR+/LrACmELS/y51gVV9J5R4vzS VjaQhLBAsMT9L3/AbBGBbkaJHWfrIIqOMkpcuPqaESTBJqAjcWr1ETCbV8BO4uvuXnYQm0VA VeLPiclMILaoQITE3YsvWCBqBCVOznwCZHNwcALVHz1RDxJmFlCX+DPvEjOELS7R9GUlK4Qt L7H97RzmCYyCs5B0z0LSMgtJyywkLQsYWVYxiqSWFuem5xYb6RUn5haX5qXrJefnbmIERtK2 Yz+37GDsehd8iFGAg1GJhzdiS3uMEGtiWXFl7iFGCQ5mJRFeixygEG9KYmVValF+fFFpTmrx IUZToN8mMkuJJucDozyvJN7Q1NDcwtLQ3Njc2MxCSZz3vEFllJBAemJJanZqakFqEUwfEwen VANj/nL18q77t2bzvE8OvX9Rez1zUVPFteRkBo3/k/TkZqv6TEjjFrJ49ErxKO+m7ym30hu+ dq1J47Vz6ucS5A55bS4hc1Wn+BnrvykmmrUlrJGCCsv7O8IPPOzScZPafbO19Fv7dFu22XNn r2gUYRebsvezR4xT1L4t732n2TE4fFnecHOtjq4SS3FGoqEWc1FxIgAvSI0XugIAAA== X-CMS-MailID: 20190312161442eucas1p24c6c317b8183eb81af8389adaf0c395c X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20190312145424epcas3p4716152a64d665ee7f7ba06417e7e08f9 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20190312145424epcas3p4716152a64d665ee7f7ba06417e7e08f9 References: <20190312145410.570-1-maxime.coquelin@redhat.com> <20190312145410.570-3-maxime.coquelin@redhat.com> Subject: Re: [dpdk-dev] [PATCH 2/2] vhost: support requests only handled by external backend 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: Tue, 12 Mar 2019 16:14:45 -0000 On 12.03.2019 17:54, Maxime Coquelin wrote: > External backends may have specific requests to handle, and so > we don't want the vhost-user lib to handle these requests as > errors. > > This patch also changes the experimental API by introducing > RTE_VHOST_MSG_RESULT_NOT_HANDLED so that vhost-user lib > can report an error if a message is handled neither by > the vhost-user library nor by the external backend. > > The logic changes a bit so that if the callback returns > with ERR, OK or REPLY, it is considered the message > is handled by the external backend so it won't be > handled by the vhost-user library. > It is still possible for an external backend to listen > to requests that have to be handled by the vhost-user > library like SET_MEM_TABLE, but the callback have to > return NOT_HANDLED in that case. > > Vhost-crypto backend is ialso adapted to this API change. > > Suggested-by: Ilya Maximets > Signed-off-by: Maxime Coquelin > Tested-by: Darek Stojaczyk > --- > lib/librte_vhost/rte_vhost.h | 16 +++++-- > lib/librte_vhost/vhost_crypto.c | 10 +++- > lib/librte_vhost/vhost_user.c | 82 +++++++++++++++++++++------------ > 3 files changed, 71 insertions(+), 37 deletions(-) > > diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h > index c9c392975..b1c5a0908 100644 > --- a/lib/librte_vhost/rte_vhost.h > +++ b/lib/librte_vhost/rte_vhost.h > @@ -121,6 +121,8 @@ enum rte_vhost_msg_result { > RTE_VHOST_MSG_RESULT_OK = 0, > /* Message handling successful and reply prepared */ > RTE_VHOST_MSG_RESULT_REPLY = 1, > + /* Message not handled */ > + RTE_VHOST_MSG_RESULT_NOT_HANDLED, > }; > > /** > @@ -135,11 +137,13 @@ enum rte_vhost_msg_result { > * If the handler requires skipping the master message handling, this variable > * shall be written 1, otherwise 0. Above statement should be updated because 'skip_master' removed. BTW, maybe it's better to squash these two typedef's as they are equal now? Comment parts that differs could be moved to the definition of the 'struct rte_vhost_user_extern_ops'. > * @return > - * VH_RESULT_OK on success, VH_RESULT_REPLY on success with reply, > - * VH_RESULT_ERR on failure > + * RTE_VHOST_MSG_RESULT_OK on success, > + * RTE_VHOST_MSG_RESULT_REPLY on success with reply, > + * RTE_VHOST_MSG_RESULT_ERR on failure, > + * RTE_VHOST_MSG_RESULT_NOT_HANDLED if message was not handled. > */ > typedef enum rte_vhost_msg_result (*rte_vhost_msg_pre_handle)(int vid, > - void *msg, uint32_t *skip_master); > + void *msg); > > /** > * Function prototype for the vhost backend to handler specific vhost user > @@ -150,8 +154,10 @@ typedef enum rte_vhost_msg_result (*rte_vhost_msg_pre_handle)(int vid, > * @param msg > * Message pointer. > * @return > - * VH_RESULT_OK on success, VH_RESULT_REPLY on success with reply, > - * VH_RESULT_ERR on failure > + * RTE_VHOST_MSG_RESULT_OK on success, > + * RTE_VHOST_MSG_RESULT_REPLY on success with reply, > + * RTE_VHOST_MSG_RESULT_ERR on failure, > + * RTE_VHOST_MSG_RESULT_NOT_HANDLED if message was not handled. > */ > typedef enum rte_vhost_msg_result (*rte_vhost_msg_post_handle)(int vid, > void *msg); > diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c > index 0f437c4a1..9b4b850e8 100644 > --- a/lib/librte_vhost/vhost_crypto.c > +++ b/lib/librte_vhost/vhost_crypto.c > @@ -453,14 +453,20 @@ vhost_crypto_msg_post_handler(int vid, void *msg) > return RTE_VHOST_MSG_RESULT_ERR; > } > > - if (vmsg->request.master == VHOST_USER_CRYPTO_CREATE_SESS) { > + switch (vmsg->request.master) { > + case VHOST_USER_CRYPTO_CREATE_SESS: > vhost_crypto_create_sess(vcrypto, > &vmsg->payload.crypto_session); > vmsg->fd_num = 0; > ret = RTE_VHOST_MSG_RESULT_REPLY; > - } else if (vmsg->request.master == VHOST_USER_CRYPTO_CLOSE_SESS) { > + break; > + case VHOST_USER_CRYPTO_CLOSE_SESS: > if (vhost_crypto_close_sess(vcrypto, vmsg->payload.u64)) > ret = RTE_VHOST_MSG_RESULT_ERR; > + break; > + default: > + ret = RTE_VHOST_MSG_RESULT_NOT_HANDLED; > + break; > } > > return ret; > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c > index 555d09ad9..39756fce7 100644 > --- a/lib/librte_vhost/vhost_user.c > +++ b/lib/librte_vhost/vhost_user.c > @@ -1910,7 +1910,7 @@ vhost_user_msg_handler(int vid, int fd) > int did = -1; > int ret; > int unlock_required = 0; > - uint32_t skip_master = 0; > + bool handled; > int request; > > dev = get_device(vid); > @@ -1928,27 +1928,30 @@ vhost_user_msg_handler(int vid, int fd) > } > > ret = read_vhost_message(fd, &msg); > - if (ret <= 0 || msg.request.master >= VHOST_USER_MAX) { > + if (ret <= 0) { > if (ret < 0) > RTE_LOG(ERR, VHOST_CONFIG, > "vhost read message failed\n"); > - else if (ret == 0) > + else > RTE_LOG(INFO, VHOST_CONFIG, > "vhost peer closed\n"); > - else > - RTE_LOG(ERR, VHOST_CONFIG, > - "vhost read incorrect message\n"); > > return -1; > } > > ret = 0; > - if (msg.request.master != VHOST_USER_IOTLB_MSG) > - RTE_LOG(INFO, VHOST_CONFIG, "read message %s\n", > - vhost_message_str[msg.request.master]); > - else > - RTE_LOG(DEBUG, VHOST_CONFIG, "read message %s\n", > - vhost_message_str[msg.request.master]); > + request = msg.request.master; > + if (request > VHOST_USER_NONE && request < VHOST_USER_MAX && > + vhost_message_str[request]) { > + if (request != VHOST_USER_IOTLB_MSG) > + RTE_LOG(INFO, VHOST_CONFIG, "read message %s\n", > + vhost_message_str[request]); > + else > + RTE_LOG(DEBUG, VHOST_CONFIG, "read message %s\n", > + vhost_message_str[request]); > + } else { > + RTE_LOG(DEBUG, VHOST_CONFIG, "External request %d\n", request); > + } > > ret = vhost_user_check_and_alloc_queue_pair(dev, &msg); > if (ret < 0) { > @@ -1964,7 +1967,7 @@ vhost_user_msg_handler(int vid, int fd) > * inactive, so it is safe. Otherwise taking the access_lock > * would cause a dead lock. > */ > - switch (msg.request.master) { > + switch (request) { > case VHOST_USER_SET_FEATURES: > case VHOST_USER_SET_PROTOCOL_FEATURES: > case VHOST_USER_SET_OWNER: > @@ -1989,19 +1992,24 @@ vhost_user_msg_handler(int vid, int fd) > > } > > + handled = false; > if (dev->extern_ops.pre_msg_handle) { > ret = (*dev->extern_ops.pre_msg_handle)(dev->vid, > - (void *)&msg, &skip_master); > - if (ret == RTE_VHOST_MSG_RESULT_ERR) > - goto skip_to_reply; > - else if (ret == RTE_VHOST_MSG_RESULT_REPLY) > + (void *)&msg); > + switch (ret) { > + case RTE_VHOST_MSG_RESULT_REPLY: > send_vhost_reply(fd, &msg); > - > - if (skip_master) > + /* Fall-through */ > + case RTE_VHOST_MSG_RESULT_ERR: > + case RTE_VHOST_MSG_RESULT_OK: > + handled = true; > goto skip_to_post_handle; > + case RTE_VHOST_MSG_RESULT_NOT_HANDLED: > + default: > + break; > + } > } > > - request = msg.request.master; > if (request > VHOST_USER_NONE && request < VHOST_USER_MAX) { > if (!vhost_message_handlers[request]) > goto skip_to_post_handle; > @@ -2012,40 +2020,54 @@ vhost_user_msg_handler(int vid, int fd) > RTE_LOG(ERR, VHOST_CONFIG, > "Processing %s failed.\n", > vhost_message_str[request]); > + handled = true; > break; > case RTE_VHOST_MSG_RESULT_OK: > RTE_LOG(DEBUG, VHOST_CONFIG, > "Processing %s succeeded.\n", > vhost_message_str[request]); > + handled = true; > break; > case RTE_VHOST_MSG_RESULT_REPLY: > RTE_LOG(DEBUG, VHOST_CONFIG, > "Processing %s succeeded and needs reply.\n", > vhost_message_str[request]); > send_vhost_reply(fd, &msg); > + handled = true; > + break; > + default: > break; > } > - } else { > - RTE_LOG(ERR, VHOST_CONFIG, > - "Requested invalid message type %d.\n", request); > - ret = RTE_VHOST_MSG_RESULT_ERR; > } > > skip_to_post_handle: > if (ret != RTE_VHOST_MSG_RESULT_ERR && > dev->extern_ops.post_msg_handle) { > - ret = (*dev->extern_ops.post_msg_handle)( > - dev->vid, (void *)&msg); > - if (ret == RTE_VHOST_MSG_RESULT_ERR) > - goto skip_to_reply; > - else if (ret == RTE_VHOST_MSG_RESULT_REPLY) > + ret = (*dev->extern_ops.post_msg_handle)(dev->vid, > + (void *)&msg); > + switch (ret) { > + case RTE_VHOST_MSG_RESULT_REPLY: > send_vhost_reply(fd, &msg); > + /* Fall-through */ > + case RTE_VHOST_MSG_RESULT_ERR: > + case RTE_VHOST_MSG_RESULT_OK: > + handled = true; > + case RTE_VHOST_MSG_RESULT_NOT_HANDLED: > + default: > + break; > + } > } > > -skip_to_reply: > if (unlock_required) > vhost_user_unlock_all_queue_pairs(dev); > > + /* If message was not handled at this stage, treat it as an error */ > + if (!handled) { > + RTE_LOG(ERR, VHOST_CONFIG, > + "vhost message (req: %d) was not handled.\n", request); > + ret = RTE_VHOST_MSG_RESULT_ERR; > + } > + > /* > * If the request required a reply that was already sent, > * this optional reply-ack won't be sent as the >