[v8,02/19] eal: enable multi process init callback

Message ID 20180702054450.29269-3-qi.z.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series enable hotplug on multi-process |

Checks

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

Commit Message

Qi Zhang July 2, 2018, 5:44 a.m. UTC
  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.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/eal_common_proc.c | 57 +++++++++++++++++++++++++++++++--
 lib/librte_eal/common/eal_private.h     |  5 +++
 lib/librte_eal/common/include/rte_eal.h | 34 ++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal.c       |  2 ++
 lib/librte_eal/rte_eal_version.map      |  1 +
 5 files changed, 97 insertions(+), 2 deletions(-)
  

Comments

Thomas Monjalon July 3, 2018, 9:27 a.m. UTC | #1
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_
  
Qi Zhang July 3, 2018, 3:16 p.m. UTC | #2
> -----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
>
  
Thomas Monjalon July 3, 2018, 9:51 p.m. UTC | #3
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.
  
Qi Zhang July 4, 2018, 1:08 a.m. UTC | #4
> -----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.
> 
>
  

Patch

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;