[dpdk-dev,v3,03/28] eal/pci: Replace pci address comparison code by eal_compare_pci_addr

Message ID 1418106629-22227-4-git-send-email-mukawa@igel.co.jp (mailing list archive)
State Superseded, archived
Headers

Commit Message

Tetsuya Mukawa Dec. 9, 2014, 6:30 a.m. UTC
  This patch replaces pci_addr_comparison() and memcmp() of pci addresses by
eal_compare_pci_addr().

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c       | 16 +---------------
 lib/librte_eal/common/eal_common_pci.c    |  2 +-
 lib/librte_eal/common/include/rte_pci.h   | 29 +++++++++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci.c     | 16 +---------------
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c |  2 +-
 5 files changed, 33 insertions(+), 32 deletions(-)
  

Comments

Michael Qiu Dec. 9, 2014, 2:22 p.m. UTC | #1
On 2014/12/9 14:32, Tetsuya Mukawa wrote:
> This patch replaces pci_addr_comparison() and memcmp() of pci addresses by
> eal_compare_pci_addr().
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  lib/librte_eal/bsdapp/eal/eal_pci.c       | 16 +---------------
>  lib/librte_eal/common/eal_common_pci.c    |  2 +-
>  lib/librte_eal/common/include/rte_pci.h   | 29 +++++++++++++++++++++++++++++
>  lib/librte_eal/linuxapp/eal/eal_pci.c     | 16 +---------------
>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c |  2 +-
>  5 files changed, 33 insertions(+), 32 deletions(-)
>
> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
> index 74ecce7..7eda513 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)
> @@ -358,7 +344,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
>  		struct rte_pci_device *dev2 = NULL;
>  
>  		TAILQ_FOREACH(dev2, &pci_device_list, next) {
> -			if (pci_addr_comparison(&dev->addr, &dev2->addr))
> +			if (eal_compare_pci_addr(&dev->addr, &dev2->addr))
>  				continue;
>  			else {
>  				TAILQ_INSERT_BEFORE(dev2, dev, next);
> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
> index f3c7f71..f01f258 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 b819539..fe374a8 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -261,6 +261,35 @@ 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.
> + */
> +static inline int
> +eal_compare_pci_addr(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;
> +

Here I prefer:

uint64_t dev_addr = (addr->domain << 24) | (addr->bus << 16) | (addr->devid << 8) | addr->function;

Thanks,
Michael
> +	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 b5f5410..23a69e9 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -200,20 +200,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,
> @@ -306,7 +292,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
>  		struct rte_pci_device *dev2 = NULL;
>  
>  		TAILQ_FOREACH(dev2, &pci_device_list, next) {
> -			if (pci_addr_comparison(&dev->addr, &dev2->addr))
> +			if (eal_compare_pci_addr(&dev->addr, &dev2->addr) != 0)
>  				continue;
>  			else {
>  				TAILQ_INSERT_BEFORE(dev2, 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++) {
  
Tetsuya Mukawa Dec. 11, 2014, 3:11 a.m. UTC | #2
Hi Michael,

(2014/12/09 23:22), Qiu, Michael wrote:
> On 2014/12/9 14:32, Tetsuya Mukawa wrote:
>> This patch replaces pci_addr_comparison() and memcmp() of pci addresses by
>> eal_compare_pci_addr().
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>> ---
>>  lib/librte_eal/bsdapp/eal/eal_pci.c       | 16 +---------------
>>  lib/librte_eal/common/eal_common_pci.c    |  2 +-
>>  lib/librte_eal/common/include/rte_pci.h   | 29 +++++++++++++++++++++++++++++
>>  lib/librte_eal/linuxapp/eal/eal_pci.c     | 16 +---------------
>>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c |  2 +-
>>  5 files changed, 33 insertions(+), 32 deletions(-)
>>
>> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
>> index 74ecce7..7eda513 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)
>> @@ -358,7 +344,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
>>  		struct rte_pci_device *dev2 = NULL;
>>  
>>  		TAILQ_FOREACH(dev2, &pci_device_list, next) {
>> -			if (pci_addr_comparison(&dev->addr, &dev2->addr))
>> +			if (eal_compare_pci_addr(&dev->addr, &dev2->addr))
>>  				continue;
>>  			else {
>>  				TAILQ_INSERT_BEFORE(dev2, dev, next);
>> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
>> index f3c7f71..f01f258 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 b819539..fe374a8 100644
>> --- a/lib/librte_eal/common/include/rte_pci.h
>> +++ b/lib/librte_eal/common/include/rte_pci.h
>> @@ -261,6 +261,35 @@ 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.
>> + */
>> +static inline int
>> +eal_compare_pci_addr(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;
>> +
> Here I prefer:
>
> uint64_t dev_addr = (addr->domain << 24) | (addr->bus << 16) | (addr->devid << 8) | addr->function;

Sure, I will change like above.

Thanks,
Tetsuya

>
> Thanks,
> Michael
>> +	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 b5f5410..23a69e9 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
>> @@ -200,20 +200,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,
>> @@ -306,7 +292,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
>>  		struct rte_pci_device *dev2 = NULL;
>>  
>>  		TAILQ_FOREACH(dev2, &pci_device_list, next) {
>> -			if (pci_addr_comparison(&dev->addr, &dev2->addr))
>> +			if (eal_compare_pci_addr(&dev->addr, &dev2->addr) != 0)
>>  				continue;
>>  			else {
>>  				TAILQ_INSERT_BEFORE(dev2, 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++) {
  

Patch

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 74ecce7..7eda513 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)
@@ -358,7 +344,7 @@  pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 		struct rte_pci_device *dev2 = NULL;
 
 		TAILQ_FOREACH(dev2, &pci_device_list, next) {
-			if (pci_addr_comparison(&dev->addr, &dev2->addr))
+			if (eal_compare_pci_addr(&dev->addr, &dev2->addr))
 				continue;
 			else {
 				TAILQ_INSERT_BEFORE(dev2, dev, next);
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index f3c7f71..f01f258 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 b819539..fe374a8 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -261,6 +261,35 @@  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.
+ */
+static inline int
+eal_compare_pci_addr(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 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 b5f5410..23a69e9 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -200,20 +200,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,
@@ -306,7 +292,7 @@  pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 		struct rte_pci_device *dev2 = NULL;
 
 		TAILQ_FOREACH(dev2, &pci_device_list, next) {
-			if (pci_addr_comparison(&dev->addr, &dev2->addr))
+			if (eal_compare_pci_addr(&dev->addr, &dev2->addr) != 0)
 				continue;
 			else {
 				TAILQ_INSERT_BEFORE(dev2, 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++) {