From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 264BE29CF for ; Fri, 6 Jan 2017 15:17:48 +0100 (CET) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga102.fm.intel.com with ESMTP; 06 Jan 2017 06:17:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,325,1477983600"; d="scan'208";a="50823248" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by fmsmga006.fm.intel.com with ESMTP; 06 Jan 2017 06:17:47 -0800 Date: Fri, 6 Jan 2017 22:19:37 +0800 From: Yuanhan Liu To: Thomas Monjalon Cc: Bruce Richardson , dev@dpdk.org, Ferruh Yigit Message-ID: <20170106141937.GZ21228@yliu-dev.sh.intel.com> References: <1482922962-21036-1-git-send-email-yuanhan.liu@linux.intel.com> <1483697780-12088-1-git-send-email-yuanhan.liu@linux.intel.com> <1483697780-12088-2-git-send-email-yuanhan.liu@linux.intel.com> <16135200.seSVc815in@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <16135200.seSVc815in@xps13> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v3 1/6] ethdev: fix port data mismatched in multiple process model 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, 06 Jan 2017 14:17:49 -0000 On Fri, Jan 06, 2017 at 02:12:48PM +0100, Thomas Monjalon wrote: > 2017-01-06 18:16, Yuanhan Liu: > > +static void > > +eth_dev_init(struct rte_eth_dev *eth_dev, uint8_t port_id, const char *name) > > +{ > > + eth_dev->data = &rte_eth_dev_data[port_id]; > > + eth_dev->attached = DEV_ATTACHED; > > + eth_dev_last_created_port = port_id; > > + nb_ports++; > > + > > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { > > + snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), > > + "%s", name); > > + eth_dev->data->port_id = port_id; > > + } > > Why not keeping eth_dev->data filling in rte_eth_dev_allocate? I could do that. > > > +} > > + > > struct rte_eth_dev * > > rte_eth_dev_allocate(const char *name) > > { > > @@ -211,12 +226,41 @@ struct rte_eth_dev * > > } > > > > eth_dev = &rte_eth_devices[port_id]; > > Why not moving this line in eth_dev_init? Ditto. > > > - eth_dev->data = &rte_eth_dev_data[port_id]; > > - snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), "%s", name); > > - eth_dev->data->port_id = port_id; > > - eth_dev->attached = DEV_ATTACHED; > > - eth_dev_last_created_port = port_id; > > - nb_ports++; > > + eth_dev_init(eth_dev, port_id, name); > > + > > + return eth_dev; > > +} > > [...] > > +/* > > + * Attach to a port already registered by the primary process, which > > + * makes sure that the same device would have the same port id both > > + * in the primary and secondary process. > > + */ > > +static struct rte_eth_dev * > > +eth_dev_attach_secondary(const char *name) > > OK, good description > > [...] > > - eth_dev = rte_eth_dev_allocate(ethdev_name); > > - if (eth_dev == NULL) > > - return -ENOMEM; > > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { > > + eth_dev = rte_eth_dev_allocate(ethdev_name); > > + if (eth_dev == NULL) > > + return -ENOMEM; > > You could merge here the rest of primary init below. Oh, right. > > > + } else { > > + eth_dev = eth_dev_attach_secondary(ethdev_name); > > + if (eth_dev == NULL) { > > + /* > > + * if we failed to attach a device, it means > > + * the device is skipped, due to some errors. > > + * Take virtio-net device as example, it could > > + * due to the device is managed by virtio-net > > + * kernel driver. For such case, we return a > > + * positive value, to let EAL skip it as well. > > + */ > > I'm not sure we need an example here. > Is the virtio case special? Not quite sure, likely not. I was thinking a detailed example helps people to understand better. I could remove it if you think it's not necessary. > nit: "it could due" looks to be a typo Oops. --yliu