From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from wes1-so2.wedos.net (wes1-so2.wedos.net [46.28.106.16]) by dpdk.org (Postfix) with ESMTP id E642C2E83 for ; Wed, 10 Feb 2016 12:08:57 +0100 (CET) Received: from pcviktorin.fit.vutbr.cz (pcviktorin.fit.vutbr.cz [147.229.13.147]) by wes1-so2.wedos.net (Postfix) with ESMTPSA id 3q0dd554fDztt; Wed, 10 Feb 2016 12:08:57 +0100 (CET) Date: Wed, 10 Feb 2016 12:10:20 +0100 From: Jan Viktorin To: David Marchand Message-ID: <20160210121020.7389c33f@pcviktorin.fit.vutbr.cz> In-Reply-To: <1454076516-21591-8-git-send-email-david.marchand@6wind.com> References: <1454076516-21591-1-git-send-email-david.marchand@6wind.com> <1454076516-21591-8-git-send-email-david.marchand@6wind.com> Organization: RehiveTech MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 7/9] pci: add a helper for device name 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: Wed, 10 Feb 2016 11:08:58 -0000 On Fri, 29 Jan 2016 15:08:34 +0100 David Marchand wrote: > eal is a better place than ethdev for naming resources. > Add a helper here, and make use of it in ethdev hotplug code. > > Signed-off-by: David Marchand > --- > lib/librte_eal/common/include/rte_pci.h | 28 ++++++++++++++++++++++++++++ > lib/librte_ether/rte_ethdev.c | 22 ++-------------------- > 2 files changed, 30 insertions(+), 20 deletions(-) > > diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h > index 334c12e..9edd5f5 100644 > --- a/lib/librte_eal/common/include/rte_pci.h > +++ b/lib/librte_eal/common/include/rte_pci.h > @@ -309,6 +309,34 @@ eal_parse_pci_DomBDF(const char *input, struct rte_pci_addr *dev_addr) > } > #undef GET_PCIADDR_FIELD > > +/** > + * Utility function to write a pci device name, this device name can later be > + * used to retrieve the corresponding rte_pci_addr using above functions. > + * > + * @param addr > + * The PCI Bus-Device-Function address > + * @param output > + * The output buffer string > + * @param size > + * The output buffer size > + * @return > + * 0 on success, negative on error. > + */ > +static inline int > +eal_pci_device_name(const struct rte_pci_addr *addr, > + char *output, int size) "size_t size" (or unsigned int) seems to be better to me. > +{ > + int ret; > + > + ret = snprintf(output, size, PCI_PRI_FMT, > + addr->domain, addr->bus, > + addr->devid, addr->function); > + if (ret < 0 || ret >= size) > + return -1; The return value is not checked (below). I think, such functions are usually missing error checking as nobody expects them to fail. Wouldn't it be better to panic here? > + > + return 0; > +} > + > /* Compare two PCI device addresses. */ > /** > * Utility function to compare two PCI device addresses. > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index 17e4f4d..5ba7479 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -214,20 +214,6 @@ rte_eth_dev_allocate(const char *name, enum rte_eth_dev_type type) > return eth_dev; > } > > -static int > -rte_eth_dev_create_unique_device_name(char *name, size_t size, > - struct rte_pci_device *pci_dev) > -{ > - int ret; > - > - ret = snprintf(name, size, "%d:%d.%d", > - pci_dev->addr.bus, pci_dev->addr.devid, > - pci_dev->addr.function); > - if (ret < 0) > - return ret; > - return 0; > -} > - > int > rte_eth_dev_release_port(struct rte_eth_dev *eth_dev) > { > @@ -251,9 +237,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv, > > eth_drv = (struct eth_driver *)pci_drv; > > - /* Create unique Ethernet device name using PCI address */ > - rte_eth_dev_create_unique_device_name(ethdev_name, > - sizeof(ethdev_name), pci_dev); > + eal_pci_device_name(&pci_dev->addr, ethdev_name, sizeof(ethdev_name)); > > eth_dev = rte_eth_dev_allocate(ethdev_name, RTE_ETH_DEV_PCI); > if (eth_dev == NULL) > @@ -304,9 +288,7 @@ rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev) > if (pci_dev == NULL) > return -EINVAL; > > - /* Create unique Ethernet device name using PCI address */ > - rte_eth_dev_create_unique_device_name(ethdev_name, > - sizeof(ethdev_name), pci_dev); > + eal_pci_device_name(&pci_dev->addr, ethdev_name, sizeof(ethdev_name)); > > eth_dev = rte_eth_dev_allocated(ethdev_name); > if (eth_dev == NULL) -- Jan Viktorin E-mail: Viktorin@RehiveTech.com System Architect Web: www.RehiveTech.com RehiveTech Brno, Czech Republic