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 A365237B0 for ; Wed, 13 Mar 2019 17:09:29 +0100 (CET) Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20190313160928euoutp024f996938d4e2c76d7995739ab67d6760~LkH5I-UL51498714987euoutp02Q for ; Wed, 13 Mar 2019 16:09:28 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20190313160928euoutp024f996938d4e2c76d7995739ab67d6760~LkH5I-UL51498714987euoutp02Q DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1552493368; bh=kDg7wrcXnppW5euhsfhEd/UIPj1ElaHAm4Nw2kzT0ZM=; h=Subject:To:From:Date:In-Reply-To:References:From; b=koDvOlpZ2o+eczBfr+pE6EZGJrSDIdVKl7vh1J1slI3hVm0d1yLyr8EUdjOzLYtg5 2kA9RNf8T+qLhJz6hwRSS4ZOBJD05TYJP9FggekgxOqi3IOcOqM15/CxiO6r6244YH 5g32hIen5lhYRMfJUIKQrX/FfT/kJDNSkQJQc91M= Received: from eusmges1new.samsung.com (unknown [203.254.199.242]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20190313160928eucas1p1e573201dad494a1c34531cc4943e2083~LkH4ox9vX1898718987eucas1p11; Wed, 13 Mar 2019 16:09:28 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges1new.samsung.com (EUCPMTA) with SMTP id 70.60.04441.73B298C5; Wed, 13 Mar 2019 16:09:27 +0000 (GMT) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20190313160927eucas1p27d48ed91797cfa4cc458a33a47a726b1~LkH3y4JE30123301233eucas1p2t; Wed, 13 Mar 2019 16:09:27 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20190313160926eusmtrp12de8e8f6ce076d5b83933d80d5eeced7~LkH3k1K421871418714eusmtrp1F; Wed, 13 Mar 2019 16:09:26 +0000 (GMT) X-AuditID: cbfec7f2-5e3ff70000001159-26-5c892b37ddd2 Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id E4.97.04128.63B298C5; Wed, 13 Mar 2019 16:09:26 +0000 (GMT) Received: from [106.109.129.180] (unknown [106.109.129.180]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20190313160926eusmtip1d5f436ff6052d4f4a7cf4cb3fab4d69f~LkH3KOryE0100501005eusmtip1g; Wed, 13 Mar 2019 16:09:26 +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: <03ff92ec-7068-8312-669d-53519878c6cd@samsung.com> Date: Wed, 13 Mar 2019 19:09:25 +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: <20190313155504.15087-3-maxime.coquelin@redhat.com> Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrEKsWRmVeSWpSXmKPExsWy7djP87rm2p0xBgcaDC0u/vzKbvH+9xpG i3eftjNZXGn/yW5xrHMPi8XWhv9MDmwevxYsZfVYvOclk8f7fVfZPPq2rGIMYInisklJzcks Sy3St0vgylhx8zdTwU7fimXNfWwNjH9tuhg5OSQETCT29d9m7WLk4hASWMEo8fzOazYI5wuj xO01d6Eynxklui5+Z4Jp6f14kBEisZxRovXYGaiqj4wSnRN+s4BUCQuESfw5cZcFJCEi0M0o MfvnRnaQBJuAjsSp1UcYQWxeATuJOT+2MIPYLAKqEstaT7KC2KICERLvn+5mgagRlDg58wmY zSngIDFv40WwemYBcYmmLytZIWx5ie1v5zCDLJMQ6GeXOHfnFNCtHECOi8TEkzkQZwtLvDq+ hR3ClpE4PbmHBcKul7jf8pIRoreDUWL6oX9Qf9pLbHl9jh1kDrOApsT6XfoQYUeJiR8+sUGM 55O48VYQ4gQ+iUnbpjNDhHklOtqEIKpVJH4fXM4MYUtJ3Hz3GeoCD4mn/xYyTWBUnIXkyVlI HpuF5LFZCDcsYGRZxSieWlqcm55abJiXWq5XnJhbXJqXrpecn7uJEZhyTv87/mkH49dLSYcY BTgYlXh4Lfg7Y4RYE8uKK3MPMUpwMCuJ8B6RBwrxpiRWVqUW5ccXleakFh9ilOZgURLnrWZ4 EC0kkJ5YkpqdmlqQWgSTZeLglGpgFDzLv1byW/XZ5qTvOqe1fwrvUJDLvJS6/Xgw25fND0+k uKzpl/LeFxg27+9+577p/7NdFy1ST1ljoHbv+ZmUNYqlQb7MG3mfeHuWFLL1ilSznHn49YLX vwsSNzmzNug7135Nm58XnKOaPE1Gxkj5hPrDH6yfT5/rYTN/z1pa5P9zg9gm8R2RSizFGYmG WsxFxYkAnMqYBDUDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrJIsWRmVeSWpSXmKPExsVy+t/xu7pm2p0xBsc2mltc/PmV3eL97zWM Fu8+bWeyuNL+k93iWOceFoutDf+ZHNg8fi1YyuqxeM9LJo/3+66yefRtWcUYwBKlZ1OUX1qS qpCRX1xiqxRtaGGkZ2hpoWdkYqlnaGwea2VkqqRvZ5OSmpNZllqkb5egl7Hi5m+mgp2+Fcua +9gaGP/adDFyckgImEj0fjzI2MXIxSEksJRR4vPbXawQCSmJH78uQNnCEn+udbFBFL1nlJjx t4cRJCEsECbx58RdFhBbRKCbUWLH2TqIopOMEtfXTQTrZhPQkTi1+ghYA6+AncScH1uYQWwW AVWJZa0nwWpEBSIk7l58wQJRIyhxcuYTMJtTwEFi3saLYPXMAuoSf+ZdgrLFJZq+rGSFsOUl tr+dwzyBUXAWkvZZSFpmIWmZhaRlASPLKkaR1NLi3PTcYiO94sTc4tK8dL3k/NxNjMBo2nbs 55YdjF3vgg8xCnAwKvHwajB2xgixJpYVV+YeYpTgYFYS4T0iDxTiTUmsrEotyo8vKs1JLT7E aAr03ERmKdHkfGCk55XEG5oamltYGpobmxubWSiJ8543qIwSEkhPLEnNTk0tSC2C6WPi4JRq YJw45/KRsEB2/k4xzqthZ0I0E9n/f3shs7KA/dac7LPz46frfpPj44rVO2DF+Ubj6nzDs8Xr l/t6S2i5qFg4dLlYLlqgaXgzZ9OhDgnjJRbJB+MdDmx8prLVcuVJphl6RhHeGw/3/tV3UV9T 7PtJcLa61irZYuZy64+xCu7u/uE+V97KHV8wX4mlOCPRUIu5qDgRAEr6yFK8AgAA X-CMS-MailID: 20190313160927eucas1p27d48ed91797cfa4cc458a33a47a726b1 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20190313155518epcas2p4eb04eee1df9960adfebddd4a0120a29c X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20190313155518epcas2p4eb04eee1df9960adfebddd4a0120a29c References: <20190313155504.15087-1-maxime.coquelin@redhat.com> <20190313155504.15087-3-maxime.coquelin@redhat.com> Subject: Re: [dpdk-dev] [PATCH v2 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: Wed, 13 Mar 2019 16:09:29 -0000 On 13.03.2019 18:55, 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 | 33 ++++--------- > lib/librte_vhost/vhost_crypto.c | 10 +++- > lib/librte_vhost/vhost_user.c | 82 +++++++++++++++++++++------------ > 3 files changed, 69 insertions(+), 56 deletions(-) > > diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h > index c9c392975..8ab4ff299 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, > }; > > /** > @@ -131,37 +133,20 @@ enum rte_vhost_msg_result { /** * Function prototype for the vhost backend to handler specific vhost user * messages prior to the master message handling Now you have definition of pre handler for a common handler type. It should be updated. Probably just cropped a bit (and s/handler/handle/): /** * Function prototype for the vhost backend to handle specific vhost user * messages. > * vhost device id > * @param msg > * Message pointer. > - * @param skip_master > - * If the handler requires skipping the master message handling, this variable > - * shall be written 1, otherwise 0. > * @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); > - > -/** > - * Function prototype for the vhost backend to handler specific vhost user > - * messages after the master message handling is done > - * > - * @param vid > - * vhost device id > - * @param msg > - * Message pointer. > - * @return > - * VH_RESULT_OK on success, VH_RESULT_REPLY on success with reply, > - * VH_RESULT_ERR on failure > - */ > -typedef enum rte_vhost_msg_result (*rte_vhost_msg_post_handle)(int vid, > - void *msg); > +typedef enum rte_vhost_msg_result (*rte_vhost_msg_handle)(int vid, void *msg); > > /** > * Optional vhost user message handlers. > */ > struct rte_vhost_user_extern_ops { > - rte_vhost_msg_pre_handle pre_msg_handle; > - rte_vhost_msg_post_handle post_msg_handle; What about some comments here? /* Called prior to the master message handling. */ > + rte_vhost_msg_handle pre_msg_handle; /* Called after the master message handling. */ > + rte_vhost_msg_handle post_msg_handle; > }; > > /** > 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 >