Re: [PATCH v2 3/7] net/mlx5: split PCI from generic probing code

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

On Sun, Jun 17, 2018 at 10:14:58AM +0000, Shahaf Shuler wrote:
> Thursday, June 14, 2018 11:35 AM, Adrien Mazarguil:
> > Subject: [PATCH v2 3/7] net/mlx5: split PCI from generic probing code
> > 
> > All the generic probing code needs is an IB device. While this device is
> > currently supplied by a PCI lookup, other methods will be added soon.
> > 
> > This patch divides the original function, which has become huge over time, as
> > follows:
> > 
> > 1. PCI-specific (mlx5_pci_probe()).
> > 2. All ports of a Verbs device (mlx5_dev_spawn()).
> > 3. A given port of a Verbs device (mlx5_dev_spawn_one()).
> > 
> > (Patch based on prior work from Yuanhan Liu)
> > 
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > --
> > v2 changes:
> > 
> > - Fixed device naming. A port suffix is now appended only if several IB
> >   ports happen to be detected.
> > - Added separate message to distinguish missing kernel drivers from other
> >   initialization errors, as it was confusing.
<snip>
> > +static int
> > +mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
> > +	       struct rte_pci_device *pci_dev) {
> > +	struct ibv_device **ibv_list;
> > +	struct rte_eth_dev **eth_list = NULL;
> > +	int vf;
> > +	int ret;
> > +
> > +	assert(pci_drv == &mlx5_driver);
> > +	switch (pci_dev->id.device_id) {
> > +	case PCI_DEVICE_ID_MELLANOX_CONNECTX4VF:
> > +	case PCI_DEVICE_ID_MELLANOX_CONNECTX4LXVF:
> > +	case PCI_DEVICE_ID_MELLANOX_CONNECTX5VF:
> > +	case PCI_DEVICE_ID_MELLANOX_CONNECTX5EXVF:
> > +		vf = 1;
> > +		break;
> > +	default:
> > +		vf = 0;
> > +	}
> 
> Even though I couldn't find any functional bug, I think it is logically more correct to determine if pci device is vf after we know this is Mellanox device. 
> Meaning the above block should be ...

<snip> 
> > +	while (ret-- > 0) {
> > +		struct rte_pci_addr pci_addr;
> > +
> > +		DRV_LOG(DEBUG, "checking device \"%s\"", ibv_list[ret]-
> > >name);
> > +		if (mlx5_ibv_device_to_pci_addr(ibv_list[ret], &pci_addr))
> > +			continue;
> > +		if (pci_dev->addr.domain != pci_addr.domain ||
> > +		    pci_dev->addr.bus != pci_addr.bus ||
> > +		    pci_dev->addr.devid != pci_addr.devid ||
> > +		    pci_dev->addr.function != pci_addr.function)
> > +			continue;
> > +		DRV_LOG(INFO, "PCI information matches, using device
> > \"%s\"",
> > +			ibv_list[ret]->name);
> > +		break;
> > +	}
> 
> Here. 

No problem, I will update.

-- 
Adrien Mazarguil
6WIND