From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id DB1242BF4 for ; Sat, 13 Jan 2018 14:41:52 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Jan 2018 05:41:52 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,353,1511856000"; d="scan'208";a="10021732" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.252.9.53]) ([10.252.9.53]) by fmsmga002.fm.intel.com with ESMTP; 13 Jan 2018 05:41:50 -0800 To: Jianfeng Tan , dev@dpdk.org Cc: bruce.richardson@intel.com, konstantin.ananyev@intel.com, thomas@monjalon.net References: <1512067450-59203-1-git-send-email-jianfeng.tan@intel.com> <1515643654-129489-1-git-send-email-jianfeng.tan@intel.com> <1515643654-129489-4-git-send-email-jianfeng.tan@intel.com> From: "Burakov, Anatoly" Message-ID: Date: Sat, 13 Jan 2018 13:41:50 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <1515643654-129489-4-git-send-email-jianfeng.tan@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2 3/4] eal: add synchronous multi-process communication X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jan 2018 13:41:53 -0000 On 11-Jan-18 4:07 AM, Jianfeng Tan wrote: > --- > lib/librte_eal/common/eal_common_proc.c | 144 +++++++++++++++++++++++++++++--- > lib/librte_eal/common/include/rte_eal.h | 73 +++++++++++++++- > lib/librte_eal/rte_eal_version.map | 2 + > 3 files changed, 206 insertions(+), 13 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c > index 70519cc..f194a52 100644 > --- a/lib/librte_eal/common/eal_common_proc.c > +++ b/lib/librte_eal/common/eal_common_proc.c > @@ -32,6 +32,7 @@ > static int mp_fd = -1; > static char *mp_sec_sockets[MAX_SECONDARY_PROCS]; > static pthread_mutex_t mp_mutex_action = PTHREAD_MUTEX_INITIALIZER; > +static pthread_mutex_t mp_mutex_request = PTHREAD_MUTEX_INITIALIZER; > > struct action_entry { > TAILQ_ENTRY(action_entry) next; /**< Next attached action entry */ > @@ -49,6 +50,10 @@ static struct action_entry_list action_entry_list = > > struct mp_msghdr { > char action_name[MAX_ACTION_NAME_LEN]; > +#define MP_MSG 0 /* Share message with peers, will not block */ > +#define MP_REQ 1 /* Request for information, Will block for a reply */ > +#define MP_REP 2 /* Reply to previously-received request */ nitpicking, but... response instead of reply? > + int type; > int fds_num; > int len_params; > char params[0]; > @@ -138,7 +143,8 @@ rte_eal_mp_action_unregister(const char *name) > } > > static int > -read_msg(int fd, char *buf, int buflen, int *fds, int fds_num) > +read_msg(int fd, char *buf, int buflen, > + int *fds, int fds_num, struct sockaddr_un *s) > + return mp_send(action_name, params, len_params, > + fds, fds_num, MP_MSG, NULL); > +} > + > +int > +rte_eal_mp_request(const char *action_name, > + void *params, > + int len_p, > + int fds[], > + int fds_in, > + int fds_out) name == NULL? name too long? > +{ > + int i, j; > + int sockfd; > + int nprocs; > + int ret = 0; > + struct mp_msghdr *req; > + struct timeval tv; > + char buf[MAX_MSG_LENGTH]; > + struct mp_msghdr *hdr; > + > + RTE_LOG(DEBUG, EAL, "request: %s\n", action_name); > + > + if (fds_in > SCM_MAX_FD || fds_out > SCM_MAX_FD) { > + RTE_LOG(ERR, EAL, "Cannot send more than %d FDs\n", SCM_MAX_FD); > + rte_errno = -E2BIG; (this also applies to previous patches) you set rte_errno to -EINVAL in format_msg when message with parameters is too big - should that be setting -E2BIG as well? Also, maybe not set rte_errno in multiple places, and put all parameter checking (or at least errno setting) in rte_eal_mp_* functions? > + return 0; > + } > + > + req = format_msg(action_name, params, len_p, fds_in, MP_REQ); > + if (req == NULL) > + return 0; > + > + if ((sockfd = open_unix_fd(0)) < 0) { > + free(req); > + return 0; > + } > + > + tv.tv_sec = 5; /* 5 Secs Timeout */ > + tv.tv_usec = 0; > + if (setsockopt(sockfd, SOL_SOCKET, SO_RCVTIMEO, > + (const void *)&tv, sizeof(struct timeval)) < 0) > + RTE_LOG(INFO, EAL, "Failed to set recv timeout\n"); > + > + /* Only allow one req at a time */ > + pthread_mutex_lock(&mp_mutex_request); > + > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { > + nprocs = 0; > + for (i = 0; i < MAX_SECONDARY_PROCS; ++i) What follows is a bit confusing, some comments explaining what happens and maybe more informative variable names would've been helpful. > + if (!mp_sec_sockets[i]) { > + j = i; > + nprocs++; > + } > + > + if (nprocs > 1) { > + RTE_LOG(ERR, EAL, > + "multi secondary processes not supported\n"); > + goto free_and_ret; > + } > + -- Thanks, Anatoly