From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-f169.google.com (mail-pd0-f169.google.com [209.85.192.169]) by dpdk.org (Postfix) with ESMTP id C3D37CE7 for ; Wed, 11 Feb 2015 03:51:25 +0100 (CET) Received: by pdjy10 with SMTP id y10so1041128pdj.13 for ; Tue, 10 Feb 2015 18:51:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=fgNVKCSpuO2i9q58h0MsvKuqsPFKiAJlSePIEfdnbCQ=; b=cLJqn+OzYNoGyuqmh5lrrkElUeT3fpPHxr0kP9JTmR0db2S+FLWopFdF8UnTCsodA6 5lgpzwHs3wcWX4O2oAzikIpmKEbFzHPwlqEN1xCxZtaApyy8LUXl0Exwf11FheU7hfxa Y9ad3jUB9vXgVpLY4UCFy02iNL7GI0SVSqgrNKD4pXgcUxC1Xpj3ttR/uyxQvYxDJtY0 Vx5mZ3yA0vBH+kbEc3kBh+OlQYeZtEDDA0N3SjVyBk/U3AadzyZmwpuRqXHnQ1cZ6iZ+ TVTtAxwmLmufjx+G1NrGp+fGgnyjLvjHqD6E7+RVbPhRftt0XY1klJxzi+ulSBtj+Pgq beDQ== X-Gm-Message-State: ALoCoQmtTlJ/zvR3Mz/vj6Hku+mm8rSS40Lzn5626NHjvE3XFwz11fg5gJ1EXBR3Lyt/V1iSVic9 X-Received: by 10.68.57.199 with SMTP id k7mr40155379pbq.163.1423623085097; Tue, 10 Feb 2015 18:51:25 -0800 (PST) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id ex3sm16981872pdb.23.2015.02.10.18.51.23 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 10 Feb 2015 18:51:24 -0800 (PST) Message-ID: <54DAC3A8.1000704@igel.co.jp> Date: Wed, 11 Feb 2015 11:51:20 +0900 From: Tetsuya Mukawa User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: "Iremonger, Bernard" , "dev@dpdk.org" References: <1422763322-13742-4-git-send-email-mukawa@igel.co.jp> <1423470639-15744-1-git-send-email-mukawa@igel.co.jp> <1423470639-15744-5-git-send-email-mukawa@igel.co.jp> <533710CFB86FA344BFBF2D6802E60286CE71EA@SHSMSX101.ccr.corp.intel.com> <8CEF83825BEC744B83065625E567D7C2049DF5CA@IRSMSX108.ger.corp.intel.com> In-Reply-To: <8CEF83825BEC744B83065625E567D7C2049DF5CA@IRSMSX108.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v7 04/14] eal/pci: Consolidate pci address comparison APIs 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, 11 Feb 2015 02:51:26 -0000 On 2015/02/11 0:08, Iremonger, Bernard wrote: >> -----Original Message----- >> From: Qiu, Michael >> Sent: Monday, February 9, 2015 1:10 PM >> To: Tetsuya Mukawa; dev@dpdk.org >> Cc: Iremonger, Bernard >> Subject: Re: [PATCH v7 04/14] eal/pci: Consolidate pci address comparison APIs >> >> On 2/9/2015 4:31 PM, Tetsuya Mukawa wrote: >>> This patch replaces pci_addr_comparison() and memcmp() of pci >>> addresses by eal_compare_pci_addr(). >>> >>> v5: >>> - Fix pci_scan_one to handle pt_driver correctly. >>> v4: >>> - Fix calculation method of eal_compare_pci_addr(). >>> - Add parameter checking. >>> >>> Signed-off-by: Tetsuya Mukawa >>> --- >>> lib/librte_eal/bsdapp/eal/eal_pci.c | 25 ++++++++--------------- >>> lib/librte_eal/common/eal_common_pci.c | 2 +- >>> lib/librte_eal/common/include/rte_pci.h | 34 +++++++++++++++++++++++++++++++ >>> lib/librte_eal/linuxapp/eal/eal_pci.c | 25 ++++++++--------------- >>> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 2 +- >>> 5 files changed, 54 insertions(+), 34 deletions(-) >>> >>> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c >>> b/lib/librte_eal/bsdapp/eal/eal_pci.c >>> index 74ecce7..c844d58 100644 >>> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c >>> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c >>> @@ -270,20 +270,6 @@ pci_uio_map_resource(struct rte_pci_device *dev) >>> return (0); >>> } >>> >>> -/* Compare two PCI device addresses. */ -static int >>> -pci_addr_comparison(struct rte_pci_addr *addr, struct rte_pci_addr >>> *addr2) -{ >>> - uint64_t dev_addr = (addr->domain << 24) + (addr->bus << 16) + (addr->devid << 8) + addr- >>> function; >>> - uint64_t dev_addr2 = (addr2->domain << 24) + (addr2->bus << 16) + (addr2->devid << 8) + >> addr2->function; >>> - >>> - if (dev_addr > dev_addr2) >>> - return 1; >>> - else >>> - return 0; >>> -} >>> - >>> - >>> /* Scan one pci sysfs entry, and fill the devices list from it. */ >>> static int pci_scan_one(int dev_pci_fd, struct pci_conf *conf) @@ >>> -356,13 +342,20 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf) >>> } >>> else { >>> struct rte_pci_device *dev2 = NULL; >>> + int ret; >>> >>> TAILQ_FOREACH(dev2, &pci_device_list, next) { >>> - if (pci_addr_comparison(&dev->addr, &dev2->addr)) >>> + ret = eal_compare_pci_addr(&dev->addr, &dev2->addr); >>> + if (ret > 0) >>> continue; >>> - else { >>> + else if (ret < 0) { >>> TAILQ_INSERT_BEFORE(dev2, dev, next); >>> return 0; >>> + } else { /* already registered */ >>> + /* update pt_driver */ >>> + dev2->pt_driver = dev->pt_driver; >>> + free(dev); >>> + return 0; >>> } >>> } >>> TAILQ_INSERT_TAIL(&pci_device_list, dev, next); diff --git >>> a/lib/librte_eal/common/eal_common_pci.c >>> b/lib/librte_eal/common/eal_common_pci.c >>> index f3c7f71..a89f5c3 100644 >>> --- a/lib/librte_eal/common/eal_common_pci.c >>> +++ b/lib/librte_eal/common/eal_common_pci.c >>> @@ -93,7 +93,7 @@ static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev) >>> if (devargs->type != RTE_DEVTYPE_BLACKLISTED_PCI && >>> devargs->type != RTE_DEVTYPE_WHITELISTED_PCI) >>> continue; >>> - if (!memcmp(&dev->addr, &devargs->pci.addr, sizeof(dev->addr))) >>> + if (!eal_compare_pci_addr(&dev->addr, &devargs->pci.addr)) >>> return devargs; >>> } >>> return NULL; >>> diff --git a/lib/librte_eal/common/include/rte_pci.h >>> b/lib/librte_eal/common/include/rte_pci.h >>> index 7f2d699..4814cd7 100644 >>> --- a/lib/librte_eal/common/include/rte_pci.h >>> +++ b/lib/librte_eal/common/include/rte_pci.h >>> @@ -269,6 +269,40 @@ eal_parse_pci_DomBDF(const char *input, struct >>> rte_pci_addr *dev_addr) } #undef GET_PCIADDR_FIELD >>> >>> +/* Compare two PCI device addresses. */ >>> +/** >>> + * Utility function to compare two PCI device addresses. >>> + * >>> + * @param addr >>> + * The PCI Bus-Device-Function address to compare >>> + * @param addr2 >>> + * The PCI Bus-Device-Function address to compare >>> + * @return >>> + * 0 on equal PCI address. >>> + * Positive on addr is greater than addr2. >>> + * Negative on addr is less than addr2, or error. >>> + */ >>> +static inline int >>> +eal_compare_pci_addr(struct rte_pci_addr *addr, struct rte_pci_addr >>> +*addr2) { >>> + uint64_t dev_addr, dev_addr2; >>> + >>> + if ((addr == NULL) || (addr2 == NULL)) >>> + return -1; >>> + >>> + dev_addr = (addr->domain << 24) | (addr->bus << 16) | >>> + (addr->devid << 8) | addr->function; >>> + dev_addr2 = (addr2->domain << 24) | (addr2->bus << 16) | >>> + (addr2->devid << 8) | addr2->function; >>> + >>> + if (dev_addr > dev_addr2) >>> + return 1; >>> + else if (dev_addr < dev_addr2) >>> + return -1; >>> + else >>> + return 0; >>> +} >>> + >>> /** >>> * Probe the PCI bus for registered drivers. >>> * >>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c >>> b/lib/librte_eal/linuxapp/eal/eal_pci.c >>> index c0ca5a5..d847102 100644 >>> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c >>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c >>> @@ -229,20 +229,6 @@ error: >>> return -1; >>> } >>> >>> -/* Compare two PCI device addresses. */ -static int >>> -pci_addr_comparison(struct rte_pci_addr *addr, struct rte_pci_addr >>> *addr2) -{ >>> - uint64_t dev_addr = (addr->domain << 24) + (addr->bus << 16) + (addr->devid << 8) + addr- >>> function; >>> - uint64_t dev_addr2 = (addr2->domain << 24) + (addr2->bus << 16) + (addr2->devid << 8) + >> addr2->function; >>> - >>> - if (dev_addr > dev_addr2) >>> - return 1; >>> - else >>> - return 0; >>> -} >>> - >>> - >>> /* Scan one pci sysfs entry, and fill the devices list from it. */ >>> static int pci_scan_one(const char *dirname, uint16_t domain, uint8_t >>> bus, @@ -353,13 +339,20 @@ pci_scan_one(const char *dirname, uint16_t >>> domain, uint8_t bus, >>> } >>> else { >>> struct rte_pci_device *dev2 = NULL; >>> + int ret; >>> >>> TAILQ_FOREACH(dev2, &pci_device_list, next) { >>> - if (pci_addr_comparison(&dev->addr, &dev2->addr)) >>> + ret = eal_compare_pci_addr(&dev->addr, &dev2->addr); >>> + if (ret > 0) >>> continue; >>> - else { >>> + else if (ret < 0) { >>> TAILQ_INSERT_BEFORE(dev2, dev, next); >>> return 0; >>> + } else { /* already registered */ >>> + /* update pt_driver */ >>> + dev2->pt_driver = dev->pt_driver; > Hi Tetsuya, > > I am seeing a problem with the librte_pmd_ixgbe code where dev->max_vfs is being lost in some scenarios. > The following line should be added here: > dev2->max_vfs = dev->max_vfs; > > numa_mode should probably be updated too (although it is not causing a problem at present). > dev2->numa_mode = dev->numa_mode; Hi Bernard, Thanks for reporting. I will add above 2 lines. Regards, Tetsuya > Regards, > > Bernard. > > > > >>> + free(dev); >>> + return 0; >>> } >>> } >>> TAILQ_INSERT_TAIL(&pci_device_list, dev, next); diff --git >>> a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >>> b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >>> index e53f06b..1da3507 100644 >>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >>> @@ -123,7 +123,7 @@ pci_uio_map_secondary(struct rte_pci_device *dev) >>> TAILQ_FOREACH(uio_res, pci_res_list, next) { >>> >>> /* skip this element if it doesn't match our PCI address */ >>> - if (memcmp(&uio_res->pci_addr, &dev->addr, sizeof(dev->addr))) >>> + if (eal_compare_pci_addr(&uio_res->pci_addr, &dev->addr)) >>> continue; >>> >>> for (i = 0; i != uio_res->nb_maps; i++) { >> Acked-by: Michael Qiu