[dpdk-dev,V16,2/4] eal: add device event monitor framework

Message ID 1522063256-3997-3-git-send-email-jia.guo@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

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

Commit Message

Guo, Jia March 26, 2018, 11:20 a.m. UTC
  This patch aims to add a general device event monitor mechanism at
EAL device layer, for device hotplug awareness and actions adopted
accordingly. It could also expand for all other type of device event
monitor, but not in this scope at the stage.

To get started, users firstly register or unregister callbacks through
the new added APIs. Callbacks can be some device specific, or for all
devices.
  -rte_dev_callback_register
  -rte_dev_callback_unregister

Then application shall call below new added APIs to enable/disable the
mechanism:
  - rte_dev_event_monitor_start
  - rte_dev_event_monitor_stop

Use hotplug case for example, when device hotplug insertion or hotplug
removal, we will get notified from kernel, then call user's callbacks
accordingly to handle it, such as detach or attach the device from the
bus, and could be benifit for futher fail-safe or live-migration.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v16->v15:
1.remove some linux related code out of eal common layer
2.fix some uneasy readble issue.
---
 lib/librte_eal/bsdapp/eal/Makefile      |   1 +
 lib/librte_eal/bsdapp/eal/eal_dev.c     |  19 +++++
 lib/librte_eal/common/eal_common_dev.c  | 145 ++++++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_private.h     |  24 ++++++
 lib/librte_eal/common/include/rte_dev.h |  92 ++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/Makefile    |   1 +
 lib/librte_eal/linuxapp/eal/eal_dev.c   |  20 +++++
 7 files changed, 302 insertions(+)
 create mode 100644 lib/librte_eal/bsdapp/eal/eal_dev.c
 create mode 100644 lib/librte_eal/linuxapp/eal/eal_dev.c
  

Comments

Jianfeng Tan March 28, 2018, 3:39 a.m. UTC | #1
> -----Original Message-----
> From: Guo, Jia
> Sent: Monday, March 26, 2018 7:21 PM
> To: stephen@networkplumber.org; Richardson, Bruce; Yigit, Ferruh;
> Ananyev, Konstantin; gaetan.rivet@6wind.com; Wu, Jingjing;
> thomas@monjalon.net; motih@mellanox.com; Van Haaren, Harry; Tan,
> Jianfeng
> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo,
> Jia; Zhang, Helin
> Subject: [PATCH V16 2/4] eal: add device event monitor framework
> 
> This patch aims to add a general device event monitor mechanism at

mechanism -> framework?
Linux uevent is a mechanism.

> EAL device layer, for device hotplug awareness and actions adopted
> accordingly. It could also expand for all other type of device event
> monitor, but not in this scope at the stage.
> 
> To get started, users firstly register or unregister callbacks through
> the new added APIs. Callbacks can be some device specific, or for all
> devices.
>   -rte_dev_callback_register
>   -rte_dev_callback_unregister
> 

New APIs shall be added into rte_eal_version.map.

And also, we shall update the release note.

> Then application shall call below new added APIs to enable/disable the
> mechanism:
>   - rte_dev_event_monitor_start
>   - rte_dev_event_monitor_stop

Do we really have the use case to keep the callbacks, but stop monitoring? I don't think we really need these two APIs to enable/disable.

Instead, if we have a callback registered, then enable it; if we don't have any callbacks, then it's definitely disabled. 

> 
> Use hotplug case for example, when device hotplug insertion or hotplug
> removal, we will get notified from kernel, then call user's callbacks
> accordingly to handle it, such as detach or attach the device from the
> bus, and could be benifit for futher fail-safe or live-migration.

Typo: "be benifit " -> "benefit"
Typo: " futher" -> "further"

> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
> v16->v15:
> 1.remove some linux related code out of eal common layer
> 2.fix some uneasy readble issue.
> ---
>  lib/librte_eal/bsdapp/eal/Makefile      |   1 +
>  lib/librte_eal/bsdapp/eal/eal_dev.c     |  19 +++++
>  lib/librte_eal/common/eal_common_dev.c  | 145
> ++++++++++++++++++++++++++++++++
>  lib/librte_eal/common/eal_private.h     |  24 ++++++
>  lib/librte_eal/common/include/rte_dev.h |  92 ++++++++++++++++++++
>  lib/librte_eal/linuxapp/eal/Makefile    |   1 +
>  lib/librte_eal/linuxapp/eal/eal_dev.c   |  20 +++++
>  7 files changed, 302 insertions(+)
>  create mode 100644 lib/librte_eal/bsdapp/eal/eal_dev.c
>  create mode 100644 lib/librte_eal/linuxapp/eal/eal_dev.c
> 
> diff --git a/lib/librte_eal/bsdapp/eal/Makefile
> b/lib/librte_eal/bsdapp/eal/Makefile
> index dd455e6..c0921dd 100644
> --- a/lib/librte_eal/bsdapp/eal/Makefile
> +++ b/lib/librte_eal/bsdapp/eal/Makefile
> @@ -33,6 +33,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) +=
> eal_lcore.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_timer.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_interrupts.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_alarm.c
> +SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_dev.c
> 
>  # from common dir
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_lcore.c
> diff --git a/lib/librte_eal/bsdapp/eal/eal_dev.c
> b/lib/librte_eal/bsdapp/eal/eal_dev.c
> new file mode 100644
> index 0000000..ad606b3
> --- /dev/null
> +++ b/lib/librte_eal/bsdapp/eal/eal_dev.c
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#include <rte_log.h>
> +
> +int __rte_experimental
> +rte_dev_event_monitor_start(void)
> +{
> +	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
> +	return -1;
> +}
> +
> +int __rte_experimental
> +rte_dev_event_monitor_stop(void)
> +{
> +	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
> +	return -1;
> +}
> diff --git a/lib/librte_eal/common/eal_common_dev.c
> b/lib/librte_eal/common/eal_common_dev.c
> index cd07144..3a1bbb6 100644
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -14,9 +14,34 @@
>  #include <rte_devargs.h>
>  #include <rte_debug.h>
>  #include <rte_log.h>
> +#include <rte_spinlock.h>
> +#include <rte_malloc.h>
> 
>  #include "eal_private.h"
> 
> +/* spinlock for device callbacks */
> +static rte_spinlock_t dev_event_lock = RTE_SPINLOCK_INITIALIZER;
> +
> +/**
> + * The device event callback description.
> + *
> + * It contains callback address to be registered by user application,
> + * the pointer to the parameters for callback, and the device name.
> + */
> +struct dev_event_callback {
> +	TAILQ_ENTRY(dev_event_callback) next; /**< Callbacks list */
> +	rte_dev_event_cb_fn cb_fn;                /**< Callback address */
> +	void *cb_arg;                           /**< Callback parameter */
> +	char *dev_name;	 /**< Callback devcie name, NULL is for all
> device */

Typo: " devcie" -> "device" 

> +	uint32_t active;                        /**< Callback is executing */
> +};
> +
> +/** @internal Structure to keep track of registered callbacks */
> +TAILQ_HEAD(dev_event_cb_list, dev_event_callback);
> +
> +/* The device event callback list for all registered callbacks. */
> +static struct dev_event_cb_list dev_event_cbs;
> +
>  static int cmp_detached_dev_name(const struct rte_device *dev,
>  	const void *_name)
>  {
> @@ -207,3 +232,123 @@ rte_eal_hotplug_remove(const char *busname,
> const char *devname)
>  	rte_eal_devargs_remove(busname, devname);
>  	return ret;
>  }
> +
> +static struct dev_event_callback * __rte_experimental

We don't have to flag an internal function as " __rte_experimental ".

> +dev_event_cb_find(const char *device_name, rte_dev_event_cb_fn cb_fn,
> +				void *cb_arg)
> +{
> +	struct dev_event_callback *event_cb = NULL;
> +
> +	TAILQ_FOREACH(event_cb, &(dev_event_cbs), next) {
> +		if (event_cb->cb_fn == cb_fn && event_cb->cb_arg == cb_arg) {
> +			if (device_name == NULL && event_cb->dev_name == NULL)
> +				break;
> +			if (device_name == NULL || event_cb->dev_name == NULL)
> +				continue;
> +			if (!strcmp(event_cb->dev_name, device_name))
> +				break;
> +		}
> +	}
> +	return event_cb;
> +}
> +
> +int __rte_experimental
> +rte_dev_callback_register(const char *device_name, rte_dev_event_cb_fn cb_fn,
> +				void *cb_arg)

"rte_dev_event_callback_register" sounds more reasonable?

> +{
> +	struct dev_event_callback *event_cb = NULL;

We don't need to initialize it to NULL.

> +
> +	if (!cb_fn)
> +		return -EINVAL;
> +
> +	rte_spinlock_lock(&dev_event_lock);
> +
> +	if (TAILQ_EMPTY(&(dev_event_cbs)))
> +		TAILQ_INIT(&(dev_event_cbs));
> +
> +	event_cb = dev_event_cb_find(device_name, cb_fn, cb_arg);
> +
> +	/* create a new callback. */
> +	if (event_cb == NULL) {
> +		event_cb = malloc(sizeof(struct dev_event_callback));
> +		if (event_cb != NULL) {
> +			event_cb->cb_fn = cb_fn;
> +			event_cb->cb_arg = cb_arg;
> +			event_cb->dev_name = strdup(device_name);
> +			if (event_cb->dev_name == NULL)
> +				return -EINVAL;
> +			TAILQ_INSERT_TAIL(&(dev_event_cbs), event_cb, next);
> +		} else {
> +			RTE_LOG(ERR, EAL,
> +				"Failed to allocate memory for device event callback");

Miss the unlock here.

> +			return -ENOMEM;
> +		}
> +	}
> +
> +	rte_spinlock_unlock(&dev_event_lock);
> +	return (event_cb == NULL) ? -EEXIST : 0;
> +}
> +
> +int __rte_experimental
> +rte_dev_callback_unregister(const char *device_name,
> rte_dev_event_cb_fn cb_fn,
> +				void *cb_arg)
> +{
> +	int ret = -1;
> +	struct dev_event_callback *event_cb = NULL;
> +
> +	if (!cb_fn)
> +		return -EINVAL;
> +
> +	rte_spinlock_lock(&dev_event_lock);
> +
> +	event_cb = dev_event_cb_find(device_name, cb_fn, cb_arg);
> +
> +	/*
> +	 * if this callback is not executing right now,
> +	 * then remove it.
> +	 */

This note is not in right place.

> +	if (event_cb != NULL) {
> +		if (event_cb->active == 0) {
> +			TAILQ_REMOVE(&(dev_event_cbs), event_cb, next);
> +			rte_free(event_cb);
> +			ret = 0;
> +		}
> +		ret = -EBUSY;

Miss "else" for busy cb.

> +	}

Miss "else" for a cb which is not existed. And print an error log here.

> +
> +	rte_spinlock_unlock(&dev_event_lock);
> +	return ret;
> +}
> +
> +int __rte_experimental
> +dev_callback_process(char *device_name, enum rte_dev_event_type event,
> +				void *cb_arg)

From interrupt thread, there is no cb_arg.

> +{
> +	struct dev_event_callback *cb_lst;
> +	int rc = 0;
> +
> +	rte_spinlock_lock(&dev_event_lock);
> +
> +	if (device_name == NULL)
> +		return -EINVAL;

Put such check out of lock. Or it's very easy to miss the unlock which is happening now.

> +
> +	TAILQ_FOREACH(cb_lst, &dev_event_cbs, next) {
> +		if (!cb_lst->dev_name)
> +			break;
> +		else if (!strcmp(cb_lst->dev_name, device_name))
> +			break;
> +	}

We invoke only one callback. But for this device, we could have many callback to call.

> +	if (cb_lst) {
> +		cb_lst->active = 1;
> +		if (cb_arg)
> +			cb_lst->cb_arg = cb_arg;

What's the reason of overwriting this cb_arg?

> +		rte_spinlock_unlock(&dev_event_lock);
> +		rc = cb_lst->cb_fn(device_name, event,
> +				cb_lst->cb_arg);
> +		rte_spinlock_lock(&dev_event_lock);
> +		cb_lst->active = 0;
> +	}
> +
> +	rte_spinlock_unlock(&dev_event_lock);
> +	return rc;

What's the reason of returning the ret of a callback? I don't think we need to return anything here.

> +}
> diff --git a/lib/librte_eal/common/eal_private.h
> b/lib/librte_eal/common/eal_private.h
> index 0b28770..d55cd68 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -9,6 +9,8 @@
>  #include <stdint.h>
>  #include <stdio.h>
> 
> +#include <rte_dev.h>
> +
>  /**
>   * Initialize the memzone subsystem (private to eal).
>   *
> @@ -205,4 +207,26 @@ struct rte_bus
> *rte_bus_find_by_device_name(const char *str);
> 
>  int rte_mp_channel_init(void);
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice

It's not an API. We don't need this.

> + *
> + * 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 device_name
> + *  The device name.
> + * @param event
> + *  the device event type
> + * @param cb_arg
> + *  callback parameter.
> + *
> + * @return
> + *  - On success, return zero.
> + *  - On failure, a negative value.
> + */
> +int __rte_experimental

It's not an API, so don't add "__rte_experimental" flag.

> +dev_callback_process(char *device_name, enum rte_dev_event_type
> event,
> +				void *cb_arg);

As mentioned above, we don't have cb_arg from interrupt thread.

>  #endif /* _EAL_PRIVATE_H_ */
> diff --git a/lib/librte_eal/common/include/rte_dev.h
> b/lib/librte_eal/common/include/rte_dev.h
> index b688f1e..8867de6 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -24,6 +24,26 @@ extern "C" {
>  #include <rte_compat.h>
>  #include <rte_log.h>
> 
> +/**
> + * The device event type.
> + */
> +enum rte_dev_event_type {
> +	RTE_DEV_EVENT_UNKNOWN,	/**< unknown event type */

Again, why do we report an "unknown" event to applications?

> +	RTE_DEV_EVENT_ADD,	/**< device being added */
> +	RTE_DEV_EVENT_REMOVE,	/**< device being removed */
> +	RTE_DEV_EVENT_MAX	/**< max value of this enum */
> +};
> +
> +struct rte_dev_event {
> +	enum rte_dev_event_type type;	/**< device event type */
> +	int subsystem;			/**< subsystem id */
> +	char *devname;			/**< device name */

I prefer to remove such note if we can already get the information from the variable name.

> +};
> +
> +typedef int (*rte_dev_event_cb_fn)(char *device_name,
> +					enum rte_dev_event_type event,
> +					void *cb_arg);

"rte_dev_event_callback" sounds better than "rte_dev_event_cb_fn" to me.

> +
>  __attribute__((format(printf, 2, 0)))
>  static inline void
>  rte_pmd_debug_trace(const char *func_name, const char *fmt, ...)
> @@ -267,4 +287,76 @@ __attribute__((used)) = str
>  }
>  #endif
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * It registers the callback for the specific device.

"the" -> "a"

Besides, "It registers a callback for the specific device or all devices"

> + * Multiple callbacks cal be registered at the same time.

"cal" -> "can"

Besides, above sentence sounds like this API can register multiple callbacks. Change to:
"Users can call this API multiple times to register multiple callbacks."

> + *
> + * @param device_name
> + *  The device name, that is the param name of the struct rte_device,
> + *  null value means for all devices.
> + * @param cb_fn
> + *  callback address.
> + * @param cb_arg
> + *  address of parameter for callback.
> + *
> + * @return
> + *  - On success, zero.
> + *  - On failure, a negative value.
> + */
> +int __rte_experimental
> +rte_dev_callback_register(const char *device_name,
> rte_dev_event_cb_fn cb_fn,
> +			void *cb_arg);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * It unregisters the callback according to the specified device.
> + *
> + * @param device_name
> + *  The device name, that is the param name of the struct rte_device,
> + *  null value means for all devices.
> + * @param cb_fn
> + *  callback address.
> + * @param cb_arg
> + *  address of parameter for callback.
> + *
> + * @return
> + *  - On success, return the number of callback entities removed.
> + *  - On failure, a negative value.
> + */
> +int __rte_experimental
> +rte_dev_callback_unregister(const char *device_name,
> rte_dev_event_cb_fn cb_fn,
> +					void *cb_arg);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Start the device event monitoring.
> + *
> + * @param none
> + * @return
> + *   - On success, zero.
> + *   - On failure, a negative value.
> + */
> +int __rte_experimental
> +rte_dev_event_monitor_start(void);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Stop the device event monitoring .
> + *
> + * @param none
> + * @return
> + *   - On success, zero.
> + *   - On failure, a negative value.
> + */
> +int __rte_experimental
> +rte_dev_event_monitor_stop(void);
>  #endif /* _RTE_DEV_H_ */
> diff --git a/lib/librte_eal/linuxapp/eal/Makefile
> b/lib/librte_eal/linuxapp/eal/Makefile
> index 7e5bbe8..8578796 100644
> --- a/lib/librte_eal/linuxapp/eal/Makefile
> +++ b/lib/librte_eal/linuxapp/eal/Makefile
> @@ -41,6 +41,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) +=
> eal_lcore.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_timer.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_interrupts.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_alarm.c
> +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_dev.c
> 
>  # from common dir
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_lcore.c
> diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c
> b/lib/librte_eal/linuxapp/eal/eal_dev.c
> new file mode 100644
> index 0000000..5ab5830
> --- /dev/null
> +++ b/lib/librte_eal/linuxapp/eal/eal_dev.c
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#include <rte_log.h>
> +#include <rte_dev.h>
> +
> +int __rte_experimental
> +rte_dev_event_monitor_start(void)
> +{
> +	/* TODO: start uevent monitor for linux */
> +	return 0;
> +}
> +
> +int __rte_experimental
> +rte_dev_event_monitor_stop(void)
> +{
> +	/* TODO: stop uevent monitor for linux */
> +	return 0;
> +}
> --
> 2.7.4
  
Guo, Jia March 28, 2018, 8:12 a.m. UTC | #2
jianfeng

will correct every typo, and comment inline.

On 3/28/2018 11:39 AM, Tan, Jianfeng wrote:
>
>> -----Original Message-----
>> From: Guo, Jia
>> Sent: Monday, March 26, 2018 7:21 PM
>> To: stephen@networkplumber.org; Richardson, Bruce; Yigit, Ferruh;
>> Ananyev, Konstantin; gaetan.rivet@6wind.com; Wu, Jingjing;
>> thomas@monjalon.net; motih@mellanox.com; Van Haaren, Harry; Tan,
>> Jianfeng
>> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo,
>> Jia; Zhang, Helin
>> Subject: [PATCH V16 2/4] eal: add device event monitor framework
>>
>> This patch aims to add a general device event monitor mechanism at
> mechanism -> framework?
> Linux uevent is a mechanism.
>> EAL device layer, for device hotplug awareness and actions adopted
>> accordingly. It could also expand for all other type of device event
>> monitor, but not in this scope at the stage.
>>
>> To get started, users firstly register or unregister callbacks through
>> the new added APIs. Callbacks can be some device specific, or for all
>> devices.
>>    -rte_dev_callback_register
>>    -rte_dev_callback_unregister
>>
> New APIs shall be added into rte_eal_version.map.
>
> And also, we shall update the release note.
>
>> Then application shall call below new added APIs to enable/disable the
>> mechanism:
>>    - rte_dev_event_monitor_start
>>    - rte_dev_event_monitor_stop
> Do we really have the use case to keep the callbacks, but stop monitoring? I don't think we really need these two APIs to enable/disable.
>
> Instead, if we have a callback registered, then enable it; if we don't have any callbacks, then it's definitely disabled.
you raise a good question, but if it is good readable to enable the 
monitor into the register function or let register into the enable 
function, should let we think about that.
>> Use hotplug case for example, when device hotplug insertion or hotplug
>> removal, we will get notified from kernel, then call user's callbacks
>> accordingly to handle it, such as detach or attach the device from the
>> bus, and could be benifit for futher fail-safe or live-migration.
> Typo: "be benifit " -> "benefit"
> Typo: " futher" -> "further"
>
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> ---
>> v16->v15:
>> 1.remove some linux related code out of eal common layer
>> 2.fix some uneasy readble issue.
>> ---
>>   lib/librte_eal/bsdapp/eal/Makefile      |   1 +
>>   lib/librte_eal/bsdapp/eal/eal_dev.c     |  19 +++++
>>   lib/librte_eal/common/eal_common_dev.c  | 145
>> ++++++++++++++++++++++++++++++++
>>   lib/librte_eal/common/eal_private.h     |  24 ++++++
>>   lib/librte_eal/common/include/rte_dev.h |  92 ++++++++++++++++++++
>>   lib/librte_eal/linuxapp/eal/Makefile    |   1 +
>>   lib/librte_eal/linuxapp/eal/eal_dev.c   |  20 +++++
>>   7 files changed, 302 insertions(+)
>>   create mode 100644 lib/librte_eal/bsdapp/eal/eal_dev.c
>>   create mode 100644 lib/librte_eal/linuxapp/eal/eal_dev.c
>>
>> diff --git a/lib/librte_eal/bsdapp/eal/Makefile
>> b/lib/librte_eal/bsdapp/eal/Makefile
>> index dd455e6..c0921dd 100644
>> --- a/lib/librte_eal/bsdapp/eal/Makefile
>> +++ b/lib/librte_eal/bsdapp/eal/Makefile
>> @@ -33,6 +33,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) +=
>> eal_lcore.c
>>   SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_timer.c
>>   SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_interrupts.c
>>   SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_alarm.c
>> +SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_dev.c
>>
>>   # from common dir
>>   SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_lcore.c
>> diff --git a/lib/librte_eal/bsdapp/eal/eal_dev.c
>> b/lib/librte_eal/bsdapp/eal/eal_dev.c
>> new file mode 100644
>> index 0000000..ad606b3
>> --- /dev/null
>> +++ b/lib/librte_eal/bsdapp/eal/eal_dev.c
>> @@ -0,0 +1,19 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause
>> + * Copyright(c) 2018 Intel Corporation
>> + */
>> +
>> +#include <rte_log.h>
>> +
>> +int __rte_experimental
>> +rte_dev_event_monitor_start(void)
>> +{
>> +	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
>> +	return -1;
>> +}
>> +
>> +int __rte_experimental
>> +rte_dev_event_monitor_stop(void)
>> +{
>> +	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
>> +	return -1;
>> +}
>> diff --git a/lib/librte_eal/common/eal_common_dev.c
>> b/lib/librte_eal/common/eal_common_dev.c
>> index cd07144..3a1bbb6 100644
>> --- a/lib/librte_eal/common/eal_common_dev.c
>> +++ b/lib/librte_eal/common/eal_common_dev.c
>> @@ -14,9 +14,34 @@
>>   #include <rte_devargs.h>
>>   #include <rte_debug.h>
>>   #include <rte_log.h>
>> +#include <rte_spinlock.h>
>> +#include <rte_malloc.h>
>>
>>   #include "eal_private.h"
>>
>> +/* spinlock for device callbacks */
>> +static rte_spinlock_t dev_event_lock = RTE_SPINLOCK_INITIALIZER;
>> +
>> +/**
>> + * The device event callback description.
>> + *
>> + * It contains callback address to be registered by user application,
>> + * the pointer to the parameters for callback, and the device name.
>> + */
>> +struct dev_event_callback {
>> +	TAILQ_ENTRY(dev_event_callback) next; /**< Callbacks list */
>> +	rte_dev_event_cb_fn cb_fn;                /**< Callback address */
>> +	void *cb_arg;                           /**< Callback parameter */
>> +	char *dev_name;	 /**< Callback devcie name, NULL is for all
>> device */
> Typo: " devcie" -> "device"
>
>> +	uint32_t active;                        /**< Callback is executing */
>> +};
>> +
>> +/** @internal Structure to keep track of registered callbacks */
>> +TAILQ_HEAD(dev_event_cb_list, dev_event_callback);
>> +
>> +/* The device event callback list for all registered callbacks. */
>> +static struct dev_event_cb_list dev_event_cbs;
>> +
>>   static int cmp_detached_dev_name(const struct rte_device *dev,
>>   	const void *_name)
>>   {
>> @@ -207,3 +232,123 @@ rte_eal_hotplug_remove(const char *busname,
>> const char *devname)
>>   	rte_eal_devargs_remove(busname, devname);
>>   	return ret;
>>   }
>> +
>> +static struct dev_event_callback * __rte_experimental
> We don't have to flag an internal function as " __rte_experimental ".
>
>> +dev_event_cb_find(const char *device_name, rte_dev_event_cb_fn cb_fn,
>> +				void *cb_arg)
>> +{
>> +	struct dev_event_callback *event_cb = NULL;
>> +
>> +	TAILQ_FOREACH(event_cb, &(dev_event_cbs), next) {
>> +		if (event_cb->cb_fn == cb_fn && event_cb->cb_arg == cb_arg) {
>> +			if (device_name == NULL && event_cb->dev_name == NULL)
>> +				break;
>> +			if (device_name == NULL || event_cb->dev_name == NULL)
>> +				continue;
>> +			if (!strcmp(event_cb->dev_name, device_name))
>> +				break;
>> +		}
>> +	}
>> +	return event_cb;
>> +}
>> +
>> +int __rte_experimental
>> +rte_dev_callback_register(const char *device_name, rte_dev_event_cb_fn cb_fn,
>> +				void *cb_arg)
> "rte_dev_event_callback_register" sounds more reasonable?
>
>> +{
>> +	struct dev_event_callback *event_cb = NULL;
> We don't need to initialize it to NULL.
>
>> +
>> +	if (!cb_fn)
>> +		return -EINVAL;
>> +
>> +	rte_spinlock_lock(&dev_event_lock);
>> +
>> +	if (TAILQ_EMPTY(&(dev_event_cbs)))
>> +		TAILQ_INIT(&(dev_event_cbs));
>> +
>> +	event_cb = dev_event_cb_find(device_name, cb_fn, cb_arg);
>> +
>> +	/* create a new callback. */
>> +	if (event_cb == NULL) {
>> +		event_cb = malloc(sizeof(struct dev_event_callback));
>> +		if (event_cb != NULL) {
>> +			event_cb->cb_fn = cb_fn;
>> +			event_cb->cb_arg = cb_arg;
>> +			event_cb->dev_name = strdup(device_name);
>> +			if (event_cb->dev_name == NULL)
>> +				return -EINVAL;
>> +			TAILQ_INSERT_TAIL(&(dev_event_cbs), event_cb, next);
>> +		} else {
>> +			RTE_LOG(ERR, EAL,
>> +				"Failed to allocate memory for device event callback");
> Miss the unlock here.
>
>> +			return -ENOMEM;
>> +		}
>> +	}
>> +
>> +	rte_spinlock_unlock(&dev_event_lock);
>> +	return (event_cb == NULL) ? -EEXIST : 0;
>> +}
>> +
>> +int __rte_experimental
>> +rte_dev_callback_unregister(const char *device_name,
>> rte_dev_event_cb_fn cb_fn,
>> +				void *cb_arg)
>> +{
>> +	int ret = -1;
>> +	struct dev_event_callback *event_cb = NULL;
>> +
>> +	if (!cb_fn)
>> +		return -EINVAL;
>> +
>> +	rte_spinlock_lock(&dev_event_lock);
>> +
>> +	event_cb = dev_event_cb_find(device_name, cb_fn, cb_arg);
>> +
>> +	/*
>> +	 * if this callback is not executing right now,
>> +	 * then remove it.
>> +	 */
> This note is not in right place.
>
>> +	if (event_cb != NULL) {
>> +		if (event_cb->active == 0) {
>> +			TAILQ_REMOVE(&(dev_event_cbs), event_cb, next);
>> +			rte_free(event_cb);
>> +			ret = 0;
>> +		}
>> +		ret = -EBUSY;
> Miss "else" for busy cb.
>
>> +	}
> Miss "else" for a cb which is not existed. And print an error log here.
>
>> +
>> +	rte_spinlock_unlock(&dev_event_lock);
>> +	return ret;
>> +}
>> +
>> +int __rte_experimental
>> +dev_callback_process(char *device_name, enum rte_dev_event_type event,
>> +				void *cb_arg)
>  From interrupt thread, there is no cb_arg.
>> +{
>> +	struct dev_event_callback *cb_lst;
>> +	int rc = 0;
>> +
>> +	rte_spinlock_lock(&dev_event_lock);
>> +
>> +	if (device_name == NULL)
>> +		return -EINVAL;
> Put such check out of lock. Or it's very easy to miss the unlock which is happening now.
>
>> +
>> +	TAILQ_FOREACH(cb_lst, &dev_event_cbs, next) {
>> +		if (!cb_lst->dev_name)
>> +			break;
>> +		else if (!strcmp(cb_lst->dev_name, device_name))
>> +			break;
>> +	}
> We invoke only one callback. But for this device, we could have many callback to call.
>
>> +	if (cb_lst) {
>> +		cb_lst->active = 1;
>> +		if (cb_arg)
>> +			cb_lst->cb_arg = cb_arg;
> What's the reason of overwriting this cb_arg?
it is no used anymore here.
>> +		rte_spinlock_unlock(&dev_event_lock);
>> +		rc = cb_lst->cb_fn(device_name, event,
>> +				cb_lst->cb_arg);
>> +		rte_spinlock_lock(&dev_event_lock);
>> +		cb_lst->active = 0;
>> +	}
>> +
>> +	rte_spinlock_unlock(&dev_event_lock);
>> +	return rc;
> What's the reason of returning the ret of a callback? I don't think we need to return anything here.
>
>> +}
>> diff --git a/lib/librte_eal/common/eal_private.h
>> b/lib/librte_eal/common/eal_private.h
>> index 0b28770..d55cd68 100644
>> --- a/lib/librte_eal/common/eal_private.h
>> +++ b/lib/librte_eal/common/eal_private.h
>> @@ -9,6 +9,8 @@
>>   #include <stdint.h>
>>   #include <stdio.h>
>>
>> +#include <rte_dev.h>
>> +
>>   /**
>>    * Initialize the memzone subsystem (private to eal).
>>    *
>> @@ -205,4 +207,26 @@ struct rte_bus
>> *rte_bus_find_by_device_name(const char *str);
>>
>>   int rte_mp_channel_init(void);
>>
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice
> It's not an API. We don't need this.
>
>> + *
>> + * 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 device_name
>> + *  The device name.
>> + * @param event
>> + *  the device event type
>> + * @param cb_arg
>> + *  callback parameter.
>> + *
>> + * @return
>> + *  - On success, return zero.
>> + *  - On failure, a negative value.
>> + */
>> +int __rte_experimental
> It's not an API, so don't add "__rte_experimental" flag.
>
>> +dev_callback_process(char *device_name, enum rte_dev_event_type
>> event,
>> +				void *cb_arg);
> As mentioned above, we don't have cb_arg from interrupt thread.
>
>>   #endif /* _EAL_PRIVATE_H_ */
>> diff --git a/lib/librte_eal/common/include/rte_dev.h
>> b/lib/librte_eal/common/include/rte_dev.h
>> index b688f1e..8867de6 100644
>> --- a/lib/librte_eal/common/include/rte_dev.h
>> +++ b/lib/librte_eal/common/include/rte_dev.h
>> @@ -24,6 +24,26 @@ extern "C" {
>>   #include <rte_compat.h>
>>   #include <rte_log.h>
>>
>> +/**
>> + * The device event type.
>> + */
>> +enum rte_dev_event_type {
>> +	RTE_DEV_EVENT_UNKNOWN,	/**< unknown event type */
> Again, why do we report an "unknown" event to applications?
>
>> +	RTE_DEV_EVENT_ADD,	/**< device being added */
>> +	RTE_DEV_EVENT_REMOVE,	/**< device being removed */
>> +	RTE_DEV_EVENT_MAX	/**< max value of this enum */
>> +};
>> +
>> +struct rte_dev_event {
>> +	enum rte_dev_event_type type;	/**< device event type */
>> +	int subsystem;			/**< subsystem id */
>> +	char *devname;			/**< device name */
> I prefer to remove such note if we can already get the information from the variable name.
>
>> +};
>> +
>> +typedef int (*rte_dev_event_cb_fn)(char *device_name,
>> +					enum rte_dev_event_type event,
>> +					void *cb_arg);
> "rte_dev_event_callback" sounds better than "rte_dev_event_cb_fn" to me.
>
>> +
>>   __attribute__((format(printf, 2, 0)))
>>   static inline void
>>   rte_pmd_debug_trace(const char *func_name, const char *fmt, ...)
>> @@ -267,4 +287,76 @@ __attribute__((used)) = str
>>   }
>>   #endif
>>
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice
>> + *
>> + * It registers the callback for the specific device.
> "the" -> "a"
>
> Besides, "It registers a callback for the specific device or all devices"
>
>> + * Multiple callbacks cal be registered at the same time.
> "cal" -> "can"
>
> Besides, above sentence sounds like this API can register multiple callbacks. Change to:
> "Users can call this API multiple times to register multiple callbacks."
>
>> + *
>> + * @param device_name
>> + *  The device name, that is the param name of the struct rte_device,
>> + *  null value means for all devices.
>> + * @param cb_fn
>> + *  callback address.
>> + * @param cb_arg
>> + *  address of parameter for callback.
>> + *
>> + * @return
>> + *  - On success, zero.
>> + *  - On failure, a negative value.
>> + */
>> +int __rte_experimental
>> +rte_dev_callback_register(const char *device_name,
>> rte_dev_event_cb_fn cb_fn,
>> +			void *cb_arg);
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice
>> + *
>> + * It unregisters the callback according to the specified device.
>> + *
>> + * @param device_name
>> + *  The device name, that is the param name of the struct rte_device,
>> + *  null value means for all devices.
>> + * @param cb_fn
>> + *  callback address.
>> + * @param cb_arg
>> + *  address of parameter for callback.
>> + *
>> + * @return
>> + *  - On success, return the number of callback entities removed.
>> + *  - On failure, a negative value.
>> + */
>> +int __rte_experimental
>> +rte_dev_callback_unregister(const char *device_name,
>> rte_dev_event_cb_fn cb_fn,
>> +					void *cb_arg);
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice
>> + *
>> + * Start the device event monitoring.
>> + *
>> + * @param none
>> + * @return
>> + *   - On success, zero.
>> + *   - On failure, a negative value.
>> + */
>> +int __rte_experimental
>> +rte_dev_event_monitor_start(void);
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice
>> + *
>> + * Stop the device event monitoring .
>> + *
>> + * @param none
>> + * @return
>> + *   - On success, zero.
>> + *   - On failure, a negative value.
>> + */
>> +int __rte_experimental
>> +rte_dev_event_monitor_stop(void);
>>   #endif /* _RTE_DEV_H_ */
>> diff --git a/lib/librte_eal/linuxapp/eal/Makefile
>> b/lib/librte_eal/linuxapp/eal/Makefile
>> index 7e5bbe8..8578796 100644
>> --- a/lib/librte_eal/linuxapp/eal/Makefile
>> +++ b/lib/librte_eal/linuxapp/eal/Makefile
>> @@ -41,6 +41,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) +=
>> eal_lcore.c
>>   SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_timer.c
>>   SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_interrupts.c
>>   SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_alarm.c
>> +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_dev.c
>>
>>   # from common dir
>>   SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_lcore.c
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c
>> b/lib/librte_eal/linuxapp/eal/eal_dev.c
>> new file mode 100644
>> index 0000000..5ab5830
>> --- /dev/null
>> +++ b/lib/librte_eal/linuxapp/eal/eal_dev.c
>> @@ -0,0 +1,20 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause
>> + * Copyright(c) 2018 Intel Corporation
>> + */
>> +
>> +#include <rte_log.h>
>> +#include <rte_dev.h>
>> +
>> +int __rte_experimental
>> +rte_dev_event_monitor_start(void)
>> +{
>> +	/* TODO: start uevent monitor for linux */
>> +	return 0;
>> +}
>> +
>> +int __rte_experimental
>> +rte_dev_event_monitor_stop(void)
>> +{
>> +	/* TODO: stop uevent monitor for linux */
>> +	return 0;
>> +}
>> --
>> 2.7.4
  

Patch

diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
index dd455e6..c0921dd 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -33,6 +33,7 @@  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_lcore.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_timer.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_interrupts.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_alarm.c
+SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_dev.c
 
 # from common dir
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_lcore.c
diff --git a/lib/librte_eal/bsdapp/eal/eal_dev.c b/lib/librte_eal/bsdapp/eal/eal_dev.c
new file mode 100644
index 0000000..ad606b3
--- /dev/null
+++ b/lib/librte_eal/bsdapp/eal/eal_dev.c
@@ -0,0 +1,19 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#include <rte_log.h>
+
+int __rte_experimental
+rte_dev_event_monitor_start(void)
+{
+	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
+	return -1;
+}
+
+int __rte_experimental
+rte_dev_event_monitor_stop(void)
+{
+	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
+	return -1;
+}
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index cd07144..3a1bbb6 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -14,9 +14,34 @@ 
 #include <rte_devargs.h>
 #include <rte_debug.h>
 #include <rte_log.h>
+#include <rte_spinlock.h>
+#include <rte_malloc.h>
 
 #include "eal_private.h"
 
+/* spinlock for device callbacks */
+static rte_spinlock_t dev_event_lock = RTE_SPINLOCK_INITIALIZER;
+
+/**
+ * The device event callback description.
+ *
+ * It contains callback address to be registered by user application,
+ * the pointer to the parameters for callback, and the device name.
+ */
+struct dev_event_callback {
+	TAILQ_ENTRY(dev_event_callback) next; /**< Callbacks list */
+	rte_dev_event_cb_fn cb_fn;                /**< Callback address */
+	void *cb_arg;                           /**< Callback parameter */
+	char *dev_name;	 /**< Callback devcie name, NULL is for all device */
+	uint32_t active;                        /**< Callback is executing */
+};
+
+/** @internal Structure to keep track of registered callbacks */
+TAILQ_HEAD(dev_event_cb_list, dev_event_callback);
+
+/* The device event callback list for all registered callbacks. */
+static struct dev_event_cb_list dev_event_cbs;
+
 static int cmp_detached_dev_name(const struct rte_device *dev,
 	const void *_name)
 {
@@ -207,3 +232,123 @@  rte_eal_hotplug_remove(const char *busname, const char *devname)
 	rte_eal_devargs_remove(busname, devname);
 	return ret;
 }
+
+static struct dev_event_callback * __rte_experimental
+dev_event_cb_find(const char *device_name, rte_dev_event_cb_fn cb_fn,
+				void *cb_arg)
+{
+	struct dev_event_callback *event_cb = NULL;
+
+	TAILQ_FOREACH(event_cb, &(dev_event_cbs), next) {
+		if (event_cb->cb_fn == cb_fn && event_cb->cb_arg == cb_arg) {
+			if (device_name == NULL && event_cb->dev_name == NULL)
+				break;
+			if (device_name == NULL || event_cb->dev_name == NULL)
+				continue;
+			if (!strcmp(event_cb->dev_name, device_name))
+				break;
+		}
+	}
+	return event_cb;
+}
+
+int __rte_experimental
+rte_dev_callback_register(const char *device_name, rte_dev_event_cb_fn cb_fn,
+				void *cb_arg)
+{
+	struct dev_event_callback *event_cb = NULL;
+
+	if (!cb_fn)
+		return -EINVAL;
+
+	rte_spinlock_lock(&dev_event_lock);
+
+	if (TAILQ_EMPTY(&(dev_event_cbs)))
+		TAILQ_INIT(&(dev_event_cbs));
+
+	event_cb = dev_event_cb_find(device_name, cb_fn, cb_arg);
+
+	/* create a new callback. */
+	if (event_cb == NULL) {
+		event_cb = malloc(sizeof(struct dev_event_callback));
+		if (event_cb != NULL) {
+			event_cb->cb_fn = cb_fn;
+			event_cb->cb_arg = cb_arg;
+			event_cb->dev_name = strdup(device_name);
+			if (event_cb->dev_name == NULL)
+				return -EINVAL;
+			TAILQ_INSERT_TAIL(&(dev_event_cbs), event_cb, next);
+		} else {
+			RTE_LOG(ERR, EAL,
+				"Failed to allocate memory for device event callback");
+			return -ENOMEM;
+		}
+	}
+
+	rte_spinlock_unlock(&dev_event_lock);
+	return (event_cb == NULL) ? -EEXIST : 0;
+}
+
+int __rte_experimental
+rte_dev_callback_unregister(const char *device_name, rte_dev_event_cb_fn cb_fn,
+				void *cb_arg)
+{
+	int ret = -1;
+	struct dev_event_callback *event_cb = NULL;
+
+	if (!cb_fn)
+		return -EINVAL;
+
+	rte_spinlock_lock(&dev_event_lock);
+
+	event_cb = dev_event_cb_find(device_name, cb_fn, cb_arg);
+
+	/*
+	 * if this callback is not executing right now,
+	 * then remove it.
+	 */
+	if (event_cb != NULL) {
+		if (event_cb->active == 0) {
+			TAILQ_REMOVE(&(dev_event_cbs), event_cb, next);
+			rte_free(event_cb);
+			ret = 0;
+		}
+		ret = -EBUSY;
+	}
+
+	rte_spinlock_unlock(&dev_event_lock);
+	return ret;
+}
+
+int __rte_experimental
+dev_callback_process(char *device_name, enum rte_dev_event_type event,
+				void *cb_arg)
+{
+	struct dev_event_callback *cb_lst;
+	int rc = 0;
+
+	rte_spinlock_lock(&dev_event_lock);
+
+	if (device_name == NULL)
+		return -EINVAL;
+
+	TAILQ_FOREACH(cb_lst, &dev_event_cbs, next) {
+		if (!cb_lst->dev_name)
+			break;
+		else if (!strcmp(cb_lst->dev_name, device_name))
+			break;
+	}
+	if (cb_lst) {
+		cb_lst->active = 1;
+		if (cb_arg)
+			cb_lst->cb_arg = cb_arg;
+		rte_spinlock_unlock(&dev_event_lock);
+		rc = cb_lst->cb_fn(device_name, event,
+				cb_lst->cb_arg);
+		rte_spinlock_lock(&dev_event_lock);
+		cb_lst->active = 0;
+	}
+
+	rte_spinlock_unlock(&dev_event_lock);
+	return rc;
+}
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 0b28770..d55cd68 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -9,6 +9,8 @@ 
 #include <stdint.h>
 #include <stdio.h>
 
+#include <rte_dev.h>
+
 /**
  * Initialize the memzone subsystem (private to eal).
  *
@@ -205,4 +207,26 @@  struct rte_bus *rte_bus_find_by_device_name(const char *str);
 
 int rte_mp_channel_init(void);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * 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 device_name
+ *  The device name.
+ * @param event
+ *  the device event type
+ * @param cb_arg
+ *  callback parameter.
+ *
+ * @return
+ *  - On success, return zero.
+ *  - On failure, a negative value.
+ */
+int __rte_experimental
+dev_callback_process(char *device_name, enum rte_dev_event_type event,
+				void *cb_arg);
 #endif /* _EAL_PRIVATE_H_ */
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index b688f1e..8867de6 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -24,6 +24,26 @@  extern "C" {
 #include <rte_compat.h>
 #include <rte_log.h>
 
+/**
+ * The device event type.
+ */
+enum rte_dev_event_type {
+	RTE_DEV_EVENT_UNKNOWN,	/**< unknown event type */
+	RTE_DEV_EVENT_ADD,	/**< device being added */
+	RTE_DEV_EVENT_REMOVE,	/**< device being removed */
+	RTE_DEV_EVENT_MAX	/**< max value of this enum */
+};
+
+struct rte_dev_event {
+	enum rte_dev_event_type type;	/**< device event type */
+	int subsystem;			/**< subsystem id */
+	char *devname;			/**< device name */
+};
+
+typedef int (*rte_dev_event_cb_fn)(char *device_name,
+					enum rte_dev_event_type event,
+					void *cb_arg);
+
 __attribute__((format(printf, 2, 0)))
 static inline void
 rte_pmd_debug_trace(const char *func_name, const char *fmt, ...)
@@ -267,4 +287,76 @@  __attribute__((used)) = str
 }
 #endif
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * It registers the callback for the specific device.
+ * Multiple callbacks cal be registered at the same time.
+ *
+ * @param device_name
+ *  The device name, that is the param name of the struct rte_device,
+ *  null value means for all devices.
+ * @param cb_fn
+ *  callback address.
+ * @param cb_arg
+ *  address of parameter for callback.
+ *
+ * @return
+ *  - On success, zero.
+ *  - On failure, a negative value.
+ */
+int __rte_experimental
+rte_dev_callback_register(const char *device_name, rte_dev_event_cb_fn cb_fn,
+			void *cb_arg);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * It unregisters the callback according to the specified device.
+ *
+ * @param device_name
+ *  The device name, that is the param name of the struct rte_device,
+ *  null value means for all devices.
+ * @param cb_fn
+ *  callback address.
+ * @param cb_arg
+ *  address of parameter for callback.
+ *
+ * @return
+ *  - On success, return the number of callback entities removed.
+ *  - On failure, a negative value.
+ */
+int __rte_experimental
+rte_dev_callback_unregister(const char *device_name, rte_dev_event_cb_fn cb_fn,
+					void *cb_arg);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Start the device event monitoring.
+ *
+ * @param none
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+int __rte_experimental
+rte_dev_event_monitor_start(void);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Stop the device event monitoring .
+ *
+ * @param none
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+int __rte_experimental
+rte_dev_event_monitor_stop(void);
 #endif /* _RTE_DEV_H_ */
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 7e5bbe8..8578796 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -41,6 +41,7 @@  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_lcore.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_timer.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_interrupts.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_alarm.c
+SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_dev.c
 
 # from common dir
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_lcore.c
diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c b/lib/librte_eal/linuxapp/eal/eal_dev.c
new file mode 100644
index 0000000..5ab5830
--- /dev/null
+++ b/lib/librte_eal/linuxapp/eal/eal_dev.c
@@ -0,0 +1,20 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#include <rte_log.h>
+#include <rte_dev.h>
+
+int __rte_experimental
+rte_dev_event_monitor_start(void)
+{
+	/* TODO: start uevent monitor for linux */
+	return 0;
+}
+
+int __rte_experimental
+rte_dev_event_monitor_stop(void)
+{
+	/* TODO: stop uevent monitor for linux */
+	return 0;
+}