[dpdk-dev] [PATCH V12 1/3] eal: add uevent monitor api and callback func
Guo, Jia
jia.guo at intel.com
Thu Jan 25 15:57:47 CET 2018
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: stephen at networkplumber.org; Richardson, Bruce <bruce.richardson at intel.com>;
>> Yigit, Ferruh <ferruh.yigit at intel.com>; gaetan.rivet at 6wind.com
>> Cc: Ananyev, Konstantin <konstantin.ananyev at intel.com>; jblunck at infradead.org;
>> shreyansh.jain at nxp.com; Wu, Jingjing <jingjing.wu at intel.com>; dev at dpdk.org; Guo, Jia
>> <jia.guo at intel.com>; thomas at monjalon.net; Zhang, Helin <helin.zhang at intel.com>;
>> motih at 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.guo at 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
More information about the dev
mailing list