From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 482D91C157 for ; Thu, 5 Apr 2018 12:15:59 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Apr 2018 03:15:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,410,1517904000"; d="scan'208";a="45119188" Received: from tanjianf-mobl.ccr.corp.intel.com (HELO [10.255.27.107]) ([10.255.27.107]) by orsmga001.jf.intel.com with ESMTP; 05 Apr 2018 03:15:54 -0700 To: Jeff Guo , stephen@networkplumber.org, bruce.richardson@intel.com, ferruh.yigit@intel.com, konstantin.ananyev@intel.com, gaetan.rivet@6wind.com, jingjing.wu@intel.com, thomas@monjalon.net, motih@mellanox.com, harry.van.haaren@intel.com References: <1522751639-2315-5-git-send-email-jia.guo@intel.com> <1522917173-15182-1-git-send-email-jia.guo@intel.com> <1522917173-15182-3-git-send-email-jia.guo@intel.com> Cc: jblunck@infradead.org, shreyansh.jain@nxp.com, dev@dpdk.org, helin.zhang@intel.com From: "Tan, Jianfeng" Message-ID: Date: Thu, 5 Apr 2018 18:15:53 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1522917173-15182-3-git-send-email-jia.guo@intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH V19 2/4] eal: add device event monitor framework X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Apr 2018 10:16:00 -0000 On 4/5/2018 4:32 PM, Jeff Guo wrote: > This patch aims to add a general device event monitor framework at > EAL device layer, for device hotplug awareness and actions adopted > accordingly. It could also expand for all other types of device event > monitor, but not in this scope at the stage. > > To get started, users firstly call below new added APIs to enable/disable > the device event monitor mechanism: > - rte_dev_event_monitor_start > - rte_dev_event_monitor_stop > > Then users shell register or unregister callbacks through the new added > APIs. Callbacks can be some device specific, or for all devices. > -rte_dev_event_callback_register > -rte_dev_event_callback_unregister > > 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 benefit further fail-safe or live-migration. > > Signed-off-by: Jeff Guo Except some trivial things, I'm ok with this patch, so Reviewed-by: Jianfeng Tan > --- > v19->v18: > clear the coding style and fix typo > --- > doc/guides/rel_notes/release_18_05.rst | 9 ++ > lib/librte_eal/bsdapp/eal/Makefile | 1 + > lib/librte_eal/bsdapp/eal/eal_dev.c | 21 +++++ > lib/librte_eal/bsdapp/eal/meson.build | 1 + > lib/librte_eal/common/eal_common_dev.c | 161 ++++++++++++++++++++++++++++++++ > lib/librte_eal/common/eal_private.h | 15 +++ > lib/librte_eal/common/include/rte_dev.h | 94 +++++++++++++++++++ > lib/librte_eal/linuxapp/eal/Makefile | 1 + > lib/librte_eal/linuxapp/eal/eal_dev.c | 22 +++++ > lib/librte_eal/linuxapp/eal/meson.build | 1 + > lib/librte_eal/rte_eal_version.map | 10 ++ > 11 files changed, 336 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/doc/guides/rel_notes/release_18_05.rst b/doc/guides/rel_notes/release_18_05.rst > index e5fac1c..d3c86bd 100644 > --- a/doc/guides/rel_notes/release_18_05.rst > +++ b/doc/guides/rel_notes/release_18_05.rst > @@ -58,6 +58,15 @@ New Features > * Added support for NVGRE, VXLAN and GENEVE filters in flow API. > * Added support for DROP action in flow API. > > +* **Added device event monitor framework.** > + > + Added a general device event monitor framework at EAL, for device dynamic management. > + Such as device hotplug awareness and actions adopted accordingly. The list of new APIs: > + > + * ``rte_dev_event_monitor_start`` and ``rte_dev_event_monitor_stop`` are for > + the event monitor enable and disable. > + * ``rte_dev_event_callback_register`` and ``rte_dev_event_callback_unregister`` > + are for the user's callbacks register and unregister. > > API Changes > ----------- > diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile > index ed1d17b..90b88eb 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..1c6c51b > --- /dev/null > +++ b/lib/librte_eal/bsdapp/eal/eal_dev.c > @@ -0,0 +1,21 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2018 Intel Corporation > + */ > + > +#include > +#include > +#include > + > +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/bsdapp/eal/meson.build b/lib/librte_eal/bsdapp/eal/meson.build > index e83fc91..6dfc533 100644 > --- a/lib/librte_eal/bsdapp/eal/meson.build > +++ b/lib/librte_eal/bsdapp/eal/meson.build > @@ -12,4 +12,5 @@ env_sources = files('eal_alarm.c', > 'eal_timer.c', > 'eal.c', > 'eal_memory.c', > + 'eal_dev.c' > ) > diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c > index cd07144..e202cf2 100644 > --- a/lib/librte_eal/common/eal_common_dev.c > +++ b/lib/librte_eal/common/eal_common_dev.c > @@ -14,9 +14,34 @@ > #include > #include > #include > +#include > +#include > > #include "eal_private.h" > > +/** > + * 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 device name, NULL is for all device */ > + uint32_t active; /**< Callback is executing */ > +}; > + > +/* spinlock for device callbacks */ > +static rte_spinlock_t dev_event_lock = RTE_SPINLOCK_INITIALIZER; It's still not in the right position; it shall be put after dev_event_callback list. > + > +/* The device event callback list for all registered callbacks. */ > +static struct dev_event_cb_list dev_event_cbs; > + > +/** @internal Structure to keep track of registered callbacks */ > +TAILQ_HEAD(dev_event_cb_list, dev_event_callback); > + > static int cmp_detached_dev_name(const struct rte_device *dev, > const void *_name) > { > @@ -207,3 +232,139 @@ rte_eal_hotplug_remove(const char *busname, const char *devname) > rte_eal_devargs_remove(busname, devname); > return ret; > } > + > +int __rte_experimental > +rte_dev_event_callback_register(const char *device_name, > + rte_dev_event_cb_fn cb_fn, > + void *cb_arg) > +{ > + struct dev_event_callback *event_cb; > + int ret; > + > + if (!cb_fn) > + return -EINVAL; > + > + rte_spinlock_lock(&dev_event_lock); > + > + if (TAILQ_EMPTY(&dev_event_cbs)) > + TAILQ_INIT(&dev_event_cbs); > + > + 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; > + } > + } > + > + /* 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->active = 0; > + if (!device_name) { > + event_cb->dev_name = NULL; > + } else { > + event_cb->dev_name = strdup(device_name); > + if (event_cb->dev_name == NULL) { > + ret = -ENOMEM; > + goto error; > + } > + } > + TAILQ_INSERT_TAIL(&dev_event_cbs, event_cb, next); > + } else { > + RTE_LOG(ERR, EAL, > + "Failed to allocate memory for device " > + "event callback."); > + ret = -ENOMEM; > + goto error; > + } > + } else { > + RTE_LOG(ERR, EAL, > + "The callback is already exist, no need " > + "to register again.\n"); > + ret = -EEXIST; > + } > + > + rte_spinlock_unlock(&dev_event_lock); > + return 0; > +error: > + free(event_cb); > + rte_spinlock_unlock(&dev_event_lock); > + return ret; > +} > + > +int __rte_experimental > +rte_dev_event_callback_unregister(const char *device_name, > + rte_dev_event_cb_fn cb_fn, > + void *cb_arg) > +{ > + int ret = 0; > + struct dev_event_callback *event_cb, *next; > + > + if (!cb_fn) > + return -EINVAL; > + > + rte_spinlock_lock(&dev_event_lock); > + /*walk through the callbacks and remove all that match. */ > + for (event_cb = TAILQ_FIRST(&dev_event_cbs); event_cb != NULL; > + event_cb = next) { > + > + next = TAILQ_NEXT(event_cb, next); > + > + if (device_name != NULL && event_cb->dev_name != NULL) { > + if (!strcmp(event_cb->dev_name, device_name)) { > + if (event_cb->cb_fn != cb_fn || > + (cb_arg != (void *)-1 && > + event_cb->cb_arg != cb_arg)) > + continue; > + } > + } else if (device_name != NULL) { > + continue; > + } > + > + /* > + * if this callback is not executing right now, > + * then remove it. > + */ > + if (event_cb->active == 0) { > + TAILQ_REMOVE(&dev_event_cbs, event_cb, next); > + free(event_cb); > + ret++; > + } else { > + continue; > + } > + } > + rte_spinlock_unlock(&dev_event_lock); > + return ret; > +} > + > +void > +dev_callback_process(char *device_name, enum rte_dev_event_type event) > +{ > + struct dev_event_callback *cb_lst; > + > + if (device_name == NULL) > + return; > + > + rte_spinlock_lock(&dev_event_lock); > + > + TAILQ_FOREACH(cb_lst, &dev_event_cbs, next) { > + if (cb_lst->dev_name) { > + if (strcmp(cb_lst->dev_name, device_name)) > + continue; > + } > + cb_lst->active = 1; > + rte_spinlock_unlock(&dev_event_lock); > + 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); > +} > diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h > index 0b28770..88e5a59 100644 > --- a/lib/librte_eal/common/eal_private.h > +++ b/lib/librte_eal/common/eal_private.h > @@ -9,6 +9,8 @@ > #include > #include > > +#include > + > /** > * Initialize the memzone subsystem (private to eal). > * > @@ -205,4 +207,17 @@ struct rte_bus *rte_bus_find_by_device_name(const char *str); > > int rte_mp_channel_init(void); > > +/** > + * 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. > + * > + */ > +void > +dev_callback_process(char *device_name, enum rte_dev_event_type event); > #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..2ed240e 100644 > --- a/lib/librte_eal/common/include/rte_dev.h > +++ b/lib/librte_eal/common/include/rte_dev.h > @@ -24,6 +24,25 @@ extern "C" { > #include > #include > > +/** > + * The device event type. > + */ > +enum rte_dev_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 void (*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 +286,79 @@ __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_event_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. Do you mean all callbacks? > + * @param cb_fn > + * callback address. > + * @param cb_arg > + * address of parameter for callback, (void *)-1 means to remove all > + * registered which has the same callback address. > + * > + * @return > + * - On success, return the number of callback entities removed. > + * - On failure, a negative value. > + */ > +int __rte_experimental > +rte_dev_event_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 b9c7727..db67001 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..9c8d1a0 > --- /dev/null > +++ b/lib/librte_eal/linuxapp/eal/eal_dev.c > @@ -0,0 +1,22 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2018 Intel Corporation > + */ > + > +#include > +#include > +#include > + > + > +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; > +} > diff --git a/lib/librte_eal/linuxapp/eal/meson.build b/lib/librte_eal/linuxapp/eal/meson.build > index 03974ff..b222571 100644 > --- a/lib/librte_eal/linuxapp/eal/meson.build > +++ b/lib/librte_eal/linuxapp/eal/meson.build > @@ -18,6 +18,7 @@ env_sources = files('eal_alarm.c', > 'eal_vfio_mp_sync.c', > 'eal.c', > 'eal_memory.c', > + 'eal_dev.c', > ) > > if has_libnuma == 1 > diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map > index dd38783..3022df1 100644 > --- a/lib/librte_eal/rte_eal_version.map > +++ b/lib/librte_eal/rte_eal_version.map > @@ -260,3 +260,13 @@ EXPERIMENTAL { > rte_socket_id_by_idx; > > } DPDK_18.02; > + > +EXPERIMENTAL { > + global: > + > + rte_dev_event_monitor_start; > + rte_dev_event_monitor_stop; > + rte_dev_event_callback_register; > + rte_dev_event_callback_unregister; Use tab instead of spaces. > + > +} DPDK_18.05;