[dpdk-dev,v3] net/bonding: support bifurcated driver in eal cli using --vdev

Message ID 502c45826dbbfd2bc3f4f743aeea87f159921ae9.1505929849.git.gowrishankar.m@linux.vnet.ibm.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Gowrishankar Sept. 20, 2017, 6:04 p.m. UTC
  From: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>

At present, creating bonding devices using --vdev is broken for PMD like
mlx5 as it is neither UIO nor VFIO based and hence PMD driver is unknown
to find_port_id_by_pci_addr(), as below.

testpmd <EAL args> --vdev 'net_bonding0,mode=1,slave=<PCI>,socket_id=0'

PMD: bond_ethdev_parse_slave_port_kvarg(150) - Invalid slave port value
 (<PCI ID>) specified
EAL: Failed to parse slave ports for bonded device net_bonding0

This patch fixes parsing PCI ID from bonding device params by verifying
it in RTE PCI bus, rather than checking dev->kdrv.

Fixes: eac901ce ("ethdev: decouple from PCI device")
Signed-off-by: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
---
v3:
 - adapt rte_bus API (with suggestions from Declan and Gaëtan)

 drivers/net/bonding/rte_eth_bond_args.c | 35 ++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)
  

Comments

Doherty, Declan Oct. 2, 2017, 11:06 a.m. UTC | #1
On 20/09/2017 7:04 PM, Gowrishankar wrote:
> From: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
> 
> At present, creating bonding devices using --vdev is broken for PMD like
> mlx5 as it is neither UIO nor VFIO based and hence PMD driver is unknown
> to find_port_id_by_pci_addr(), as below.
> 
> testpmd <EAL args> --vdev 'net_bonding0,mode=1,slave=<PCI>,socket_id=0'
> 
> PMD: bond_ethdev_parse_slave_port_kvarg(150) - Invalid slave port value
>   (<PCI ID>) specified
> EAL: Failed to parse slave ports for bonded device net_bonding0
> 
> This patch fixes parsing PCI ID from bonding device params by verifying
> it in RTE PCI bus, rather than checking dev->kdrv.
> 
> Fixes: eac901ce ("ethdev: decouple from PCI device")
> Signed-off-by: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
> ---
> v3:
>   - adapt rte_bus API (with suggestions from Declan and Gaëtan)
> 
...
> 

Acked-by: Declan Doherty <declan.doherty@intel.com>
  
Gaëtan Rivet Oct. 2, 2017, 12:09 p.m. UTC | #2
Hi Gowrishankar,

There will be a trivial conflict with my PCI patchset on the
pci_addr_cmp function. I don't know the best way to solve this.

It depends on my patchset being accepted as-is or not, and which
address namespace has precedence over the other.

You could rename pci_addr_cmp with a reference to the bonding namespace,
it is always nice when debugging to know that we are within the bonding
PMD, even if the function is static inlined...

Anyway, on the principle anyway the code seems ok to me, so

Reviewed-by: Gaetan Rivet <gaetan.rivet@6wind.com>

On Wed, Sep 20, 2017 at 11:34:58PM +0530, Gowrishankar wrote:
> From: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
> 
> At present, creating bonding devices using --vdev is broken for PMD like
> mlx5 as it is neither UIO nor VFIO based and hence PMD driver is unknown
> to find_port_id_by_pci_addr(), as below.
> 
> testpmd <EAL args> --vdev 'net_bonding0,mode=1,slave=<PCI>,socket_id=0'
> 
> PMD: bond_ethdev_parse_slave_port_kvarg(150) - Invalid slave port value
>  (<PCI ID>) specified
> EAL: Failed to parse slave ports for bonded device net_bonding0
> 
> This patch fixes parsing PCI ID from bonding device params by verifying
> it in RTE PCI bus, rather than checking dev->kdrv.
> 
> Fixes: eac901ce ("ethdev: decouple from PCI device")
> Signed-off-by: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
> ---
> v3:
>  - adapt rte_bus API (with suggestions from Declan and Gaëtan)
> 
>  drivers/net/bonding/rte_eth_bond_args.c | 35 ++++++++++++++++++++++-----------
>  1 file changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_args.c b/drivers/net/bonding/rte_eth_bond_args.c
> index bb634c6..7c65dda 100644
> --- a/drivers/net/bonding/rte_eth_bond_args.c
> +++ b/drivers/net/bonding/rte_eth_bond_args.c
> @@ -61,16 +61,6 @@
>  	unsigned i;
>  
>  	for (i = 0; i < rte_eth_dev_count(); i++) {
> -
> -		/* Currently populated by rte_eth_copy_pci_info().
> -		 *
> -		 * TODO: Once the PCI bus has arrived we should have a better
> -		 * way to test for being a PCI device or not.
> -		 */
> -		if (rte_eth_devices[i].data->kdrv == RTE_KDRV_UNKNOWN ||
> -		    rte_eth_devices[i].data->kdrv == RTE_KDRV_NONE)
> -			continue;
> -
>  		pci_dev = RTE_ETH_DEV_TO_PCI(&rte_eth_devices[i]);
>  		eth_pci_addr = &pci_dev->addr;
>  
> @@ -98,6 +88,16 @@
>  	return -1;
>  }
>  
> +static inline int
> +pci_addr_cmp(const struct rte_device *dev, const void *_pci_addr)
> +{
> +	struct rte_pci_device *pdev;
> +	const struct rte_pci_addr *paddr = _pci_addr;
> +
> +	pdev = RTE_DEV_TO_PCI(*(struct rte_device **)(void *)&dev);
> +	return rte_eal_compare_pci_addr(&pdev->addr, paddr);
> +}
> +
>  /**
>   * Parses a port identifier string to a port id by pci address, then by name,
>   * and finally port id.
> @@ -106,10 +106,23 @@
>  parse_port_id(const char *port_str)
>  {
>  	struct rte_pci_addr dev_addr;
> +	struct rte_bus *pci_bus;
> +	struct rte_device *dev;
>  	int port_id;
>  
> +	pci_bus = rte_bus_find_by_name("pci");
> +	if (pci_bus == NULL) {
> +		RTE_LOG(ERR, PMD, "unable to find PCI bus\n");
> +		return -1;
> +	}
> +
>  	/* try parsing as pci address, physical devices */
> -	if (eal_parse_pci_DomBDF(port_str, &dev_addr) == 0) {
> +	if (pci_bus->parse(port_str, &dev_addr) == 0) {
> +		dev = pci_bus->find_device(NULL, pci_addr_cmp, &dev_addr);
> +		if (dev == NULL) {
> +			RTE_LOG(ERR, PMD, "unable to find PCI device\n");
> +			return -1;
> +		}
>  		port_id = find_port_id_by_pci_addr(&dev_addr);
>  		if (port_id < 0)
>  			return -1;
> -- 
> 1.9.1
>
  
Ferruh Yigit Oct. 2, 2017, 11:32 p.m. UTC | #3
On 10/2/2017 12:06 PM, Doherty, Declan wrote:
> On 20/09/2017 7:04 PM, Gowrishankar wrote:
>> From: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
>>
>> At present, creating bonding devices using --vdev is broken for PMD like
>> mlx5 as it is neither UIO nor VFIO based and hence PMD driver is unknown
>> to find_port_id_by_pci_addr(), as below.
>>
>> testpmd <EAL args> --vdev 'net_bonding0,mode=1,slave=<PCI>,socket_id=0'
>>
>> PMD: bond_ethdev_parse_slave_port_kvarg(150) - Invalid slave port value
>>   (<PCI ID>) specified
>> EAL: Failed to parse slave ports for bonded device net_bonding0
>>
>> This patch fixes parsing PCI ID from bonding device params by verifying
>> it in RTE PCI bus, rather than checking dev->kdrv.
>>
>> Fixes: eac901ce ("ethdev: decouple from PCI device")
>> Signed-off-by: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
>> ---
<...>
> Acked-by: Declan Doherty <declan.doherty@intel.com>

Reviewed-by: Gaetan Rivet <gaetan.rivet@6wind.com>

Applied to dpdk-next-net/master, thanks.

(Gaetan, we may ask your help during merge on this.)
  

Patch

diff --git a/drivers/net/bonding/rte_eth_bond_args.c b/drivers/net/bonding/rte_eth_bond_args.c
index bb634c6..7c65dda 100644
--- a/drivers/net/bonding/rte_eth_bond_args.c
+++ b/drivers/net/bonding/rte_eth_bond_args.c
@@ -61,16 +61,6 @@ 
 	unsigned i;
 
 	for (i = 0; i < rte_eth_dev_count(); i++) {
-
-		/* Currently populated by rte_eth_copy_pci_info().
-		 *
-		 * TODO: Once the PCI bus has arrived we should have a better
-		 * way to test for being a PCI device or not.
-		 */
-		if (rte_eth_devices[i].data->kdrv == RTE_KDRV_UNKNOWN ||
-		    rte_eth_devices[i].data->kdrv == RTE_KDRV_NONE)
-			continue;
-
 		pci_dev = RTE_ETH_DEV_TO_PCI(&rte_eth_devices[i]);
 		eth_pci_addr = &pci_dev->addr;
 
@@ -98,6 +88,16 @@ 
 	return -1;
 }
 
+static inline int
+pci_addr_cmp(const struct rte_device *dev, const void *_pci_addr)
+{
+	struct rte_pci_device *pdev;
+	const struct rte_pci_addr *paddr = _pci_addr;
+
+	pdev = RTE_DEV_TO_PCI(*(struct rte_device **)(void *)&dev);
+	return rte_eal_compare_pci_addr(&pdev->addr, paddr);
+}
+
 /**
  * Parses a port identifier string to a port id by pci address, then by name,
  * and finally port id.
@@ -106,10 +106,23 @@ 
 parse_port_id(const char *port_str)
 {
 	struct rte_pci_addr dev_addr;
+	struct rte_bus *pci_bus;
+	struct rte_device *dev;
 	int port_id;
 
+	pci_bus = rte_bus_find_by_name("pci");
+	if (pci_bus == NULL) {
+		RTE_LOG(ERR, PMD, "unable to find PCI bus\n");
+		return -1;
+	}
+
 	/* try parsing as pci address, physical devices */
-	if (eal_parse_pci_DomBDF(port_str, &dev_addr) == 0) {
+	if (pci_bus->parse(port_str, &dev_addr) == 0) {
+		dev = pci_bus->find_device(NULL, pci_addr_cmp, &dev_addr);
+		if (dev == NULL) {
+			RTE_LOG(ERR, PMD, "unable to find PCI device\n");
+			return -1;
+		}
 		port_id = find_port_id_by_pci_addr(&dev_addr);
 		if (port_id < 0)
 			return -1;