[dpdk-dev,RFC,2/4] Add the new common device header and C file.

Message ID 1428526720-50221-3-git-send-email-keith.wiles@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Wiles, Keith April 8, 2015, 8:58 p.m. UTC
  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 <keith.wiles@intel.com>
---
 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
  

Comments

Neil Horman April 9, 2015, 11:53 a.m. UTC | #1
On Wed, Apr 08, 2015 at 03:58:38PM -0500, Keith Wiles wrote:
> 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 <keith.wiles@intel.com>
> ---
>  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
> 
> diff --git a/lib/librte_eal/common/eal_common_device.c b/lib/librte_eal/common/eal_common_device.c
> new file mode 100644
> index 0000000..a9ef925
> --- /dev/null
> +++ b/lib/librte_eal/common/eal_common_device.c
> @@ -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)
> +			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.
 
> diff --git a/lib/librte_eal/common/include/rte_common_device.h b/lib/librte_eal/common/include/rte_common_device.h
> new file mode 100644
> index 0000000..5baefb6
> --- /dev/null
> +++ b/lib/librte_eal/common/include/rte_common_device.h
> @@ -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)
> +
> +/**
> + * @internal
> + * Common set of members for all devices included at the top of 'struct rte_xyz_dev'
> + */
> +#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. */   \
> +    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

> +    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                                     \
> +     */                                                                                     \
> +    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.                 \
> +     */                                                                                     \
> +    struct rte_dev_rxtx_callback **pre_tx_burst_cbs;                                     	\
> +    enum rte_dev_type       	dev_type;       /**< Flag indicating the 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?

> +/**
> + * @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
> +
> +#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 */       \
> +    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.

> +    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;
> +};
> +
> +/**
> + * @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.
> + */
> +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.

> +/**
> + * 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;
> +
> +    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
  
Wiles, Keith April 10, 2015, 7:25 p.m. UTC | #2
On 4/9/15, 6:53 AM, "Neil Horman" <nhorman@tuxdriver.com> wrote:

>On Wed, Apr 08, 2015 at 03:58:38PM -0500, Keith Wiles wrote:
>> 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 <keith.wiles@intel.com>
>> ---
>>  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
>> 
>> diff --git a/lib/librte_eal/common/eal_common_device.c
>>b/lib/librte_eal/common/eal_common_device.c
>> new file mode 100644
>> index 0000000..a9ef925
>> --- /dev/null
>> +++ b/lib/librte_eal/common/eal_common_device.c
>> @@ -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)
>> +			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.

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.

> 
>> diff --git a/lib/librte_eal/common/include/rte_common_device.h
>>b/lib/librte_eal/common/include/rte_common_device.h
>> new file mode 100644
>> index 0000000..5baefb6
>> --- /dev/null
>> +++ b/lib/librte_eal/common/include/rte_common_device.h
>> @@ -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)
>> +
>> +/**
>> + * @internal
>> + * Common set of members for all devices included at the top of
>>'struct rte_xyz_dev'
>> + */
>> +#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. */   \
>> +    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

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.

>
>> +    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
>>                     \
>> +     */        
>>                     \
>> +    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.                 \
>> +     */        
>>                     \
>> +    struct rte_dev_rxtx_callback **pre_tx_burst_cbs;
>>                  	\
>> +    enum rte_dev_type       	dev_type;       /**< Flag indicating the
>>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?

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.

>
>> +/**
>> + * @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
>> +
>> +#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 */
>>\
>> +    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.

>
>> +    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;
>> +};
>> +
>> +/**
>> + * @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.
>> + */
>> +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.
>
>> +/**
>> + * 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;
>> +
>> +    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.

>
>Neil
>
  
Neil Horman April 12, 2015, 1:08 p.m. UTC | #3
On Fri, Apr 10, 2015 at 07:25:32PM +0000, Wiles, Keith wrote:
> 
> 
> On 4/9/15, 6:53 AM, "Neil Horman" <nhorman@tuxdriver.com> wrote:
> 
> >On Wed, Apr 08, 2015 at 03:58:38PM -0500, Keith Wiles wrote:
> >> 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 <keith.wiles@intel.com>
> >> ---
> >>  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
> >> 
> >> diff --git a/lib/librte_eal/common/eal_common_device.c
> >>b/lib/librte_eal/common/eal_common_device.c
> >> new file mode 100644
> >> index 0000000..a9ef925
> >> --- /dev/null
> >> +++ b/lib/librte_eal/common/eal_common_device.c
> >> @@ -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)
> >> +			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.
> 
> 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.
> 
Its ok, ignore me here.  I hadn't looked ahead to how you used this in the
ethdev case, that makes sense.  Though I'm still a bit concerned about how this
might be used in other device cases, but that can wait until you post the QAT
RFC patches.

> > 
> >> diff --git a/lib/librte_eal/common/include/rte_common_device.h
> >>b/lib/librte_eal/common/include/rte_common_device.h
> >> new file mode 100644
> >> index 0000000..5baefb6
> >> --- /dev/null
> >> +++ b/lib/librte_eal/common/include/rte_common_device.h
> >> @@ -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)
> >> +
> >> +/**
> >> + * @internal
> >> + * Common set of members for all devices included at the top of
> >>'struct rte_xyz_dev'
> >> + */
> >> +#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. */   \
> >> +    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
> 
> 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.
> 
Well, yes, that would make more sense to me.  The macro is called
RTE_COMMON_DEV.  It seems like it should only create members that are common to
all devices, with common names.

> >
> >> +    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
> >>                     \
> >> +     */        
> >>                     \
> >> +    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.                 \
> >> +     */        
> >>                     \
> >> +    struct rte_dev_rxtx_callback **pre_tx_burst_cbs;
> >>                  	\
> >> +    enum rte_dev_type       	dev_type;       /**< Flag indicating the
> >>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?
> 
> 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.
> 

I know what your goal is.  All I'm saying is that, in pursuing that goal,
you seem to have decided that all the data in the rte_ethdev is common to all
devices.

> >
> >> +/**
> >> + * @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
> >> +
> >> +#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 */
> >>\
> >> +    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.
> 
Thank you.

> >
> >> +    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;
> >> +};
> >> +
> >> +/**
> >> + * @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.
> >> + */
> >> +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.
> >
> >> +/**
> >> + * 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;
> >> +
> >> +    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.
> 
Ok, I get that, but if the caller has to know how to cast this to an appropriate
type, then they will need to know the type of device they are communicating
with, and understand all its member data and methods, which defeats the ability
for a user of the device to write truly generic code to access devices, which,
when we spoke offline (and I think earlier in this thread), you indicated was a
goal of yours.

Neil
  

Patch

diff --git a/lib/librte_eal/common/eal_common_device.c b/lib/librte_eal/common/eal_common_device.c
new file mode 100644
index 0000000..a9ef925
--- /dev/null
+++ b/lib/librte_eal/common/eal_common_device.c
@@ -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)
+			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);
+}
diff --git a/lib/librte_eal/common/include/rte_common_device.h b/lib/librte_eal/common/include/rte_common_device.h
new file mode 100644
index 0000000..5baefb6
--- /dev/null
+++ b/lib/librte_eal/common/include/rte_common_device.h
@@ -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)
+
+/**
+ * @internal
+ * Common set of members for all devices included at the top of 'struct rte_xyz_dev'
+ */
+#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. */   \
+    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 */           \
+    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                                     \
+     */                                                                                     \
+    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.                 \
+     */                                                                                     \
+    struct rte_dev_rxtx_callback **pre_tx_burst_cbs;                                     	\
+    enum rte_dev_type       	dev_type;       /**< Flag indicating the device type */     \
+    uint8_t                     attached        /**< Flag indicating the port is attached */
+
+/**
+ * @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
+
+#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 */       \
+    struct rte_##_t##dev_data * data;  /**< Device private data */     \
+    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;
+};
+
+/**
+ * @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.
+ */
+struct rte_dev_global {
+    RTE_COMMON_GLOBAL();
+};
+
+/**
+ * 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;
+
+    return (void *)((uint8_t *)gbl->devs + (port_id * gbl->dev_size));
+}
+
+/**
+ * Get the number of device ports in the system.
+ *
+ * @param gbl
+ *  The pointer to the current global data structure for xyzdev.
+ *
+ * @return
+ *   - Number of ports found in the system.
+ */
+static inline uint8_t
+rte_pkt_dev_count(void * _gbl)
+{
+    struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
+    return gbl->nb_ports;
+}
+
+/**
+ * Validate if the port number is valid
+ *
+ * @param   gbl The pointer to the current global data structure for xyzdev.
+ * @param   port_id Port ID value to select the device.
+ *
+ * @return
+ *   - Number of ports found in the system.
+ */
+static inline int
+rte_dev_is_valid_port(void * _gbl, uint8_t port_id)
+{
+    struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
+    struct rte_dev * dev;
+
+    if (port_id >= gbl->nb_ports)
+        return 0;
+
+    dev = rte_get_dev(gbl, port_id);
+    if (dev->attached != DEV_ATTACHED)
+        return 0;
+    else
+        return 1;
+}
+
+/**
+ * Get the number of device ports in the system.
+ *
+ * @param gbl
+ *  The pointer to the current global data structure for xyzdev.
+ *
+ * @return
+ *   - Number of ports found in the system.
+ */
+static inline uint8_t
+rte_dev_count(void * _gbl)
+{
+    struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
+    return gbl->nb_ports;
+}
+
+/**
+ * Get the rte_pkt_dev structure device pointer for the device by name.
+ *
+ * @param   gbl     pointer to device specific global structure.
+ * @param   name    Name of device.
+ *
+ * @return
+ *   - The rte_dev structure pointer for the given name.
+ */
+static inline struct rte_dev *
+rte_get_named_dev(void * _gbl, const char *name)
+{
+    struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
+    struct rte_dev *dev;
+    int i;
+
+    if (name == NULL)
+        return NULL;
+
+    for (i = 0; i < gbl->nb_ports; i++) {
+        dev = (struct rte_dev *)((uint8_t *)gbl->devs +
+                (i * gbl->dev_size));
+
+        if (strcmp(dev->data->name, name) == 0)
+            return dev;
+    }
+
+    return NULL;
+}
+
+/**
+ * Get the rte_pkt_dev structure device pointer for the PCI name.
+ *
+ * @param   gbl     pointer to device specific global structure.
+ * @param   name    Name of PCI device.
+ *
+ * @return
+ *   - The rte_dev structure pointer for the given PCIname.
+ */
+static inline struct rte_dev *
+rte_get_pci_named_dev(void * _gbl, struct rte_pci_addr *pci_addr)
+{
+    struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
+    struct rte_dev *dev;
+    int i;
+
+    if (pci_addr == NULL)
+        return NULL;
+
+    for (i = 0; i < gbl->nb_ports; i++) {
+        dev = (struct rte_dev *)((uint8_t *)gbl->devs +
+                (i * gbl->dev_size));
+
+        if (rte_eal_compare_pci_addr(&dev->pci_dev->addr, pci_addr) == 0)
+            return dev;
+    }
+
+    return NULL;
+}
+
+/*
+ * Return the NUMA socket to which a device is connected
+ *
+ * @param gbl
+ *   The global structure pointer for a given xyzdev.
+ * @param port_id
+ *   The port identifier of the device
+ * @return
+ *   The NUMA socket id to which the device is connected or
+ *   a default of zero if the socket could not be determined.
+ *   -1 is returned is the port_id value is out of range.
+ */
+static inline int
+rte_dev_socket_id(void * _gbl, uint8_t port_id)
+{
+    struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
+    struct rte_dev * dev;
+
+    if (!rte_dev_is_valid_port(gbl, port_id))
+        return -1;
+
+    dev = rte_get_dev(gbl, port_id);
+
+    if ( dev->pci_dev )
+        return dev->pci_dev->numa_node;
+    else
+        return 0;
+}
+/**
+ * Function type used for RX packet processing packet a callback.
+ *
+ * The callback function is called on RX with a burst of packets that have
+ * been received on the given port and queue.
+ *
+ * @param port
+ *   The Ethernet port on which RX is being performed.
+ * @param queue
+ *   The queue on the Ethernet port which is being used to receive the packets.
+ * @param pkts
+ *   The burst of packets that have just been received.
+ * @param nb_pkts
+ *   The number of packets in the burst pointed to by "pkts".
+ * @param max_pkts
+ *   The max number of packets that can be stored in the "pkts" array.
+ * @param user_param
+ *   The arbitrary user parameter passed in by the application when the callback
+ *   was originally configured.
+ * @return
+ *   The number of packets returned to the user.
+ */
+typedef uint16_t (*rte_rx_callback_fn)(uint8_t port, uint16_t queue,
+	struct rte_mbuf *pkts[], uint16_t nb_pkts, uint16_t max_pkts,
+	void *user_param);
+
+/**
+ * Function type used for TX packet processing packet a callback.
+ *
+ * The callback function is called on TX with a burst of packets immediately
+ * before the packets are put onto the hardware queue for transmission.
+ *
+ * @param port
+ *   The Ethernet port on which TX is being performed.
+ * @param queue
+ *   The queue on the Ethernet port which is being used to transmit the packets.
+ * @param pkts
+ *   The burst of packets that are about to be transmitted.
+ * @param nb_pkts
+ *   The number of packets in the burst pointed to by "pkts".
+ * @param user_param
+ *   The arbitrary user parameter passed in by the application when the callback
+ *   was originally configured.
+ * @return
+ *   The number of packets to be written to the NIC.
+ */
+typedef uint16_t (*rte_tx_callback_fn)(uint8_t port, uint16_t queue,
+	struct rte_mbuf *pkts[], uint16_t nb_pkts, void *user_param);
+
+/**
+ * @internal
+ * Union used to hold the callback function pointers for RX and TX callback.
+ */
+union rte_dev_callback_fn {
+	void * vp;						/**< Allow for a generic function pointer to avoid casting in common routines */
+	rte_rx_callback_fn rx;			/**< Ethernet or packet based callback function for RX packets */
+	rte_tx_callback_fn tx;			/**< Ethernet or packet based callback function for TX packets */
+	/**< Add other device callback prototypes here */
+};
+
+/**
+ * @internal
+ * Structure used to hold information about a callback to be called for a
+ * queue on RX and TX.
+ */
+struct rte_dev_rxtx_callback {
+	struct rte_dev_rxtx_callback *next;
+	union rte_dev_callback_fn fn;
+	void *param;
+};
+
+/**
+ * The user application callback description.
+ *
+ * It contains callback address to be registered by user application,
+ * the pointer to the parameters for callback, and the event type.
+ */
+struct rte_dev_callback {
+	TAILQ_ENTRY(rte_dev_callback) next;		/**< Callback list */
+	rte_dev_cb_fn cb_fn;                	/**< Callback address */
+	void *cb_arg;                           /**< Parameter for callback */
+	enum rte_dev_event_type event;          /**< Interrupt event type */
+	uint32_t active;                        /**< Callback is executing */
+};
+
+/**
+ * Register a callback function for specific port id.
+ *
+ * @param cbp
+ *  Pointer to the ret_dev_cb_list structure.
+ * @param lock
+ *  Pointer to the rte_spinlock_t structure.
+ * @param event
+ *  Event interested.
+ * @param cb_fn
+ *  User supplied callback function to be called.
+ * @param cb_arg
+ *  Pointer to the parameters for the registered callback.
+ *
+ * @return
+ *  - On success, zero.
+ *  - On failure, a negative value.
+ */
+int rte_dev_callback_register(struct rte_dev_cb_list * cbp, rte_spinlock_t * lock,
+		enum rte_dev_event_type event,
+		rte_dev_cb_fn cb_fn, void *cb_arg);
+
+/**
+ * Unregister a callback function for specific port id.
+ *
+ * @param cb_list
+ *  Pointer to the ret_dev_cb_list structure.
+ * @param lock
+ *  Pointer to the rte_spinlock_t structure.
+ * @param event
+ *  Event interested.
+ * @param cb_fn
+ *  User supplied callback function to be called.
+ * @param cb_arg
+ *  Pointer to the parameters for the registered callback. -1 means to
+ *  remove all for the same callback address and same event.
+ *
+ * @return
+ *  - On success, zero.
+ *  - On failure, a negative value.
+ */
+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);
+
+/**
+ * @internal Executes all the user application registered callbacks for
+ * the specific device. It is for DPDK internal user only. User
+ * application should not call it directly.
+ *
+ * @param cb_list
+ *  Pointer to struct rte_dev_cb_list.
+ * @param port_id
+ *  Port_id or unit_id value.
+ * @param event
+ *  Eth device interrupt event type.
+ * @param lock
+ *  Pointer to the lock structure.
+ *
+ * @return
+ *  void
+ */
+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);
+
+/**
+ * Add a callback to be called on packet RX or TX on a given port and queue.
+ *
+ * This API configures a function to be called for each burst of
+ * packets received on a given NIC port queue. The return value is a pointer
+ * that can be used to later remove the callback using
+ * rte_dev_remove_callback().
+ *
+ * @param cbp
+ *  Pointer to a pointer of the ret_dev_cb_list structure.
+ * @param fn
+ *   The callback function
+ * @param user_param
+ *   A generic pointer parameter which will be passed to each invocation of the
+ *   callback function on this port and queue.
+ *
+ * @return
+ *   NULL on error.
+ *   On success, a pointer value which can later be used to remove the callback.
+ */
+void *rte_dev_add_callback(struct rte_dev_rxtx_callback ** cbp,
+		void * fn, void *user_param);
+
+/**
+ * Remove an RX or TX packet callback from a given port and queue.
+ *
+ * This function is used to removed a callback that were added to a NIC port
+ * queue using rte_dev_add_callback().
+ *
+ * Note: the callback is removed from the callback list but it isn't freed
+ * since the it may still be in use. The memory for the callback can be
+ * subsequently freed back by the application by calling rte_free():
+ *
+ *  - Immediately - if the port is stopped, or the user knows that no
+ *    callback's are in flight e.g. if called from the thread doing RX/TX
+ *    on that queue.
+ *
+ * - After a short delay - where the delay is sufficient to allow any
+ *   in-flight callback's to complete.
+ *
+ * @param cbp
+ *   Pointer to a pointer to use as the head of the list.
+ * @param user_cb
+ *   User supplied callback created via rte_dev_add_callback().
+ *
+ * @return
+ *   - 0: Success. Callback was removed.
+ *   - -ENOTSUP: Callback support is not available.
+ *   - -EINVAL:  The port_id or the queue_id is out of range, or the callback
+ *               is NULL or not found for the port/queue.
+ */
+int rte_dev_remove_callback(struct rte_dev_rxtx_callback ** cbp,
+		struct rte_dev_rxtx_callback *user_cb);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_COMMON_DEVICE_H_ */