From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id C76D9CFBC for ; Fri, 20 Apr 2018 16:28:18 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Apr 2018 07:28:17 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,301,1520924400"; d="scan'208";a="52332882" Received: from tanjianf-mobl.ccr.corp.intel.com (HELO [10.249.235.3]) ([10.249.235.3]) by orsmga002.jf.intel.com with ESMTP; 20 Apr 2018 07:28:16 -0700 To: "Burakov, Anatoly" , dev@dpdk.org References: <1520177405-59091-1-git-send-email-jianfeng.tan@intel.com> <1524156618-81402-1-git-send-email-jianfeng.tan@intel.com> <1524156618-81402-4-git-send-email-jianfeng.tan@intel.com> <59ed38b8-6ee5-5bc9-089c-c9a437c030c1@intel.com> Cc: thomas@monjalon.net From: "Tan, Jianfeng" Message-ID: <9553c783-9d56-786e-87ed-6fc96982e251@intel.com> Date: Fri, 20 Apr 2018 22:28:15 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <59ed38b8-6ee5-5bc9-089c-c9a437c030c1@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 3/5] bus/vdev: bus scan by multi-process channel 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: Fri, 20 Apr 2018 14:28:19 -0000 On 4/20/2018 4:41 PM, Burakov, Anatoly wrote: > On 19-Apr-18 5:50 PM, Jianfeng Tan wrote: >> To scan the vdevs in primary, we send request to primary process >> to obtain the names for vdevs. >> >> Only the name is shared from the primary. In probe(), the device >> driver is supposed to locate (or request more) the detail >> information from the primary. >> >> Signed-off-by: Jianfeng Tan >> Reviewed-by: Qi Zhang >> --- > > <...> > >> +static int >> +vdev_action(const struct rte_mp_msg *mp_msg, const void *peer) >> +{ >> + struct rte_vdev_device *dev; >> + struct rte_mp_msg mp_resp; >> + struct vdev_param *ou = (struct vdev_param *)&mp_resp.param; >> + const struct vdev_param *in = (const struct vdev_param >> *)mp_msg->param; >> + const char *devname; >> + int num; >> + >> + strcpy(mp_resp.name, "vdev"); >> + mp_resp.len_param = sizeof(*ou); >> + mp_resp.num_fds = 0; >> + >> + switch (in->type) { >> + case VDEV_SCAN_REQ: >> + ou->type = VDEV_SCAN_ONE; >> + ou->num = 1; >> + num = 0; >> + >> + rte_spinlock_lock(&vdev_device_list_lock); >> + TAILQ_FOREACH(dev, &vdev_device_list, next) { >> + devname = rte_vdev_device_name(dev); >> + if (strlen(devname) == 0) >> + VDEV_LOG(INFO, "vdev with no name is not sent"); >> + VDEV_LOG(INFO, "send vdev, %s", devname); >> + strncpy(ou->name, devname, RTE_DEV_NAME_MAX_LEN); > > Probably better use strlcpy as it always null-terminates. Yep. > >> + if (rte_mp_sendmsg(&mp_resp) < 0) >> + VDEV_LOG(ERR, "send vdev, %s, failed, %s", >> + devname, strerror(rte_errno)); >> + num++; > > Some comments on what is going on here (why are we sending messages in > response? why multiple? who will receive these messages?) would be nice. Yep, will explain that below. > I have a sneaking suspicion that you could've packed the response into > one single message, but i'm not completely sure what is going on here, > so maybe what you have here makes sense... What's happening here is that: a. Secondary process sends a sync request to ask for vdev in primary. b. Primary process receives the request, and send vdevs one by one. c. Primary process sends back reply, which indicates how many vdevs are sent. The reason we don't pack all vdevs in the reply message is that, the message length is RTE_MP_MAX_PARAM_LEN (256) in length. It's possible that we cannot pack all vdevs in the single reply message. Thanks, Jianfeng >> + } >> + rte_spinlock_unlock(&vdev_device_list_lock); >> + >> + ou->type = VDEV_SCAN_REP; >> + ou->num = num; >> + if (rte_mp_reply(&mp_resp, peer) < 0) >> + VDEV_LOG(ERR, "Failed to reply a scan request"); >> + break; > > <...> >