From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 340334C9D for ; Sat, 31 Mar 2018 06:00:09 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 Mar 2018 21:00:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,383,1517904000"; d="scan'208";a="42805807" Received: from tanjianf-mobl.ccr.corp.intel.com (HELO [10.255.29.250]) ([10.255.29.250]) by fmsmga001.fm.intel.com with ESMTP; 30 Mar 2018 21:00:07 -0700 To: Junjie Chen , maxime.coquelin@redhat.com, mtetsuyah@gmail.com References: <1519293583-54933-1-git-send-email-junjie.j.chen@intel.com> <1522334234-98309-1-git-send-email-junjie.j.chen@intel.com> Cc: dev@dpdk.org From: "Tan, Jianfeng" Message-ID: Date: Sat, 31 Mar 2018 12:00:04 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1522334234-98309-1-git-send-email-junjie.j.chen@intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] vhost: add support for interrupt mode 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: Sat, 31 Mar 2018 04:00:10 -0000 On 3/29/2018 10:37 PM, Junjie Chen wrote: > In some cases we want vhost dequeue work in interrupt mode to > release cpus to others when no data to transmit. So we install > interrupt handler of vhost device and interrupt vectors for each > rx queue when creating new backend according to vhost intrerupt > configuration. Thus, applications could register a epoll event fd > to associate rx queues with interrupt vectors. > > Signed-off-by: Junjie Chen > --- > Changes in v2: > - update rx queue index > - fill efd_counter_size for intr handler > - update log > > drivers/net/vhost/rte_eth_vhost.c | 130 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 130 insertions(+) > > diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c > index 3aae01c..b2d925e 100644 > --- a/drivers/net/vhost/rte_eth_vhost.c > +++ b/drivers/net/vhost/rte_eth_vhost.c > @@ -552,12 +552,129 @@ update_queuing_status(struct rte_eth_dev *dev) > } > > static int > +eth_rxq_intr_enable(struct rte_eth_dev *dev, uint16_t qid) > +{ > + struct rte_vhost_vring vring; > + struct vhost_queue *vq; > + int ret = 0; > + > + vq = dev->data->rx_queues[qid]; > + if (!vq) { > + RTE_LOG(ERR, PMD, "rxq%d is not setup yet\n", qid); > + return -1; > + } > + > + ret = rte_vhost_get_vhost_vring(vq->vid, (qid << 1) + 1, &vring); > + if (ret < 0) { > + RTE_LOG(ERR, PMD, "Failed to get rxq%d's vring\n", qid); > + return ret; > + } > + RTE_LOG(INFO, PMD, "Enable interrupt for rxq%d\n", qid); > + vring.used->flags &= (~VRING_USED_F_NO_NOTIFY); Instead, you might want to use the API, rte_vhost_enable_guest_notification. > + rte_wmb(); > + > + return ret; > +} > + > +static int > +eth_rxq_intr_disable(struct rte_eth_dev *dev, uint16_t qid) > +{ > + struct rte_vhost_vring vring; > + struct vhost_queue *vq; > + int ret = 0; > + > + vq = dev->data->rx_queues[qid]; > + if (!vq) { > + RTE_LOG(ERR, PMD, "rxq%d is not setup yet\n", qid); > + return -1; > + } > + > + ret = rte_vhost_get_vhost_vring(vq->vid, (qid << 1) + 1, &vring); > + if (ret < 0) { > + RTE_LOG(ERR, PMD, "Failed to get rxq%d's vring", qid); > + return ret; > + } > + RTE_LOG(INFO, PMD, "Disable interrupt for rxq%d\n", qid); > + vring.used->flags |= VRING_USED_F_NO_NOTIFY; > + rte_wmb(); Ditto. > + > + return 0; > +} > + > +static void > +eth_vhost_uninstall_intr(struct rte_eth_dev *dev) > +{ > + struct rte_intr_handle *intr_handle = dev->intr_handle; > + > + if (intr_handle) { > + if (intr_handle->intr_vec) > + free(intr_handle->intr_vec); > + free(intr_handle); > + } > + > + dev->intr_handle = NULL; > +} > + > +static int > +eth_vhost_install_intr(struct rte_eth_dev *dev) > +{ > + struct rte_vhost_vring vring; > + struct vhost_queue *vq; > + int count = 0; > + int nb_rxq = dev->data->nb_rx_queues; > + int i; > + int ret; > + > + /* uninstall firstly if we are reconnecting */ > + if (dev->intr_handle) > + eth_vhost_uninstall_intr(dev); > + > + dev->intr_handle = malloc(sizeof(*dev->intr_handle)); > + if (!dev->intr_handle) { > + RTE_LOG(ERR, PMD, "Fail to allocate intr_handle\n"); > + return -ENOMEM; > + } > + memset(dev->intr_handle, 0, sizeof(*dev->intr_handle)); > + > + dev->intr_handle->efd_counter_size = sizeof(uint64_t); I did not read-clean this for virtio-user. But we'd better keep consistent, to avoid confusion. If we need read-clean this, we could fix virtio-user. > + > + dev->intr_handle->intr_vec = > + malloc(nb_rxq * sizeof(dev->intr_handle->intr_vec[0])); > + > + if (!dev->intr_handle->intr_vec) { > + RTE_LOG(ERR, PMD, > + "Failed to allocate memory for interrupt vector\n"); For completeness, we need to uninstall before return. > + return -ENOMEM; > + } > + > + for (i = 0; i < nb_rxq; i++) { > + vq = dev->data->rx_queues[i]; > + if (!vq) > + continue; Put such check at the entry of this function? > + ret = rte_vhost_get_vhost_vring(vq->vid, (i << 1) + 1, &vring); > + if (ret < 0) Does that mean we shall take care of this failure? > + continue; > + RTE_LOG(INFO, PMD, "Install intr vec for rxq%d\n", i); > + dev->intr_handle->intr_vec[i] = RTE_INTR_VEC_RXTX_OFFSET + i; > + dev->intr_handle->efds[i] = vring.kickfd; Before use of the kickfd, we need to check the validation? > + count++; > + } > + > + dev->intr_handle->nb_efd = count; > + dev->intr_handle->max_intr = count + 1; > + dev->intr_handle->type = RTE_INTR_HANDLE_VDEV; > + > + return 0; > +} > + > +static int > new_device(int vid) > { > struct rte_eth_dev *eth_dev; > struct internal_list *list; > struct pmd_internal *internal; > struct vhost_queue *vq; > + struct rte_eth_conf *dev_conf; > unsigned i; > char ifname[PATH_MAX]; > #ifdef RTE_LIBRTE_VHOST_NUMA > @@ -609,6 +726,15 @@ new_device(int vid) > > RTE_LOG(INFO, PMD, "Vhost device %d created\n", vid); > > + dev_conf = ð_dev->data->dev_conf; > + > + if (dev_conf->intr_conf.rxq) { > + if (eth_vhost_install_intr(eth_dev) < 0) { > + RTE_LOG(INFO, PMD, "Failed to prepare intr handler."); > + return -1; > + } > + } > + > _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL); > > return 0; > @@ -663,6 +789,8 @@ destroy_device(int vid) > > RTE_LOG(INFO, PMD, "Vhost device %d destroyed\n", vid); > > + eth_vhost_uninstall_intr(eth_dev); > + > _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL); > } > > @@ -1007,6 +1135,8 @@ static const struct eth_dev_ops ops = { > .xstats_reset = vhost_dev_xstats_reset, > .xstats_get = vhost_dev_xstats_get, > .xstats_get_names = vhost_dev_xstats_get_names, > + .rx_queue_intr_enable = eth_rxq_intr_enable, > + .rx_queue_intr_disable = eth_rxq_intr_disable, > }; > > static struct rte_vdev_driver pmd_vhost_drv;