ethdev: get rxq interrupt fd

Message ID 20180928034331.25147-1-xiaoyun.li@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: get rxq interrupt fd |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Li, Xiaoyun Sept. 28, 2018, 3:43 a.m. UTC
Some users want to use their own epoll instances to control both
DPDK rxq interrupt fds and their own other fds. So added a function
to get rxq interrupt fd based on port id and queue id.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
 lib/librte_ethdev/rte_ethdev.c | 37 ++++++++++++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev.h |  3 +++
 2 files changed, 40 insertions(+)
  

Comments

Ferruh Yigit Sept. 28, 2018, 8:33 a.m. UTC | #1
On 9/28/2018 4:43 AM, Xiaoyun Li wrote:
> Some users want to use their own epoll instances to control both
> DPDK rxq interrupt fds and their own other fds. So added a function
> to get rxq interrupt fd based on port id and queue id.
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>

<...>

> @@ -2719,6 +2719,9 @@ int rte_eth_dev_rx_intr_ctl(uint16_t port_id, int epfd, int op, void *data);
>  int rte_eth_dev_rx_intr_ctl_q(uint16_t port_id, uint16_t queue_id,
>  			      int epfd, int op, void *data);
>  
> +int rte_eth_dev_rx_intr_ctl_q_get_fd(uint16_t port_id, uint16_t queue_id,
> +				     int *fd);


Hi Xiaoyun,

API is missing documentation, please add doxygen comments.

New APIs need to be experimental, at least for one release.
Also can you please add it to .map file otherwise shared lib build will fail
  
Li, Xiaoyun Sept. 28, 2018, 9:08 a.m. UTC | #2
OK. Thanks.

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, September 28, 2018 16:33
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; thomas@monjalon.net; Zhang, Helin
> <helin.zhang@intel.com>; damarion@cisco.com; Kinsella, Ray
> <ray.kinsella@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH] ethdev: get rxq interrupt fd
> 
> On 9/28/2018 4:43 AM, Xiaoyun Li wrote:
> > Some users want to use their own epoll instances to control both DPDK
> > rxq interrupt fds and their own other fds. So added a function to get
> > rxq interrupt fd based on port id and queue id.
> >
> > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> 
> <...>
> 
> > @@ -2719,6 +2719,9 @@ int rte_eth_dev_rx_intr_ctl(uint16_t port_id,
> > int epfd, int op, void *data);  int rte_eth_dev_rx_intr_ctl_q(uint16_t
> port_id, uint16_t queue_id,
> >  			      int epfd, int op, void *data);
> >
> > +int rte_eth_dev_rx_intr_ctl_q_get_fd(uint16_t port_id, uint16_t
> queue_id,
> > +				     int *fd);
> 
> 
> Hi Xiaoyun,
> 
> API is missing documentation, please add doxygen comments.
> 
> New APIs need to be experimental, at least for one release.
> Also can you please add it to .map file otherwise shared lib build will fail
  
Stephen Hemminger Sept. 28, 2018, 12:46 p.m. UTC | #3
In general, an API is less error prone if it only does return by value.
What about just returning fd or -1?


On Fri, Sep 28, 2018, 5:55 AM Xiaoyun Li <xiaoyun.li@intel.com> wrote:

> Some users want to use their own epoll instances to control both
> DPDK rxq interrupt fds and their own other fds. So added a function
> to get rxq interrupt fd based on port id and queue id.
>
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> ---
>  lib/librte_ethdev/rte_ethdev.c | 37 ++++++++++++++++++++++++++++++++++
>  lib/librte_ethdev/rte_ethdev.h |  3 +++
>  2 files changed, 40 insertions(+)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c
> b/lib/librte_ethdev/rte_ethdev.c
> index ef99f7068..c21124e32 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -3433,6 +3433,43 @@ rte_eth_dev_rx_intr_ctl(uint16_t port_id, int epfd,
> int op, void *data)
>         return 0;
>  }
>
> +int
> +rte_eth_dev_rx_intr_ctl_q_get_fd(uint16_t port_id, uint16_t queue_id,
> +                                int *fd)
> +{
> +       struct rte_intr_handle *intr_handle;
> +       struct rte_eth_dev *dev;
> +       unsigned int efd_idx;
> +       uint32_t vec;
> +
> +       RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> +       dev = &rte_eth_devices[port_id];
> +
> +       if (queue_id >= dev->data->nb_rx_queues) {
> +               RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n", queue_id);
> +               return -EINVAL;
> +       }
> +
> +       if (!dev->intr_handle) {
> +               RTE_ETHDEV_LOG(ERR, "RX Intr handle unset\n");
> +               return -ENOTSUP;
> +       }
> +
> +       intr_handle = dev->intr_handle;
> +       if (!intr_handle->intr_vec) {
> +               RTE_ETHDEV_LOG(ERR, "RX Intr vector unset\n");
> +               return -EPERM;
> +       }
> +
> +       vec = intr_handle->intr_vec[queue_id];
> +       efd_idx = (vec >= RTE_INTR_VEC_RXTX_OFFSET) ?
> +               (vec - RTE_INTR_VEC_RXTX_OFFSET) : vec;
> +       *fd = intr_handle->efds[efd_idx];
> +
> +       return 0;
> +}
> +
>  const struct rte_memzone *
>  rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char
> *ring_name,
>                          uint16_t queue_id, size_t size, unsigned align,
> diff --git a/lib/librte_ethdev/rte_ethdev.h
> b/lib/librte_ethdev/rte_ethdev.h
> index 012577b0a..3670d7249 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -2719,6 +2719,9 @@ int rte_eth_dev_rx_intr_ctl(uint16_t port_id, int
> epfd, int op, void *data);
>  int rte_eth_dev_rx_intr_ctl_q(uint16_t port_id, uint16_t queue_id,
>                               int epfd, int op, void *data);
>
> +int rte_eth_dev_rx_intr_ctl_q_get_fd(uint16_t port_id, uint16_t queue_id,
> +                                    int *fd);
> +
>  /**
>   * Turn on the LED on the Ethernet device.
>   * This function turns on the LED on the Ethernet device.
> --
> 2.17.1
>
>
  
Li, Xiaoyun Sept. 29, 2018, 1:57 a.m. UTC | #4
Sure. Anyway there is an error log to indicate the error.
I will send v3 later. Thanks.

From: Stephen Hemminger [mailto:stephen@networkplumber.org]
Sent: Friday, September 28, 2018 20:47
To: Li, Xiaoyun <xiaoyun.li@intel.com>
Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net>; Zhang, Helin <helin.zhang@intel.com>; damarion@cisco.com; Kinsella, Ray <ray.kinsella@intel.com>; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] ethdev: get rxq interrupt fd

In general, an API is less error prone if it only does return by value. What about just returning fd or -1?


On Fri, Sep 28, 2018, 5:55 AM Xiaoyun Li <xiaoyun.li@intel.com<mailto:xiaoyun.li@intel.com>> wrote:
Some users want to use their own epoll instances to control both
DPDK rxq interrupt fds and their own other fds. So added a function
to get rxq interrupt fd based on port id and queue id.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com<mailto:xiaoyun.li@intel.com>>
---
 lib/librte_ethdev/rte_ethdev.c | 37 ++++++++++++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev.h |  3 +++
 2 files changed, 40 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index ef99f7068..c21124e32 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -3433,6 +3433,43 @@ rte_eth_dev_rx_intr_ctl(uint16_t port_id, int epfd, int op, void *data)
        return 0;
 }

+int
+rte_eth_dev_rx_intr_ctl_q_get_fd(uint16_t port_id, uint16_t queue_id,
+                                int *fd)
+{
+       struct rte_intr_handle *intr_handle;
+       struct rte_eth_dev *dev;
+       unsigned int efd_idx;
+       uint32_t vec;
+
+       RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+       dev = &rte_eth_devices[port_id];
+
+       if (queue_id >= dev->data->nb_rx_queues) {
+               RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n", queue_id);
+               return -EINVAL;
+       }
+
+       if (!dev->intr_handle) {
+               RTE_ETHDEV_LOG(ERR, "RX Intr handle unset\n");
+               return -ENOTSUP;
+       }
+
+       intr_handle = dev->intr_handle;
+       if (!intr_handle->intr_vec) {
+               RTE_ETHDEV_LOG(ERR, "RX Intr vector unset\n");
+               return -EPERM;
+       }
+
+       vec = intr_handle->intr_vec[queue_id];
+       efd_idx = (vec >= RTE_INTR_VEC_RXTX_OFFSET) ?
+               (vec - RTE_INTR_VEC_RXTX_OFFSET) : vec;
+       *fd = intr_handle->efds[efd_idx];
+
+       return 0;
+}
+
 const struct rte_memzone *
 rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
                         uint16_t queue_id, size_t size, unsigned align,
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 012577b0a..3670d7249 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -2719,6 +2719,9 @@ int rte_eth_dev_rx_intr_ctl(uint16_t port_id, int epfd, int op, void *data);
 int rte_eth_dev_rx_intr_ctl_q(uint16_t port_id, uint16_t queue_id,
                              int epfd, int op, void *data);

+int rte_eth_dev_rx_intr_ctl_q_get_fd(uint16_t port_id, uint16_t queue_id,
+                                    int *fd);
+
 /**
  * Turn on the LED on the Ethernet device.
  * This function turns on the LED on the Ethernet device.
--
2.17.1
  

Patch

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index ef99f7068..c21124e32 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -3433,6 +3433,43 @@  rte_eth_dev_rx_intr_ctl(uint16_t port_id, int epfd, int op, void *data)
 	return 0;
 }
 
+int
+rte_eth_dev_rx_intr_ctl_q_get_fd(uint16_t port_id, uint16_t queue_id,
+				 int *fd)
+{
+	struct rte_intr_handle *intr_handle;
+	struct rte_eth_dev *dev;
+	unsigned int efd_idx;
+	uint32_t vec;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+	dev = &rte_eth_devices[port_id];
+
+	if (queue_id >= dev->data->nb_rx_queues) {
+		RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n", queue_id);
+		return -EINVAL;
+	}
+
+	if (!dev->intr_handle) {
+		RTE_ETHDEV_LOG(ERR, "RX Intr handle unset\n");
+		return -ENOTSUP;
+	}
+
+	intr_handle = dev->intr_handle;
+	if (!intr_handle->intr_vec) {
+		RTE_ETHDEV_LOG(ERR, "RX Intr vector unset\n");
+		return -EPERM;
+	}
+
+	vec = intr_handle->intr_vec[queue_id];
+	efd_idx = (vec >= RTE_INTR_VEC_RXTX_OFFSET) ?
+		(vec - RTE_INTR_VEC_RXTX_OFFSET) : vec;
+	*fd = intr_handle->efds[efd_idx];
+
+	return 0;
+}
+
 const struct rte_memzone *
 rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 			 uint16_t queue_id, size_t size, unsigned align,
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 012577b0a..3670d7249 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -2719,6 +2719,9 @@  int rte_eth_dev_rx_intr_ctl(uint16_t port_id, int epfd, int op, void *data);
 int rte_eth_dev_rx_intr_ctl_q(uint16_t port_id, uint16_t queue_id,
 			      int epfd, int op, void *data);
 
+int rte_eth_dev_rx_intr_ctl_q_get_fd(uint16_t port_id, uint16_t queue_id,
+				     int *fd);
+
 /**
  * Turn on the LED on the Ethernet device.
  * This function turns on the LED on the Ethernet device.