From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from stargate3.asicdesigners.com (stargate.chelsio.com [67.207.112.58]) by dpdk.org (Postfix) with ESMTP id F0242CE7 for ; Mon, 20 Jul 2015 14:08:08 +0200 (CEST) Received: from localhost (scalar.blr.asicdesigners.com [10.193.185.94]) by stargate3.asicdesigners.com (8.13.8/8.13.8) with ESMTP id t6KC7irZ029158; Mon, 20 Jul 2015 05:07:44 -0700 Date: Mon, 20 Jul 2015 17:37:27 +0530 From: Rahul Lakkireddy To: David Marchand Message-ID: <20150720120725.GA3771@scalar.blr.asicdesigners.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Cc: "dev@dpdk.org" , Felix Marti , Nirranjan Kirubaharan , Kumar Sanghvi Subject: Re: [dpdk-dev] [PATCH v2 1/3] nic_uio: Fix to allow any device to be bound to nic_uio X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 20 Jul 2015 12:08:09 -0000 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. > >  lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 48 > +++++++++------------------------ >  1 file changed, 13 insertions(+), 35 deletions(-) > > 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) >  { >         int i; > > -       for (i = 0; i < NUM_DEVICES; i++) > -               if (pci_get_vendor(dev) == devices[i].vend && > -                       pci_get_device(dev) == devices[i].dev) { > - > -                       device_set_desc(dev, "Intel(R) DPDK PCI > Device"); > +       for (i = 0; i < num_detached; i++) > +               if (pci_get_vendor(dev) == > pci_get_vendor(detached_devices[i]) && > +                   pci_get_device(dev) == > pci_get_device(detached_devices[i])) { > +                       device_set_desc(dev, "DPDK PCI Device"); >                         return BUS_PROBE_SPECIFIC; >                 } > > When going through the probe stuff, the device vendor and type are used as > the matching criteria. > > @@ -256,7 +237,6 @@ static void >  nic_uio_load(void) >  { >         uint32_t bus, device, function; > -       int i; >         device_t dev; >         char bdf_str[256]; >         char *token, *remaining; > @@ -295,17 +275,15 @@ nic_uio_load(void) >                 if (dev == NULL) >                         continue; > > -               for (i = 0; i < NUM_DEVICES; i++) > -                       if (pci_get_vendor(dev) == devices[i].vend && > -                                       pci_get_device(dev) == > devices[i].dev) { > -                                               if (num_detached < > MAX_DETACHED_DEVICES) { > -                                                      >  printf("nic_uio_load: detaching and storing dev=%p\n", dev); > -                                                      >  detached_devices[num_detached++] = dev; > -                                               } else > -                                                      >  printf("nic_uio_load: reached MAX_DETACHED_DEVICES=%d. dev=%p won't be > reattached\n", > -                                                              >  MAX_DETACHED_DEVICES, dev); > -                                               device_detach(dev); > -                       } > +               if (num_detached < MAX_DETACHED_DEVICES) { > +                       printf("nic_uio_load: detaching and storing > dev=%p\n", > +                              dev); > +                       detached_devices[num_detached++] = dev; > +               } else { > +                       printf("nic_uio_load: reached > MAX_DETACHED_DEVICES=%d. dev=%p won't be reattached\n", > +                              MAX_DETACHED_DEVICES, dev); > +               } > +               device_detach(dev); >         } >  } > > 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 what > 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. > > Btw, with this change, we would then be limited to MAX_DETACHED_DEVICES > devices even if 128 pci devices looks quite big enough to me. > This part could be reworked (later). > -- > David Marchand Thanks, Rahul