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 C5187234 for ; Thu, 29 Jan 2015 06:06:46 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP; 28 Jan 2015 21:00:32 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="446830759" Received: from pgsmsx105.gar.corp.intel.com ([10.221.44.96]) by FMSMGA003.fm.intel.com with ESMTP; 28 Jan 2015 20:52:56 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by PGSMSX105.gar.corp.intel.com (10.221.44.96) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 29 Jan 2015 13:06:42 +0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.253]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.168]) with mapi id 14.03.0195.001; Thu, 29 Jan 2015 13:06:41 +0800 From: "Qiu, Michael" To: "Zhou, Danny" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v1 4/5] eal: add per rx queue interrupt handling based on VFIO Thread-Index: AQHQOuApxy7rc+GviEapFzOtR/yBOw== Date: Thu, 29 Jan 2015 05:06:40 +0000 Message-ID: <533710CFB86FA344BFBF2D6802E60286CCFB9F@SHSMSX101.ccr.corp.intel.com> References: <1422438631-7853-1-git-send-email-danny.zhou@intel.com> <1422438631-7853-5-git-send-email-danny.zhou@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: 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 v1 4/5] eal: add per rx queue interrupt handling based on VFIO X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Jan 2015 05:06:47 -0000 On 1/28/2015 5:52 PM, Danny Zhou wrote:=0A= > Signed-off-by: Danny Zhou =0A= > Signed-off-by: Yong Liu =0A= > ---=0A= =0A= [...]=0A= =0A= > +static void=0A= > +eal_intr_process_rx_interrupts(uint8_t port_id, struct epoll_event *even= ts, int nfds)=0A= > +{=0A= > + int n, bytes_read;=0A= > + union rte_intr_read_buffer buf;=0A= > + struct rte_intr_handle intr_handle =3D rte_eth_devices[port_id].pci_dev= ->intr_handle;=0A= > +=0A= > + for (n =3D 0; n < nfds; n++) {=0A= > + /* set the length to be read dor different handle type */=0A= > + switch (intr_handle.type) {=0A= > + case RTE_INTR_HANDLE_UIO:=0A= > + bytes_read =3D sizeof(buf.uio_intr_count);=0A= > + break;=0A= > + case RTE_INTR_HANDLE_ALARM:=0A= > + bytes_read =3D sizeof(buf.timerfd_num);=0A= > + break;=0A= > +#ifdef VFIO_PRESENT=0A= > + case RTE_INTR_HANDLE_VFIO_MSIX:=0A= > + case RTE_INTR_HANDLE_VFIO_MSI:=0A= > + case RTE_INTR_HANDLE_VFIO_LEGACY:=0A= > + bytes_read =3D sizeof(buf.vfio_intr_count);=0A= > + break;=0A= > +#endif=0A= > + default:=0A= > + bytes_read =3D 1;=0A= > + break;=0A= > + }=0A= > +=0A= > + /**=0A= > + * read out to clear the ready-to-be-read flag=0A= > + * for epoll_wait.=0A= > + */=0A= > + bytes_read =3D read(events[n].data.fd, &buf, bytes_read);=0A= > + if (bytes_read < 0)=0A= > + RTE_LOG(ERR, EAL, "Error reading from file "=0A= > + "descriptor %d: %s\n", events[n].data.fd,=0A= > + strerror(errno));=0A= > + else if (bytes_read =3D=3D 0)=0A= > + RTE_LOG(ERR, EAL, "Read nothing from file "=0A= > + "descriptor %d\n", events[n].data.fd);=0A= =0A= Is there any issue while bytes_read is not equal to the count need to be=0A= read?=0A= =0A= > + }=0A= > +}=0A= > +=0A= > +static void=0A= > +eal_intr_handle_rx_interrupts(uint8_t port_id, int pfd, unsigned totalfd= s)=0A= > +{=0A= > + struct epoll_event events[totalfds];=0A= > + int nfds =3D 0;=0A= > +=0A= > +m_wait:=0A= > + nfds =3D epoll_wait(pfd, events, totalfds,=0A= > + EAL_INTR_EPOLL_WAIT_FOREVER);=0A= > + /* epoll_wait fail */=0A= > + if (nfds < 0) {=0A= > + RTE_LOG(ERR, EAL,=0A= > + "epoll_wait returns with fail\n");=0A= > + return;=0A= > + }=0A= > + /* epoll_wait timeout, will never happens here */=0A= > + else if (nfds =3D=3D 0)=0A= > + goto m_wait;=0A= > + /* epoll_wait has at least one fd ready to read */=0A= > + eal_intr_process_rx_interrupts(port_id, events, nfds);=0A= > +}=0A= > +=0A= > +int=0A= > +rte_eal_wait_rx_intr(uint8_t port_id, uint8_t queue_id)=0A= > +{=0A= > + struct rte_intr_handle intr_handle =3D rte_eth_devices[port_id].pci_dev= ->intr_handle;=0A= > + struct epoll_event ev;=0A= > + unsigned numfds =3D 0;=0A= > +=0A= > + /* create epoll fd */=0A= > + int pfd =3D epoll_create(1);=0A= > + if (pfd < 0) {=0A= > + RTE_LOG(ERR, EAL, "Cannot create epoll instance\n");=0A= > + return -1;=0A= > + }=0A= > +=0A= > + rte_spinlock_lock(&intr_lock);=0A= > +=0A= > + ev.events =3D EPOLLIN | EPOLLPRI;=0A= > + switch (intr_handle.type) {=0A= > + case RTE_INTR_HANDLE_UIO:=0A= > + ev.data.fd =3D intr_handle.fd;=0A= > + break;=0A= > +#ifdef VFIO_PRESENT=0A= > + case RTE_INTR_HANDLE_VFIO_MSIX:=0A= > + ev.data.fd =3D intr_handle.queue_fd[queue_id];=0A= > + break;=0A= > + case RTE_INTR_HANDLE_VFIO_MSI:=0A= > + ev.data.fd =3D intr_handle.queue_fd[queue_id];=0A= > + break;=0A= > + case RTE_INTR_HANDLE_VFIO_LEGACY:=0A= > + ev.data.fd =3D intr_handle.queue_fd[queue_id];=0A= > + break;=0A= =0A= As those three branches are all the same, why not combine to one?=0A= =0A= + case RTE_INTR_HANDLE_VFIO_MSIX:=0A= + case RTE_INTR_HANDLE_VFIO_MSI:=0A= + case RTE_INTR_HANDLE_VFIO_LEGACY:=0A= + ev.data.fd =3D intr_handle.queue_fd[queue_id];=0A= + break;=0A= =0A= =0A= > +#endif=0A= > + default:=0A= > + break;=0A= > + close(pfd);=0A= > + return -1;=0A= =0A= Here Steve has pointed out, but indentation should be a issue.=0A= =0A= > + }=0A= > +=0A= > + if (epoll_ctl(pfd, EPOLL_CTL_ADD, ev.data.fd, &ev) < 0) {=0A= > + RTE_LOG(ERR, EAL, "Error adding fd %d epoll_ctl, %s\n",=0A= > + intr_handle.queue_fd[queue_id], strerror(errno));=0A= > + } else=0A= > + numfds++;=0A= > +=0A= > + rte_spinlock_unlock(&intr_lock);=0A= > + /* serve the interrupt */=0A= > + eal_intr_handle_rx_interrupts(port_id, pfd, numfds);=0A= > +=0A= > + /**=0A= > + * when we return, we need to rebuild the=0A= > + * list of fds to monitor.=0A= > + */=0A= > + close(pfd);=0A= > +=0A= > + return 0;=0A= > +}=0A= > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/= linuxapp/eal/eal_pci_vfio.c=0A= > index 20e0977..63d0ae8 100644=0A= > --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c=0A= > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c=0A= > @@ -283,7 +283,6 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev,= int vfio_dev_fd)=0A= > =0A= > dev->intr_handle.fd =3D fd;=0A= > dev->intr_handle.vfio_dev_fd =3D vfio_dev_fd;=0A= > -=0A= > switch (i) {=0A= > case VFIO_PCI_MSIX_IRQ_INDEX:=0A= > internal_config.vfio_intr_mode =3D RTE_INTR_MODE_MSIX;=0A= > @@ -302,6 +301,16 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev= , int vfio_dev_fd)=0A= > return -1;=0A= > }=0A= > =0A= > + for (i =3D 0; i < VFIO_MAX_QUEUE_ID; i++) {=0A= > + fd =3D eventfd(0, 0);=0A= > + if (fd < 0) {=0A= > + RTE_LOG(ERR, EAL, " cannot set up eventfd, "=0A= > + "error %i (%s)\n", errno, strerror(errno));=0A= > + return -1;=0A= > + }=0A= > + dev->intr_handle.queue_fd[i] =3D fd;=0A= > + }=0A= > +=0A= > return 0;=0A= > }=0A= > =0A= > diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.= h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h=0A= > index 23eafd9..c6982cf 100644=0A= > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h=0A= > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h=0A= > @@ -38,6 +38,8 @@=0A= > #ifndef _RTE_LINUXAPP_INTERRUPTS_H_=0A= > #define _RTE_LINUXAPP_INTERRUPTS_H_=0A= > =0A= > +#define VFIO_MAX_QUEUE_ID 32=0A= > +=0A= > enum rte_intr_handle_type {=0A= > RTE_INTR_HANDLE_UNKNOWN =3D 0,=0A= > RTE_INTR_HANDLE_UIO, /**< uio device handle */=0A= > @@ -52,6 +54,8 @@ enum rte_intr_handle_type {=0A= > struct rte_intr_handle {=0A= > int vfio_dev_fd; /**< VFIO device file descriptor */=0A= > int fd; /**< file descriptor */=0A= > + int max_intr; /**< max interrupt requested */=0A= > + int queue_fd[VFIO_MAX_QUEUE_ID]; /**< rx and tx queue interrupt file de= scriptor */=0A= > enum rte_intr_handle_type type; /**< handle type */=0A= > };=0A= > =0A= =0A=