thanks for your review. please check v13. On 1/24/2018 10:52 PM, Wu, Jingjing wrote:
-----Original Message----- From: Guo, Jia Sent: Thursday, January 18, 2018 12:12 PM To: step...@networkplumber.org; Richardson, Bruce <bruce.richard...@intel.com>; Yigit, Ferruh <ferruh.yi...@intel.com>; gaetan.ri...@6wind.com Cc: Ananyev, Konstantin <konstantin.anan...@intel.com>; jblu...@infradead.org; shreyansh.j...@nxp.com; Wu, Jingjing <jingjing...@intel.com>; dev@dpdk.org; Guo, Jia <jia....@intel.com>; tho...@monjalon.net; Zhang, Helin <helin.zh...@intel.com>; mo...@mellanox.com Subject: [PATCH V12 1/3] eal: add uevent monitor api and callback func This patch aim to add a general uevent mechanism in eal device layer, to enable all linux kernel object uevent monitoring, user could use these APIs to monitor and read out the device status info that sent from the kernel side, then corresponding to handle it, such as when detect hotplug uevent type, user could detach or attach the device, and more it benefit to use to do smoothly fail safe work. About uevent monitoring: a: add one epolling to poll the netlink socket, to monitor the uevent of the device. b: add enum of rte_eal_dev_event_type and struct of rte_eal_uevent. c: add below APIs in rte eal device layer. rte_dev_callback_register rte_dev_callback_unregister _rte_dev_callback_process rte_dev_event_monitor_start rte_dev_event_monitor_stop Signed-off-by: Jeff Guo <jia....@intel.com> --- v12->v11: identify null param in callback for monitor all devices uevent --- lib/librte_eal/bsdapp/eal/eal_dev.c | 38 ++++++ lib/librte_eal/common/eal_common_dev.c | 128 ++++++++++++++++++ lib/librte_eal/common/include/rte_dev.h | 119 +++++++++++++++++ lib/librte_eal/linuxapp/eal/Makefile | 1 + lib/librte_eal/linuxapp/eal/eal_dev.c | 223 ++++++++++++++++++++++++++++++++ 5 files changed, 509 insertions(+) create mode 100644 lib/librte_eal/bsdapp/eal/eal_dev.c create mode 100644 lib/librte_eal/linuxapp/eal/eal_dev.c[......]+int +rte_dev_callback_register(char *device_name, rte_dev_event_cb_fn cb_fn, + void *cb_arg) +{ + struct rte_dev_event_callback *event_cb = NULL; + + rte_spinlock_lock(&rte_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 && + !strcmp(event_cb->dev_name, device_name))device_name = NULL means means for all devices, right? Can strcmp accept NULL arguments?
got it.
+ break; + } + + /* create a new callback. */ + if (event_cb == NULL) { + /* allocate a new user callback entity */ + event_cb = malloc(sizeof(struct rte_dev_event_callback)); + if (event_cb != NULL) { + event_cb->cb_fn = cb_fn; + event_cb->cb_arg = cb_arg; + event_cb->dev_name = device_name; + }Is that OK to call TAILQ_INSERT_TAIL below if event_cb == NULL?
yes, that might be wrong.
+ TAILQ_INSERT_TAIL(&(dev_event_cbs), event_cb, next); + } + + rte_spinlock_unlock(&rte_dev_event_lock); + return (event_cb == NULL) ? -1 : 0; +} + +int +rte_dev_callback_unregister(char *device_name, rte_dev_event_cb_fn cb_fn, + void *cb_arg) +{ + int ret; + struct rte_dev_event_callback *event_cb, *next; + + if (!cb_fn || device_name == NULL) + return -EINVAL; + + rte_spinlock_lock(&rte_dev_event_lock); + + ret = 0; + + for (event_cb = TAILQ_FIRST(&(dev_event_cbs)); event_cb != NULL; + event_cb = next) { + + next = TAILQ_NEXT(event_cb, next); + + if (event_cb->cb_fn != cb_fn || + (event_cb->cb_arg != (void *)-1 && + event_cb->cb_arg != cb_arg) || + strcmp(event_cb->dev_name, device_name))The same comments as above.
ok.
+ 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); + rte_free(event_cb); + } else { + ret = -EAGAIN; + } + } + + rte_spinlock_unlock(&rte_dev_event_lock); + return ret; +} +[......]+int +rte_dev_event_monitor_start(void) +{ + int ret; + struct rte_service_spec service; + uint32_t id; + const uint32_t sid = 0; + + if (!service_no_init) + return 0; + + uint32_t slcore_1 = rte_get_next_lcore(/* start core */ -1, + /* skip master */ 1, + /* wrap */ 0); + + ret = rte_service_lcore_add(slcore_1); + if (ret) { + RTE_LOG(ERR, EAL, "dev event monitor lcore add fail"); + return ret; + } + + memset(&service, 0, sizeof(service)); + snprintf(service.name, sizeof(service.name), DEV_EV_MNT_SERVICE_NAME); + + service.socket_id = rte_socket_id(); + service.callback = dev_uev_monitoring; + service.callback_userdata = NULL; + service.capabilities = 0; + ret = rte_service_component_register(&service, &id); + if (ret) { + RTE_LOG(ERR, EAL, "Failed to register service %s " + "err = %" PRId32, + service.name, ret); + return ret; + } + ret = rte_service_runstate_set(sid, 1); + if (ret) { + RTE_LOG(ERR, EAL, "Failed to set the runstate of " + "the service");Any rollback need to be done when fails?
yes, should be handle fails.
+ return ret; + } + ret = rte_service_component_runstate_set(id, 1); + if (ret) { + RTE_LOG(ERR, EAL, "Failed to set the backend runstate" + " of a component"); + return ret; + } + ret = rte_service_map_lcore_set(sid, slcore_1, 1); + if (ret) { + RTE_LOG(ERR, EAL, "Failed to enable lcore 1 on " + "dev event monitor service"); + return ret; + } + rte_service_lcore_start(slcore_1); + service_no_init = false; + return 0; +} + +int +rte_dev_event_monitor_stop(void) +{ + service_exit = true; + service_no_init = true; + return 0;Are start and stop peer functions to call? If we call rte_dev_event_monitor_start to start monitor and then call rte_dev_event_monitor_stop to stop it, and then how to start again?
sure. should peer control.
+} -- 2.7.4
