On 1/28/2015 5:52 PM, Danny Zhou wrote:
> Signed-off-by: Danny Zhou <danny.zhou at intel.com>
> Signed-off-by: Yong Liu <yong.liu at 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?

> +     }
> +}
> +
> +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;


> +#endif
> +     default:
> +             break;
> +                     close(pfd);
> +                     return -1;

Here Steve has pointed out, but indentation should be a issue.

> +     }
> +
> +     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 via email to