From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id E0335255 for ; Wed, 28 Jan 2015 19:10:53 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP; 28 Jan 2015 10:06:10 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="446590573" Received: from kmsmsx151.gar.corp.intel.com ([172.21.73.86]) by FMSMGA003.fm.intel.com with ESMTP; 28 Jan 2015 09:56:43 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by KMSMSX151.gar.corp.intel.com (172.21.73.86) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 29 Jan 2015 02:10:27 +0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.238]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.231]) with mapi id 14.03.0195.001; Thu, 29 Jan 2015 02:10:26 +0800 From: "Liang, Cunming" 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: AQHQOuApTtQdCcsW4E6KPhaGUw1Wq5zVr7rA Date: Wed, 28 Jan 2015 18:10:26 +0000 Message-ID: References: <1422438631-7853-1-git-send-email-danny.zhou@intel.com> <1422438631-7853-5-git-send-email-danny.zhou@intel.com> In-Reply-To: <1422438631-7853-5-git-send-email-danny.zhou@intel.com> Accept-Language: zh-CN, 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: Wed, 28 Jan 2015 18:10:54 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Danny Zhou > Sent: Wednesday, January 28, 2015 2:51 AM > To: dev@dpdk.org > Subject: [dpdk-dev] [PATCH v1 4/5] eal: add per rx queue interrupt handli= ng > based on VFIO >=20 > Signed-off-by: Danny Zhou > Signed-off-by: Yong Liu > --- > lib/librte_eal/common/include/rte_eal.h | 9 + > lib/librte_eal/linuxapp/eal/eal_interrupts.c | 186 > ++++++++++++++++----- > lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 11 +- > .../linuxapp/eal/include/exec-env/rte_interrupts.h | 4 + > 4 files changed, 168 insertions(+), 42 deletions(-) >=20 > diff --git a/lib/librte_eal/common/include/rte_eal.h > b/lib/librte_eal/common/include/rte_eal.h > index f4ecd2e..5f31aa5 100644 > --- a/lib/librte_eal/common/include/rte_eal.h > +++ b/lib/librte_eal/common/include/rte_eal.h > @@ -150,6 +150,15 @@ int rte_eal_iopl_init(void); > * - On failure, a negative error value. > */ > int rte_eal_init(int argc, char **argv); > + > +/** > + * @param port_id > + * the port id > + * @return > + * - On success, return 0 [LCM] It has changes to return -1. > + */ > +int rte_eal_wait_rx_intr(uint8_t port_id, uint8_t queue_id); > + > /** > * Usage function typedef used by the application usage function. > * > diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c > b/lib/librte_eal/linuxapp/eal/eal_interrupts.c > index dc2668a..b120303 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c > +++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c > @@ -64,6 +64,7 @@ > #include > #include > #include > +#include >=20 > #include "eal_private.h" > #include "eal_vfio.h" > @@ -127,6 +128,7 @@ static pthread_t intr_thread; > #ifdef VFIO_PRESENT >=20 > #define IRQ_SET_BUF_LEN (sizeof(struct vfio_irq_set) + sizeof(int)) > +#define MSIX_IRQ_SET_BUF_LEN (sizeof(struct vfio_irq_set) + sizeof(int) = * > (VFIO_MAX_QUEUE_ID + 1)) >=20 > /* enable legacy (INTx) interrupts */ > static int > @@ -221,7 +223,7 @@ vfio_disable_intx(struct rte_intr_handle *intr_handle= ) { > /* enable MSI-X interrupts */ > static int > vfio_enable_msi(struct rte_intr_handle *intr_handle) { > - int len, ret; > + int len, ret, max_intr; > char irq_set_buf[IRQ_SET_BUF_LEN]; > struct vfio_irq_set *irq_set; > int *fd_ptr; > @@ -230,12 +232,19 @@ vfio_enable_msi(struct rte_intr_handle *intr_handle= ) > { >=20 > irq_set =3D (struct vfio_irq_set *) irq_set_buf; > irq_set->argsz =3D len; > - irq_set->count =3D 1; > + if ((!intr_handle->max_intr) || > + (intr_handle->max_intr > VFIO_MAX_QUEUE_ID)) > + max_intr =3D VFIO_MAX_QUEUE_ID + 1; > + else > + max_intr =3D intr_handle->max_intr; > + > + irq_set->count =3D max_intr; > irq_set->flags =3D VFIO_IRQ_SET_DATA_EVENTFD | > VFIO_IRQ_SET_ACTION_TRIGGER; > irq_set->index =3D VFIO_PCI_MSI_IRQ_INDEX; > irq_set->start =3D 0; > fd_ptr =3D (int *) &irq_set->data; > - *fd_ptr =3D intr_handle->fd; > + memcpy(fd_ptr, intr_handle->queue_fd, sizeof(intr_handle->queue_fd)); > + fd_ptr[max_intr - 1] =3D intr_handle->fd; >=20 > ret =3D ioctl(intr_handle->vfio_dev_fd, VFIO_DEVICE_SET_IRQS, irq_set); >=20 > @@ -244,23 +253,6 @@ vfio_enable_msi(struct rte_intr_handle *intr_handle)= { > intr_handle->fd); > return -1; > } > - > - /* manually trigger interrupt to enable it */ > - memset(irq_set, 0, len); > - len =3D sizeof(struct vfio_irq_set); > - irq_set->argsz =3D len; > - irq_set->count =3D 1; > - irq_set->flags =3D VFIO_IRQ_SET_DATA_NONE | > VFIO_IRQ_SET_ACTION_TRIGGER; > - irq_set->index =3D VFIO_PCI_MSI_IRQ_INDEX; > - irq_set->start =3D 0; > - > - ret =3D ioctl(intr_handle->vfio_dev_fd, VFIO_DEVICE_SET_IRQS, irq_set); > - > - if (ret) { > - RTE_LOG(ERR, EAL, "Error triggering MSI interrupts for fd %d\n", > - intr_handle->fd); > - return -1; > - } > return 0; > } >=20 > @@ -292,8 +284,8 @@ vfio_disable_msi(struct rte_intr_handle *intr_handle)= { > /* enable MSI-X interrupts */ > static int > vfio_enable_msix(struct rte_intr_handle *intr_handle) { > - int len, ret; > - char irq_set_buf[IRQ_SET_BUF_LEN]; > + int len, ret, max_intr; > + char irq_set_buf[MSIX_IRQ_SET_BUF_LEN]; > struct vfio_irq_set *irq_set; > int *fd_ptr; >=20 > @@ -301,12 +293,19 @@ vfio_enable_msix(struct rte_intr_handle *intr_handl= e) > { >=20 > irq_set =3D (struct vfio_irq_set *) irq_set_buf; > irq_set->argsz =3D len; > - irq_set->count =3D 1; > + if ((!intr_handle->max_intr) || > + (intr_handle->max_intr > VFIO_MAX_QUEUE_ID)) > + max_intr =3D VFIO_MAX_QUEUE_ID + 1; > + else > + max_intr =3D intr_handle->max_intr; > + > + irq_set->count =3D max_intr; > irq_set->flags =3D VFIO_IRQ_SET_DATA_EVENTFD | > VFIO_IRQ_SET_ACTION_TRIGGER; > irq_set->index =3D VFIO_PCI_MSIX_IRQ_INDEX; > irq_set->start =3D 0; > fd_ptr =3D (int *) &irq_set->data; > - *fd_ptr =3D intr_handle->fd; > + memcpy(fd_ptr, intr_handle->queue_fd, sizeof(intr_handle->queue_fd)); > + fd_ptr[max_intr - 1] =3D intr_handle->fd; >=20 > ret =3D ioctl(intr_handle->vfio_dev_fd, VFIO_DEVICE_SET_IRQS, irq_set); >=20 > @@ -316,22 +315,6 @@ vfio_enable_msix(struct rte_intr_handle *intr_handle= ) > { > return -1; > } >=20 > - /* manually trigger interrupt to enable it */ > - memset(irq_set, 0, len); > - len =3D sizeof(struct vfio_irq_set); > - irq_set->argsz =3D len; > - irq_set->count =3D 1; > - irq_set->flags =3D VFIO_IRQ_SET_DATA_NONE | > VFIO_IRQ_SET_ACTION_TRIGGER; > - irq_set->index =3D VFIO_PCI_MSIX_IRQ_INDEX; > - irq_set->start =3D 0; > - > - ret =3D ioctl(intr_handle->vfio_dev_fd, VFIO_DEVICE_SET_IRQS, irq_set); > - > - if (ret) { > - RTE_LOG(ERR, EAL, "Error triggering MSI-X interrupts for fd %d\n", > - intr_handle->fd); > - return -1; > - } > return 0; > } >=20 > @@ -339,7 +322,7 @@ vfio_enable_msix(struct rte_intr_handle *intr_handle)= { > static int > vfio_disable_msix(struct rte_intr_handle *intr_handle) { > struct vfio_irq_set *irq_set; > - char irq_set_buf[IRQ_SET_BUF_LEN]; > + char irq_set_buf[MSIX_IRQ_SET_BUF_LEN]; > int len, ret; >=20 > len =3D sizeof(struct vfio_irq_set); > @@ -824,3 +807,124 @@ rte_eal_intr_init(void) > return -ret; > } >=20 > +static void > +eal_intr_process_rx_interrupts(uint8_t port_id, struct epoll_event *even= ts, int > nfds) > +{ > + int n, bytes_read; > + union rte_intr_read_buffer buf; > + struct rte_intr_handle intr_handle =3D > rte_eth_devices[port_id].pci_dev->intr_handle; > + > + for (n =3D 0; n < nfds; n++) { > + /* set the length to be read dor different handle type */ > + switch (intr_handle.type) { > + case RTE_INTR_HANDLE_UIO: > + bytes_read =3D sizeof(buf.uio_intr_count); > + break; > + case RTE_INTR_HANDLE_ALARM: > + bytes_read =3D sizeof(buf.timerfd_num); > + break; > +#ifdef VFIO_PRESENT > + case RTE_INTR_HANDLE_VFIO_MSIX: > + case RTE_INTR_HANDLE_VFIO_MSI: > + case RTE_INTR_HANDLE_VFIO_LEGACY: > + bytes_read =3D sizeof(buf.vfio_intr_count); > + break; > +#endif > + default: > + bytes_read =3D 1; > + break; > + } > + > + /** > + * read out to clear the ready-to-be-read flag > + * for epoll_wait. > + */ > + bytes_read =3D read(events[n].data.fd, &buf, bytes_read); > + if (bytes_read < 0) > + RTE_LOG(ERR, EAL, "Error reading from file " > + "descriptor %d: %s\n", events[n].data.fd, > + strerror(errno)); > + else if (bytes_read =3D=3D 0) > + RTE_LOG(ERR, EAL, "Read nothing from file " > + "descriptor %d\n", events[n].data.fd); > + } > +} > + > +static void > +eal_intr_handle_rx_interrupts(uint8_t port_id, int pfd, unsigned totalfd= s) > +{ > + struct epoll_event events[totalfds]; [LCM] I don't think it's a way to define array by var. > + int nfds =3D 0; > + > +m_wait: [LCM] do { > + nfds =3D epoll_wait(pfd, events, totalfds, > + EAL_INTR_EPOLL_WAIT_FOREVER); > + /* epoll_wait fail */ > + if (nfds < 0) { > + RTE_LOG(ERR, EAL, > + "epoll_wait returns with fail\n"); > + return; > + } > + /* epoll_wait timeout, will never happens here */ [LCM] } while (nfds =3D=3D 0); > + else if (nfds =3D=3D 0) > + goto m_wait; [LCM] Then we don't need lable 'm_wait' there. > + /* epoll_wait has at least one fd ready to read */ > + eal_intr_process_rx_interrupts(port_id, events, nfds); > +} > + > +int > +rte_eal_wait_rx_intr(uint8_t port_id, uint8_t queue_id) > +{ > + struct rte_intr_handle intr_handle =3D > rte_eth_devices[port_id].pci_dev->intr_handle; > + struct epoll_event ev; > + unsigned numfds =3D 0; > + > + /* create epoll fd */ > + int pfd =3D epoll_create(1); > + if (pfd < 0) { > + RTE_LOG(ERR, EAL, "Cannot create epoll instance\n"); > + return -1; > + } > + > + rte_spinlock_lock(&intr_lock); > + > + ev.events =3D EPOLLIN | EPOLLPRI; > + switch (intr_handle.type) { > + case RTE_INTR_HANDLE_UIO: > + ev.data.fd =3D intr_handle.fd; > + break; > +#ifdef VFIO_PRESENT > + case RTE_INTR_HANDLE_VFIO_MSIX: > + ev.data.fd =3D intr_handle.queue_fd[queue_id]; > + break; > + case RTE_INTR_HANDLE_VFIO_MSI: > + ev.data.fd =3D intr_handle.queue_fd[queue_id]; > + break; > + case RTE_INTR_HANDLE_VFIO_LEGACY: > + ev.data.fd =3D intr_handle.queue_fd[queue_id]; > + break; > +#endif > + default: > + break; > + close(pfd); > + return -1; [LCM] It makes confusing here. If directly return, 'break' won't require an= d need to add unlock. > + } > + > + if (epoll_ctl(pfd, EPOLL_CTL_ADD, ev.data.fd, &ev) < 0) { > + RTE_LOG(ERR, EAL, "Error adding fd %d epoll_ctl, %s\n", > + intr_handle.queue_fd[queue_id], strerror(errno)); > + } else > + numfds++; > + > + rte_spinlock_unlock(&intr_lock); > + /* serve the interrupt */ > + eal_intr_handle_rx_interrupts(port_id, pfd, numfds); > + > + /** > + * when we return, we need to rebuild the > + * list of fds to monitor. > + */ > + close(pfd); > + > + return 0; > +} > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > index 20e0977..63d0ae8 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > @@ -283,7 +283,6 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev, > int vfio_dev_fd) >=20 > dev->intr_handle.fd =3D fd; > dev->intr_handle.vfio_dev_fd =3D vfio_dev_fd; > - > switch (i) { > case VFIO_PCI_MSIX_IRQ_INDEX: > internal_config.vfio_intr_mode =3D RTE_INTR_MODE_MSIX; > @@ -302,6 +301,16 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev= , > int vfio_dev_fd) > return -1; > } >=20 > + for (i =3D 0; i < VFIO_MAX_QUEUE_ID; i++) { > + fd =3D eventfd(0, 0); > + if (fd < 0) { > + RTE_LOG(ERR, EAL, " cannot set up eventfd, " > + "error %i (%s)\n", errno, strerror(errno)); > + return -1; > + } > + dev->intr_handle.queue_fd[i] =3D fd; > + } > + > return 0; > } >=20 > 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 > index 23eafd9..c6982cf 100644 > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h > @@ -38,6 +38,8 @@ > #ifndef _RTE_LINUXAPP_INTERRUPTS_H_ > #define _RTE_LINUXAPP_INTERRUPTS_H_ >=20 > +#define VFIO_MAX_QUEUE_ID 32 > + > enum rte_intr_handle_type { > RTE_INTR_HANDLE_UNKNOWN =3D 0, > RTE_INTR_HANDLE_UIO, /**< uio device handle */ > @@ -52,6 +54,8 @@ enum rte_intr_handle_type { > struct rte_intr_handle { > int vfio_dev_fd; /**< VFIO device file descriptor */ > int fd; /**< file descriptor */ > + int max_intr; /**< max interrupt requested */ > + int queue_fd[VFIO_MAX_QUEUE_ID]; /**< rx and tx queue interrupt file > descriptor */ > enum rte_intr_handle_type type; /**< handle type */ > }; >=20 > -- > 1.8.1.4