2015-02-16 13:14, Tetsuya Mukawa:
> This patch replaces pci_addr_comparison() and memcmp() of pci addresses by
> eal_compare_pci_addr().
> 
> v8:
> - Fix pci_scan_one() to update sysfs values.
>   (Thanks to Qiu, Michael and Iremonger, Bernard)
> 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 <mukawa at igel.co.jp>
> ---
>  lib/librte_eal/bsdapp/eal/eal_pci.c       | 29 ++++++++++++--------------
>  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     | 30 +++++++++++++--------------
>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c |  2 +-
>  5 files changed, 63 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..7dbdccd 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,24 @@ 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;
> +                             dev2->max_vfs = dev->max_vfs;
> +                             memmove(dev2->mem_resource,
> +                                     dev->mem_resource,
> +                                     sizeof(dev->mem_resource));
> +                             free(dev);
> +                             return 0;

Could you comment this "else part" please?

[...]
> --- 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)

I think that this function should be prefixed with rte_

[...]
>               /* 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))

Why memcmp is not sufficient to compare PCI addresses?
The only exception I see is endianness for natural sorting.

>                       continue;
>  
>               for (i = 0; i != uio_res->nb_maps; i++) {
> 

Reply via email to