[dpdk-dev] [PATCH v10 11/25] eal/pci: helpers for device name parsing/update

2016-10-26 Thread Shreyansh Jain
Hello Reshma,

On Tuesday 25 October 2016 09:19 PM, Pattan, Reshma wrote:
> Hi Shreyansh,
>
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Shreyansh Jain
>> Sent: Friday, September 16, 2016 5:30 AM
>> To: dev at dpdk.org
>> Cc: viktorin at rehivetech.com; David Marchand ;
>> hemant.agrawal at nxp.com; Thomas Monjalon
>> ; Shreyansh Jain 
>> Subject: [dpdk-dev] [PATCH v10 11/25] eal/pci: helpers for device name
>> parsing/update
>>
>> From: David Marchand 
>>
>> - Move rte_eth_dev_create_unique_device_name() from ether/rte_ethdev.c to
>>   common/include/rte_pci.h as rte_eal_pci_device_name(). Being a common
>>   method, can be used across crypto/net PCI PMDs.
>> - Remove crypto specific routine and fallback to common name function.
>> - Introduce a eal private Update function for PCI device naming.
>>
>> Signed-off-by: David Marchand 
>> [Shreyansh: Merge crypto/pci helper patches]
>> Signed-off-by: Shreyansh Jain 
>> ---
>>  lib/librte_cryptodev/rte_cryptodev.c| 27 +++---
>>  lib/librte_eal/bsdapp/eal/eal_pci.c | 49
>> +
>>  lib/librte_eal/common/eal_private.h | 13 +
>>  lib/librte_eal/common/include/rte_pci.h | 24 
>>  lib/librte_eal/linuxapp/eal/eal_pci.c   | 13 +
>>  lib/librte_ether/rte_ethdev.c   | 24 +++-
>>  6 files changed, 107 insertions(+), 43 deletions(-)
>>
>> diff --git a/lib/librte_cryptodev/rte_cryptodev.c
>> b/lib/librte_cryptodev/rte_cryptodev.c
>> index 2a3b649..c81e366 100644
>> --- a/lib/librte_cryptodev/rte_cryptodev.c
>> +++ b/lib/librte_cryptodev/rte_cryptodev.c
>> @@ -365,23 +365,6 @@ rte_cryptodev_pmd_allocate(const char *name, int
>> socket_id)
>>  return cryptodev;
>>  }
>>
>>   *
>>   * This function is private to EAL.
>> diff --git a/lib/librte_eal/common/include/rte_pci.h
>> b/lib/librte_eal/common/include/rte_pci.h
>> index cf81898..e1f695f 100644
>> --- a/lib/librte_eal/common/include/rte_pci.h
>> +++ b/lib/librte_eal/common/include/rte_pci.h
>> @@ -82,6 +82,7 @@ extern "C" {
>>  /** Formatting string for PCI device identifier: Ex: :00:01.0 */  
>> #define
>> PCI_PRI_FMT "%.4" PRIx16 ":%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8
>> +#define PCI_PRI_STR_SIZE sizeof(":XX:XX.X")
>>
>>  /** Short formatting string, without domain, for PCI device: Ex: 00:01.0 */
>> #define PCI_SHORT_PRI_FMT "%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8 @@ -308,6
>>
>> +static inline void
>> +rte_eal_pci_device_name(const struct rte_pci_addr *addr,
>> +char *output, size_t size)
>> +{
>> +RTE_VERIFY(size >= PCI_PRI_STR_SIZE);
>> +RTE_VERIFY(snprintf(output, size, PCI_PRI_FMT,
>> +addr->domain, addr->bus,
>> +addr->devid, addr->function) >= 0); }
>> +
>>
>> +int
>> +pci_update_device(const struct rte_pci_addr *addr) {
>> +char filename[PATH_MAX];
>> +
>> +snprintf(filename, sizeof(filename), "%s/" PCI_PRI_FMT,
>> + pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
>> + addr->function);
>> +
>> +return pci_scan_one(filename, addr->domain, addr->bus, addr->devid,
>> +addr->function);
>> +}
>> +
>
>
> Earlier device names were created in the format "bus:deviceid.function" as 
> per the below ethdev API.
> Now after above new eal API the name format is "domain:bus:deviceid.func" was 
> that intentional  and why is that so.

Yes, this is intentional.
It is to bring the naming in sync with the device name being used for 
scanning on the bus (/sys/bus/pci/devices/:BB:CC.D/).
Also, it was proposed in a separate patch [1] but merged in this series.

[1] http://dpdk.org/ml/archives/dev/2016-July/044614.html

(Just as a note: I am not the original author of this patch but above is 
what I understood and acked it).

>
>> -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;
>> -}
>> -
>

-
Shreyansh


[dpdk-dev] [PATCH v10 11/25] eal/pci: helpers for device name parsing/update

2016-10-26 Thread Pattan, Reshma
Hi,


> -Original Message-
> From: Shreyansh Jain [mailto:shreyansh.jain at nxp.com]
> Sent: Wednesday, October 26, 2016 7:23 AM
> To: Pattan, Reshma 
> Cc: dev at dpdk.org; viktorin at rehivetech.com; David Marchand
> ; hemant.agrawal at nxp.com; Thomas Monjalon
> 
> Subject: Re: [PATCH v10 11/25] eal/pci: helpers for device name parsing/update
> 
> Hello Reshma,
> 
> On Tuesday 25 October 2016 09:19 PM, Pattan, Reshma wrote:
> > Hi Shreyansh,
> >
> >> -Original Message-
> >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Shreyansh Jain
> >> Sent: Friday, September 16, 2016 5:30 AM
> >> To: dev at dpdk.org
> >> Cc: viktorin at rehivetech.com; David Marchand
> >> ; hemant.agrawal at nxp.com; Thomas
> Monjalon
> >> ; Shreyansh Jain 
> >> Subject: [dpdk-dev] [PATCH v10 11/25] eal/pci: helpers for device
> >> name parsing/update
> >>
> >> From: David Marchand 
> >>
> >> - Move rte_eth_dev_create_unique_device_name() from ether/rte_ethdev.c
> to
> >>   common/include/rte_pci.h as rte_eal_pci_device_name(). Being a common
> >>   method, can be used across crypto/net PCI PMDs.
> >> - Remove crypto specific routine and fallback to common name function.
> >> - Introduce a eal private Update function for PCI device naming.
> >>
> >> Signed-off-by: David Marchand 
> >> [Shreyansh: Merge crypto/pci helper patches]
> >> Signed-off-by: Shreyansh Jain 
> >> ---
> >>  lib/librte_cryptodev/rte_cryptodev.c| 27 +++---
> >>  lib/librte_eal/bsdapp/eal/eal_pci.c | 49
> >> +
> >>  lib/librte_eal/common/eal_private.h | 13 +
> >>  lib/librte_eal/common/include/rte_pci.h | 24 
> >>  lib/librte_eal/linuxapp/eal/eal_pci.c   | 13 +
> >>  lib/librte_ether/rte_ethdev.c   | 24 +++-
> >>  6 files changed, 107 insertions(+), 43 deletions(-)
> >>
> >> diff --git a/lib/librte_cryptodev/rte_cryptodev.c
> >> b/lib/librte_cryptodev/rte_cryptodev.c
> >> index 2a3b649..c81e366 100644
> >> --- a/lib/librte_cryptodev/rte_cryptodev.c
> >> +++ b/lib/librte_cryptodev/rte_cryptodev.c
> >> @@ -365,23 +365,6 @@ rte_cryptodev_pmd_allocate(const char *name, int
> >> socket_id)
> >>return cryptodev;
> >>  }
> >>
> >>   *
> >>   * This function is private to EAL.
> >> diff --git a/lib/librte_eal/common/include/rte_pci.h
> >> b/lib/librte_eal/common/include/rte_pci.h
> >> index cf81898..e1f695f 100644
> >> --- a/lib/librte_eal/common/include/rte_pci.h
> >> +++ b/lib/librte_eal/common/include/rte_pci.h
> >> @@ -82,6 +82,7 @@ extern "C" {
> >>  /** Formatting string for PCI device identifier: Ex: :00:01.0 */
> >> #define PCI_PRI_FMT "%.4" PRIx16 ":%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8
> >> +#define PCI_PRI_STR_SIZE sizeof(":XX:XX.X")
> >>
> >>  /** Short formatting string, without domain, for PCI device: Ex:
> >> 00:01.0 */ #define PCI_SHORT_PRI_FMT "%.2" PRIx8 ":%.2" PRIx8 ".%"
> >> PRIx8 @@ -308,6
> >>
> >> +static inline void
> >> +rte_eal_pci_device_name(const struct rte_pci_addr *addr,
> >> +  char *output, size_t size)
> >> +{
> >> +  RTE_VERIFY(size >= PCI_PRI_STR_SIZE);
> >> +  RTE_VERIFY(snprintf(output, size, PCI_PRI_FMT,
> >> +  addr->domain, addr->bus,
> >> +  addr->devid, addr->function) >= 0); }
> >> +
> >>
> >> +int
> >> +pci_update_device(const struct rte_pci_addr *addr) {
> >> +  char filename[PATH_MAX];
> >> +
> >> +  snprintf(filename, sizeof(filename), "%s/" PCI_PRI_FMT,
> >> +   pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
> >> +   addr->function);
> >> +
> >> +  return pci_scan_one(filename, addr->domain, addr->bus, addr->devid,
> >> +  addr->function);
> >> +}
> >> +
> >
> >
> > Earlier device names were created in the format "bus:deviceid.function" as 
> > per
> the below ethdev API.
> > Now after above new eal API the name format is "domain:bus:deviceid.func"
> was that intentional  and why is that so.
> 
> Yes, this is intentional.
> It is to bring the naming in sync with the device name being used for 
> scanning on
> the bus (/sys/bus/pci/devices/:BB:CC.D/).

Fair enough and thanks for clarification.

Thanks,
Reshma


[dpdk-dev] [PATCH v10 11/25] eal/pci: helpers for device name parsing/update

2016-10-25 Thread Pattan, Reshma
Hi Shreyansh,

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Shreyansh Jain
> Sent: Friday, September 16, 2016 5:30 AM
> To: dev at dpdk.org
> Cc: viktorin at rehivetech.com; David Marchand ;
> hemant.agrawal at nxp.com; Thomas Monjalon
> ; Shreyansh Jain 
> Subject: [dpdk-dev] [PATCH v10 11/25] eal/pci: helpers for device name
> parsing/update
> 
> From: David Marchand 
> 
> - Move rte_eth_dev_create_unique_device_name() from ether/rte_ethdev.c to
>   common/include/rte_pci.h as rte_eal_pci_device_name(). Being a common
>   method, can be used across crypto/net PCI PMDs.
> - Remove crypto specific routine and fallback to common name function.
> - Introduce a eal private Update function for PCI device naming.
> 
> Signed-off-by: David Marchand 
> [Shreyansh: Merge crypto/pci helper patches]
> Signed-off-by: Shreyansh Jain 
> ---
>  lib/librte_cryptodev/rte_cryptodev.c| 27 +++---
>  lib/librte_eal/bsdapp/eal/eal_pci.c | 49
> +
>  lib/librte_eal/common/eal_private.h | 13 +
>  lib/librte_eal/common/include/rte_pci.h | 24 
>  lib/librte_eal/linuxapp/eal/eal_pci.c   | 13 +
>  lib/librte_ether/rte_ethdev.c   | 24 +++-
>  6 files changed, 107 insertions(+), 43 deletions(-)
> 
> diff --git a/lib/librte_cryptodev/rte_cryptodev.c
> b/lib/librte_cryptodev/rte_cryptodev.c
> index 2a3b649..c81e366 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.c
> +++ b/lib/librte_cryptodev/rte_cryptodev.c
> @@ -365,23 +365,6 @@ rte_cryptodev_pmd_allocate(const char *name, int
> socket_id)
>   return cryptodev;
>  }
> 
>   *
>   * This function is private to EAL.
> diff --git a/lib/librte_eal/common/include/rte_pci.h
> b/lib/librte_eal/common/include/rte_pci.h
> index cf81898..e1f695f 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -82,6 +82,7 @@ extern "C" {
>  /** Formatting string for PCI device identifier: Ex: :00:01.0 */  #define
> PCI_PRI_FMT "%.4" PRIx16 ":%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8
> +#define PCI_PRI_STR_SIZE sizeof(":XX:XX.X")
> 
>  /** Short formatting string, without domain, for PCI device: Ex: 00:01.0 */
> #define PCI_SHORT_PRI_FMT "%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8 @@ -308,6
> 
> +static inline void
> +rte_eal_pci_device_name(const struct rte_pci_addr *addr,
> + char *output, size_t size)
> +{
> + RTE_VERIFY(size >= PCI_PRI_STR_SIZE);
> + RTE_VERIFY(snprintf(output, size, PCI_PRI_FMT,
> + addr->domain, addr->bus,
> + addr->devid, addr->function) >= 0); }
> +
> 
> +int
> +pci_update_device(const struct rte_pci_addr *addr) {
> + char filename[PATH_MAX];
> +
> + snprintf(filename, sizeof(filename), "%s/" PCI_PRI_FMT,
> +  pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
> +  addr->function);
> +
> + return pci_scan_one(filename, addr->domain, addr->bus, addr->devid,
> + addr->function);
> +}
> +


Earlier device names were created in the format "bus:deviceid.function" as per 
the below ethdev API.
Now after above new eal API the name format is "domain:bus:deviceid.func" was 
that intentional  and why is that so.

> -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;
> -}
> -


[dpdk-dev] [PATCH v10 11/25] eal/pci: helpers for device name parsing/update

2016-09-16 Thread Shreyansh Jain
From: David Marchand 

- Move rte_eth_dev_create_unique_device_name() from ether/rte_ethdev.c to
  common/include/rte_pci.h as rte_eal_pci_device_name(). Being a common
  method, can be used across crypto/net PCI PMDs.
- Remove crypto specific routine and fallback to common name function.
- Introduce a eal private Update function for PCI device naming.

Signed-off-by: David Marchand 
[Shreyansh: Merge crypto/pci helper patches]
Signed-off-by: Shreyansh Jain 
---
 lib/librte_cryptodev/rte_cryptodev.c| 27 +++---
 lib/librte_eal/bsdapp/eal/eal_pci.c | 49 +
 lib/librte_eal/common/eal_private.h | 13 +
 lib/librte_eal/common/include/rte_pci.h | 24 
 lib/librte_eal/linuxapp/eal/eal_pci.c   | 13 +
 lib/librte_ether/rte_ethdev.c   | 24 +++-
 6 files changed, 107 insertions(+), 43 deletions(-)

diff --git a/lib/librte_cryptodev/rte_cryptodev.c 
b/lib/librte_cryptodev/rte_cryptodev.c
index 2a3b649..c81e366 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -365,23 +365,6 @@ rte_cryptodev_pmd_allocate(const char *name, int socket_id)
return cryptodev;
 }

-static inline int
-rte_cryptodev_create_unique_device_name(char *name, size_t size,
-   struct rte_pci_device *pci_dev)
-{
-   int ret;
-
-   if ((name == NULL) || (pci_dev == NULL))
-   return -EINVAL;
-
-   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_cryptodev_pmd_release_device(struct rte_cryptodev *cryptodev)
 {
@@ -444,9 +427,8 @@ rte_cryptodev_pci_probe(struct rte_pci_driver *pci_drv,
if (cryptodrv == NULL)
return -ENODEV;

-   /* Create unique Crypto device name using PCI address */
-   rte_cryptodev_create_unique_device_name(cryptodev_name,
-   sizeof(cryptodev_name), pci_dev);
+   rte_eal_pci_device_name(_dev->addr, cryptodev_name,
+   sizeof(cryptodev_name));

cryptodev = rte_cryptodev_pmd_allocate(cryptodev_name, rte_socket_id());
if (cryptodev == NULL)
@@ -501,9 +483,8 @@ rte_cryptodev_pci_remove(struct rte_pci_device *pci_dev)
if (pci_dev == NULL)
return -EINVAL;

-   /* Create unique device name using PCI address */
-   rte_cryptodev_create_unique_device_name(cryptodev_name,
-   sizeof(cryptodev_name), pci_dev);
+   rte_eal_pci_device_name(_dev->addr, cryptodev_name,
+   sizeof(cryptodev_name));

cryptodev = rte_cryptodev_pmd_get_named_dev(cryptodev_name);
if (cryptodev == NULL)
diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c 
b/lib/librte_eal/bsdapp/eal/eal_pci.c
index a73cbb0..1d91c78 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -406,6 +406,55 @@ error:
return -1;
 }

+int
+pci_update_device(const struct rte_pci_addr *addr)
+{
+   int fd;
+   struct pci_conf matches[2];
+   struct pci_match_conf match = {
+   .pc_sel = {
+   .pc_domain = addr->domain,
+   .pc_bus = addr->bus,
+   .pc_dev = addr->devid,
+   .pc_func = addr->function,
+   },
+   };
+   struct pci_conf_io conf_io = {
+   .pat_buf_len = 0,
+   .num_patterns = 1,
+   .patterns = ,
+   .match_buf_len = sizeof(matches),
+   .matches = [0],
+   };
+
+   fd = open("/dev/pci", O_RDONLY);
+   if (fd < 0) {
+   RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);
+   goto error;
+   }
+
+   if (ioctl(fd, PCIOCGETCONF, _io) < 0) {
+   RTE_LOG(ERR, EAL, "%s(): error with ioctl on /dev/pci: %s\n",
+   __func__, strerror(errno));
+   goto error;
+   }
+
+   if (conf_io.num_matches != 1)
+   goto error;
+
+   if (pci_scan_one(fd, [0]) < 0)
+   goto error;
+
+   close(fd);
+
+   return 0;
+
+error:
+   if (fd >= 0)
+   close(fd);
+   return -1;
+}
+
 /* Read PCI config space. */
 int rte_eal_pci_read_config(const struct rte_pci_device *dev,
void *buf, size_t len, off_t offset)
diff --git a/lib/librte_eal/common/eal_private.h 
b/lib/librte_eal/common/eal_private.h
index ca1aec6..431d6c2 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -130,6 +130,19 @@ struct rte_pci_driver;
 struct rte_pci_device;

 /**
+ * Update a pci device object by asking the kernel for the latest information.
+ *
+ * This function is private to EAL.
+ *
+ * @param