From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from wes1-so1.wedos.net (wes1-so1.wedos.net [46.28.106.15]) by dpdk.org (Postfix) with ESMTP id 25FC247CD for ; Thu, 14 Jul 2016 18:51:42 +0200 (CEST) Received: from jvn (dynamic-109-81-211-32.ipv4.broadband.iol.cz [109.81.211.32]) by wes1-so1.wedos.net (Postfix) with ESMTPSA id 3rr1v14zVbzBgf; Thu, 14 Jul 2016 18:51:41 +0200 (CEST) Date: Thu, 14 Jul 2016 18:51:48 +0200 From: Jan Viktorin To: Shreyansh Jain Cc: , , Message-ID: <20160714185148.738f1fa3@jvn> In-Reply-To: <1468303282-2806-17-git-send-email-shreyansh.jain@nxp.com> References: <1466510566-9240-1-git-send-email-shreyansh.jain@nxp.com> <1468303282-2806-1-git-send-email-shreyansh.jain@nxp.com> <1468303282-2806-17-git-send-email-shreyansh.jain@nxp.com> Organization: RehiveTech X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-unknown-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v6 16/17] ethdev: convert to eal hotplug X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Jul 2016 16:51:42 -0000 On Tue, 12 Jul 2016 11:31:21 +0530 Shreyansh Jain wrote: > Remove bus logic from ethdev hotplug by using eal for this. > > Current api is preserved: > - the last port that has been created is tracked to return it to the > application when attaching, > - the internal device name is reused when detaching. > > We can not get rid of ethdev hotplug yet since we still need some mechanism > to inform applications of port creation/removal to substitute for ethdev > hotplug api. > > dev_type field in struct rte_eth_dev and rte_eth_dev_allocate are kept as > is, but this information is not needed anymore and is removed in the following > commit. > > Signed-off-by: David Marchand > Signed-off-by: Shreyansh Jain > --- > lib/librte_ether/rte_ethdev.c | 207 +++++++----------------------------------- > 1 file changed, 33 insertions(+), 174 deletions(-) > > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index a667012..8d14fd7 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -72,6 +72,7 @@ > static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data"; > struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS]; > static struct rte_eth_dev_data *rte_eth_dev_data; > +static uint8_t eth_dev_last_created_port; > static uint8_t nb_ports; > [...] > - > /* attach the new device, then store port_id of the device */ > int > rte_eth_dev_attach(const char *devargs, uint8_t *port_id) > { > - struct rte_pci_addr addr; > int ret = -1; > + int current = eth_dev_last_created_port; > + char *name = NULL; > + char *args = NULL; > > if ((devargs == NULL) || (port_id == NULL)) { > ret = -EINVAL; > goto err; > } > > - if (eal_parse_pci_DomBDF(devargs, &addr) == 0) { > - ret = rte_eth_dev_attach_pdev(&addr, port_id); > - if (ret < 0) > - goto err; > - } else { > - ret = rte_eth_dev_attach_vdev(devargs, port_id); > - if (ret < 0) > - goto err; > + /* parse devargs, then retrieve device name and args */ > + if (rte_eal_parse_devargs_str(devargs, &name, &args)) > + goto err; > + > + ret = rte_eal_dev_attach(name, args); > + if (ret < 0) > + goto err; > + > + /* no point looking at eth_dev_last_created_port if no port exists */ I am not sure about this comment. What is "no point"? Isn't this also a potential bug? (Like the one below.) How could it happen there is no port after a successful attach? > + if (!nb_ports) { > + ret = -1; > + goto err; > + } > + /* if nothing happened, there is a bug here, since some driver told us > + * it did attach a device, but did not create a port */ > + if (current == eth_dev_last_created_port) { > + ret = -1; > + goto err; Should we log this? Or call some kind panic? > } > + *port_id = eth_dev_last_created_port; > + ret = 0; > > - return 0; > err: > - RTE_LOG(ERR, EAL, "Driver, cannot attach the device\n"); > + free(name); > + free(args); > return ret; > } > > @@ -590,7 +464,6 @@ err: > int > rte_eth_dev_detach(uint8_t port_id, char *name) > { > - struct rte_pci_addr addr; > int ret = -1; > > if (name == NULL) { > @@ -598,33 +471,19 @@ rte_eth_dev_detach(uint8_t port_id, char *name) > goto err; > } > > - /* check whether the driver supports detach feature, or not */ > + /* FIXME: move this to eal, once device flags are relocated there */ > if (rte_eth_dev_is_detachable(port_id)) > goto err; > > - if (rte_eth_dev_get_device_type(port_id) == RTE_ETH_DEV_PCI) { > - ret = rte_eth_dev_get_addr_by_port(port_id, &addr); > - if (ret < 0) > - goto err; > - > - ret = rte_eth_dev_detach_pdev(port_id, &addr); > - if (ret < 0) > - goto err; > - > - snprintf(name, RTE_ETH_NAME_MAX_LEN, > - "%04x:%02x:%02x.%d", > - addr.domain, addr.bus, > - addr.devid, addr.function); > - } else { > - ret = rte_eth_dev_detach_vdev(port_id, name); > - if (ret < 0) > - goto err; > - } > + snprintf(name, sizeof(rte_eth_devices[port_id].data->name), > + "%s", rte_eth_devices[port_id].data->name); > + ret = rte_eal_dev_detach(name); > + if (ret < 0) > + goto err; > > return 0; > > err: > - RTE_LOG(ERR, EAL, "Driver, cannot detach the device\n"); I'd be more specific about the failing device, we have its name. > return ret; > } >