From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 0EF581E35 for ; Mon, 2 Apr 2018 10:51:34 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 Apr 2018 01:51:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,394,1517904000"; d="scan'208";a="40027711" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga003.jf.intel.com with ESMTP; 02 Apr 2018 01:51:32 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 2 Apr 2018 01:51:32 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.166]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.129]) with mapi id 14.03.0319.002; Mon, 2 Apr 2018 16:51:30 +0800 From: "Chen, Junjie J" To: "Tan, Jianfeng" , "maxime.coquelin@redhat.com" , "mtetsuyah@gmail.com" CC: "dev@dpdk.org" Thread-Topic: [PATCH v2] vhost: add support for interrupt mode Thread-Index: AQHTxytnTkBUiyzYG0yScu1fB3bO76PpNQwAgAP38bA= Date: Mon, 2 Apr 2018 08:51:30 +0000 Message-ID: References: <1519293583-54933-1-git-send-email-junjie.j.chen@intel.com> <1522334234-98309-1-git-send-email-junjie.j.chen@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZDdhZjg4NmEtN2FkNS00MzZkLTg2OWMtMTc1ZDVjYzk0ZDkyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJ0N3pVYnJcL1VSZjBBcmI3WXdWbFVPbU1tRXFCVzNUREFFNDByYmRsTlowSWZ3czNuNnhmYWhMTkEyYjY2MEpMZSJ9 x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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: Mon, 02 Apr 2018 08:51:36 -0000 >=20 >=20 >=20 > 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 =3D 0; > > + > > + vq =3D dev->data->rx_queues[qid]; > > + if (!vq) { > > + RTE_LOG(ERR, PMD, "rxq%d is not setup yet\n", qid); > > + return -1; > > + } > > + > > + ret =3D 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 &=3D (~VRING_USED_F_NO_NOTIFY); >=20 > Instead, you might want to use the API, rte_vhost_enable_guest_notificati= on. Will do. >=20 > > + 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 =3D 0; > > + > > + vq =3D dev->data->rx_queues[qid]; > > + if (!vq) { > > + RTE_LOG(ERR, PMD, "rxq%d is not setup yet\n", qid); > > + return -1; > > + } > > + > > + ret =3D 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 |=3D VRING_USED_F_NO_NOTIFY; > > + rte_wmb(); >=20 > Ditto. Will do. >=20 > > + > > + return 0; > > +} > > + > > +static void > > +eth_vhost_uninstall_intr(struct rte_eth_dev *dev) { > > + struct rte_intr_handle *intr_handle =3D dev->intr_handle; > > + > > + if (intr_handle) { > > + if (intr_handle->intr_vec) > > + free(intr_handle->intr_vec); > > + free(intr_handle); > > + } > > + > > + dev->intr_handle =3D NULL; > > +} > > + > > +static int > > +eth_vhost_install_intr(struct rte_eth_dev *dev) { > > + struct rte_vhost_vring vring; > > + struct vhost_queue *vq; > > + int count =3D 0; > > + int nb_rxq =3D 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 =3D 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 =3D sizeof(uint64_t); >=20 > I did not read-clean this for virtio-user. But we'd better keep consisten= t, to > avoid confusion. If we need read-clean this, we could fix virtio-user. So, do you want to change this in this patch? Or in other separated one? >=20 > > + > > + dev->intr_handle->intr_vec =3D > > + 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"); >=20 > For completeness, we need to uninstall before return. Will do. >=20 > > + return -ENOMEM; > > + } > > + > > + for (i =3D 0; i < nb_rxq; i++) { > > + vq =3D dev->data->rx_queues[i]; > > + if (!vq) > > + continue; >=20 > Put such check at the entry of this function? >=20 > > + ret =3D rte_vhost_get_vhost_vring(vq->vid, (i << 1) + 1, &vring); > > + if (ret < 0) >=20 > Does that mean we shall take care of this failure? For above two checks, I had a fix to check overall queue setup in patch: ht= tps://dpdk.org/dev/patchwork/patch/36766/ recently. So I would like to put install_intr function into queue_setup helper functi= on in patch 36766, and for internal vq, vring, and kickfd check of install_= intr, I'd like to skip setup if check fail. How do you think?=20 >=20 > > + continue; > > + RTE_LOG(INFO, PMD, "Install intr vec for rxq%d\n", i); > > + dev->intr_handle->intr_vec[i] =3D RTE_INTR_VEC_RXTX_OFFSET + i; > > + dev->intr_handle->efds[i] =3D vring.kickfd; >=20 > Before use of the kickfd, we need to check the validation? Will do. >=20 > > + count++; > > + } > > + > > + dev->intr_handle->nb_efd =3D count; > > + dev->intr_handle->max_intr =3D count + 1; > > + dev->intr_handle->type =3D 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 =3D ð_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 =3D { > > .xstats_reset =3D vhost_dev_xstats_reset, > > .xstats_get =3D vhost_dev_xstats_get, > > .xstats_get_names =3D vhost_dev_xstats_get_names, > > + .rx_queue_intr_enable =3D eth_rxq_intr_enable, > > + .rx_queue_intr_disable =3D eth_rxq_intr_disable, > > }; > > > > static struct rte_vdev_driver pmd_vhost_drv;