DPDK patches and discussions
 help / color / Atom feed
* [dpdk-dev] [PATCH v2] eal: fix to set the rte_device ptr's device args before hotplug
@ 2020-02-14  6:43 Somnath Kotur
  2020-02-14  8:24 ` Gaetan Rivet
       [not found] ` <CAOBf=mvnb_o90qfN43i3zBYRCLEDQx7W40G+q_41j3f+c7J5Zg@mail.gmail.com>
  0 siblings, 2 replies; 4+ messages in thread
From: Somnath Kotur @ 2020-02-14  6:43 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

As per the comments in this code section, since there is a matching device,
it is now its responsibility to manage the devargs we've just inserted.
But the matching device ptr's devargs is still uninitialized or not pointing
to the newest dev_args that were passed as a parameter to local_dev_probe().
This is needed particularly in the case when *probe is called again* on an
already probed device as part of adding a representor port to an OVS switch(OVS-DPDK)

Fixes: 7e8b26650146 ("eal: fix hotplug add / remove")
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
v1->v2: Incorporated suggestions from Gaetan Rivet
 drivers/bus/pci/linux/pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 740a2cd..71b0a30 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -377,6 +377,11 @@
 						 */
 						RTE_LOG(ERR, EAL, "Unexpected device scan at %s!\n",
 							filename);
+					else if (dev2->device.devargs !=
+						 dev->device.devargs) {
+						rte_devargs_remove(dev2->device.devargs);
+						pci_name_set(dev2);
+					}
 				}
 				free(dev);
 			}
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: fix to set the rte_device ptr's device args before hotplug
  2020-02-14  6:43 [dpdk-dev] [PATCH v2] eal: fix to set the rte_device ptr's device args before hotplug Somnath Kotur
@ 2020-02-14  8:24 ` Gaetan Rivet
  2020-03-31  0:52   ` Thomas Monjalon
       [not found] ` <CAOBf=mvnb_o90qfN43i3zBYRCLEDQx7W40G+q_41j3f+c7J5Zg@mail.gmail.com>
  1 sibling, 1 reply; 4+ messages in thread
From: Gaetan Rivet @ 2020-02-14  8:24 UTC (permalink / raw)
  To: Somnath Kotur, dev; +Cc: ferruh.yigit

On 14/02/2020 07:43, Somnath Kotur wrote:
> As per the comments in this code section, since there is a matching device,
> it is now its responsibility to manage the devargs we've just inserted.
> But the matching device ptr's devargs is still uninitialized or not pointing
> to the newest dev_args that were passed as a parameter to local_dev_probe().
> This is needed particularly in the case when *probe is called again* on an
> already probed device as part of adding a representor port to an OVS switch(OVS-DPDK)
> 
> Fixes: 7e8b26650146 ("eal: fix hotplug add / remove")
> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
> ---
> v1->v2: Incorporated suggestions from Gaetan Rivet
>   drivers/bus/pci/linux/pci.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> index 740a2cd..71b0a30 100644
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -377,6 +377,11 @@
>   						 */
>   						RTE_LOG(ERR, EAL, "Unexpected device scan at %s!\n",
>   							filename);
> +					else if (dev2->device.devargs !=
> +						 dev->device.devargs) {
> +						rte_devargs_remove(dev2->device.devargs);
> +						pci_name_set(dev2);
> +					}
>   				}
>   				free(dev);
>   			}
> 

Hi Somnath,

I see that this is already pretty similar in BSD (minus the rte_devargs_remove()),
so if you have tested and validated that this works properly I'm fine with this patch.

This might miss a Cc: stable@dpdk.org, otherwise,

Acked-by: Gaetan Rivet <grive@u256.net>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: fix to set the rte_device ptr's device args before hotplug
       [not found]     ` <2210365.oX9e4DgFVH@xps>
@ 2020-03-20  4:21       ` Somnath Kotur
  0 siblings, 0 replies; 4+ messages in thread
From: Somnath Kotur @ 2020-03-20  4:21 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Gaetan Rivet, Ferruh Yigit, dev

On Mon, Feb 17, 2020 at 3:36 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 17/02/2020 11:02, Ferruh Yigit:
> > On 2/17/2020 3:18 AM, Somnath Kotur wrote:
> > > On Fri, Feb 14, 2020 at 1:54 PM Gaetan Rivet <grive@u256.net> wrote:
> > >>
> > >> On 14/02/2020 07:43, Somnath Kotur wrote:
> > >>> As per the comments in this code section, since there is a matching device,
> > >>> it is now its responsibility to manage the devargs we've just inserted.
> > >>> But the matching device ptr's devargs is still uninitialized or not pointing
> > >>> to the newest dev_args that were passed as a parameter to local_dev_probe().
> > >>> This is needed particularly in the case when *probe is called again* on an
> > >>> already probed device as part of adding a representor port to an OVS switch(OVS-DPDK)
> > >>>
> > >>> Fixes: 7e8b26650146 ("eal: fix hotplug add / remove")
> > >>> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
> > >>> ---
> > >>> v1->v2: Incorporated suggestions from Gaetan Rivet
> > >>>   drivers/bus/pci/linux/pci.c | 5 +++++
> > >>>   1 file changed, 5 insertions(+)
> > >>>
> > >>> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> > >>> index 740a2cd..71b0a30 100644
> > >>> --- a/drivers/bus/pci/linux/pci.c
> > >>> +++ b/drivers/bus/pci/linux/pci.c
> > >>> @@ -377,6 +377,11 @@
> > >>>                                                */
> > >>>                                               RTE_LOG(ERR, EAL, "Unexpected device scan at %s!\n",
> > >>>                                                       filename);
> > >>> +                                     else if (dev2->device.devargs !=
> > >>> +                                              dev->device.devargs) {
> > >>> +                                             rte_devargs_remove(dev2->device.devargs);
> > >>> +                                             pci_name_set(dev2);
> > >>> +                                     }
> > >>>                               }
> > >>>                               free(dev);
> > >>>                       }
> > >>>
> > >>
> > >> Hi Somnath,
> > >>
> > >> I see that this is already pretty similar in BSD (minus the rte_devargs_remove()),
> > >> so if you have tested and validated that this works properly I'm fine with this patch.
> > >>
> > >> This might miss a Cc: stable@dpdk.org, otherwise,
> > >>
> > >> Acked-by: Gaetan Rivet <grive@u256.net>
> > >
> > > Off the list.
> > > Thanks Gaetan. Ferruh : Anything else you waiting from my side or is
> > > this done ?
> >
> > Hi Somnath,
> >
> > The patch is for the main repo, it is Thomas who will merge it, cc'ed.
>
> I won't take any risk changing this critical code in the last days of 20.02.
> I will take time to review it carefully post-20.02.
>
Thomas,
               Now that 20.02 is out and we are already in the 20.05
window, could you please merge this in or pls give me an ETA by when
you think you'll be able to do it?

Thank you so much!

Som

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: fix to set the rte_device ptr's device args before hotplug
  2020-02-14  8:24 ` Gaetan Rivet
@ 2020-03-31  0:52   ` Thomas Monjalon
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Monjalon @ 2020-03-31  0:52 UTC (permalink / raw)
  To: Somnath Kotur; +Cc: dev, ferruh.yigit, Gaetan Rivet

14/02/2020 09:24, Gaetan Rivet:
> On 14/02/2020 07:43, Somnath Kotur wrote:
> > As per the comments in this code section, since there is a matching device,
> > it is now its responsibility to manage the devargs we've just inserted.
> > But the matching device ptr's devargs is still uninitialized or not pointing
> > to the newest dev_args that were passed as a parameter to local_dev_probe().
> > This is needed particularly in the case when *probe is called again* on an
> > already probed device as part of adding a representor port to an OVS switch(OVS-DPDK)
> > 
> > Fixes: 7e8b26650146 ("eal: fix hotplug add / remove")
> > Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
> > ---
> > --- a/drivers/bus/pci/linux/pci.c
> > +++ b/drivers/bus/pci/linux/pci.c
> > +					else if (dev2->device.devargs !=
> > +						 dev->device.devargs) {
> > +						rte_devargs_remove(dev2->device.devargs);
> > +						pci_name_set(dev2);
> > +					}
> 
> I see that this is already pretty similar in BSD (minus the rte_devargs_remove()),

We really need to review this kind of code for Linux and FreeBSD,
and share the common code.

> so if you have tested and validated that this works properly I'm fine with this patch.
> 
> This might miss a Cc: stable@dpdk.org, otherwise,
> 
> Acked-by: Gaetan Rivet <grive@u256.net>

I don't like how complicate this function is becoming,
but because it's tested and acked,
Applied, thanks

Title updated: bus/pci: fix devargs on probing again



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14  6:43 [dpdk-dev] [PATCH v2] eal: fix to set the rte_device ptr's device args before hotplug Somnath Kotur
2020-02-14  8:24 ` Gaetan Rivet
2020-03-31  0:52   ` Thomas Monjalon
     [not found] ` <CAOBf=mvnb_o90qfN43i3zBYRCLEDQx7W40G+q_41j3f+c7J5Zg@mail.gmail.com>
     [not found]   ` <e835ea74-38d9-e594-1223-f392c88bdbb6@intel.com>
     [not found]     ` <2210365.oX9e4DgFVH@xps>
2020-03-20  4:21       ` Somnath Kotur

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox