Re: [PATCH v3] net/bonding: support bifurcated driver in eal cli using --vdev

["Gaëtan Rivet" <gaetan.rivet@6wind.com>]

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
> 

-- 
Gaëtan Rivet
6WIND