From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id C9B785B36 for ; Tue, 10 Jul 2018 07:57:23 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Jul 2018 22:57:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,333,1526367600"; d="scan'208";a="214754680" Received: from debian.sh.intel.com (HELO debian) ([10.67.104.228]) by orsmga004.jf.intel.com with ESMTP; 09 Jul 2018 22:57:10 -0700 Date: Tue, 10 Jul 2018 13:56:57 +0800 From: Tiwei Bie To: Nikolay Nikolaev Cc: maxime.coquelin@redhat.com, zhihong.wang@intel.com, dev@dpdk.org Message-ID: <20180710055657.GB10600@debian> References: <153002988259.22089.8523468795459281187.stgit@T460> <153002997827.22089.15583649737397622971.stgit@T460> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <153002997827.22089.15583649737397622971.stgit@T460> User-Agent: Mutt/1.10.0 (2018-05-17) Subject: Re: [dpdk-dev] [PATCH v1 5/5] vhost: message handling implemented as a callback array 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, 10 Jul 2018 05:57:24 -0000 On Tue, Jun 26, 2018 at 07:19:38PM +0300, Nikolay Nikolaev wrote: > Introduce vhost_message_handlers, which maps the message request > type to the message handler. Then replace the switch construct > with a map and call. > > Signed-off-by: Nikolay Nikolaev > --- > lib/librte_vhost/vhost_user.c | 144 ++++++++++++++++------------------------- > 1 file changed, 55 insertions(+), 89 deletions(-) > > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c > index dd47d84c7..e62164d63 100644 > --- a/lib/librte_vhost/vhost_user.c > +++ b/lib/librte_vhost/vhost_user.c > @@ -1379,6 +1379,36 @@ vhost_user_iotlb_msg(struct virtio_net **pdev, VhostUserMsg *msg) > return VH_RESULT_OK; > } > > +typedef int (*message_handler)(struct virtio_net **pdev, VhostUserMsg * msg); s/message_handler/vhost_message_handler_t/ > +static message_handler vhost_message_handlers[VHOST_USER_MAX] = { > + [VHOST_USER_NONE] = NULL, > + [VHOST_USER_GET_FEATURES] = vhost_user_get_features, > + [VHOST_USER_SET_FEATURES] = vhost_user_set_features, > + [VHOST_USER_SET_OWNER] = vhost_user_set_owner, > + [VHOST_USER_RESET_OWNER] = vhost_user_reset_owner, > + [VHOST_USER_SET_MEM_TABLE] = vhost_user_set_mem_table, > + [VHOST_USER_SET_LOG_BASE] = vhost_user_set_log_base, > + [VHOST_USER_SET_LOG_FD] = vhost_user_set_log_fd, > + [VHOST_USER_SET_VRING_NUM] = vhost_user_set_vring_num, > + [VHOST_USER_SET_VRING_ADDR] = vhost_user_set_vring_addr, > + [VHOST_USER_SET_VRING_BASE] = vhost_user_set_vring_base, > + [VHOST_USER_GET_VRING_BASE] = vhost_user_get_vring_base, > + [VHOST_USER_SET_VRING_KICK] = vhost_user_set_vring_kick, > + [VHOST_USER_SET_VRING_CALL] = vhost_user_set_vring_call, > + [VHOST_USER_SET_VRING_ERR] = vhost_user_set_vring_err, > + [VHOST_USER_GET_PROTOCOL_FEATURES] = vhost_user_get_protocol_features, > + [VHOST_USER_SET_PROTOCOL_FEATURES] = vhost_user_set_protocol_features, > + [VHOST_USER_GET_QUEUE_NUM] = vhost_user_get_queue_num, > + [VHOST_USER_SET_VRING_ENABLE] = vhost_user_set_vring_enable, > + [VHOST_USER_SEND_RARP] = vhost_user_send_rarp, > + [VHOST_USER_NET_SET_MTU] = vhost_user_net_set_mtu, > + [VHOST_USER_SET_SLAVE_REQ_FD] = vhost_user_set_req_fd, > + [VHOST_USER_IOTLB_MSG] = vhost_user_iotlb_msg, Please keep using one space between "]" and "=". > + [VHOST_USER_CRYPTO_CREATE_SESS] = NULL, > + [VHOST_USER_CRYPTO_CLOSE_SESS] = NULL, There is no need to zero above two explicitly. > +}; > + > + > /* return bytes# of read on success or negative val on failure. */ > static int > read_vhost_message(int sockfd, VhostUserMsg *msg) > @@ -1623,97 +1653,33 @@ vhost_user_msg_handler(int vid, int fd) > goto skip_to_post_handle; > } > > - switch (msg.request.master) { > - case VHOST_USER_GET_FEATURES: > - ret = vhost_user_get_features(&dev, &msg); > - send_vhost_reply(fd, &msg); > - break; > - case VHOST_USER_SET_FEATURES: > - ret = vhost_user_set_features(&dev, &msg); > - if (ret) > - return -1; > - break; > - > - case VHOST_USER_GET_PROTOCOL_FEATURES: > - ret = vhost_user_get_protocol_features(&dev, &msg); > - send_vhost_reply(fd, &msg); > - break; > - case VHOST_USER_SET_PROTOCOL_FEATURES: > - ret = vhost_user_set_protocol_features(&dev, &msg); > - break; > - > - case VHOST_USER_SET_OWNER: > - ret = vhost_user_set_owner(&dev, &msg); > - break; > - case VHOST_USER_RESET_OWNER: > - ret = vhost_user_reset_owner(&dev, &msg); > - break; > - > - case VHOST_USER_SET_MEM_TABLE: > - ret = vhost_user_set_mem_table(&dev, &msg); > - break; > - > - case VHOST_USER_SET_LOG_BASE: > - ret = vhost_user_set_log_base(&dev, &msg); > - send_vhost_reply(fd, &msg); > - break; > - case VHOST_USER_SET_LOG_FD: > - ret = vhost_user_set_log_fd(&dev, &msg); > - break; > - > - case VHOST_USER_SET_VRING_NUM: > - ret = vhost_user_set_vring_num(&dev, &msg); > - break; > - case VHOST_USER_SET_VRING_ADDR: > - ret = vhost_user_set_vring_addr(&dev, &msg); > - break; > - case VHOST_USER_SET_VRING_BASE: > - ret = vhost_user_set_vring_base(&dev, &msg); > - break; > Please also remove above blank line. > - case VHOST_USER_GET_VRING_BASE: > - ret = vhost_user_get_vring_base(&dev, &msg); > - send_vhost_reply(fd, &msg); > - break; > + int request = msg.request.master; Please define variable at the beginning of the function. > + if (vhost_message_handlers[request]) { The `request` needs to be checked with VHOST_USER_MAX first. > + ret = vhost_message_handlers[request](&dev, &msg); > [...] > - > - default: > - ret = -1; > - break; > + switch (ret) { > + case VH_RESULT_ERR: > + RTE_LOG(ERR, VHOST_CONFIG, > + "Processing %s failed.\n", > + vhost_message_str[request]); > + return -1; Queue pairs need to be unlocked when necessary. > + case VH_RESULT_OK: > + RTE_LOG(DEBUG, VHOST_CONFIG, > + "Processing %s succeeded.\n", > + vhost_message_str[request]); > + break; > + case VH_RESULT_REPLY: > + RTE_LOG(INFO, VHOST_CONFIG, > + "Processing %s succeeded and needs reply.\n", > + vhost_message_str[request]); > + send_vhost_reply(fd, &msg); > + break; > + } > + } else { > + RTE_LOG(ERR, VHOST_CONFIG, > + "Requested invalid message type %d.\n", request); > + return -1; The old behaviour is changed, VHOST_USER_CRYPTO_CREATE_SESS and VHOST_USER_CRYPTO_CLOSE_SESS will be broken. > } > > skip_to_post_handle: >