| Message ID | 1424932400-66862-1-git-send-email-razamir22@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 8E803106B; Sun, 1 Mar 2015 12:09:45 +0100 (CET) Received: from localhost.my.domain (84.95.210.61.forward.012.net.il [84.95.210.61]) by dpdk.org (Postfix) with ESMTP id 9C420959 for <dev@dpdk.org>; Sun, 1 Mar 2015 12:09:43 +0100 (CET) Received: from localhost.my.domain (localhost [127.0.0.1]) by localhost.my.domain (8.14.9/8.14.9) with ESMTP id t1Q6fEc9066915 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 26 Feb 2015 06:41:14 GMT (envelope-from root@localhost.my.domain) Received: (from root@localhost) by localhost.my.domain (8.14.9/8.14.9/Submit) id t1Q6fEsj066914; Thu, 26 Feb 2015 06:41:14 GMT (envelope-from root) From: Raz Amir <razamir22@gmail.com> To: dev@dpdk.org Date: Thu, 26 Feb 2015 06:33:20 +0000 Message-Id: <1424932400-66862-1-git-send-email-razamir22@gmail.com> X-Mailer: git-send-email 2.1.2 Subject: [dpdk-dev] [PATCH] pci: save list of detached devices, and re-probe during driver unload X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list 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> |
Commit Message
Raz Amir
Feb. 26, 2015, 6:33 a.m. UTC
Added code that saves the pointers to the detached devices, during
driver loading, and during driver unloading, go over the list,
and re-attach them by calling device_probe_and_attach
on each device.
Signed-off-by: Raz Amir <razamir22@gmail.com>
---
lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
Comments
On Thu, Feb 26, 2015 at 06:33:20AM +0000, Raz Amir wrote: > Added code that saves the pointers to the detached devices, during > driver loading, and during driver unloading, go over the list, > and re-attach them by calling device_probe_and_attach > on each device. > > Signed-off-by: Raz Amir <razamir22@gmail.com> > --- > lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > index 5ae8560..7d702a5 100644 > --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > @@ -55,6 +55,9 @@ __FBSDID("$FreeBSD$"); > > #define MAX_BARS (PCIR_MAX_BAR_0 + 1) > > +#define MAX_DETACHED_DEVICES 128 > +static device_t detached_devices[MAX_DETACHED_DEVICES] = {}; > +static int last_detached = 0; > > struct nic_uio_softc { > device_t dev_t; > @@ -291,14 +294,35 @@ nic_uio_load(void) > if (dev != NULL) > for (i = 0; i < NUM_DEVICES; i++) > if (pci_get_vendor(dev) == devices[i].vend && > - pci_get_device(dev) == devices[i].dev) > + pci_get_device(dev) == devices[i].dev) { > + if (last_detached+1 < MAX_DETACHED_DEVICES) { > + printf("nic_uio_load: detaching and storing dev=%p\n", dev); No printfs. Use the logging facilities > + detached_devices[last_detached++] = dev; > + } > + else { > + printf("nic_uio_load: reached MAX_DETACHED_DEVICES=%d. dev=%p won't be reattached\n", > + MAX_DETACHED_DEVICES, dev); Dittto > + } > + > device_detach(dev); > + } > } > } > > static void > nic_uio_unload(void) > { > + int i; > + printf("nic_uio_unload: entered ... \n"); > + > + for (i = 0; i < last_detached; i++) { > + printf("nic_uio_unload: calling to device_probe_and_attach for dev=%p...\n", > + detached_devices[i]); > + device_probe_and_attach(detached_devices[i]); Where is this defined? It doesn't appear to be in the latest dpdk. > + printf("nic_uio_unload: done.\n"); > + } > + > + printf("nic_uio_unload: leaving ... \n"); > } > > static int > -- > 2.1.2 > >
Can you refer me to the logging facilities you are referring to for this Freebsd driver? device_probe_and_attach is an API in Freebsd kernel which is called during boot for finding the relevant driver for each device. I added manual call to it in the driver unload for re-probing and re-matching the devices that were detached by the driver. Before this change, you had to reboot in order to get the devices back. -----Original Message----- From: Neil Horman [mailto:nhorman@tuxdriver.com] Sent: 01 March 2015 15:48 To: Raz Amir Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, and re-probe during driver unload On Thu, Feb 26, 2015 at 06:33:20AM +0000, Raz Amir wrote: > Added code that saves the pointers to the detached devices, during > driver loading, and during driver unloading, go over the list, > and re-attach them by calling device_probe_and_attach > on each device. > > Signed-off-by: Raz Amir <razamir22@gmail.com> > --- > lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > index 5ae8560..7d702a5 100644 > --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > @@ -55,6 +55,9 @@ __FBSDID("$FreeBSD$"); > > #define MAX_BARS (PCIR_MAX_BAR_0 + 1) > > +#define MAX_DETACHED_DEVICES 128 > +static device_t detached_devices[MAX_DETACHED_DEVICES] = {}; > +static int last_detached = 0; > > struct nic_uio_softc { > device_t dev_t; > @@ -291,14 +294,35 @@ nic_uio_load(void) > if (dev != NULL) > for (i = 0; i < NUM_DEVICES; i++) > if (pci_get_vendor(dev) == devices[i].vend && > - pci_get_device(dev) == devices[i].dev) > + pci_get_device(dev) == devices[i].dev) { > + if (last_detached+1 < MAX_DETACHED_DEVICES) { > + printf("nic_uio_load: detaching and storing dev=%p\n", dev); No printfs. Use the logging facilities > + detached_devices[last_detached++] = dev; > + } > + else { > + printf("nic_uio_load: reached MAX_DETACHED_DEVICES=%d. dev=%p won't be reattached\n", > + MAX_DETACHED_DEVICES, dev); Dittto > + } > + > device_detach(dev); > + } > } > } > > static void > nic_uio_unload(void) > { > + int i; > + printf("nic_uio_unload: entered ... \n"); > + > + for (i = 0; i < last_detached; i++) { > + printf("nic_uio_unload: calling to device_probe_and_attach for dev=%p...\n", > + detached_devices[i]); > + device_probe_and_attach(detached_devices[i]); Where is this defined? It doesn't appear to be in the latest dpdk. > + printf("nic_uio_unload: done.\n"); > + } > + > + printf("nic_uio_unload: leaving ... \n"); > } > > static int > -- > 2.1.2 > >
On Sun, Mar 01, 2015 at 04:21:10PM +0200, Raz Amir wrote: > Can you refer me to the logging facilities you are referring to for this > Freebsd driver? > device_probe_and_attach is an API in Freebsd kernel which is called during rte_log and friends. > boot for finding the relevant driver for each device. > I added manual call to it in the driver unload for re-probing and > re-matching the devices that were detached by the driver. > Before this change, you had to reboot in order to get the devices back. > my bad, I didn't see it defined and was thinking it was removed from an earlier version of the dpdk. Neil > -----Original Message----- > From: Neil Horman [mailto:nhorman@tuxdriver.com] > Sent: 01 March 2015 15:48 > To: Raz Amir > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, and > re-probe during driver unload > > On Thu, Feb 26, 2015 at 06:33:20AM +0000, Raz Amir wrote: > > Added code that saves the pointers to the detached devices, during > > driver loading, and during driver unloading, go over the list, > > and re-attach them by calling device_probe_and_attach > > on each device. > > > > Signed-off-by: Raz Amir <razamir22@gmail.com> > > --- > > lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 26 +++++++++++++++++++++++++- > > 1 file changed, 25 insertions(+), 1 deletion(-) > > > > diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > index 5ae8560..7d702a5 100644 > > --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > @@ -55,6 +55,9 @@ __FBSDID("$FreeBSD$"); > > > > #define MAX_BARS (PCIR_MAX_BAR_0 + 1) > > > > +#define MAX_DETACHED_DEVICES 128 > > +static device_t detached_devices[MAX_DETACHED_DEVICES] = {}; > > +static int last_detached = 0; > > > > struct nic_uio_softc { > > device_t dev_t; > > @@ -291,14 +294,35 @@ nic_uio_load(void) > > if (dev != NULL) > > for (i = 0; i < NUM_DEVICES; i++) > > if (pci_get_vendor(dev) == devices[i].vend > && > > - pci_get_device(dev) == > devices[i].dev) > > + pci_get_device(dev) == > devices[i].dev) { > > + if (last_detached+1 > < MAX_DETACHED_DEVICES) { > > + > printf("nic_uio_load: detaching and storing dev=%p\n", dev); > No printfs. Use the logging facilities > > > + > detached_devices[last_detached++] = dev; > > + } > > + else { > > + > printf("nic_uio_load: reached MAX_DETACHED_DEVICES=%d. dev=%p won't be > reattached\n", > > + > MAX_DETACHED_DEVICES, dev); > Dittto > > > + } > > + > > device_detach(dev); > > + } > > } > > } > > > > static void > > nic_uio_unload(void) > > { > > + int i; > > + printf("nic_uio_unload: entered ... \n"); > > + > > + for (i = 0; i < last_detached; i++) { > > + printf("nic_uio_unload: calling to device_probe_and_attach > for dev=%p...\n", > > + detached_devices[i]); > > + device_probe_and_attach(detached_devices[i]); > Where is this defined? It doesn't appear to be in the latest dpdk. > > > + printf("nic_uio_unload: done.\n"); > > + } > > + > > + printf("nic_uio_unload: leaving ... \n"); > > } > > > > static int > > -- > > 2.1.2 > > > > > >
The patch I suggest is in the nic_uio freebsd kernel driver. I don't think that rte_log can be used there. I checked the contigmem freebsd driver and it uses printf. I can either remove those printfs, or keep them. I suggest keeping them as they were very helpful for understanding the flow, and they don't provide too much output (depending on the amount of interfaces connected to the nic_uio driver). How should I continue with this patch? BTW, I don't see it in incoming patches page at http://dpdk.org/dev/patchwork/project/dpdk/list/?page=1 Is it because I missed something in the code contribution instruction? -----Original Message----- From: Neil Horman [mailto:nhorman@tuxdriver.com] Sent: 01 March 2015 19:17 To: Raz Amir Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, and re-probe during driver unload On Sun, Mar 01, 2015 at 04:21:10PM +0200, Raz Amir wrote: > Can you refer me to the logging facilities you are referring to for > this Freebsd driver? > device_probe_and_attach is an API in Freebsd kernel which is called > during rte_log and friends. > boot for finding the relevant driver for each device. > I added manual call to it in the driver unload for re-probing and > re-matching the devices that were detached by the driver. > Before this change, you had to reboot in order to get the devices back. > my bad, I didn't see it defined and was thinking it was removed from an earlier version of the dpdk. Neil > -----Original Message----- > From: Neil Horman [mailto:nhorman@tuxdriver.com] > Sent: 01 March 2015 15:48 > To: Raz Amir > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, > and re-probe during driver unload > > On Thu, Feb 26, 2015 at 06:33:20AM +0000, Raz Amir wrote: > > Added code that saves the pointers to the detached devices, during > > driver loading, and during driver unloading, go over the list, and > > re-attach them by calling device_probe_and_attach on each device. > > > > Signed-off-by: Raz Amir <razamir22@gmail.com> > > --- > > lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 26 > > +++++++++++++++++++++++++- > > 1 file changed, 25 insertions(+), 1 deletion(-) > > > > diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > index 5ae8560..7d702a5 100644 > > --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > @@ -55,6 +55,9 @@ __FBSDID("$FreeBSD$"); > > > > #define MAX_BARS (PCIR_MAX_BAR_0 + 1) > > > > +#define MAX_DETACHED_DEVICES 128 > > +static device_t detached_devices[MAX_DETACHED_DEVICES] = {}; static > > +int last_detached = 0; > > > > struct nic_uio_softc { > > device_t dev_t; > > @@ -291,14 +294,35 @@ nic_uio_load(void) > > if (dev != NULL) > > for (i = 0; i < NUM_DEVICES; i++) > > if (pci_get_vendor(dev) == devices[i].vend > && > > - pci_get_device(dev) == > devices[i].dev) > > + pci_get_device(dev) == > devices[i].dev) { > > + if (last_detached+1 > < MAX_DETACHED_DEVICES) { > > + > printf("nic_uio_load: detaching and storing dev=%p\n", dev); No > printfs. Use the logging facilities > > > + > detached_devices[last_detached++] = dev; > > + } > > + else { > > + > printf("nic_uio_load: reached MAX_DETACHED_DEVICES=%d. dev=%p won't be > reattached\n", > > + > MAX_DETACHED_DEVICES, dev); > Dittto > > > + } > > + > > device_detach(dev); > > + } > > } > > } > > > > static void > > nic_uio_unload(void) > > { > > + int i; > > + printf("nic_uio_unload: entered ... \n"); > > + > > + for (i = 0; i < last_detached; i++) { > > + printf("nic_uio_unload: calling to device_probe_and_attach > for dev=%p...\n", > > + detached_devices[i]); > > + device_probe_and_attach(detached_devices[i]); > Where is this defined? It doesn't appear to be in the latest dpdk. > > > + printf("nic_uio_unload: done.\n"); > > + } > > + > > + printf("nic_uio_unload: leaving ... \n"); > > } > > > > static int > > -- > > 2.1.2 > > > > > >
On Mon, Mar 02, 2015 at 10:18:42AM +0200, Raz Amir wrote: > The patch I suggest is in the nic_uio freebsd kernel driver. I don't think > that rte_log can be used there. > I checked the contigmem freebsd driver and it uses printf. > I can either remove those printfs, or keep them. > I suggest keeping them as they were very helpful for understanding the flow, > and they don't provide too much output (depending on the amount of > interfaces connected to the nic_uio driver). > You're right you can't use it, I thought it was the user space support for the uio driver, not the kernel module > How should I continue with this patch? > BTW, I don't see it in incoming patches page at > http://dpdk.org/dev/patchwork/project/dpdk/list/?page=1 > Is it because I missed something in the code contribution instruction? > > -----Original Message----- > From: Neil Horman [mailto:nhorman@tuxdriver.com] > Sent: 01 March 2015 19:17 > To: Raz Amir > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, and > re-probe during driver unload > > On Sun, Mar 01, 2015 at 04:21:10PM +0200, Raz Amir wrote: > > Can you refer me to the logging facilities you are referring to for > > this Freebsd driver? > > device_probe_and_attach is an API in Freebsd kernel which is called > > during > rte_log and friends. > > > boot for finding the relevant driver for each device. > > I added manual call to it in the driver unload for re-probing and > > re-matching the devices that were detached by the driver. > > Before this change, you had to reboot in order to get the devices back. > > > my bad, I didn't see it defined and was thinking it was removed from an > earlier version of the dpdk. > Neil > > > -----Original Message----- > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > Sent: 01 March 2015 15:48 > > To: Raz Amir > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, > > and re-probe during driver unload > > > > On Thu, Feb 26, 2015 at 06:33:20AM +0000, Raz Amir wrote: > > > Added code that saves the pointers to the detached devices, during > > > driver loading, and during driver unloading, go over the list, and > > > re-attach them by calling device_probe_and_attach on each device. > > > > > > Signed-off-by: Raz Amir <razamir22@gmail.com> > > > --- > > > lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 26 > > > +++++++++++++++++++++++++- > > > 1 file changed, 25 insertions(+), 1 deletion(-) > > > > > > diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > index 5ae8560..7d702a5 100644 > > > --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > @@ -55,6 +55,9 @@ __FBSDID("$FreeBSD$"); > > > > > > #define MAX_BARS (PCIR_MAX_BAR_0 + 1) > > > > > > +#define MAX_DETACHED_DEVICES 128 > > > +static device_t detached_devices[MAX_DETACHED_DEVICES] = {}; static > > > +int last_detached = 0; > > > > > > struct nic_uio_softc { > > > device_t dev_t; > > > @@ -291,14 +294,35 @@ nic_uio_load(void) > > > if (dev != NULL) > > > for (i = 0; i < NUM_DEVICES; i++) > > > if (pci_get_vendor(dev) == devices[i].vend > > && > > > - pci_get_device(dev) == > > devices[i].dev) > > > + pci_get_device(dev) == > > devices[i].dev) { > > > + if (last_detached+1 > > < MAX_DETACHED_DEVICES) { > > > + > > printf("nic_uio_load: detaching and storing dev=%p\n", dev); No > > printfs. Use the logging facilities > > > > > + > > detached_devices[last_detached++] = dev; > > > + } > > > + else { > > > + > > printf("nic_uio_load: reached MAX_DETACHED_DEVICES=%d. dev=%p won't be > > reattached\n", > > > + > > MAX_DETACHED_DEVICES, dev); > > Dittto > > > > > + } > > > + > > > device_detach(dev); > > > + } > > > } > > > } > > > > > > static void > > > nic_uio_unload(void) > > > { > > > + int i; > > > + printf("nic_uio_unload: entered ... \n"); > > > + > > > + for (i = 0; i < last_detached; i++) { > > > + printf("nic_uio_unload: calling to device_probe_and_attach > > for dev=%p...\n", > > > + detached_devices[i]); > > > + device_probe_and_attach(detached_devices[i]); > > Where is this defined? It doesn't appear to be in the latest dpdk. > > > > > + printf("nic_uio_unload: done.\n"); > > > + } > > > + > > > + printf("nic_uio_unload: leaving ... \n"); > > > } > > > > > > static int > > > -- > > > 2.1.2 > > > > > > > > > > > >
Thanks. How should I continue with this patch? BTW, I don't see it in incoming patches page at http://dpdk.org/dev/patchwork/project/dpdk/list/?page=1 Is it because I missed something in the code contribution instruction? -----Original Message----- From: Neil Horman [mailto:nhorman@tuxdriver.com] Sent: 02 March 2015 13:37 To: Raz Amir Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, and re-probe during driver unload On Mon, Mar 02, 2015 at 10:18:42AM +0200, Raz Amir wrote: > The patch I suggest is in the nic_uio freebsd kernel driver. I don't > think that rte_log can be used there. > I checked the contigmem freebsd driver and it uses printf. > I can either remove those printfs, or keep them. > I suggest keeping them as they were very helpful for understanding the > flow, and they don't provide too much output (depending on the amount > of interfaces connected to the nic_uio driver). > You're right you can't use it, I thought it was the user space support for the uio driver, not the kernel module > How should I continue with this patch? > BTW, I don't see it in incoming patches page at > http://dpdk.org/dev/patchwork/project/dpdk/list/?page=1 > Is it because I missed something in the code contribution instruction? > > -----Original Message----- > From: Neil Horman [mailto:nhorman@tuxdriver.com] > Sent: 01 March 2015 19:17 > To: Raz Amir > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, > and re-probe during driver unload > > On Sun, Mar 01, 2015 at 04:21:10PM +0200, Raz Amir wrote: > > Can you refer me to the logging facilities you are referring to for > > this Freebsd driver? > > device_probe_and_attach is an API in Freebsd kernel which is called > > during > rte_log and friends. > > > boot for finding the relevant driver for each device. > > I added manual call to it in the driver unload for re-probing and > > re-matching the devices that were detached by the driver. > > Before this change, you had to reboot in order to get the devices back. > > > my bad, I didn't see it defined and was thinking it was removed from > an earlier version of the dpdk. > Neil > > > -----Original Message----- > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > Sent: 01 March 2015 15:48 > > To: Raz Amir > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, > > and re-probe during driver unload > > > > On Thu, Feb 26, 2015 at 06:33:20AM +0000, Raz Amir wrote: > > > Added code that saves the pointers to the detached devices, during > > > driver loading, and during driver unloading, go over the list, and > > > re-attach them by calling device_probe_and_attach on each device. > > > > > > Signed-off-by: Raz Amir <razamir22@gmail.com> > > > --- > > > lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 26 > > > +++++++++++++++++++++++++- > > > 1 file changed, 25 insertions(+), 1 deletion(-) > > > > > > diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > index 5ae8560..7d702a5 100644 > > > --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > @@ -55,6 +55,9 @@ __FBSDID("$FreeBSD$"); > > > > > > #define MAX_BARS (PCIR_MAX_BAR_0 + 1) > > > > > > +#define MAX_DETACHED_DEVICES 128 > > > +static device_t detached_devices[MAX_DETACHED_DEVICES] = {}; > > > +static int last_detached = 0; > > > > > > struct nic_uio_softc { > > > device_t dev_t; > > > @@ -291,14 +294,35 @@ nic_uio_load(void) > > > if (dev != NULL) > > > for (i = 0; i < NUM_DEVICES; i++) > > > if (pci_get_vendor(dev) == devices[i].vend > > && > > > - pci_get_device(dev) == > > devices[i].dev) > > > + pci_get_device(dev) == > > devices[i].dev) { > > > + if (last_detached+1 > > < MAX_DETACHED_DEVICES) { > > > + > > printf("nic_uio_load: detaching and storing dev=%p\n", dev); No > > printfs. Use the logging facilities > > > > > + > > detached_devices[last_detached++] = dev; > > > + } > > > + else { > > > + > > printf("nic_uio_load: reached MAX_DETACHED_DEVICES=%d. dev=%p won't > > be reattached\n", > > > + > > MAX_DETACHED_DEVICES, dev); > > Dittto > > > > > + } > > > + > > > device_detach(dev); > > > + } > > > } > > > } > > > > > > static void > > > nic_uio_unload(void) > > > { > > > + int i; > > > + printf("nic_uio_unload: entered ... \n"); > > > + > > > + for (i = 0; i < last_detached; i++) { > > > + printf("nic_uio_unload: calling to device_probe_and_attach > > for dev=%p...\n", > > > + detached_devices[i]); > > > + device_probe_and_attach(detached_devices[i]); > > Where is this defined? It doesn't appear to be in the latest dpdk. > > > > > + printf("nic_uio_unload: done.\n"); > > > + } > > > + > > > + printf("nic_uio_unload: leaving ... \n"); > > > } > > > > > > static int > > > -- > > > 2.1.2 > > > > > > > > > > > >
2015-03-02 13:58, Raz Amir: > BTW, I don't see it in incoming patches page at > http://dpdk.org/dev/patchwork/project/dpdk/list/?page=1 > Is it because I missed something in the code contribution instruction? It's here: http://dpdk.org/dev/patchwork/patch/3800/
Thanks. What's next with the suggested parch? On Mar 2, 2015, at 3:29 PM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote: 2015-03-02 13:58, Raz Amir: > BTW, I don't see it in incoming patches page at > http://dpdk.org/dev/patchwork/project/dpdk/list/?page=1 > Is it because I missed something in the code contribution instruction? It's here: http://dpdk.org/dev/patchwork/patch/3800/
On Tue, Mar 03, 2015 at 01:30:29PM +0200, Raz Amir wrote: > Thanks. > What's next with the suggested parch? > Hi, This patch needs to be reviewed and acked before it can be merged. I'll take a look at it and test it today, I hope, if nobody else reviews and comments on it before I get to it. Regards, /Bruce > On Mar 2, 2015, at 3:29 PM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote: > > 2015-03-02 13:58, Raz Amir: > > BTW, I don't see it in incoming patches page at > > http://dpdk.org/dev/patchwork/project/dpdk/list/?page=1 > > Is it because I missed something in the code contribution instruction? > > It's here: http://dpdk.org/dev/patchwork/patch/3800/ >
Thank you -----Original Message----- From: Bruce Richardson [mailto:bruce.richardson@intel.com] Sent: 03 March 2015 13:45 To: Raz Amir Cc: Thomas Monjalon; dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, and re-probe during driver unload On Tue, Mar 03, 2015 at 01:30:29PM +0200, Raz Amir wrote: > Thanks. > What's next with the suggested parch? > Hi, This patch needs to be reviewed and acked before it can be merged. I'll take a look at it and test it today, I hope, if nobody else reviews and comments on it before I get to it. Regards, /Bruce > On Mar 2, 2015, at 3:29 PM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote: > > 2015-03-02 13:58, Raz Amir: > > BTW, I don't see it in incoming patches page at > > http://dpdk.org/dev/patchwork/project/dpdk/list/?page=1 > > Is it because I missed something in the code contribution instruction? > > It's here: http://dpdk.org/dev/patchwork/patch/3800/ >
On Thu, Feb 26, 2015 at 06:33:20AM +0000, Raz Amir wrote: > Added code that saves the pointers to the detached devices, during > driver loading, and during driver unloading, go over the list, > and re-attach them by calling device_probe_and_attach > on each device. > > Signed-off-by: Raz Amir <razamir22@gmail.com> Couple of minor comments below. Otherwise all looks good to me. Acked-by: Bruce Richardson <bruce.richardson@intel.com> > --- > lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > index 5ae8560..7d702a5 100644 > --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > @@ -55,6 +55,9 @@ __FBSDID("$FreeBSD$"); > > #define MAX_BARS (PCIR_MAX_BAR_0 + 1) > > +#define MAX_DETACHED_DEVICES 128 > +static device_t detached_devices[MAX_DETACHED_DEVICES] = {}; > +static int last_detached = 0; Maybe num_detached/nb_detached or even just "detached" instead of "last_detached". > > struct nic_uio_softc { > device_t dev_t; > @@ -291,14 +294,35 @@ nic_uio_load(void) > if (dev != NULL) We are getting into some serious levels of indentation below, so maybe flip this condition around and put in a "continue" instead, so that we can dedent everything below that follows it. > for (i = 0; i < NUM_DEVICES; i++) > if (pci_get_vendor(dev) == devices[i].vend && > - pci_get_device(dev) == devices[i].dev) > + pci_get_device(dev) == devices[i].dev) { > + if (last_detached+1 < MAX_DETACHED_DEVICES) { I don't think you need the +1 here. > + printf("nic_uio_load: detaching and storing dev=%p\n", dev); > + detached_devices[last_detached++] = dev; > + } > + else { > + printf("nic_uio_load: reached MAX_DETACHED_DEVICES=%d. dev=%p won't be reattached\n", > + MAX_DETACHED_DEVICES, dev); > + } DPDK coding style is not to put braces around single-line statements like this. > + Remove whitespace from this new line. > device_detach(dev); > + } > } > } > > static void > nic_uio_unload(void) > { > + int i; > + printf("nic_uio_unload: entered ... \n"); > + > + for (i = 0; i < last_detached; i++) { > + printf("nic_uio_unload: calling to device_probe_and_attach for dev=%p...\n", > + detached_devices[i]); > + device_probe_and_attach(detached_devices[i]); > + printf("nic_uio_unload: done.\n"); > + } > + > + printf("nic_uio_unload: leaving ... \n"); > } > > static int > -- > 2.1.2 >
Thank you. See answers inline (mostly ack, but not only), and I will send the updated patch soon. > -----Original Message----- > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: 03 March 2015 15:33 > To: Raz Amir > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, and re- > probe during driver unload > > On Thu, Feb 26, 2015 at 06:33:20AM +0000, Raz Amir wrote: > > Added code that saves the pointers to the detached devices, during > > driver loading, and during driver unloading, go over the list, and > > re-attach them by calling device_probe_and_attach on each device. > > > > Signed-off-by: Raz Amir < <mailto:razamir22@gmail.com> razamir22@gmail.com> > > Couple of minor comments below. Otherwise all looks good to me. > > Acked-by: Bruce Richardson < <mailto:bruce.richardson@intel.com> bruce.richardson@intel.com> > > --- > > lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 26 > > +++++++++++++++++++++++++- > > 1 file changed, 25 insertions(+), 1 deletion(-) > > > > diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > index 5ae8560..7d702a5 100644 > > --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > @@ -55,6 +55,9 @@ __FBSDID("$FreeBSD$"); > > > > #define MAX_BARS (PCIR_MAX_BAR_0 + 1) > > > > +#define MAX_DETACHED_DEVICES 128 > > +static device_t detached_devices[MAX_DETACHED_DEVICES] = {}; static > > +int last_detached = 0; > Maybe num_detached/nb_detached or even just "detached" instead of > "last_detached". Ack. > > > > > struct nic_uio_softc { > > device_t dev_t; > > @@ -291,14 +294,35 @@ nic_uio_load(void) > > if (dev != NULL) > > We are getting into some serious levels of indentation below, so maybe flip > this condition around and put in a "continue" instead, so that we can dedent > everything below that follows it. > Ack. > > for (i = 0; i < NUM_DEVICES; i++) > > if (pci_get_vendor(dev) == devices[i].vend > && > > - pci_get_device(dev) == > devices[i].dev) > > + pci_get_device(dev) == > devices[i].dev) { > > + if (last_detached+1 < > MAX_DETACHED_DEVICES) { > I don't think you need the +1 here. It is needed, otherwise the last object will be added at MAX_DETACHED_DEVICES position while the last position is MAX_DETACHED_DEVICES-1. However, I did change the code to: if (last_detached < MAX_DETACHED_DEVICES-1) { to better reflect that. You will see it in the next patch I will send. > > > + > printf("nic_uio_load: detaching and storing dev=%p\n", dev); > > + > detached_devices[last_detached++] = dev; > > + } > > + else { > > + > printf("nic_uio_load: reached MAX_DETACHED_DEVICES=%d. > dev=%p won't be reattached\n", > > + > MAX_DETACHED_DEVICES, dev); > > + } > DPDK coding style is not to put braces around single-line statements like this. Ack. > > > > + > Remove whitespace from this new line. > Ack. > > device_detach(dev); > > + } > > } > > } > > > > static void > > nic_uio_unload(void) > > { > > + int i; > > + printf("nic_uio_unload: entered ... \n"); > > + > > + for (i = 0; i < last_detached; i++) { > > + printf("nic_uio_unload: calling to device_probe_and_attach > for dev=%p...\n", > > + detached_devices[i]); > > + device_probe_and_attach(detached_devices[i]); > > + printf("nic_uio_unload: done.\n"); > > + } > > + > > + printf("nic_uio_unload: leaving ... \n"); > > } > > > > static int > > -- > > 2.1.2 > >
On Wed, Mar 04, 2015 at 11:07:41AM +0200, Raz Amir wrote: > Thank you. > > See answers inline (mostly ack, but not only), and I will send the updated > patch soon. > > > > > -----Original Message----- > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > Sent: 03 March 2015 15:33 > > > To: Raz Amir > > > Cc: dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, and > re- > > > probe during driver unload > > > > > > On Thu, Feb 26, 2015 at 06:33:20AM +0000, Raz Amir wrote: > > > > Added code that saves the pointers to the detached devices, during > > > > driver loading, and during driver unloading, go over the list, and > > > > re-attach them by calling device_probe_and_attach on each device. > > > > > > > > Signed-off-by: Raz Amir < <mailto:razamir22@gmail.com> > razamir22@gmail.com> > > > > > > Couple of minor comments below. Otherwise all looks good to me. > > > > > > Acked-by: Bruce Richardson < <mailto:bruce.richardson@intel.com> > bruce.richardson@intel.com> > > > > --- > > > > lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 26 > > > > +++++++++++++++++++++++++- > > > > 1 file changed, 25 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > > b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > > index 5ae8560..7d702a5 100644 > > > > --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > > +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > > @@ -55,6 +55,9 @@ __FBSDID("$FreeBSD$"); > > > > > > > > #define MAX_BARS (PCIR_MAX_BAR_0 + 1) > > > > > > > > +#define MAX_DETACHED_DEVICES 128 > > > > +static device_t detached_devices[MAX_DETACHED_DEVICES] = {}; static > > > > +int last_detached = 0; > > > Maybe num_detached/nb_detached or even just "detached" instead of > > > "last_detached". > > Ack. > > > > > > > > > > > > > struct nic_uio_softc { > > > > device_t dev_t; > > > > @@ -291,14 +294,35 @@ nic_uio_load(void) > > > > if (dev != NULL) > > > > > > We are getting into some serious levels of indentation below, so maybe > flip > > > this condition around and put in a "continue" instead, so that we can > dedent > > > everything below that follows it. > > > > > Ack. > > > > > > for (i = 0; i < NUM_DEVICES; > i++) > > > > if > (pci_get_vendor(dev) == devices[i].vend > > > && > > > > - > pci_get_device(dev) == > > > devices[i].dev) > > > > + > pci_get_device(dev) == > > > devices[i].dev) { > > > > + > if (last_detached+1 < > > > MAX_DETACHED_DEVICES) { > > > I don't think you need the +1 here. > > It is needed, otherwise the last object will be added at > MAX_DETACHED_DEVICES position while the last position is > MAX_DETACHED_DEVICES-1. Yes, the last position is MAX_DETACHED_DEVICES-1, but you do the addition of the element to the array using "detached_devices[last_detached++]", i.e. a post-increment, so when last_detached == (MAX_DETACHED_DEVICES-1), you still can fill in an entry. Next time around, when last_detached == MAX_DETACHED_DEVICES it's no longer safe to add, and the condition "last_detached < MAX_DETACHED_DEVICES) will now fail. No +1 or -1 necessary to prevent this. /Bruce
Understood. I already sent the updated patch, so I will fix this and resend it soon. -----Original Message----- From: Bruce Richardson [mailto:bruce.richardson@intel.com] Sent: 04 March 2015 12:13 To: Raz Amir Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, and re-probe during driver unload On Wed, Mar 04, 2015 at 11:07:41AM +0200, Raz Amir wrote: > Thank you. > > See answers inline (mostly ack, but not only), and I will send the > updated patch soon. > > > > > -----Original Message----- > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > Sent: 03 March 2015 15:33 > > > To: Raz Amir > > > Cc: dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, > > and > re- > > > probe during driver unload > > > > > > On Thu, Feb 26, 2015 at 06:33:20AM +0000, Raz Amir wrote: > > > > Added code that saves the pointers to the detached devices, during > > > > driver loading, and during driver unloading, go over the list, and > > > > re-attach them by calling device_probe_and_attach on each device. > > > > > > > > Signed-off-by: Raz Amir < <mailto:razamir22@gmail.com> > razamir22@gmail.com> > > > > > > Couple of minor comments below. Otherwise all looks good to me. > > > > > > Acked-by: Bruce Richardson < <mailto:bruce.richardson@intel.com> > bruce.richardson@intel.com> > > > > --- > > > > lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 26 > > > > +++++++++++++++++++++++++- > > > > 1 file changed, 25 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > > b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > > index 5ae8560..7d702a5 100644 > > > > --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > > +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c > > > > @@ -55,6 +55,9 @@ __FBSDID("$FreeBSD$"); > > > > > > > > #define MAX_BARS (PCIR_MAX_BAR_0 + 1) > > > > > > > > +#define MAX_DETACHED_DEVICES 128 > > > > +static device_t detached_devices[MAX_DETACHED_DEVICES] = {}; > > > +static > > > > +int last_detached = 0; > > > Maybe num_detached/nb_detached or even just "detached" instead of > > > "last_detached". > > Ack. > > > > > > > > > > > > > struct nic_uio_softc { > > > > device_t dev_t; > > > > @@ -291,14 +294,35 @@ nic_uio_load(void) > > > > if (dev != NULL) > > > > > > We are getting into some serious levels of indentation below, so > > maybe > flip > > > this condition around and put in a "continue" instead, so that we > > can > dedent > > > everything below that follows it. > > > > > Ack. > > > > > > for (i = 0; i < > > > NUM_DEVICES; > i++) > > > > if > (pci_get_vendor(dev) == devices[i].vend > > > && > > > > - > pci_get_device(dev) == > > > devices[i].dev) > > > > + > pci_get_device(dev) == > > > devices[i].dev) { > > > > + > if (last_detached+1 < > > > MAX_DETACHED_DEVICES) { > > > I don't think you need the +1 here. > > It is needed, otherwise the last object will be added at > MAX_DETACHED_DEVICES position while the last position is > MAX_DETACHED_DEVICES-1. Yes, the last position is MAX_DETACHED_DEVICES-1, but you do the addition of the element to the array using "detached_devices[last_detached++]", i.e. a post-increment, so when last_detached == (MAX_DETACHED_DEVICES-1), you still can fill in an entry. Next time around, when last_detached == MAX_DETACHED_DEVICES it's no longer safe to add, and the condition "last_detached < MAX_DETACHED_DEVICES) will now fail. No +1 or -1 necessary to prevent this. /Bruce
diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c index 5ae8560..7d702a5 100644 --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c @@ -55,6 +55,9 @@ __FBSDID("$FreeBSD$"); #define MAX_BARS (PCIR_MAX_BAR_0 + 1) +#define MAX_DETACHED_DEVICES 128 +static device_t detached_devices[MAX_DETACHED_DEVICES] = {}; +static int last_detached = 0; struct nic_uio_softc { device_t dev_t; @@ -291,14 +294,35 @@ nic_uio_load(void) if (dev != NULL) for (i = 0; i < NUM_DEVICES; i++) if (pci_get_vendor(dev) == devices[i].vend && - pci_get_device(dev) == devices[i].dev) + pci_get_device(dev) == devices[i].dev) { + if (last_detached+1 < MAX_DETACHED_DEVICES) { + printf("nic_uio_load: detaching and storing dev=%p\n", dev); + detached_devices[last_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); + } } } static void nic_uio_unload(void) { + int i; + printf("nic_uio_unload: entered ... \n"); + + for (i = 0; i < last_detached; i++) { + printf("nic_uio_unload: calling to device_probe_and_attach for dev=%p...\n", + detached_devices[i]); + device_probe_and_attach(detached_devices[i]); + printf("nic_uio_unload: done.\n"); + } + + printf("nic_uio_unload: leaving ... \n"); } static int