From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id C0BE02C7A for ; Tue, 10 Feb 2015 16:10:45 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP; 10 Feb 2015 07:04:56 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,551,1418112000"; d="scan'208";a="452619285" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by FMSMGA003.fm.intel.com with ESMTP; 10 Feb 2015 06:54:05 -0800 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.64]) by IRSMSX101.ger.corp.intel.com ([169.254.1.244]) with mapi id 14.03.0195.001; Tue, 10 Feb 2015 15:08:36 +0000 From: "Iremonger, Bernard" To: Tetsuya Mukawa , "dev@dpdk.org" Thread-Topic: [PATCH v7 04/14] eal/pci: Consolidate pci address comparison APIs Thread-Index: AQHQRELJadkSvYPLqkeLS8tHXgQYqpzp/Hkg Date: Tue, 10 Feb 2015 15:08:35 +0000 Message-ID: <8CEF83825BEC744B83065625E567D7C2049DF5CA@IRSMSX108.ger.corp.intel.com> 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> In-Reply-To: <533710CFB86FA344BFBF2D6802E60286CE71EA@SHSMSX101.ccr.corp.intel.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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: Tue, 10 Feb 2015 15:10:46 -0000 > -----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 >=20 > 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 =3D (addr->domain << 24) + (addr->bus << 16) + (add= r->devid << 8) + addr- > >function; > > - uint64_t dev_addr2 =3D (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 =3D NULL; > > + int ret; > > > > TAILQ_FOREACH(dev2, &pci_device_list, next) { > > - if (pci_addr_comparison(&dev->addr, &dev2->addr)) > > + ret =3D 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 =3D 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 !=3D RTE_DEVTYPE_BLACKLISTED_PCI && > > devargs->type !=3D 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 =3D=3D NULL) || (addr2 =3D=3D NULL)) > > + return -1; > > + > > + dev_addr =3D (addr->domain << 24) | (addr->bus << 16) | > > + (addr->devid << 8) | addr->function; > > + dev_addr2 =3D (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 =3D (addr->domain << 24) + (addr->bus << 16) + (add= r->devid << 8) + addr- > >function; > > - uint64_t dev_addr2 =3D (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 =3D NULL; > > + int ret; > > > > TAILQ_FOREACH(dev2, &pci_device_list, next) { > > - if (pci_addr_comparison(&dev->addr, &dev2->addr)) > > + ret =3D 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 =3D 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 =3D dev->max_vfs; numa_mode should probably be updated too (although it is not causing a prob= lem at present). dev2->numa_mode =3D dev->numa_mode; 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 =3D 0; i !=3D uio_res->nb_maps; i++) { > Acked-by: Michael Qiu