Extending DPDK to have more devices supported

Move a number of device specific define, structures and functions into a generic device base set of files for all device not just Ethernet.

Signed-off-by Keith Wiles <[email protected]>

---
 lib/librte_eal/common/eal_common_device.c         | 185 +++++++
 lib/librte_eal/common/include/rte_common_device.h | 617 ++++++++++++++++++++++
 2 files changed, 802 insertions(+)
 create mode 100644 lib/librte_eal/common/eal_common_device.c
 create mode 100644 lib/librte_eal/common/include/rte_common_device.h

lib/librte_eal/common/eal_common_device.c new file +175 -0

new file mode 100644
index 0000000..a9ef925
@@ -0,0 +1,185 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2014 6WIND S.A.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <inttypes.h>
+#include <sys/queue.h>
+
+#include <rte_dev.h>
+#include <rte_devargs.h>
+#include <rte_debug.h>
+#include <rte_devargs.h>
+#include <rte_log.h>
+#include <rte_malloc.h>
+#include <rte_errno.h>
+
+#include "rte_common_device.h"
+
+void *
+rte_dev_add_callback(struct rte_dev_rxtx_callback ** cbp,
+		void * fn, void *user_param)
+{
+	struct rte_dev_rxtx_callback *cb;
+
+	cb = rte_zmalloc(NULL, sizeof(*cb), 0);
+
+	if (cb == NULL) {
+		rte_errno = ENOMEM;
+		return NULL;
+	}
+
+	cb->fn.vp = fn;
+	cb->param = user_param;
+	cb->next = *cbp;
+	*cbp = cb;
+	return cb;
+}
+
+int
+rte_dev_remove_callback(struct rte_dev_rxtx_callback ** cbp,
+		struct rte_dev_rxtx_callback *user_cb)
+{
+	struct rte_dev_rxtx_callback *cb = *cbp;
+	struct rte_dev_rxtx_callback *prev_cb;
+
+	/* Reset head pointer and remove user cb if first in the list. */
+	if (cb == user_cb) {
+		*cbp = user_cb->next;
+		return 0;
+	}
+
+	/* Remove the user cb from the callback list. */
+	do {
+		prev_cb = cb;
+		cb = cb->next;
+
+		if (cb == user_cb) {
+			prev_cb->next = user_cb->next;
+			return 0;
+		}
+	} while (cb != NULL);
+
+	/* Callback wasn't found. */
+	return (-EINVAL);
+}
+
+int
+rte_dev_callback_register(struct rte_dev_cb_list * cb_list,
+		rte_spinlock_t * lock,
+		enum rte_dev_event_type event,
+		rte_dev_cb_fn cb_fn, void *cb_arg)
+{
+	struct rte_dev_callback *cb;
+
+	rte_spinlock_lock(lock);
+
+	TAILQ_FOREACH(cb, cb_list, next) {
+		if (cb->cb_fn == cb_fn &&
+			cb->cb_arg == cb_arg &&
+			cb->event == event) {
+			break;
+		}
+	}
+
+	/* create a new callback. */
+	if (cb == NULL && (cb = rte_zmalloc("INTR_USER_CALLBACK",
+			sizeof(struct rte_dev_callback), 0)) != NULL) {
+		cb->cb_fn = cb_fn;
+		cb->cb_arg = cb_arg;
+		cb->event = event;
+		TAILQ_INSERT_TAIL(cb_list, cb, next);
+	}
+
+	rte_spinlock_unlock(lock);
+	return ((cb == NULL) ? -ENOMEM : 0);
+}
+
+int
+rte_dev_callback_unregister(struct rte_dev_cb_list * cb_list,
+		rte_spinlock_t * lock,
+		enum rte_dev_event_type event,
+		rte_dev_cb_fn cb_fn, void *cb_arg)
+{
+	int ret;
+	struct rte_dev_callback *cb, *next;
+
+	rte_spinlock_lock(lock);
+
+	ret = 0;
+	for (cb = TAILQ_FIRST(cb_list); cb != NULL; cb = next) {
+
+		next = TAILQ_NEXT(cb, next);
+
+		if (cb->cb_fn != cb_fn || cb->event != event ||
+				(cb->cb_arg != (void *)-1 &&
+				cb->cb_arg != cb_arg))
+			continue;
+
+		/*
+		 * if this callback is not executing right now,
+		 * then remove it.
+		 */
+		if (cb->active == 0) {
+			TAILQ_REMOVE(cb_list, cb, next);
+			rte_free(cb);
+		} else {
+			ret = -EAGAIN;
+		}
+	}
+
+	rte_spinlock_unlock(lock);
+	return (ret);
+}
+
+void
+rte_dev_callback_process(struct rte_dev_cb_list * cb_list, uint8_t port_id,
+	enum rte_dev_event_type event, rte_spinlock_t *lock)
+{
+	struct rte_dev_callback *cb;
+	struct rte_dev_callback dev_cb;
+
+	rte_spinlock_lock(lock);
+	TAILQ_FOREACH(cb, cb_list, next) {
+		if (cb->cb_fn == NULL || cb->event != event)

lib/librte_eal/common/include/rte_common_device.h new file +128 -0

new file mode 100644
index 0000000..5baefb6
@@ -0,0 +1,617 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHPDF IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_COMMON_DEVICE_H_
+#define _RTE_COMMON_DEVICE_H_
+
+/**
+ * @file
+ *
+ * Common Device Helpers in RTE
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <rte_spinlock.h>
+
+/* Macros for checking for restricting functions to primary instance only */
+#define PROC_PRIMARY_OR_ERR_RET(retval) do { \
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
+		PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
+		return (retval); \
+	} \
+} while(0)
+#define PROC_PRIMARY_OR_RET() do { \
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
+		PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
+		return; \
+	} \
+} while(0)
+
+/* Macros to check for invalid function pointers in dev_ops structure */
+#define FUNC_PTR_OR_ERR_RET(func, retval) do { \
+	if ((func) == NULL) { \
+		PMD_DEBUG_TRACE("Function not supported\n"); \
+		return (retval); \
+	} \
+} while(0)
+#define FUNC_PTR_OR_RET(func) do { \
+	if ((func) == NULL) { \
+		PMD_DEBUG_TRACE("Function not supported\n"); \
+		return; \
+	} \
+} while(0)
+
+enum {
+	DEV_DETACHED = 0,
+	DEV_ATTACHED
+};
+
+/*
+ * The device type
+ */
+enum rte_dev_type {
+	RTE_DEV_UNKNOWN,	/**< unknown device type */
+	RTE_DEV_PCI,
+		/**< Physical function and Virtual function of PCI devices */
+	RTE_DEV_VIRTUAL,	/**< non hardware device */
+	RTE_DEV_MAX			/**< max value of this enum */
+};
+
+/**
+ * The device event type for interrupt, and maybe others in the future.
+ */
+enum rte_dev_event_type {
+	RTE_DEV_EVENT_UNKNOWN,  /**< unknown event type */
+	RTE_DEV_EVENT_INTR_LSC, /**< lsc interrupt event */
+	RTE_DEV_EVENT_MAX       /**< max value of this enum */
+};
+
+struct rte_dev_callback;
+
+/** @internal Structure to keep track of registered callback */
+TAILQ_HEAD(rte_dev_cb_list, rte_dev_callback);
+
+struct rte_mbuf;
+
+typedef uint16_t (*dev_rx_burst_t)(void *rxq,
+				   struct rte_mbuf **rx_pkts,
+				   uint16_t nb_pkts);
+/**< @internal Retrieve input packets from a receive queue of a device. */
+
+typedef uint16_t (*dev_tx_burst_t)(void *txq,
+				   struct rte_mbuf **tx_pkts,
+				   uint16_t nb_pkts);
+/**< @internal Send output packets on a transmit queue of a device. */
+
+typedef void (*rte_dev_cb_fn)(uint8_t port_id, \
+		enum rte_dev_event_type event, void *cb_arg);
+/**< user application callback to be registered for interrupts */
+
+#define RTE_DEV_NAME_MAX_LEN (32)
+
+/**

lib/librte_eal/common/include/rte_common_device.h new file +6 -0

+    struct rte_pci_device       *pci_dev;       /**< PCI info. supplied by probing */       \
+    /** User application callback for interrupts if present */                              \
+    struct rte_dev_cb_list   link_intr_cbs;                                          		\
+    /**                                                                                     \
+     * User-supplied functions called from rx_burst to post-process                         \
+     * received packets before passing them to the user                                     \

lib/librte_eal/common/include/rte_common_device.h new file +32 -0

+/**
+ * @internal
+ * Common set of members for all devices included at the top of 'struct rte_xyz_dev_data'
+ */
+#define RTE_COMMON_DEV_DATA                                                 	\
+    char name[RTE_DEV_NAME_MAX_LEN]; /**< Unique identifier name */             \
+                                                                                \
+    void **rx_queues;               /**< Array of pointers to RX queues. */     \
+    void **tx_queues;               /**< Array of pointers to TX queues. */     \
+    uint16_t nb_rx_queues;          /**< Number of RX queues. */                \
+    uint16_t nb_tx_queues;          /**< Number of TX queues. */                \
+                                                                                \
+    uint16_t mtu;                   /**< Maximum Transmission Unit. */          \
+    uint8_t unit_id;                /**< Unit/Port ID for this instance */      \
+    uint8_t _pad0;             		/* alignment filler */                      \
+                                                                                \
+    /* 64bit alignment starts here */                                           \
+    void    *dev_private;           /**< PMD-specific private data */           \
+    uint64_t rx_mbuf_alloc_failed;  /**< RX ring mbuf allocation failures. */   \
+    uint32_t min_rx_buf_size;       /**< Common rx buffer size handled by all queues */ \
+    uint32_t _pad1					/**< Align to a 64bit boundary */
+
+/**
+ * @internal
+ * Common set of members for all devices included at the top of 'struct rte_xyz_dev_info'
+ */
+#define RTE_COMMON_DEV_INFO                                                 	\
+    struct rte_pci_device   *pci_dev;       /**< Device PCI information. */     \
+    const char              *driver_name;   /**< Device Driver name. */         \
+    unsigned int            if_index;       /**< Index to bound host interface, or 0 if none. */ \
+        /* Use if_indextoname() to translate into an interface name. */         \
+    uint32_t _pad0

lib/librte_eal/common/include/rte_common_device.h new file +40 -0

+    uint8_t     nb_ports;        /**< Number of ports/units found */    \
+    uint8_t     max_ports;       /**< Max number of ports or units */   \
+    uint16_t    dflt_mtu;        /**< Default MTU if needed by device */\
+    uint16_t    dev_size;        /**< Internal size of device struct */	\
+    uint16_t    data_size;       /**< Internal size of data structure */\
+    const char * mz_dev_data     /**< String constant for device data */
+
+/**
+ * @internal
+ * Common overlay type structures with all devices.
+ */
+struct rte_dev_info {
+    RTE_COMMON_DEV_INFO;
+};
+
+/**
+ * @internal
+ * The generic data structure associated with each device.
+ *
+ * Pointers to burst-oriented packet receive and transmit functions are
+ * located at the beginning of the structure, along with the pointer to
+ * where all the data elements for the particular device are stored in shared
+ * memory. This split allows the function pointer and driver data to be per-
+ * process, while the actual configuration data for the device is shared.
+ */
+struct rte_dev {
+    RTE_COMMON_DEV();
+};
+
+/**
+ * @internal
+ * The data part, with no function pointers, associated with each device.
+ *
+ * This structure is safe to place in shared memory to be common among different
+ * processes in a multi-process configuration.
+ */
+struct rte_dev_data {
+    RTE_COMMON_DEV_DATA;
+};
+

lib/librte_eal/common/include/rte_common_device.h new file +7 -0

+/**
+ * Get the rte_pkt_dev structure device pointer for the device.
+ *
+ * @param   gbl     pointer to device specific global structure.
+ * @param   port_id Port ID value to select the device structure.
+ *
+ * @return

Quoted 208 lines · Keith Wiles

Move a number of device specific define, structures and functions into a generic device base set of files for all device not just Ethernet.

Signed-off-by Keith Wiles <[email protected]>

---
 lib/librte_eal/common/eal_common_device.c         | 185 +++++++
 lib/librte_eal/common/include/rte_common_device.h | 617
++++++++++++++++++++++
 2 files changed, 802 insertions(+)
 create mode 100644 lib/librte_eal/common/eal_common_device.c
 create mode 100644 lib/librte_eal/common/include/rte_common_device.h

lib/librte_eal/common/eal_common_device.c new file +14 -0

new file mode 100644
index 0000000..a9ef925
@@ -0,0 +1,185 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2014 6WIND S.A.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above

copyright
+ * notice, this list of conditions and the following disclaimer
in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products
derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
DAMAGE.
+ */

lib/librte_eal/common/eal_common_device.c new file +134 -0

+
+#include <stdio.h>
+#include <string.h>
+#include <inttypes.h>
+#include <sys/queue.h>
+
+#include <rte_dev.h>
+#include <rte_devargs.h>
+#include <rte_debug.h>
+#include <rte_devargs.h>
+#include <rte_log.h>
+#include <rte_malloc.h>
+#include <rte_errno.h>
+
+#include "rte_common_device.h"
+
+void *
+rte_dev_add_callback(struct rte_dev_rxtx_callback ** cbp,
+		void * fn, void *user_param)
+{
+	struct rte_dev_rxtx_callback *cb;
+
+	cb = rte_zmalloc(NULL, sizeof(*cb), 0);
+
+	if (cb == NULL) {
+		rte_errno = ENOMEM;
+		return NULL;
+	}
+
+	cb->fn.vp = fn;
+	cb->param = user_param;
+	cb->next = *cbp;
+	*cbp = cb;
+	return cb;
+}
+
+int
+rte_dev_remove_callback(struct rte_dev_rxtx_callback ** cbp,
+		struct rte_dev_rxtx_callback *user_cb)
+{
+	struct rte_dev_rxtx_callback *cb = *cbp;
+	struct rte_dev_rxtx_callback *prev_cb;
+
+	/* Reset head pointer and remove user cb if first in the list. */
+	if (cb == user_cb) {
+		*cbp = user_cb->next;
+		return 0;
+	}
+
+	/* Remove the user cb from the callback list. */
+	do {
+		prev_cb = cb;
+		cb = cb->next;
+
+		if (cb == user_cb) {
+			prev_cb->next = user_cb->next;
+			return 0;
+		}
+	} while (cb != NULL);
+
+	/* Callback wasn't found. */
+	return (-EINVAL);
+}
+
+int
+rte_dev_callback_register(struct rte_dev_cb_list * cb_list,
+		rte_spinlock_t * lock,
+		enum rte_dev_event_type event,
+		rte_dev_cb_fn cb_fn, void *cb_arg)
+{
+	struct rte_dev_callback *cb;
+
+	rte_spinlock_lock(lock);
+
+	TAILQ_FOREACH(cb, cb_list, next) {
+		if (cb->cb_fn == cb_fn &&
+			cb->cb_arg == cb_arg &&
+			cb->event == event) {
+			break;
+		}
+	}
+
+	/* create a new callback. */
+	if (cb == NULL && (cb = rte_zmalloc("INTR_USER_CALLBACK",
+			sizeof(struct rte_dev_callback), 0)) != NULL) {
+		cb->cb_fn = cb_fn;
+		cb->cb_arg = cb_arg;
+		cb->event = event;
+		TAILQ_INSERT_TAIL(cb_list, cb, next);
+	}
+
+	rte_spinlock_unlock(lock);
+	return ((cb == NULL) ? -ENOMEM : 0);
+}
+
+int
+rte_dev_callback_unregister(struct rte_dev_cb_list * cb_list,
+		rte_spinlock_t * lock,
+		enum rte_dev_event_type event,
+		rte_dev_cb_fn cb_fn, void *cb_arg)
+{
+	int ret;
+	struct rte_dev_callback *cb, *next;
+
+	rte_spinlock_lock(lock);
+
+	ret = 0;
+	for (cb = TAILQ_FIRST(cb_list); cb != NULL; cb = next) {
+
+		next = TAILQ_NEXT(cb, next);
+
+		if (cb->cb_fn != cb_fn || cb->event != event ||
+				(cb->cb_arg != (void *)-1 &&
+				cb->cb_arg != cb_arg))
+			continue;
+
+		/*
+		 * if this callback is not executing right now,
+		 * then remove it.
+		 */
+		if (cb->active == 0) {
+			TAILQ_REMOVE(cb_list, cb, next);
+			rte_free(cb);
+		} else {
+			ret = -EAGAIN;
+		}
+	}
+
+	rte_spinlock_unlock(lock);
+	return (ret);
+}
+
+void
+rte_dev_callback_process(struct rte_dev_cb_list * cb_list, uint8_t

port_id,

lib/librte_eal/common/eal_common_device.c new file +8 -0

+	enum rte_dev_event_type event, rte_spinlock_t *lock)
+{
+	struct rte_dev_callback *cb;
+	struct rte_dev_callback dev_cb;
+
+	rte_spinlock_lock(lock);
+	TAILQ_FOREACH(cb, cb_list, next) {
+		if (cb->cb_fn == NULL || cb->event != event)

lib/librte_eal/common/eal_common_device.c line 185

Quoted 10 lines

lib/librte_eal/common/eal_common_device.c new file +10 -0

+			continue;
+		dev_cb = *cb;
+		cb->active = 1;
+		rte_spinlock_unlock(lock);
+		dev_cb.cb_fn(port_id, dev_cb.event, dev_cb.cb_arg);
+		rte_spinlock_lock(lock);
+		cb->active = 0;
+	}
+	rte_spinlock_unlock(lock);
+}

This is a bit...odd. The rte_eth callbacks had some context because you
knew
when the callback was going to be issued (at the end of an rx and tx

operation).
Here, by making the process routine generic, you're removed that context, and so
the caller has no guarantee as to when their callbacks will be called. If its
to be done at the end of the generic transmit and routine functions, that would
be fine, but I don't see that happening here.

Quoted 160 lines

lib/librte_eal/common/include/rte_common_device.h new file +13 -0

new file mode 100644
index 0000000..5baefb6
@@ -0,0 +1,617 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above

copyright
+ * notice, this list of conditions and the following disclaimer
in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products
derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
ANY
+ * THEORY OF LIABILITY, WHPDF IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
DAMAGE.
+ */

lib/librte_eal/common/include/rte_common_device.h new file +20 -0

+
+#ifndef _RTE_COMMON_DEVICE_H_
+#define _RTE_COMMON_DEVICE_H_
+
+/**
+ * @file
+ *
+ * Common Device Helpers in RTE
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <rte_spinlock.h>
+
+/* Macros for checking for restricting functions to primary instance

only */

lib/librte_eal/common/include/rte_common_device.h new file +14 -0

+#define PROC_PRIMARY_OR_ERR_RET(retval) do { \
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
+		PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
+		return (retval); \
+	} \
+} while(0)
+#define PROC_PRIMARY_OR_RET() do { \
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
+		PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
+		return; \
+	} \
+} while(0)
+
+/* Macros to check for invalid function pointers in dev_ops structure

*/

lib/librte_eal/common/include/rte_common_device.h new file +49 -0

+#define FUNC_PTR_OR_ERR_RET(func, retval) do { \
+	if ((func) == NULL) { \
+		PMD_DEBUG_TRACE("Function not supported\n"); \
+		return (retval); \
+	} \
+} while(0)
+#define FUNC_PTR_OR_RET(func) do { \
+	if ((func) == NULL) { \
+		PMD_DEBUG_TRACE("Function not supported\n"); \
+		return; \
+	} \
+} while(0)
+
+enum {
+	DEV_DETACHED = 0,
+	DEV_ATTACHED
+};
+
+/*
+ * The device type
+ */
+enum rte_dev_type {
+	RTE_DEV_UNKNOWN,	/**< unknown device type */
+	RTE_DEV_PCI,
+		/**< Physical function and Virtual function of PCI devices */
+	RTE_DEV_VIRTUAL,	/**< non hardware device */
+	RTE_DEV_MAX			/**< max value of this enum */
+};
+
+/**
+ * The device event type for interrupt, and maybe others in the future.
+ */
+enum rte_dev_event_type {
+	RTE_DEV_EVENT_UNKNOWN,  /**< unknown event type */
+	RTE_DEV_EVENT_INTR_LSC, /**< lsc interrupt event */
+	RTE_DEV_EVENT_MAX       /**< max value of this enum */
+};
+
+struct rte_dev_callback;
+
+/** @internal Structure to keep track of registered callback */
+TAILQ_HEAD(rte_dev_cb_list, rte_dev_callback);
+
+struct rte_mbuf;
+
+typedef uint16_t (*dev_rx_burst_t)(void *rxq,
+				   struct rte_mbuf **rx_pkts,
+				   uint16_t nb_pkts);
+/**< @internal Retrieve input packets from a receive queue of a

device. */

lib/librte_eal/common/include/rte_common_device.h new file +15 -0

+
+typedef uint16_t (*dev_tx_burst_t)(void *txq,
+				   struct rte_mbuf **tx_pkts,
+				   uint16_t nb_pkts);
+/**< @internal Send output packets on a transmit queue of a device. */
+
+typedef void (*rte_dev_cb_fn)(uint8_t port_id, \
+		enum rte_dev_event_type event, void *cb_arg);
+/**< user application callback to be registered for interrupts */
+
+#define RTE_DEV_NAME_MAX_LEN (32)
+
+/**
+ * @internal
+ * Common set of members for all devices included at the top of

'struct rte_xyz_dev'
+ */

lib/librte_eal/common/include/rte_common_device.h new file +2 -0

+#define RTE_COMMON_DEV(_t)
                	\
+    dev_rx_burst_t              rx_pkt_burst;   /**< Pointer to PMD

receive function. */ \
+ dev_tx_burst_t tx_pkt_burst; /**< Pointer to PMD
transmit function. */ \

Quoted 8 lines

+ struct rte_##_t##dev_data *data; /**< Pointer to device
data */ \
+ struct rte_##_t##global *gbl_data; /**< Pointer to device
global data */ \
+ const struct _t##driver *driver; /**< Driver for this
device */ \
+ struct _t##dev_ops *dev_ops; /**< Functions
exported by PMD */ \

This doesn't make any sense to me. You've made a generic macro that creates

what is ostensibly supposed to be common code point, but then you include some
string concatenation so that the common parts are actually unique to a given
device class. Why would you do that? At that point the common parts aren't
common by definition, and should go in a structure specific to that device class

Quoted 28 lines

+ struct rte_pci_device *pci_dev; /**< PCI info.
supplied by probing */ \

lib/librte_eal/common/include/rte_common_device.h new file +10 -0

+    /** User application callback for interrupts if present */
                    \
+    struct rte_dev_cb_list   link_intr_cbs;
             		\
+    /**        
                    \
+     * User-supplied functions called from rx_burst to post-process
                    \
+     * received packets before passing them to the user
                    \
+     */        
                    \
+    struct rte_dev_rxtx_callback **post_rx_burst_cbs;
                 	\
+    /**        
                    \
+     * User-supplied functions called from tx_burst to pre-process
                    \
+     * received packets before passing them to the driver for

transmission. \

lib/librte_eal/common/include/rte_common_device.h new file +3 -0

+     */        
                    \
+    struct rte_dev_rxtx_callback **pre_tx_burst_cbs;
                 	\
+    enum rte_dev_type       	dev_type;       /**< Flag indicating the

Quoted 4 lines

device type */ \
+ uint8_t attached /**< Flag indicating
the port is attached */
+

I'm also confused here. Looking at this, this is effectively a complete renaming of the rte_eth_dev device structure. Once again, why not just leave
rte_eth_dev alone, and develop the crypto device from scratch? If your intent
is for it to have so much in common with an ethernet device that you can effectively just rename the existing structure, why not just call it an ethernet
device?

Quoted 67 lines

+/**
+ * @internal
+ * Common set of members for all devices included at the top of 'struct rte_xyz_dev_data'
+ */

lib/librte_eal/common/include/rte_common_device.h new file +4 -0

+#define RTE_COMMON_DEV_DATA
    	\
+    char name[RTE_DEV_NAME_MAX_LEN]; /**< Unique identifier name */
        \
+               
        \
+    void **rx_queues;               /**< Array of pointers to RX

queues. */ \
+ void **tx_queues; /**< Array of pointers to TX
queues. */ \

lib/librte_eal/common/include/rte_common_device.h new file +5 -0

+    uint16_t nb_rx_queues;          /**< Number of RX queues. */
        \
+    uint16_t nb_tx_queues;          /**< Number of TX queues. */
        \
+               
        \
+    uint16_t mtu;                   /**< Maximum Transmission Unit. */
        \
+    uint8_t unit_id;                /**< Unit/Port ID for this

instance */ \

lib/librte_eal/common/include/rte_common_device.h new file +5 -0

+    uint8_t _pad0;             		/* alignment filler */
     \
+               
        \
+    /* 64bit alignment starts here */
        \
+    void    *dev_private;           /**< PMD-specific private data */
        \
+    uint64_t rx_mbuf_alloc_failed;  /**< RX ring mbuf allocation

failures. */ \
+ uint32_t min_rx_buf_size; /**< Common rx buffer size handled
by all queues */ \

lib/librte_eal/common/include/rte_common_device.h new file +5 -0

+    uint32_t _pad1					/**< Align to a 64bit boundary */
+
+/**
+ * @internal
+ * Common set of members for all devices included at the top of

'struct rte_xyz_dev_info'
+ */

lib/librte_eal/common/include/rte_common_device.h new file +2 -0

+#define RTE_COMMON_DEV_INFO
    	\
+    struct rte_pci_device   *pci_dev;       /**< Device PCI

information. */ \
+ const char *driver_name; /**< Device Driver name.
*/ \
+ unsigned int if_index; /**< Index to bound host
interface, or 0 if none. */ \
+ /* Use if_indextoname() to translate into an interface name.
*/ \

lib/librte_eal/common/include/rte_common_device.h new file +7 -0

+    uint32_t _pad0
+
+#define port_id		unit_id
+
+/**
+ * @internal
+ * Common set of members for all devices included at the top of

'struct rte_xyz_global'
+ */
+#define RTE_COMMON_GLOBAL(_t)
\
+ struct rte_##_t##dev * devs;/**< Device information array */

Quoted 3 lines

\
+ struct rte_##_t##dev_data * data; /**< Device private data */
\

Again, if you're going to make something common, It doesn't make sense to then
give it a name that is going to be unique to the device class instantiating the
macro.

Quoted 60 lines

+ uint8_t nb_ports; /**< Number of ports/units found */
\
+ uint8_t max_ports; /**< Max number of ports or units */
\
+ uint16_t dflt_mtu; /**< Default MTU if needed by device
*/\
+ uint16_t dev_size; /**< Internal size of device struct
*/ \
+ uint16_t data_size; /**< Internal size of data structure
*/\
+ const char * mz_dev_data /**< String constant for device data
*/

lib/librte_eal/common/include/rte_common_device.h new file +16 -0

+
+/**
+ * @internal
+ * Common overlay type structures with all devices.
+ */
+struct rte_dev_info {
+    RTE_COMMON_DEV_INFO;
+};
+
+/**
+ * @internal
+ * The generic data structure associated with each device.
+ *
+ * Pointers to burst-oriented packet receive and transmit functions are
+ * located at the beginning of the structure, along with the pointer to
+ * where all the data elements for the particular device are stored in

shared
+ * memory. This split allows the function pointer and driver data to be per-
+ * process, while the actual configuration data for the device is shared.
+ */

lib/librte_eal/common/include/rte_common_device.h new file +7 -0

+struct rte_dev {
+    RTE_COMMON_DEV();
+};
+
+/**
+ * @internal
+ * The data part, with no function pointers, associated with each

device.
+ *
+ * This structure is safe to place in shared memory to be common among different
+ * processes in a multi-process configuration.
+ */

lib/librte_eal/common/include/rte_common_device.h new file +7 -0

+struct rte_dev_data {
+    RTE_COMMON_DEV_DATA;
+};
+
+/**
+ * @internal
+ * This structure is attempting to mirror the rte_xyz_global

structures.
+ *
+ * Be careful as this structure is over layered on top of the xyzdev structures.
+ */

lib/librte_eal/common/include/rte_common_device.h line 246

Quoted 4 lines

lib/librte_eal/common/include/rte_common_device.h new file +4 -0

+struct rte_dev_global {
+    RTE_COMMON_GLOBAL();
+};
+

I don't understand, you created macros that allow for naming specifics, then
don't use it? If your intent was for that data to be generically named (which
would make sense), then you should remove the parameter from the macro so that
can't change.

Quoted 13 lines

lib/librte_eal/common/include/rte_common_device.h new file +13 -0

+/**
+ * Get the rte_pkt_dev structure device pointer for the device.
+ *
+ * @param   gbl     pointer to device specific global structure.
+ * @param   port_id Port ID value to select the device structure.
+ *
+ * @return
+ *   - The rte_pkt_dev structure pointer for the given port ID.
+ */
+static inline void *
+rte_get_dev(void * _gbl, uint8_t port_id)
+{
+    struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;

lib/librte_eal/common/include/rte_common_device.h line 263

Quoted 4 lines

lib/librte_eal/common/include/rte_common_device.h new file +4 -0

+
+    return (void *)((uint8_t *)gbl->devs + (port_id * gbl->dev_size));
+}
+

How will this be helpful? the caller will need to know what type of struct to
cast this to (since it can be named different for different dev classes). doesn't that sort of thing undo the notion of commonality?

Neil

Quoted 224 lines · Neil Horman

Quoted 208 lines · Keith Wiles

Move a number of device specific define, structures and functions into a generic device base set of files for all device not just Ethernet.

Signed-off-by Keith Wiles <[email protected]>

---
 lib/librte_eal/common/eal_common_device.c         | 185 +++++++
 lib/librte_eal/common/include/rte_common_device.h | 617
++++++++++++++++++++++
 2 files changed, 802 insertions(+)
 create mode 100644 lib/librte_eal/common/eal_common_device.c
 create mode 100644 lib/librte_eal/common/include/rte_common_device.h

lib/librte_eal/common/eal_common_device.c new file +14 -0

new file mode 100644
index 0000000..a9ef925
@@ -0,0 +1,185 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2014 6WIND S.A.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above

copyright
+ * notice, this list of conditions and the following disclaimer
in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products
derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
DAMAGE.
+ */

lib/librte_eal/common/eal_common_device.c new file +134 -0

+
+#include <stdio.h>
+#include <string.h>
+#include <inttypes.h>
+#include <sys/queue.h>
+
+#include <rte_dev.h>
+#include <rte_devargs.h>
+#include <rte_debug.h>
+#include <rte_devargs.h>
+#include <rte_log.h>
+#include <rte_malloc.h>
+#include <rte_errno.h>
+
+#include "rte_common_device.h"
+
+void *
+rte_dev_add_callback(struct rte_dev_rxtx_callback ** cbp,
+		void * fn, void *user_param)
+{
+	struct rte_dev_rxtx_callback *cb;
+
+	cb = rte_zmalloc(NULL, sizeof(*cb), 0);
+
+	if (cb == NULL) {
+		rte_errno = ENOMEM;
+		return NULL;
+	}
+
+	cb->fn.vp = fn;
+	cb->param = user_param;
+	cb->next = *cbp;
+	*cbp = cb;
+	return cb;
+}
+
+int
+rte_dev_remove_callback(struct rte_dev_rxtx_callback ** cbp,
+		struct rte_dev_rxtx_callback *user_cb)
+{
+	struct rte_dev_rxtx_callback *cb = *cbp;
+	struct rte_dev_rxtx_callback *prev_cb;
+
+	/* Reset head pointer and remove user cb if first in the list. */
+	if (cb == user_cb) {
+		*cbp = user_cb->next;
+		return 0;
+	}
+
+	/* Remove the user cb from the callback list. */
+	do {
+		prev_cb = cb;
+		cb = cb->next;
+
+		if (cb == user_cb) {
+			prev_cb->next = user_cb->next;
+			return 0;
+		}
+	} while (cb != NULL);
+
+	/* Callback wasn't found. */
+	return (-EINVAL);
+}
+
+int
+rte_dev_callback_register(struct rte_dev_cb_list * cb_list,
+		rte_spinlock_t * lock,
+		enum rte_dev_event_type event,
+		rte_dev_cb_fn cb_fn, void *cb_arg)
+{
+	struct rte_dev_callback *cb;
+
+	rte_spinlock_lock(lock);
+
+	TAILQ_FOREACH(cb, cb_list, next) {
+		if (cb->cb_fn == cb_fn &&
+			cb->cb_arg == cb_arg &&
+			cb->event == event) {
+			break;
+		}
+	}
+
+	/* create a new callback. */
+	if (cb == NULL && (cb = rte_zmalloc("INTR_USER_CALLBACK",
+			sizeof(struct rte_dev_callback), 0)) != NULL) {
+		cb->cb_fn = cb_fn;
+		cb->cb_arg = cb_arg;
+		cb->event = event;
+		TAILQ_INSERT_TAIL(cb_list, cb, next);
+	}
+
+	rte_spinlock_unlock(lock);
+	return ((cb == NULL) ? -ENOMEM : 0);
+}
+
+int
+rte_dev_callback_unregister(struct rte_dev_cb_list * cb_list,
+		rte_spinlock_t * lock,
+		enum rte_dev_event_type event,
+		rte_dev_cb_fn cb_fn, void *cb_arg)
+{
+	int ret;
+	struct rte_dev_callback *cb, *next;
+
+	rte_spinlock_lock(lock);
+
+	ret = 0;
+	for (cb = TAILQ_FIRST(cb_list); cb != NULL; cb = next) {
+
+		next = TAILQ_NEXT(cb, next);
+
+		if (cb->cb_fn != cb_fn || cb->event != event ||
+				(cb->cb_arg != (void *)-1 &&
+				cb->cb_arg != cb_arg))
+			continue;
+
+		/*
+		 * if this callback is not executing right now,
+		 * then remove it.
+		 */
+		if (cb->active == 0) {
+			TAILQ_REMOVE(cb_list, cb, next);
+			rte_free(cb);
+		} else {
+			ret = -EAGAIN;
+		}
+	}
+
+	rte_spinlock_unlock(lock);
+	return (ret);
+}
+
+void
+rte_dev_callback_process(struct rte_dev_cb_list * cb_list, uint8_t

port_id,

lib/librte_eal/common/eal_common_device.c new file +8 -0

+	enum rte_dev_event_type event, rte_spinlock_t *lock)
+{
+	struct rte_dev_callback *cb;
+	struct rte_dev_callback dev_cb;
+
+	rte_spinlock_lock(lock);
+	TAILQ_FOREACH(cb, cb_list, next) {
+		if (cb->cb_fn == NULL || cb->event != event)

lib/librte_eal/common/eal_common_device.c line 185

Quoted 10 lines

lib/librte_eal/common/eal_common_device.c new file +10 -0

+			continue;
+		dev_cb = *cb;
+		cb->active = 1;
+		rte_spinlock_unlock(lock);
+		dev_cb.cb_fn(port_id, dev_cb.event, dev_cb.cb_arg);
+		rte_spinlock_lock(lock);
+		cb->active = 0;
+	}
+	rte_spinlock_unlock(lock);
+}

This is a bit...odd. The rte_eth callbacks had some context because you
knew
when the callback was going to be issued (at the end of an rx and tx

Quoted 8 lines

operation).
Here, by making the process routine generic, you're removed that context, and so
the caller has no guarantee as to when their callbacks will be called. If its
to be done at the end of the generic transmit and routine functions, that would
be fine, but I don't see that happening here.

No able to follow you here as this routine is called from the rte_ethdev.c _rte_eth_dev_callback_process(). We could remove the _rte_eth_dev_callback_process function and have the rest of the code call this routine instead. No context is lost here and the callbacks are called in the same location as before.

Quoted 171 lines

Quoted 160 lines

lib/librte_eal/common/include/rte_common_device.h new file +13 -0

new file mode 100644
index 0000000..5baefb6
@@ -0,0 +1,617 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above

copyright
+ * notice, this list of conditions and the following disclaimer
in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products
derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
ANY
+ * THEORY OF LIABILITY, WHPDF IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
DAMAGE.
+ */

lib/librte_eal/common/include/rte_common_device.h new file +20 -0

+
+#ifndef _RTE_COMMON_DEVICE_H_
+#define _RTE_COMMON_DEVICE_H_
+
+/**
+ * @file
+ *
+ * Common Device Helpers in RTE
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <rte_spinlock.h>
+
+/* Macros for checking for restricting functions to primary instance

only */

lib/librte_eal/common/include/rte_common_device.h new file +14 -0

+#define PROC_PRIMARY_OR_ERR_RET(retval) do { \
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
+		PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
+		return (retval); \
+	} \
+} while(0)
+#define PROC_PRIMARY_OR_RET() do { \
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
+		PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
+		return; \
+	} \
+} while(0)
+
+/* Macros to check for invalid function pointers in dev_ops structure

*/

lib/librte_eal/common/include/rte_common_device.h new file +49 -0

+#define FUNC_PTR_OR_ERR_RET(func, retval) do { \
+	if ((func) == NULL) { \
+		PMD_DEBUG_TRACE("Function not supported\n"); \
+		return (retval); \
+	} \
+} while(0)
+#define FUNC_PTR_OR_RET(func) do { \
+	if ((func) == NULL) { \
+		PMD_DEBUG_TRACE("Function not supported\n"); \
+		return; \
+	} \
+} while(0)
+
+enum {
+	DEV_DETACHED = 0,
+	DEV_ATTACHED
+};
+
+/*
+ * The device type
+ */
+enum rte_dev_type {
+	RTE_DEV_UNKNOWN,	/**< unknown device type */
+	RTE_DEV_PCI,
+		/**< Physical function and Virtual function of PCI devices */
+	RTE_DEV_VIRTUAL,	/**< non hardware device */
+	RTE_DEV_MAX			/**< max value of this enum */
+};
+
+/**
+ * The device event type for interrupt, and maybe others in the future.
+ */
+enum rte_dev_event_type {
+	RTE_DEV_EVENT_UNKNOWN,  /**< unknown event type */
+	RTE_DEV_EVENT_INTR_LSC, /**< lsc interrupt event */
+	RTE_DEV_EVENT_MAX       /**< max value of this enum */
+};
+
+struct rte_dev_callback;
+
+/** @internal Structure to keep track of registered callback */
+TAILQ_HEAD(rte_dev_cb_list, rte_dev_callback);
+
+struct rte_mbuf;
+
+typedef uint16_t (*dev_rx_burst_t)(void *rxq,
+				   struct rte_mbuf **rx_pkts,
+				   uint16_t nb_pkts);
+/**< @internal Retrieve input packets from a receive queue of a

device. */

lib/librte_eal/common/include/rte_common_device.h new file +15 -0

+
+typedef uint16_t (*dev_tx_burst_t)(void *txq,
+				   struct rte_mbuf **tx_pkts,
+				   uint16_t nb_pkts);
+/**< @internal Send output packets on a transmit queue of a device. */
+
+typedef void (*rte_dev_cb_fn)(uint8_t port_id, \
+		enum rte_dev_event_type event, void *cb_arg);
+/**< user application callback to be registered for interrupts */
+
+#define RTE_DEV_NAME_MAX_LEN (32)
+
+/**
+ * @internal
+ * Common set of members for all devices included at the top of

'struct rte_xyz_dev'
+ */

lib/librte_eal/common/include/rte_common_device.h new file +2 -0

+#define RTE_COMMON_DEV(_t)
                	\
+    dev_rx_burst_t              rx_pkt_burst;   /**< Pointer to PMD

receive function. */ \
+ dev_tx_burst_t tx_pkt_burst; /**< Pointer to PMD
transmit function. */ \

Quoted 8 lines

+ struct rte_##_t##dev_data *data; /**< Pointer to device
data */ \
+ struct rte_##_t##global *gbl_data; /**< Pointer to device
global data */ \
+ const struct _t##driver *driver; /**< Driver for this
device */ \
+ struct _t##dev_ops *dev_ops; /**< Functions
exported by PMD */ \

This doesn't make any sense to me. You've made a generic macro that creates

Quoted 8 lines

what is ostensibly supposed to be common code point, but then you include some
string concatenation so that the common parts are actually unique to a given
device class. Why would you do that? At that point the common parts aren't
common by definition, and should go in a structure specific to that device class

The reason for these being specific to the device is the device may have extra members private to the device. I could have added a private pointer to the structure to make then generic to allow the device to have its own private members.

Quoted 33 lines

Quoted 28 lines

+ struct rte_pci_device *pci_dev; /**< PCI info.
supplied by probing */ \

lib/librte_eal/common/include/rte_common_device.h new file +10 -0

+    /** User application callback for interrupts if present */
                    \
+    struct rte_dev_cb_list   link_intr_cbs;
             		\
+    /**        
                    \
+     * User-supplied functions called from rx_burst to post-process
                    \
+     * received packets before passing them to the user
                    \
+     */        
                    \
+    struct rte_dev_rxtx_callback **post_rx_burst_cbs;
                 	\
+    /**        
                    \
+     * User-supplied functions called from tx_burst to pre-process
                    \
+     * received packets before passing them to the driver for

transmission. \

lib/librte_eal/common/include/rte_common_device.h new file +3 -0

+     */        
                    \
+    struct rte_dev_rxtx_callback **pre_tx_burst_cbs;
                 	\
+    enum rte_dev_type       	dev_type;       /**< Flag indicating the

Quoted 4 lines

device type */ \
+ uint8_t attached /**< Flag indicating
the port is attached */
+

Quoted 9 lines

I'm also confused here. Looking at this, this is effectively a complete renaming of the rte_eth_dev device structure. Once again, why not just leave
rte_eth_dev alone, and develop the crypto device from scratch? If your intent
is for it to have so much in common with an ethernet device that you can effectively just rename the existing structure, why not just call it an ethernet
device?

My goal was not to have everything common to a ethdev, but to move items out of the ethdev we can have in common. I feel you are not understanding something and I can not figure out what it is. If I remove the concat part maybe this will get my point across better.

Quoted 67 lines

Quoted 67 lines

+/**
+ * @internal
+ * Common set of members for all devices included at the top of 'struct rte_xyz_dev_data'
+ */

lib/librte_eal/common/include/rte_common_device.h new file +4 -0

+#define RTE_COMMON_DEV_DATA
    	\
+    char name[RTE_DEV_NAME_MAX_LEN]; /**< Unique identifier name */
        \
+               
        \
+    void **rx_queues;               /**< Array of pointers to RX

queues. */ \
+ void **tx_queues; /**< Array of pointers to TX
queues. */ \

lib/librte_eal/common/include/rte_common_device.h new file +5 -0

+    uint16_t nb_rx_queues;          /**< Number of RX queues. */
        \
+    uint16_t nb_tx_queues;          /**< Number of TX queues. */
        \
+               
        \
+    uint16_t mtu;                   /**< Maximum Transmission Unit. */
        \
+    uint8_t unit_id;                /**< Unit/Port ID for this

instance */ \

lib/librte_eal/common/include/rte_common_device.h new file +5 -0

+    uint8_t _pad0;             		/* alignment filler */
     \
+               
        \
+    /* 64bit alignment starts here */
        \
+    void    *dev_private;           /**< PMD-specific private data */
        \
+    uint64_t rx_mbuf_alloc_failed;  /**< RX ring mbuf allocation

failures. */ \
+ uint32_t min_rx_buf_size; /**< Common rx buffer size handled
by all queues */ \

lib/librte_eal/common/include/rte_common_device.h new file +5 -0

+    uint32_t _pad1					/**< Align to a 64bit boundary */
+
+/**
+ * @internal
+ * Common set of members for all devices included at the top of

'struct rte_xyz_dev_info'
+ */

lib/librte_eal/common/include/rte_common_device.h new file +2 -0

+#define RTE_COMMON_DEV_INFO
    	\
+    struct rte_pci_device   *pci_dev;       /**< Device PCI

information. */ \
+ const char *driver_name; /**< Device Driver name.
*/ \
+ unsigned int if_index; /**< Index to bound host
interface, or 0 if none. */ \
+ /* Use if_indextoname() to translate into an interface name.
*/ \

lib/librte_eal/common/include/rte_common_device.h new file +7 -0

+    uint32_t _pad0
+
+#define port_id		unit_id
+
+/**
+ * @internal
+ * Common set of members for all devices included at the top of

'struct rte_xyz_global'
+ */
+#define RTE_COMMON_GLOBAL(_t)
\
+ struct rte_##_t##dev * devs;/**< Device information array */

Quoted 8 lines

Quoted 3 lines

\
+ struct rte_##_t##dev_data * data; /**< Device private data */
\

Again, if you're going to make something common, It doesn't make sense to then
give it a name that is going to be unique to the device class instantiating the
macro.

OK, will try to update the code to be non-specific to a given device.

Quoted 85 lines

Quoted 60 lines

+ uint8_t nb_ports; /**< Number of ports/units found */
\
+ uint8_t max_ports; /**< Max number of ports or units */
\
+ uint16_t dflt_mtu; /**< Default MTU if needed by device
*/\
+ uint16_t dev_size; /**< Internal size of device struct
*/ \
+ uint16_t data_size; /**< Internal size of data structure
*/\
+ const char * mz_dev_data /**< String constant for device data
*/

lib/librte_eal/common/include/rte_common_device.h new file +16 -0

+
+/**
+ * @internal
+ * Common overlay type structures with all devices.
+ */
+struct rte_dev_info {
+    RTE_COMMON_DEV_INFO;
+};
+
+/**
+ * @internal
+ * The generic data structure associated with each device.
+ *
+ * Pointers to burst-oriented packet receive and transmit functions are
+ * located at the beginning of the structure, along with the pointer to
+ * where all the data elements for the particular device are stored in

shared
+ * memory. This split allows the function pointer and driver data to be per-
+ * process, while the actual configuration data for the device is shared.
+ */

lib/librte_eal/common/include/rte_common_device.h new file +7 -0

+struct rte_dev {
+    RTE_COMMON_DEV();
+};
+
+/**
+ * @internal
+ * The data part, with no function pointers, associated with each

device.
+ *
+ * This structure is safe to place in shared memory to be common among different
+ * processes in a multi-process configuration.
+ */

lib/librte_eal/common/include/rte_common_device.h new file +7 -0

+struct rte_dev_data {
+    RTE_COMMON_DEV_DATA;
+};
+
+/**
+ * @internal
+ * This structure is attempting to mirror the rte_xyz_global

structures.
+ *
+ * Be careful as this structure is over layered on top of the xyzdev structures.
+ */

lib/librte_eal/common/include/rte_common_device.h line 246

Quoted 4 lines

lib/librte_eal/common/include/rte_common_device.h new file +4 -0

+struct rte_dev_global {
+    RTE_COMMON_GLOBAL();
+};
+

I don't understand, you created macros that allow for naming specifics, then
don't use it? If your intent was for that data to be generically named (which
would make sense), then you should remove the parameter from the macro so that
can't change.

Quoted 13 lines

lib/librte_eal/common/include/rte_common_device.h new file +13 -0

+/**
+ * Get the rte_pkt_dev structure device pointer for the device.
+ *
+ * @param   gbl     pointer to device specific global structure.
+ * @param   port_id Port ID value to select the device structure.
+ *
+ * @return
+ *   - The rte_pkt_dev structure pointer for the given port ID.
+ */
+static inline void *
+rte_get_dev(void * _gbl, uint8_t port_id)
+{
+    struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;

Quoted 8 lines

lib/librte_eal/common/include/rte_common_device.h line 263

Quoted 4 lines

lib/librte_eal/common/include/rte_common_device.h new file +4 -0

+
+    return (void *)((uint8_t *)gbl->devs + (port_id * gbl->dev_size));
+}
+

How will this be helpful? the caller will need to know what type of struct to
cast this to (since it can be named different for different dev classes). doesn't that sort of thing undo the notion of commonality?

Not really the point was to have a routine able to return the correct device structure pointer and the developer should understand which global structure he is passing into the routine and what type is being returned.

Hi All,

Here is a set of RFC patches to update DPDK to support a generic set of devices. The patches that follow create two files eal_common_device.h and rte_common_device.c and then a few items in rte_ethdev.[ch] were moved to cleanup those files by moving the device generic values and functions into a common set of device generic files.

In the rte_common_device.h file are a couple of macros to abstract a few common items from rte_ethdev.h which are not ethernet specific. These generic device specific structure members are place into macros to be placed at the top of each new device type crypto, hardware offload, dpi, compression and possible others to be defined later.
Note: could have used nested structures, but required a larger change set.

I did not pull the Rx/Tx Routines into a common function, but it could be done if everyone agrees. It does not mean every device will use a common Rx/Tx routine. Without pulling Rx/Tx routines into a common file allows the device writer to have similar or different set of routines.

These patches do not contain any new devices as these are being work on today.

More cleanup of ethdev could be done to remove NIC specific features not supported in all devices and someone needs to do that cleanup IMO.

The code is untested and I wanted to get something our for others to poke at
today and more could be pulled out of ethdev as well. I/We will be looking at testing the code as we get more completed.

I have not finished up the crypto APIs yet, but I am planning to work on those additions today. The crypto code we are using is the Quick Assist code
found on 01.org, but we need to update the code to be move DPDK friendly.

The QAT code does have a modified like API similar to Linux Kernel crypto API
and I want to review that code first.

Regards,
++Keith

Keith Wiles (4):
Rename of device types to be generic device names.
Add the new common device header and C file.
Update files to build new device generic common files and headers. Update the rte_ethdev.[ch] files for the new device generic changes.

app/test/test_link_bonding.c                      |  10 +-
app/test/virtual_pmd.c                            |   4 +-
examples/link_status_interrupt/main.c             |   6 +-
lib/librte_eal/bsdapp/eal/Makefile                |   1 +
lib/librte_eal/common/Makefile                    |   1 +
lib/librte_eal/common/eal_common_device.c         | 185 +++++++
lib/librte_eal/common/include/rte_common_device.h | 617
++++++++++++++++++++++
lib/librte_eal/common/include/rte_log.h           |   1 +
lib/librte_eal/linuxapp/eal/Makefile              |   1 +
lib/librte_ether/rte_ethdev.c                     | 290 ++--------
lib/librte_ether/rte_ethdev.h                     | 225 +++-----
lib/librte_pmd_af_packet/rte_eth_af_packet.c      |   2 +-
lib/librte_pmd_bond/rte_eth_bond_api.c            |   6 +-
lib/librte_pmd_bond/rte_eth_bond_pmd.c            |  12 +-
lib/librte_pmd_bond/rte_eth_bond_private.h        |   2 +-
lib/librte_pmd_e1000/em_ethdev.c                  |   8 +-
lib/librte_pmd_e1000/em_rxtx.c                    |   4 +-
lib/librte_pmd_e1000/igb_ethdev.c                 |   2 +-
lib/librte_pmd_i40e/i40e_ethdev.c                 |   4 +-
lib/librte_pmd_ixgbe/ixgbe_ethdev.c               |   2 +-
lib/librte_pmd_mlx4/mlx4.c                        |   2 +-
lib/librte_pmd_null/rte_eth_null.c                |   2 +-
lib/librte_pmd_pcap/rte_eth_pcap.c                |   2 +-
lib/librte_pmd_ring/rte_eth_ring.c                |   2 +-
lib/librte_pmd_virtio/virtio_ethdev.c             |   2 +-
lib/librte_pmd_xenvirt/rte_eth_xenvirt.c          |   2 +-
26 files changed, 969 insertions(+), 426 deletions(-)

create mode 100644 lib/librte_eal/common/eal_common_device.c create mode 100644 lib/librte_eal/common/include/rte_common_device.h

Signature · 1 line

2.3.0