On 10/09/2018 10:03 AM, Tiwei Bie wrote:
On Mon, Oct 08, 2018 at 05:25:39PM +0200, 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: 2f270595c05d ("vhost: rework message handling as a callback array")Fixline should be: Fixes: 0bff510b5ea6 ("vhost: unify message handling function signature")Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com> Acked-by: Ilya Maximets <i.maxim...@samsung.com> --- 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) {- 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;We also need to take care of the return value of .pre_msg_handle() and .post_msg_handle(). Otherwise, errors could be ignored.
Right. What I plan to do is setting ret to VH_RESULT_ERR if pre/post_msg_handler() return is negative. Does that sound reasonable to you? I see that vhost-crypto does not respect the callbacks documentation wrt to return value, and we might want to change the callbacks documentation so that it returns an enum vh_result.
-- 2.17.1
