From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <cunming.liang@intel.com>
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
by dpdk.org (Postfix) with ESMTP id E0335255
for <dev@dpdk.org>; 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" <cunming.liang@intel.com>
To: "Zhou, Danny" <danny.zhou@intel.com>, "dev@dpdk.org" <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: <D0158A423229094DA7ABF71CF2FA0DA3118ABD36@shsmsx102.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>
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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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 <danny.zhou@intel.com>
> Signed-off-by: Yong Liu <yong.liu@intel.com>
> ---
> 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 <rte_malloc.h>
> #include <rte_errno.h>
> #include <rte_spinlock.h>
> +#include <rte_ethdev.h>
>=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