| Message ID | 20180702054450.29269-3-qi.z.zhang@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Thomas Monjalon |
| Headers | show |
| Series | enable hotplug on multi-process | expand |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | success | coding style OK |
| ci/Intel-compilation | fail | Compilation issues |
Hi Qi, Sorry I do not understand neither the commit log, nor the doxygen. If it can help, consider your reader is in a bad mood and needs a pleasant description. 02/07/2018 07:44, Qi Zhang: > Introduce new API rte_eal_register_mp_init that help to register > a callback function which will be invoked right after multi-process > channel be established (rte_mp_channel_init). Typically the API > will be used by other module that want it's mp channel action callbacks > can be registered during rte_eal_init automatically. [...] > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice > + * > + * Register a callback function that will be invoked right after > + * multi-process channel be established (rte_mp_channel_init). Typically > + * the function is used by other module that want it's mp channel > + * action callbacks can be registered during rte_eal_init automatically. > + * > + * @note > + * This function only take effect when be called before rte_eal_init, > + * and all registered callback will be clear during rte_eal_cleanup. > + * > + * @param callback > + * function be called at that moment. > + * > + * @return > + * - 0 on success. > + * - (<0) on failure. > + */ > +int __rte_experimental > +rte_eal_register_mp_init(rte_eal_mp_init_callback_t callback); It looks the prefix of this function should be rte_mp_
> -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Tuesday, July 3, 2018 5:28 PM > To: Zhang, Qi Z <qi.z.zhang@intel.com> > Cc: dev@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>; Ananyev, > Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce > <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Shelton, > Benjamin H <benjamin.h.shelton@intel.com>; Vangati, Narender > <narender.vangati@intel.com> > Subject: Re: [dpdk-dev] [PATCH v8 02/19] eal: enable multi process init callback > > Hi Qi, > > Sorry I do not understand neither the commit log, nor the doxygen. > If it can help, consider your reader is in a bad mood and needs a pleasant > description. OK, I think is not a grammar issue since it almost passed a grammar check :) So more explanation: Basically we need to register mp action callback in ethdev layer, but this should happens during rte_eal_init ethdev don't have a general init function that can be invoked by eal during rte_eal_init, actually I guess, all no-eal module don't have in vdev bus, it register the mp action at the first bus scan happen. but in ethdev, we can't do that way, because, we don' t know when device will be attached or detached. so we need that function to help register a callback function which will be invoked during rte_eal_init. > > 02/07/2018 07:44, Qi Zhang: > > Introduce new API rte_eal_register_mp_init that help to register a > > callback function which will be invoked right after multi-process > > channel be established (rte_mp_channel_init). Typically the API will > > be used by other module that want it's mp channel action callbacks can > > be registered during rte_eal_init automatically. > [...] > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this API may change without prior notice > > + * > > + * Register a callback function that will be invoked right after > > + * multi-process channel be established (rte_mp_channel_init). > > +Typically > > + * the function is used by other module that want it's mp channel > > + * action callbacks can be registered during rte_eal_init automatically. > > + * > > + * @note > > + * This function only take effect when be called before rte_eal_init, > > + * and all registered callback will be clear during rte_eal_cleanup. > > + * > > + * @param callback > > + * function be called at that moment. > > + * > > + * @return > > + * - 0 on success. > > + * - (<0) on failure. > > + */ > > +int __rte_experimental > > +rte_eal_register_mp_init(rte_eal_mp_init_callback_t callback); > > It looks the prefix of this function should be rte_mp_ OK, will fix. Regards Qi >
03/07/2018 17:16, Zhang, Qi Z: > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > > > Hi Qi, > > > > Sorry I do not understand neither the commit log, nor the doxygen. > > If it can help, consider your reader is in a bad mood and needs a pleasant > > description. > > OK, I think is not a grammar issue since it almost passed a grammar check :) > > So more explanation: > Basically we need to register mp action callback in ethdev layer, What is "mp action callback"? Why do you need it in ethdev? > but this should happens during rte_eal_init Why it should be done in rte_eal_init? > ethdev don't have a general init function that can be invoked by eal > during rte_eal_init, actually I guess, all no-eal module don't have What is "no-eal module"? > in vdev bus, it register the mp action at the first bus scan happen. > but in ethdev, we can't do that way, because, we don' t know > when device will be attached or detached. > so we need that function to help register a callback function which will be invoked during rte_eal_init. > > > > > > 02/07/2018 07:44, Qi Zhang: > > > Introduce new API rte_eal_register_mp_init that help to register a > > > callback function which will be invoked right after multi-process > > > channel be established (rte_mp_channel_init). Typically the API will > > > be used by other module that want it's mp channel action callbacks can > > > be registered during rte_eal_init automatically. > > [...] > > > +/** > > > + * @warning > > > + * @b EXPERIMENTAL: this API may change without prior notice > > > + * > > > + * Register a callback function that will be invoked right after > > > + * multi-process channel be established (rte_mp_channel_init). > > > +Typically > > > + * the function is used by other module that want it's mp channel > > > + * action callbacks can be registered during rte_eal_init automatically. > > > + * > > > + * @note > > > + * This function only take effect when be called before rte_eal_init, > > > + * and all registered callback will be clear during rte_eal_cleanup.
> -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Wednesday, July 4, 2018 5:51 AM > To: Zhang, Qi Z <qi.z.zhang@intel.com> > Cc: dev@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>; Ananyev, > Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce > <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Shelton, > Benjamin H <benjamin.h.shelton@intel.com>; Vangati, Narender > <narender.vangati@intel.com> > Subject: Re: [dpdk-dev] [PATCH v8 02/19] eal: enable multi process init callback > > 03/07/2018 17:16, Zhang, Qi Z: > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > > > > > Hi Qi, > > > > > > Sorry I do not understand neither the commit log, nor the doxygen. > > > If it can help, consider your reader is in a bad mood and needs a > > > pleasant description. > > > > OK, I think is not a grammar issue since it almost passed a grammar > > check :) > > > > So more explanation: > > Basically we need to register mp action callback in ethdev layer, > > What is "mp action callback"? rte_mp_action_register(ETH_DEV_MP_ACTION_REQUEST, handle_secondary_request); in this case ETH_DEV_MP_ACTION_REQUEST is the action, and handle_secondary_request is the callback which will be invoked when the action received from another process. > Why do you need it in ethdev? Based on current implementation, IPC is required during dev_attach/dev_detach. so we need to make sure callback is registered before that happen. > > > but this should happens during rte_eal_init > > Why it should be done in rte_eal_init? Either we can expose an ethdev API like rte_eth_dev_mp_init, to let application do this explicitly, or we can let application assume all necessary callback has been registered when DPDK is initialized. (rte_eal_init) > > > ethdev don't have a general init function that can be invoked by eal > > during rte_eal_init, actually I guess, all no-eal module don't have > > What is "no-eal module"? A module like lib_ethdev which lib_eal should not depend on. > > > in vdev bus, it register the mp action at the first bus scan happen. > > but in ethdev, we can't do that way, because, we don' t know when > > device will be attached or detached. > > so we need that function to help register a callback function which will be > invoked during rte_eal_init. > > > > > > > > > > 02/07/2018 07:44, Qi Zhang: > > > > Introduce new API rte_eal_register_mp_init that help to register a > > > > callback function which will be invoked right after multi-process > > > > channel be established (rte_mp_channel_init). Typically the API > > > > will be used by other module that want it's mp channel action > > > > callbacks can be registered during rte_eal_init automatically. > > > [...] > > > > +/** > > > > + * @warning > > > > + * @b EXPERIMENTAL: this API may change without prior notice > > > > + * > > > > + * Register a callback function that will be invoked right after > > > > + * multi-process channel be established (rte_mp_channel_init). > > > > +Typically > > > > + * the function is used by other module that want it's mp channel > > > > + * action callbacks can be registered during rte_eal_init automatically. > > > > + * > > > > + * @note > > > > + * This function only take effect when be called before rte_eal_init, > > > > + * and all registered callback will be clear during rte_eal_cleanup. > >
diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c index f010ef59e..f6d7c83e4 100644 --- a/lib/librte_eal/common/eal_common_proc.c +++ b/lib/librte_eal/common/eal_common_proc.c @@ -619,11 +619,47 @@ unlink_sockets(const char *filter) return 0; } +struct mp_init_entry { + TAILQ_ENTRY(mp_init_entry) next; + rte_eal_mp_init_callback_t callback; +}; + +TAILQ_HEAD(mp_init_entry_list, mp_init_entry); +static struct mp_init_entry_list mp_init_entry_list = + TAILQ_HEAD_INITIALIZER(mp_init_entry_list); + +static int process_mp_init_callbacks(void) +{ + struct mp_init_entry *entry; + int ret; + + TAILQ_FOREACH(entry, &mp_init_entry_list, next) { + ret = entry->callback(); + if (ret) + return ret; + } + return 0; +} + +int __rte_experimental +rte_eal_register_mp_init(rte_eal_mp_init_callback_t callback) +{ + struct mp_init_entry *entry = calloc(1, sizeof(struct mp_init_entry)); + + if (entry == NULL) + return -ENOMEM; + + entry->callback = callback; + TAILQ_INSERT_TAIL(&mp_init_entry_list, entry, next); + + return 0; +} + int rte_mp_channel_init(void) { char path[PATH_MAX]; - int dir_fd; + int dir_fd, ret; pthread_t mp_handle_tid, async_reply_handle_tid; /* create filter path */ @@ -686,7 +722,24 @@ rte_mp_channel_init(void) flock(dir_fd, LOCK_UN); close(dir_fd); - return 0; + ret = process_mp_init_callbacks(); + if (ret) + RTE_LOG(ERR, EAL, "failed to process mp init callbacks\n"); + + return ret; +} + +void rte_mp_init_callback_cleanup(void) +{ + struct mp_init_entry *entry; + + while (!TAILQ_EMPTY(&mp_init_entry_list)) { + TAILQ_FOREACH(entry, &mp_init_entry_list, next) { + TAILQ_REMOVE(&mp_init_entry_list, entry, next); + free(entry); + break; + } + } } /** diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h index bdadc4d50..bc230ee23 100644 --- a/lib/librte_eal/common/eal_private.h +++ b/lib/librte_eal/common/eal_private.h @@ -247,6 +247,11 @@ struct rte_bus *rte_bus_find_by_device_name(const char *str); int rte_mp_channel_init(void); /** + * Cleanup all mp channel init callbacks. + */ +void rte_mp_init_callback_cleanup(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. diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h index 8de5d69e8..506f17f34 100644 --- a/lib/librte_eal/common/include/rte_eal.h +++ b/lib/librte_eal/common/include/rte_eal.h @@ -512,6 +512,40 @@ __rte_deprecated const char * rte_eal_mbuf_default_mempool_ops(void); +/** + * Callback function right after multi-process channel be established. + * Typical implementation of these functions is to register mp channel + * action callbacks + * + * @return + * - 0 on success. + * - (<0) on failure. + */ +typedef int (*rte_eal_mp_init_callback_t)(void); + +/** + * @warning + * @b EXPERIMENTAL: this API may change without prior notice + * + * Register a callback function that will be invoked right after + * multi-process channel be established (rte_mp_channel_init). Typically + * the function is used by other module that want it's mp channel + * action callbacks can be registered during rte_eal_init automatically. + * + * @note + * This function only take effect when be called before rte_eal_init, + * and all registered callback will be clear during rte_eal_cleanup. + * + * @param callback + * function be called at that moment. + * + * @return + * - 0 on success. + * - (<0) on failure. + */ +int __rte_experimental +rte_eal_register_mp_init(rte_eal_mp_init_callback_t callback); + #ifdef __cplusplus } #endif diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c index 8655b8691..45cccff7e 100644 --- a/lib/librte_eal/linuxapp/eal/eal.c +++ b/lib/librte_eal/linuxapp/eal/eal.c @@ -1048,6 +1048,8 @@ int __rte_experimental rte_eal_cleanup(void) { rte_service_finalize(); + rte_mp_init_callback_cleanup(); + return 0; } diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map index f7dd0e7bc..67e3548e8 100644 --- a/lib/librte_eal/rte_eal_version.map +++ b/lib/librte_eal/rte_eal_version.map @@ -260,6 +260,7 @@ EXPERIMENTAL { rte_eal_hotplug_add; rte_eal_hotplug_remove; rte_eal_mbuf_user_pool_ops; + rte_eal_register_mp_init; rte_fbarray_attach; rte_fbarray_destroy; rte_fbarray_detach;