From mboxrd@z Thu Jan  1 00:00:00 1970
From: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
Subject: Re: [PATCH v2 1/3] nic_uio: Fix to allow any device to
 be bound to nic_uio
Date: Mon, 20 Jul 2015 17:37:27 +0530
Message-ID: <20150720120725.GA3771@scalar.blr.asicdesigners.com>
References: <cover.1437156076.git.rahul.lakkireddy@chelsio.com>
 <cover.1437373175.git.rahul.lakkireddy@chelsio.com>
 <a4e5a65c70f8828cc9ce607fbcce263e2fae12b8.1437373175.git.rahul.lakkireddy@chelsio.com>
 <CALwxeUt5hFdpCjhizf1EPMV53+kXJ0VsM0p5yJ5W1udJf4oj5w@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: quoted-printable
Cc: "dev@dpdk.org" <dev@dpdk.org>, Felix Marti <felix@chelsio.com>,
 Nirranjan Kirubaharan <nirranjan@chelsio.com>,
 Kumar Sanghvi <kumaras@chelsio.com>
To: David Marchand <david.marchand@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from stargate3.asicdesigners.com (stargate.chelsio.com
 [67.207.112.58]) by dpdk.org (Postfix) with ESMTP id F0242CE7
 for <dev@dpdk.org>; Mon, 20 Jul 2015 14:08:08 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <CALwxeUt5hFdpCjhizf1EPMV53+kXJ0VsM0p5yJ5W1udJf4oj5w@mail.gmail.com>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi David,

On Mon, Jul 20, 2015 at 09:43:57 +0200, David Marchand wrote:
>    Hum, what bothers me is that you do not rely on the same criteria to
>    re-attach the devices to nic_uio.
>    See below.
>=20
>      =A0lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 48
>      +++++++++------------------------
>      =A01 file changed, 13 insertions(+), 35 deletions(-)
>=20
>      diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
>      b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
>      index 2354e84..f868dc8 100644
>      --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
>      +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
>      [snip]
>      @@ -195,11 +177,10 @@ nic_uio_probe (device_t dev)
>      =A0{
>      =A0 =A0 =A0 =A0 int i;
>=20
>      -=A0 =A0 =A0 =A0for (i =3D 0; i < NUM_DEVICES; i++)
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (pci_get_vendor(dev) =3D=3D dev=
ices[i].vend &&
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pci_get_device(dev=
) =3D=3D devices[i].dev) {
>      -
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0device_set_desc(de=
v, "Intel(R) DPDK PCI
>      Device");
>      +=A0 =A0 =A0 =A0for (i =3D 0; i < num_detached; i++)
>      +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (pci_get_vendor(dev) =3D=3D
>      pci_get_vendor(detached_devices[i]) &&
>      +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pci_get_device(dev) =3D=3D
>      pci_get_device(detached_devices[i])) {
>      +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0device_set_desc(de=
v, "DPDK PCI Device");
>      =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return BUS_PROBE_S=
PECIFIC;
>      =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
>=20
>    When going through the probe stuff, the device vendor and type are u=
sed as
>    the matching criteria.
>=20
>      @@ -256,7 +237,6 @@ static void
>      =A0nic_uio_load(void)
>      =A0{
>      =A0 =A0 =A0 =A0 uint32_t bus, device, function;
>      -=A0 =A0 =A0 =A0int i;
>      =A0 =A0 =A0 =A0 device_t dev;
>      =A0 =A0 =A0 =A0 char bdf_str[256];
>      =A0 =A0 =A0 =A0 char *token, *remaining;
>      @@ -295,17 +275,15 @@ nic_uio_load(void)
>      =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (dev =3D=3D NULL)
>      =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 continue;
>=20
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0for (i =3D 0; i < NUM_DEVICES; i++=
)
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (pci_get_vendor=
(dev) =3D=3D devices[i].vend &&
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0pci_get_device(dev) =3D=3D
>      devices[i].dev) {
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (num_detached <
>      MAX_DETACHED_DEVICES) {
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0
>      =A0printf("nic_uio_load: detaching and storing dev=3D%p\n", dev);
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0
>      =A0detached_devices[num_detached++] =3D dev;
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0
>      =A0printf("nic_uio_load: reached MAX_DETACHED_DEVICES=3D%d. dev=3D=
%p won't be
>      reattached\n",
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0
>      =A0MAX_DETACHED_DEVICES, dev);
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0device_detach(dev);
>      -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>      +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (num_detached < MAX_DETACHED_DE=
VICES) {
>      +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("nic_uio_lo=
ad: detaching and storing
>      dev=3D%p\n",
>      +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev);
>      +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0detached_devices[n=
um_detached++] =3D dev;
>      +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else {
>      +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("nic_uio_lo=
ad: reached
>      MAX_DETACHED_DEVICES=3D%d. dev=3D%p won't be reattached\n",
>      +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 MAX_D=
ETACHED_DEVICES, dev);
>      +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>      +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0device_detach(dev);
>      =A0 =A0 =A0 =A0 }
>      =A0}
>=20
>    But here at init time, the bdfs informations are used to detach the =
pci
>    devices.
>    I would say this is safer we have the same criteria in both cases.
>    I think that the pci addresses are the best criteria since this is w=
hat
>    the user gives.
>    Don't we have them in the dev pointer ?

It looks like we can get them via pci_get_bus(), pci_get_slot(), and
pci_get_function().  Will add check for these 3 info instead of vendor
and device in probe to make it consistent.

>=20
>    Btw, with this change, we would then be limited to MAX_DETACHED_DEVI=
CES
>    devices even if 128 pci devices looks quite big enough to me.
>    This part could be reworked (later).
>    --
>    David Marchand

Thanks,
Rahul