[dpdk-dev,v14,06/13] eal/linux: standalone intr event fd create support

Message ID 1437113775-32199-7-git-send-email-cunming.liang@intel.com (mailing list archive)
State Changes Requested, archived
Headers

Commit Message

Cunming Liang July 17, 2015, 6:16 a.m. UTC
  The patch exposes intr event fd create and release for PMD.
The device driver can assign the number of event associated with interrupt vector.
It also provides misc functions to check 1) allows other slowpath intr(e.g. lsc);
2) intr event on fastpath is enabled or not.

Signed-off-by: Cunming Liang <cunming.liang@intel.com>
---
v14 changes
 - per-patch basis ABI compatibility rework
 - minor changes on API decription comments

v13 changes
 - version map cleanup for v2.1

v11 changes
 - typo cleanup

 lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 57 ++++++++++++++
 .../linuxapp/eal/include/exec-env/rte_interrupts.h | 87 ++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map    |  4 +
 3 files changed, 148 insertions(+)
  

Comments

Thomas Monjalon July 19, 2015, 11:35 p.m. UTC | #1
2015-07-17 14:16, Cunming Liang:
> +#ifdef RTE_NEXT_ABI
> +extern int
> +rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd);
> +#else
> +static inline int
> +rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd)
> +{
> +	RTE_SET_USED(intr_handle);
> +	RTE_SET_USED(nb_efd);
> +	return 0;
> +}
> +#endif
[...]
> +#ifdef RTE_NEXT_ABI
> +extern void
> +rte_intr_efd_disable(struct rte_intr_handle *intr_handle);
> +#else
> +static inline void
> +rte_intr_efd_disable(struct rte_intr_handle *intr_handle)
> +{
> +	RTE_SET_USED(intr_handle);
> +}
> +#endif
[...]
> +#ifdef RTE_NEXT_ABI
> +static inline int
> +rte_intr_dp_is_en(struct rte_intr_handle *intr_handle)
> +{
> +	return !(!intr_handle->nb_efd);
> +}
> +#else
> +static inline int
> +rte_intr_dp_is_en(struct rte_intr_handle *intr_handle)
> +{
> +	RTE_SET_USED(intr_handle);
> +	return 0;
> +}
> +#endif
[...]
> +#ifdef RTE_NEXT_ABI
> +static inline int
> +rte_intr_allow_others(struct rte_intr_handle *intr_handle)
> +{
> +	return !!(intr_handle->max_intr - intr_handle->nb_efd);
> +}
> +#else
> +static inline int
> +rte_intr_allow_others(struct rte_intr_handle *intr_handle)
> +{
> +	RTE_SET_USED(intr_handle);
> +	return 1;
> +}
> +#endif

Why these #else cases?

> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -117,6 +117,10 @@ DPDK_2.1 {
>  
>  	rte_epoll_ctl;
>  	rte_epoll_wait;
> +	rte_intr_allow_others;
> +	rte_intr_dp_is_en;
> +	rte_intr_efd_enable;
> +	rte_intr_efd_disable;

If RTE_NEXT_ABI is disabled, these symbols do not exist.
An alternate .map would be needed.
  
Thomas Monjalon July 19, 2015, 11:39 p.m. UTC | #2
2015-07-20 01:35, Thomas Monjalon:
> 2015-07-17 14:16, Cunming Liang:
> > +#ifdef RTE_NEXT_ABI
> > +extern int
> > +rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd);
> > +#else
> > +static inline int
> > +rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd)
> > +{
> > +	RTE_SET_USED(intr_handle);
> > +	RTE_SET_USED(nb_efd);
> > +	return 0;
> > +}
> > +#endif
> [...]
> > +#ifdef RTE_NEXT_ABI
> > +extern void
> > +rte_intr_efd_disable(struct rte_intr_handle *intr_handle);
> > +#else
> > +static inline void
> > +rte_intr_efd_disable(struct rte_intr_handle *intr_handle)
> > +{
> > +	RTE_SET_USED(intr_handle);
> > +}
> > +#endif
> [...]
> > +#ifdef RTE_NEXT_ABI
> > +static inline int
> > +rte_intr_dp_is_en(struct rte_intr_handle *intr_handle)
> > +{
> > +	return !(!intr_handle->nb_efd);
> > +}
> > +#else
> > +static inline int
> > +rte_intr_dp_is_en(struct rte_intr_handle *intr_handle)
> > +{
> > +	RTE_SET_USED(intr_handle);
> > +	return 0;
> > +}
> > +#endif
> [...]
> > +#ifdef RTE_NEXT_ABI
> > +static inline int
> > +rte_intr_allow_others(struct rte_intr_handle *intr_handle)
> > +{
> > +	return !!(intr_handle->max_intr - intr_handle->nb_efd);
> > +}
> > +#else
> > +static inline int
> > +rte_intr_allow_others(struct rte_intr_handle *intr_handle)
> > +{
> > +	RTE_SET_USED(intr_handle);
> > +	return 1;
> > +}
> > +#endif
> 
> Why these #else cases?

Why not totally removing these ifdef in the header and use them in .c?

> > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > @@ -117,6 +117,10 @@ DPDK_2.1 {
> >  
> >  	rte_epoll_ctl;
> >  	rte_epoll_wait;
> > +	rte_intr_allow_others;
> > +	rte_intr_dp_is_en;
> > +	rte_intr_efd_enable;
> > +	rte_intr_efd_disable;
> 
> If RTE_NEXT_ABI is disabled, these symbols do not exist.
> An alternate .map would be needed.
  
Cunming Liang July 20, 2015, 2:08 a.m. UTC | #3
On 7/20/2015 7:39 AM, Thomas Monjalon wrote:
> 2015-07-20 01:35, Thomas Monjalon:
>> 2015-07-17 14:16, Cunming Liang:
>>> +#ifdef RTE_NEXT_ABI
>>> +extern int
>>> +rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd);
>>> +#else
>>> +static inline int
>>> +rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd)
>>> +{
>>> +	RTE_SET_USED(intr_handle);
>>> +	RTE_SET_USED(nb_efd);
>>> +	return 0;
>>> +}
>>> +#endif
>> [...]
>>> +#ifdef RTE_NEXT_ABI
>>> +extern void
>>> +rte_intr_efd_disable(struct rte_intr_handle *intr_handle);
>>> +#else
>>> +static inline void
>>> +rte_intr_efd_disable(struct rte_intr_handle *intr_handle)
>>> +{
>>> +	RTE_SET_USED(intr_handle);
>>> +}
>>> +#endif
>> [...]
>>> +#ifdef RTE_NEXT_ABI
>>> +static inline int
>>> +rte_intr_dp_is_en(struct rte_intr_handle *intr_handle)
>>> +{
>>> +	return !(!intr_handle->nb_efd);
>>> +}
>>> +#else
>>> +static inline int
>>> +rte_intr_dp_is_en(struct rte_intr_handle *intr_handle)
>>> +{
>>> +	RTE_SET_USED(intr_handle);
>>> +	return 0;
>>> +}
>>> +#endif
>> [...]
>>> +#ifdef RTE_NEXT_ABI
>>> +static inline int
>>> +rte_intr_allow_others(struct rte_intr_handle *intr_handle)
>>> +{
>>> +	return !!(intr_handle->max_intr - intr_handle->nb_efd);
>>> +}
>>> +#else
>>> +static inline int
>>> +rte_intr_allow_others(struct rte_intr_handle *intr_handle)
>>> +{
>>> +	RTE_SET_USED(intr_handle);
>>> +	return 1;
>>> +}
>>> +#endif
>> Why these #else cases?
> Why not totally removing these ifdef in the header and use them in .c?
That's a good idea, will take it. Thanks.
>
>>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> @@ -117,6 +117,10 @@ DPDK_2.1 {
>>>   
>>>   	rte_epoll_ctl;
>>>   	rte_epoll_wait;
>>> +	rte_intr_allow_others;
>>> +	rte_intr_dp_is_en;
>>> +	rte_intr_efd_enable;
>>> +	rte_intr_efd_disable;
>> If RTE_NEXT_ABI is disabled, these symbols do not exist.
>> An alternate .map would be needed.
You're right, will take your suggestion above.
  

Patch

diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index b18ab86..0266d98 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -44,6 +44,7 @@ 
 #include <sys/epoll.h>
 #include <sys/signalfd.h>
 #include <sys/ioctl.h>
+#include <sys/eventfd.h>
 
 #include <rte_common.h>
 #include <rte_interrupts.h>
@@ -68,6 +69,7 @@ 
 #include "eal_vfio.h"
 
 #define EAL_INTR_EPOLL_WAIT_FOREVER (-1)
+#define NB_OTHER_INTR               1
 
 static RTE_DEFINE_PER_LCORE(int, _epfd) = -1; /**< epoll fd per thread */
 
@@ -1121,4 +1123,59 @@  rte_intr_rx_ctl(struct rte_intr_handle *intr_handle, int epfd,
 
 	return rc;
 }
+
+int
+rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd)
+{
+	uint32_t i;
+	int fd;
+	uint32_t n = RTE_MIN(nb_efd, (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
+
+	if (intr_handle->type == RTE_INTR_HANDLE_VFIO_MSIX) {
+		for (i = 0; i < n; i++) {
+			fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
+			if (fd < 0) {
+				RTE_LOG(ERR, EAL,
+					"cannot setup eventfd,"
+					"error %i (%s)\n",
+					errno, strerror(errno));
+				return -1;
+			}
+			intr_handle->efds[i] = fd;
+		}
+		intr_handle->nb_efd   = n;
+		intr_handle->max_intr = NB_OTHER_INTR + n;
+	} else {
+		intr_handle->efds[0]  = intr_handle->fd;
+		intr_handle->nb_efd   = RTE_MIN(nb_efd, 1U);
+		intr_handle->max_intr = NB_OTHER_INTR;
+	}
+
+	return 0;
+}
+
+void
+rte_intr_efd_disable(struct rte_intr_handle *intr_handle)
+{
+	uint32_t i;
+	struct rte_epoll_event *rev;
+
+	for (i = 0; i < intr_handle->nb_efd; i++) {
+		rev = &intr_handle->elist[i];
+		if (rev->status == RTE_EPOLL_INVALID)
+			continue;
+		if (rte_epoll_ctl(rev->epfd, EPOLL_CTL_DEL, rev->fd, rev)) {
+			/* force free if the entry valid */
+			eal_epoll_data_safe_free(rev);
+			rev->status = RTE_EPOLL_INVALID;
+		}
+	}
+
+	if (intr_handle->max_intr > intr_handle->nb_efd) {
+		for (i = 0; i < intr_handle->nb_efd; i++)
+			close(intr_handle->efds[i]);
+	}
+	intr_handle->nb_efd = 0;
+	intr_handle->max_intr = 0;
+}
 #endif
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 918246f..3f17f29 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
@@ -191,4 +191,91 @@  rte_intr_rx_ctl(struct rte_intr_handle *intr_handle,
 }
 #endif
 
+/**
+ * It enables the packet I/O interrupt event if it's necessary.
+ * It creates event fd for each interrupt vector when MSIX is used,
+ * otherwise it multiplexes a single event fd.
+ *
+ * @param intr_handle
+ *   Pointer to the interrupt handle.
+ * @param nb_vec
+ *   Number of interrupt vector trying to enable.
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+#ifdef RTE_NEXT_ABI
+extern int
+rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd);
+#else
+static inline int
+rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd)
+{
+	RTE_SET_USED(intr_handle);
+	RTE_SET_USED(nb_efd);
+	return 0;
+}
+#endif
+
+/**
+ * It disables the packet I/O interrupt event.
+ * It deletes registered eventfds and closes the open fds.
+ *
+ * @param intr_handle
+ *   Pointer to the interrupt handle.
+ */
+#ifdef RTE_NEXT_ABI
+extern void
+rte_intr_efd_disable(struct rte_intr_handle *intr_handle);
+#else
+static inline void
+rte_intr_efd_disable(struct rte_intr_handle *intr_handle)
+{
+	RTE_SET_USED(intr_handle);
+}
+#endif
+
+/**
+ * The packet I/O interrupt on datapath is enabled or not.
+ *
+ * @param intr_handle
+ *   Pointer to the interrupt handle.
+ */
+#ifdef RTE_NEXT_ABI
+static inline int
+rte_intr_dp_is_en(struct rte_intr_handle *intr_handle)
+{
+	return !(!intr_handle->nb_efd);
+}
+#else
+static inline int
+rte_intr_dp_is_en(struct rte_intr_handle *intr_handle)
+{
+	RTE_SET_USED(intr_handle);
+	return 0;
+}
+#endif
+
+/**
+ * The interrupt handle instance allows other causes or not.
+ * Other causes stand for any none packet I/O interrupts.
+ *
+ * @param intr_handle
+ *   Pointer to the interrupt handle.
+ */
+#ifdef RTE_NEXT_ABI
+static inline int
+rte_intr_allow_others(struct rte_intr_handle *intr_handle)
+{
+	return !!(intr_handle->max_intr - intr_handle->nb_efd);
+}
+#else
+static inline int
+rte_intr_allow_others(struct rte_intr_handle *intr_handle)
+{
+	RTE_SET_USED(intr_handle);
+	return 1;
+}
+#endif
+
 #endif /* _RTE_LINUXAPP_INTERRUPTS_H_ */
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 1cd4cc5..a0d9cb2 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -117,6 +117,10 @@  DPDK_2.1 {
 
 	rte_epoll_ctl;
 	rte_epoll_wait;
+	rte_intr_allow_others;
+	rte_intr_dp_is_en;
+	rte_intr_efd_enable;
+	rte_intr_efd_disable;
 	rte_intr_rx_ctl;
 	rte_intr_tls_epfd;
 	rte_memzone_free;