From mboxrd@z Thu Jan  1 00:00:00 1970
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Subject: Re: [PATCH v2 3/7] net/mlx5: split PCI from generic
	probing code
Date: Wed, 27 Jun 2018 15:31:48 +0200
Message-ID: <20180627133148.GS4025@6wind.com>
References: <20180525161814.13873-1-adrien.mazarguil@6wind.com>
 <20180614083047.10812-1-adrien.mazarguil@6wind.com>
 <20180614083047.10812-4-adrien.mazarguil@6wind.com>
 <DB7PR05MB4426658A7B634342D0739177C3720@DB7PR05MB4426.eurprd05.prod.outlook.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: "dev@dpdk.org" <dev@dpdk.org>
To: Shahaf Shuler <shahafs@mellanox.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66])
 by dpdk.org (Postfix) with ESMTP id A34091BDDA
 for <dev@dpdk.org>; Wed, 27 Jun 2018 15:32:05 +0200 (CEST)
Received: by mail-wm0-f66.google.com with SMTP id e69-v6so2931358wme.2
 for <dev@dpdk.org>; Wed, 27 Jun 2018 06:32:05 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <DB7PR05MB4426658A7B634342D0739177C3720@DB7PR05MB4426.eurprd05.prod.outlook.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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