DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Zhou, Danny" <danny.zhou@intel.com>
To: "Qiu, Michael" <michael.qiu@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v1 4/5] eal: add per rx queue interrupt handling	based on VFIO
Date: Thu, 29 Jan 2015 05:24:15 +0000	[thread overview]
Message-ID: <DFDF335405C17848924A094BC35766CF0AA592F2@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <533710CFB86FA344BFBF2D6802E60286CCFB9F@SHSMSX101.ccr.corp.intel.com>



> -----Original Message-----
> From: Qiu, Michael
> Sent: Thursday, January 29, 2015 1:07 PM
> To: Zhou, Danny; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 4/5] eal: add per rx queue interrupt handling based on VFIO
> 
> On 1/28/2015 5:52 PM, Danny Zhou wrote:
> > Signed-off-by: Danny Zhou <danny.zhou@intel.com>
> > Signed-off-by: Yong Liu <yong.liu@intel.com>
> > ---
> 
> [...]
> 
> > +static void
> > +eal_intr_process_rx_interrupts(uint8_t port_id, struct epoll_event *events, int nfds)
> > +{
> > +	int n, bytes_read;
> > +	union rte_intr_read_buffer buf;
> > +	struct rte_intr_handle intr_handle = rte_eth_devices[port_id].pci_dev->intr_handle;
> > +
> > +	for (n = 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 = sizeof(buf.uio_intr_count);
> > +			break;
> > +		case RTE_INTR_HANDLE_ALARM:
> > +			bytes_read = 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 = sizeof(buf.vfio_intr_count);
> > +			break;
> > +#endif
> > +		default:
> > +			bytes_read = 1;
> > +			break;
> > +		}
> > +
> > +		/**
> > +		* read out to clear the ready-to-be-read flag
> > +		* for epoll_wait.
> > +		*/
> > +		bytes_read = 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 == 0)
> > +			RTE_LOG(ERR, EAL, "Read nothing from file "
> > +				"descriptor %d\n", events[n].data.fd);
> 
> Is there any issue while bytes_read is not equal to the count need to be
> read?
> 

Not a problem as long as bytes_read > 0.

> > +	}
> > +}
> > +
> > +static void
> > +eal_intr_handle_rx_interrupts(uint8_t port_id, int pfd, unsigned totalfds)
> > +{
> > +	struct epoll_event events[totalfds];
> > +	int nfds = 0;
> > +
> > +m_wait:
> > +	nfds = 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 */
> > +	else if (nfds == 0)
> > +		goto m_wait;
> > +	/* 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 = rte_eth_devices[port_id].pci_dev->intr_handle;
> > +	struct epoll_event ev;
> > +	unsigned numfds = 0;
> > +
> > +	/* create epoll fd */
> > +	int pfd = epoll_create(1);
> > +	if (pfd < 0) {
> > +		RTE_LOG(ERR, EAL, "Cannot create epoll instance\n");
> > +		return -1;
> > +	}
> > +
> > +	rte_spinlock_lock(&intr_lock);
> > +
> > +	ev.events = EPOLLIN | EPOLLPRI;
> > +	switch (intr_handle.type) {
> > +	case RTE_INTR_HANDLE_UIO:
> > +		ev.data.fd = intr_handle.fd;
> > +		break;
> > +#ifdef VFIO_PRESENT
> > +	case RTE_INTR_HANDLE_VFIO_MSIX:
> > +		ev.data.fd = intr_handle.queue_fd[queue_id];
> > +		break;
> > +	case RTE_INTR_HANDLE_VFIO_MSI:
> > +		ev.data.fd = intr_handle.queue_fd[queue_id];
> > +		break;
> > +	case RTE_INTR_HANDLE_VFIO_LEGACY:
> > +		ev.data.fd = intr_handle.queue_fd[queue_id];
> > +		break;
> 
> As those three branches are all the same, why not combine to one?
> 
> +	case RTE_INTR_HANDLE_VFIO_MSIX:
> +	case RTE_INTR_HANDLE_VFIO_MSI:
> +	case RTE_INTR_HANDLE_VFIO_LEGACY:
> +		ev.data.fd = intr_handle.queue_fd[queue_id];
> +		break;
> 

Accepted.

> 
> > +#endif
> > +	default:
> > +		break;
> > +			close(pfd);
> > +			return -1;
> 
> Here Steve has pointed out, but indentation should be a issue.
> 

To be fixed in V2.

> > +	}
> > +
> > +	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)
> >
> >  		dev->intr_handle.fd = fd;
> >  		dev->intr_handle.vfio_dev_fd = vfio_dev_fd;
> > -
> >  		switch (i) {
> >  		case VFIO_PCI_MSIX_IRQ_INDEX:
> >  			internal_config.vfio_intr_mode = RTE_INTR_MODE_MSIX;
> > @@ -302,6 +301,16 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev, int vfio_dev_fd)
> >  			return -1;
> >  		}
> >
> > +		for (i = 0; i < VFIO_MAX_QUEUE_ID; i++) {
> > +			fd = 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] = fd;
> > +		}
> > +
> >  		return 0;
> >  	}
> >
> > 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_
> >
> > +#define VFIO_MAX_QUEUE_ID 32
> > +
> >  enum rte_intr_handle_type {
> >  	RTE_INTR_HANDLE_UNKNOWN = 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 */
> >  };
> >

  reply	other threads:[~2015-01-29  5:24 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-28  9:50 [dpdk-dev] [PATCH v1 0/5] Interrupt mode for PMD Danny Zhou
2015-01-28  9:50 ` [dpdk-dev] [PATCH v1 1/5] ethdev: add rx interrupt enable/disable functions Danny Zhou
2015-01-29  2:22   ` Qiu, Michael
2015-01-28  9:50 ` [dpdk-dev] [PATCH v1 2/5] ixgbe: enable rx queue interrupts for both PF and VF Danny Zhou
2015-01-29  3:40   ` Qiu, Michael
2015-01-29  5:39     ` Zhou, Danny
2015-01-29 10:13       ` Qiu, Michael
2015-01-28  9:50 ` [dpdk-dev] [PATCH v1 3/5] igb: enable rx queue interrupts for PF Danny Zhou
2015-01-28  9:50 ` [dpdk-dev] [PATCH v1 4/5] eal: add per rx queue interrupt handling based on VFIO Danny Zhou
2015-01-28 18:10   ` Liang, Cunming
2015-01-29  4:01     ` Zhou, Danny
2015-01-29  5:06   ` Qiu, Michael
2015-01-29  5:24     ` Zhou, Danny [this message]
2015-01-28  9:50 ` [dpdk-dev] [PATCH v1 5/5] L3fwd-power: enable one-shot rx interrupt and polling/interrupt mode switch Danny Zhou
2015-01-28 18:34   ` Liang, Cunming
2015-01-29  4:07     ` Zhou, Danny

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DFDF335405C17848924A094BC35766CF0AA592F2@SHSMSX104.ccr.corp.intel.com \
    --to=danny.zhou@intel.com \
    --cc=dev@dpdk.org \
    --cc=michael.qiu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).