From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yuanhan Liu <yliu@fridaylinux.org>
Subject: Re: [PATCH v2 12/12] net/vhost: support to run in the
 secondary process
Date: Fri, 29 Sep 2017 16:28:17 +0800
Message-ID: <20170929082817.GN2251@yliu-home>
References: <1503654052-84730-1-git-send-email-jianfeng.tan@intel.com>
 <1506606959-76230-1-git-send-email-jianfeng.tan@intel.com>
 <1506606959-76230-13-git-send-email-jianfeng.tan@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org, bruce.richardson@intel.com, konstantin.ananyev@intel.com,
 pablo.de.lara.guarch@intel.com, thomas@monjalon.net,
 maxime.coquelin@redhat.com, mtetsuyah@gmail.com, ferruh.yigit@intel.com
To: Jianfeng Tan <jianfeng.tan@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-pf0-f175.google.com (mail-pf0-f175.google.com
 [209.85.192.175]) by dpdk.org (Postfix) with ESMTP id 68F821B1B8
 for <dev@dpdk.org>; Fri, 29 Sep 2017 10:28:26 +0200 (CEST)
Received: by mail-pf0-f175.google.com with SMTP id l188so397817pfc.6
 for <dev@dpdk.org>; Fri, 29 Sep 2017 01:28:26 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <1506606959-76230-13-git-send-email-jianfeng.tan@intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Thu, Sep 28, 2017 at 01:55:59PM +0000, Jianfeng Tan wrote:
>  static int
> +share_device(int vid)
> +{
> +	uint32_t i, vring_num;
> +	int len;
> +	int fds[8];
> +	struct rte_vhost_memory *mem;
> +	struct vhost_params *params;
> +	struct rte_vhost_vring vring;
> +
> +	/* share mem table */
> +	if (rte_vhost_get_mem_table(vid, &mem) < 0) {
> +		RTE_LOG(ERR, PMD, "Failed to get mem table\n");
> +		return 0;
> +	}
> +	for (i = 0; i < mem->nregions; ++i)
> +		fds[i] = mem->regions[i].fd;
> +
> +	len = sizeof(struct rte_vhost_mem_region) * mem->nregions;
> +	params = malloc(sizeof(*params) + len);
> +	if (params == NULL) {
> +		RTE_LOG(ERR, PMD, "Failed to allocate memory\n");
> +		return -1;
> +	}
> +
> +	params->type = VHOST_MSG_TYPE_REGIONS;
> +	params->vid = vid;
> +	memcpy(params->regions, mem->regions, len);
> +
> +	if (rte_eal_mp_sendmsg("vhost pmd", params, sizeof(*params) + len,

To me, it's not a good idea to identify an object by a string. The common
practice is to use a handler, which could either be a struct or a nubmer.

> +			       fds, mem->nregions) < 0) {
> +		RTE_LOG(ERR, PMD, "Failed to share mem table\n");
> +		free(params);
> +		return -1;
> +	}
> +
> +	/* share callfd and kickfd */
> +	params->type = VHOST_MSG_TYPE_SET_FDS;
> +	vring_num = rte_vhost_get_vring_num(vid);
> +	for (i = 0; i < vring_num; i++) {
> +		if (rte_vhost_get_vhost_vring(vid, i, &vring) < 0) {

If you save the fds here, you don't have to get it every time when there
is a new secondary process attached. Then as I have suggested firstly,
you don't have to introduce callfd_pri in the vhost lib.

	--yliu