[dpdk-dev] [PATCH v2 1/4] eal: add channel for multi-process communication
Burakov, Anatoly
anatoly.burakov at intel.com
Sat Jan 13 13:57:29 CET 2018
On 11-Jan-18 4:07 AM, Jianfeng Tan wrote:
> diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
> index 40fa982..d700e9e 100644
<snip>
> +
> int
> rte_eal_primary_proc_alive(const char *config_file_path)
> {
> @@ -31,3 +75,347 @@ rte_eal_primary_proc_alive(const char *config_file_path)
>
> return !!ret;
> }
> +
> +static struct action_entry *
> +find_action_entry_by_name(const char *name)
> +{
> + int len = strlen(name);
Why do strlen() here? You already have MAX_ACTION_NAME_LEN, strncmp will
take care of the rest, no?
> + struct action_entry *entry;
> +
> + TAILQ_FOREACH(entry, &action_entry_list, next) {
> + if (strncmp(entry->action_name, name, len) == 0)
> + break;
> + }
> +
> + return entry;
> +}
> +
> +int
> +rte_eal_mp_action_register(const char *action_name, rte_eal_mp_t action)
> +{
> + struct action_entry *entry = malloc(sizeof(struct action_entry));
> +
> + if (entry == NULL) {
> + rte_errno = -ENOMEM;
> + return -1;
> + }
> +
> + if (strlen(action_name) > MAX_ACTION_NAME_LEN) {
> + rte_errno = -E2BIG;
> + return -1;
> + }
strnlen perhaps? strnlen(action_name) == MAX_ACTION_NAME_LEN will be an
error condition, and unlike strlen you won't have to scan the entire
memory if your string was corrupted.
> +
> + pthread_mutex_lock(&mp_mutex_action);
> + if (find_action_entry_by_name(action_name) != NULL) {
> + free(entry);
> + rte_errno = -EEXIST;
> + return -1;
> + }
> + strncpy(entry->action_name, action_name, MAX_ACTION_NAME_LEN);
> + entry->action = action;
> + TAILQ_INSERT_TAIL(&action_entry_list, entry, next);
> + pthread_mutex_unlock(&mp_mutex_action);
> + return 0;
> +}
> +
> +void
> +rte_eal_mp_action_unregister(const char *name)
> +{
name == NULL? You do a find_action_entry_by_name with it, which calls
strlen, which IIRC segfaults on NULL pointer. Also, maybe add an strlen
(or better yet, strnlen) check here like in action_register, so that
find_action_entry_by_name doesn't need to care about string lengths and
can work off MAX_ACTION_NAME_LEN instead.
> + struct action_entry *entry;
> +
> + pthread_mutex_lock(&mp_mutex_action);
> + entry = find_action_entry_by_name(name);
entry == NULL?
> + TAILQ_REMOVE(&action_entry_list, entry, next);
> + free(entry);
> + pthread_mutex_unlock(&mp_mutex_action);
> +}
> +
> +static int
> +read_msg(int fd, char *buf, int buflen, int *fds, int fds_num)
> +{
> + int ret;
> + struct iovec iov;
> + struct msghdr msgh;
<snip>
> + return -1;
> + }
> +
> + params_len = len - sizeof(struct mp_msghdr);
> + ret = entry->action(hdr->params, params_len, fds, hdr->fds_num);
> + pthread_mutex_unlock(&mp_mutex_action);
> + return ret;
> +
unnecessary newline.
> +}
> +
> +static void *
> +mp_handle(void *arg __rte_unused)
> +{
> + int len;
> + int fds[SCM_MAX_FD];
> + char buf[MAX_MSG_LENGTH];
> +
> + while (1) {
<snip>
> + goto error;
> + }
> +
> + return 0;
> +error:
> + close(mp_fd);
> + mp_fd = -1;
> + return -1;
> +}
> +
> +static inline struct mp_msghdr *
> +format_msg(const char *act_name, const void *p, int len_params, int fds_num)
The name is slightly misleading, as this function actually *creates* a
message, not just formats it. create_msg? alloc_msg?
> +{
> + int len_msg;
> + struct mp_msghdr *msg;
> +
> + len_msg = sizeof(struct mp_msghdr) + len_params;
> + if (len_msg > MAX_MSG_LENGTH) {
> + RTE_LOG(ERR, EAL, "Message is too long\n");
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> +
> + msg = malloc(len_msg);
> + if (!msg) {
> + RTE_LOG(ERR, EAL, "Cannot alloc memory for msg\n");
> + rte_errno = -ENOMEM;
> + return NULL;
> + }
> + memset(msg, 0, len_msg);
> + strcpy(msg->action_name, act_name);
strncpy?
> + msg->fds_num = fds_num;
> + msg->len_params = len_params;
> + memcpy(msg->params, p, len_params);
> + return msg;
> +}
> +
> +static int
> +send_msg(int fd, const char *dst_path, struct mp_msghdr *msg, int fds[])
> +{
> + int ret;
> + struct msghdr msgh;
> + struct iovec iov;
> + size_t fd_size = msg->fds_num * sizeof(int);
<snip>
> + return mp_send(action_name, params, len_params, fds, fds_num);
> +}
> diff --git a/lib/librte_eal/common/eal_filesystem.h b/lib/librte_eal/common/eal_filesystem.h
> index e8959eb..e95399b 100644
> --- a/lib/librte_eal/common/eal_filesystem.h
> +++ b/lib/librte_eal/common/eal_filesystem.h
> @@ -38,6 +38,23 @@ eal_runtime_config_path(void)
> return buffer;
> }
>
> +/** Path of primary/secondary communication unix socket file. */
> +#define MP_UNIX_PATH_FMT "%s/.%s_unix"
> +static inline const char *
> +eal_mp_unix_path(void)
perhaps eal_mp_socket_path would've been more descriptive? API doesn't
need to care what kind of socket it is.
> +{
> + static char buffer[PATH_MAX]; /* static so auto-zeroed */
> + const char *directory = default_config_dir;
> + const char *home_dir = getenv("HOME");
> +
> + if (getuid() != 0 && home_dir != NULL)
> + directory = home_dir;
<snip>
--
Thanks,
Anatoly
More information about the dev
mailing list