[dpdk-dev] [PATCH v4 04/24] eal: enable multi process init callback

[Burakov, Anatoly]

On 26-Jun-18 8:08 AM, Qi Zhang wrote:
> 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 at intel.com>
> ---
>   lib/librte_eal/common/eal_common_proc.c | 51 ++++++++++++++++++++++++++++++++-
>   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 ++
>   4 files changed, 91 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
> index 707d8ab30..fc0eb4d17 100644
> --- a/lib/librte_eal/common/eal_common_proc.c
> +++ b/lib/librte_eal/common/eal_common_proc.c
> @@ -619,6 +619,42 @@ 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)
>   {
> @@ -686,7 +722,20 @@ rte_mp_channel_init(void)
>   	flock(dir_fd, LOCK_UN);
>   	close(dir_fd);
>   
> -	return 0;
> +	return process_mp_init_callbacks();
> +}
> +

Perhaps some kind of log message in case of failure would be useful? 
Otherwise this failure would be pretty silent - neither 
process_mp_init_callbacks() nor rte_mp_channel_init() logs any errors.

> +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);

This is a good idea, however i should note that none of the other 
services (EAL or otherwise) clean up after themselves. They leave open 
file descriptors, malloc'd memory etc. all over the place. Maybe this is 
something to fix for the future...

> +
> +/**
>    * 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);

Otherwise,

Acked-by: Anatoly Burakov <anatoly.burakov at intel.com>

-- 
Thanks,
Anatoly