Re: [PATCH] igb_uio: remove device reset in open

Re: [PATCH] igb_uio: remove device reset in open

[Patil, Harish]

-----Original Message-----
From: dev <dev-bounces@dpdk.org> on behalf of Harish Patil
<Harish.Patil@cavium.com>
Date: Friday, October 20, 2017 at 3:18 PM
To: Ferruh Yigit <ferruh.yigit@intel.com>, Jingjing Wu
<jingjing.wu@intel.com>, "Thotton, Shijith" <Shijith.Thotton@cavium.com>,
Gregory Etelson <gregory@weka.io>
Cc: Thomas Monjalon <thomas@monjalon.net>, "dev@dpdk.org" <dev@dpdk.org>,
"stable@dpdk.org" <stable@dpdk.org>, Jianfeng Tan
<jianfeng.tan@intel.com>, George Prekas <george.prekas@epfl.ch>, Sergio
Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Subject: Re: [dpdk-dev] [PATCH] igb_uio: remove device reset in open

>[This sender failed our fraud detection checks and may not be who they
>appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
>
>-----Original Message-----
>From: Ferruh Yigit <ferruh.yigit@intel.com>
>Date: Friday, October 20, 2017 at 9:57 AM
>To: Jingjing Wu <jingjing.wu@intel.com>, "Thotton, Shijith"
><Shijith.Thotton@cavium.com>, Gregory Etelson <gregory@weka.io>, Harish
>Patil <Harish.Patil@cavium.com>
>Cc: Thomas Monjalon <thomas@monjalon.net>, "dev@dpdk.org" <dev@dpdk.org>,
>"stable@dpdk.org" <stable@dpdk.org>, Jianfeng Tan
><jianfeng.tan@intel.com>, George Prekas <george.prekas@epfl.ch>, Sergio
>Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>Subject: Re: [PATCH] igb_uio: remove device reset in open
>
>>On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
>>> Remove device reset during application start, the reset for application
>>> exit still there.
>>>
>>> Reset in open removed because of following comments:
>>> 1- Device reset not completed when VF driver loaded, which cause VF PMD
>>>    initialization error.
>>>    Adding delay can solve the issue but will increase driver load time.
>>>
>>> 2- Reset will be issues all devices unconditionally, not very efficient
>>>    way.
>>>
>>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of
>>>device file")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>>Hi Jingjing, Shijith, Gregory, Harish,
>>
>>Can you please test this on top of current master (which has already
>>Jingjin's
>>fix) ?
>>
>>Thanks,
>>Ferruh
>
>
>Ferruh,
>Sure, will try and get back to you.
>Thanks.

Ferruh and all,
This takes care of our SR-IOV VF issue.
Thanks.

>>
>>> ---
>>> Cc: Jianfeng Tan <jianfeng.tan@intel.com>
>>> Cc: Jingjing Wu <jingjing.wu@intel.com>
>>> Cc: Shijith Thotton <shijith.thotton@caviumnetworks.com>
>>> Cc: Gregory Etelson <gregory@weka.io>
>>> Cc: Harish Patil <harish.patil@cavium.com>
>>> Cc: George Prekas <george.prekas@epfl.ch>
>>> Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>>> ---
>>>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>>b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>> index f7ef82554..fd320d87d 100644
>>> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>> @@ -336,8 +336,6 @@ igbuio_pci_open(struct uio_info *info, struct inode
>>>*inode)
>>>      struct pci_dev *dev = udev->pdev;
>>>      int err;
>>>
>>> -    pci_reset_function(dev);
>>> -
>>>      /* set bus master, which was cleared by the reset function */
>>>      pci_set_master(dev);
>>>
>>>
>>
>

Re: [PATCH] igb_uio: revert open and release operations

[Ferruh Yigit]

On 10/20/2017 8:26 AM, Tan, Jianfeng wrote:
> Hi Ferruh & Harish,
> 
> 
> On 10/20/2017 9:15 AM, Ferruh Yigit wrote:
>> On 10/19/2017 3:43 PM, Patil, Harish wrote:
>>> -----Original Message-----
>>> From: Harish Patil <Harish.Patil@cavium.com>
>>> Date: Tuesday, October 17, 2017 at 9:50 PM
>>> To: Thomas Monjalon <thomas@monjalon.net>, Ferruh Yigit
>>> <ferruh.yigit@intel.com>
>>> Cc: "dev@dpdk.org" <dev@dpdk.org>, Jianfeng Tan <jianfeng.tan@intel.com>,
>>> Jingjing Wu <jingjing.wu@intel.com>, "Thotton, Shijith"
>>> <Shijith.Thotton@cavium.com>, Gregory Etelson <gregory@weka.io>, George
>>> Prekas <george.prekas@epfl.ch>, "stable@dpdk.org" <stable@dpdk.org>
>>> Subject: Re: [PATCH] igb_uio: revert open and release operations
>>>>
>>>> -----Original Message-----
>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>> Date: Tuesday, October 17, 2017 at 1:33 PM
>>>> To: Ferruh Yigit <ferruh.yigit@intel.com>, Harish Patil
>>>> <Harish.Patil@cavium.com>
>>>> Cc: "dev@dpdk.org" <dev@dpdk.org>, Jianfeng Tan <jianfeng.tan@intel.com>,
>>>> Jingjing Wu <jingjing.wu@intel.com>, "Thotton, Shijith"
>>>> <Shijith.Thotton@cavium.com>, Gregory Etelson <gregory@weka.io>, George
>>>> Prekas <george.prekas@epfl.ch>, "stable@dpdk.org" <stable@dpdk.org>
>>>> Subject: Re: [PATCH] igb_uio: revert open and release operations
>>>>
>>>>> 17/10/2017 22:14, Ferruh Yigit:
>>>>>> There were bug reports about terminated application may leave device in
>>>>>> undesired state:
>>>>>> http://dpdk.org/ml/archives/dev/2016-November/049745.html
>>>>>> http://dpdk.org/ml/archives/dev/2016-November/050932.html
>>>>>>
>>>>>> And a proposal to fix:
>>>>>> http://dpdk.org/ml/archives/dev/2016-December/051844.html
>>>>>>
>>>>>> Later another proposal triggered the discussion:
>>>>>> http://dpdk.org/ml/archives/dev/2017-May/066317.html
>>>>>>
>>>>>> Finally a fix patch pushed into v17.08:
>>>>>> Commit: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of
>>>>>> device file")
>>>>>>
>>>>>> Later a regression report sent related to the pushed patch:
>>>>>> http://dpdk.org/ml/archives/dev/2017-September/075236.html
>>>>>>
>>>>>> And a fix for regression integrated into v17.11-rc1:
>>>>>> http://dpdk.org/ml/archives/dev/2017-October/079166.html
>>>>>> Commit: 5f6ff30dc507 ("igb_uio: fix interrupt enablement after FLR in
>>>>>> VM")
>>>>>> Commit: 6b9ed026a870 ("igb_uio: fix build with kernel <= 3.17")
>>>>>>
>>>>>> Even after the fix qede PMD reported to be broken:
>>>>>> http://dpdk.org/ml/archives/dev/2017-October/079359.html
>>>>>>
>>>>>> So this patch reverts original fix and related commits. The related
>>>>>> igb_uio code part turns back to v17.05 base.
>>>>> [...]
>>>>>> ---
>>>>>> It would be nice to solve this issue in LTS release, but being close to
>>>>>> the release and the error report without details makes it hard to work
>>>>>> more on this issue.
>>>>> With this revert, we are back to the original issue.
>>>>> We must really try the proposed solution.
>>>>> Harish, please describe your issue and think how it could be fixed.
>>>>> Jingjing made it work for i40e.
>>>>> I know it is less effort to request a simple revert.
>>>>> Please let's try to fix it once for all.
>>>> Hi Ferruh/Thomas,
>>>> I’m discussing it internally, so please hold on committing this patch till
>>>> I revert back to you.
>>>>
>>>> Thanks.
>>>> [Harish]
>>>> 1) With the introduction of:
>>>> Commit: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of
>>>> device file”)
>>>     We saw failures with qede PF & SR-IOV VF initialization in PCI passthru
>>> mode.
>>>
>>>> PF PCI passthru mode initialization failure was resolved by:
>>>> “Commit: 5f6ff30dc507 ("igb_uio: fix interrupt enablement after FLR in
>>>> VM")
>> Thank you for the update.
>>
>>>> SR-IOV VF PCI passthru mode initialization issue is that PCI FLR and
>>>> related device cleanup is not completed by the time VF driver starts
>>>> loading. It results in the mbox command failure sent over the HW channel
>>>> between VF and PF.
>> This seems same reason why i40e added a check and wait loop.
>>
>>>> Even though pci_reset_function() waits for the stipulated amount of time
>>>> per standards, VF FLR takes longer than that and pci_reset_function() &
>>>> igb_uio_open() call returns before FLR completes and VF PMD driver tries
>>>> to load before FLR completes leading to VF PMD initialization failure.
>>>>
>>>>
>>>> We can work around this problem by adding driver delay/retry logic since
>>>> there is no deterministic way of detecting FLR completion. But this is
>>>> going to increase the driver load time.
>>>>
>>>> 2) With the above patch ("igb_uio: issue FLR during open and release of
>>>> device file), FLR is going to be issued unconditionally on all devices
>>>> during igb_uio_open. We think it’s an over kill. FLR is required only for
>>>> previous abnormal app termination. We already handle the abnormal app
>>>> termination by doing necessary cleanup in the driver during load. This
>>>> cleanup is more efficient as it is done only when required. So we feel
>>>> that the drivers/devices needing such cleanup (the two cases listed
>>>> below)  should do it conditionally when required rather than
>>>> igb_uio_open() unconditionally performing FLR all the time.
>>>>
>>>> e.g.,
>>>     - cdb166963cae ("net/liquidio: add API for VF FLR”)
>> Both 1) and 2) related to the pci_reset during open().
>> But the main functionality we are looking for is the pci_reset in release().
>> So we can remove reset during open() [1].
>>
>> Will disabling pci_reset in open() [2] solve your problems?
>>
>> Jingjing, Jianfeng,
>> What do you think?
> 
> If [2] can work for all drivers, I agree with this option.

OK, I will send this as a patch to make it easy to test and discuss.

> 
> Thanks,
> Jianfeng
> 
>>
>>
>> [1]
>> Perhaps will need to revert or partially revert liquidio patch after this.
>>
>> [2]
>> disable line "pci_reset_function(dev);" in the igbuio_pci_open()
>> http://dpdk.org/browse/dpdk/tree/lib/librte_eal/linuxapp/igb_uio/igb_uio.c#n339
>>
> 

[PATCH] igb_uio: remove device reset in open

[Ferruh Yigit]

Remove device reset during application start, the reset for application
exit still there.

Reset in open removed because of following comments:
1- Device reset not completed when VF driver loaded, which cause VF PMD
   initialization error.
   Adding delay can solve the issue but will increase driver load time.

2- Reset will be issues all devices unconditionally, not very efficient
   way.

Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of device file")
Cc: stable@dpdk.org

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Jianfeng Tan <jianfeng.tan@intel.com>
Cc: Jingjing Wu <jingjing.wu@intel.com>
Cc: Shijith Thotton <shijith.thotton@caviumnetworks.com>
Cc: Gregory Etelson <gregory@weka.io>
Cc: Harish Patil <harish.patil@cavium.com>
Cc: George Prekas <george.prekas@epfl.ch>
Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index f7ef82554..fd320d87d 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -336,8 +336,6 @@ igbuio_pci_open(struct uio_info *info, struct inode *inode)
 	struct pci_dev *dev = udev->pdev;
 	int err;
 
-	pci_reset_function(dev);
-
 	/* set bus master, which was cleared by the reset function */
 	pci_set_master(dev);
 
-- 
2.13.6

Re: [PATCH] igb_uio: remove device reset in open

[Ferruh Yigit]

On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
> Remove device reset during application start, the reset for application
> exit still there.
> 
> Reset in open removed because of following comments:
> 1- Device reset not completed when VF driver loaded, which cause VF PMD
>    initialization error.
>    Adding delay can solve the issue but will increase driver load time.
> 
> 2- Reset will be issues all devices unconditionally, not very efficient
>    way.
> 
> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of device file")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Hi Jingjing, Shijith, Gregory, Harish,

Can you please test this on top of current master (which has already Jingjin's
fix) ?

Thanks,
ferruh

> ---
> Cc: Jianfeng Tan <jianfeng.tan@intel.com>
> Cc: Jingjing Wu <jingjing.wu@intel.com>
> Cc: Shijith Thotton <shijith.thotton@caviumnetworks.com>
> Cc: Gregory Etelson <gregory@weka.io>
> Cc: Harish Patil <harish.patil@cavium.com>
> Cc: George Prekas <george.prekas@epfl.ch>
> Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> ---
>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> index f7ef82554..fd320d87d 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> @@ -336,8 +336,6 @@ igbuio_pci_open(struct uio_info *info, struct inode *inode)
>  	struct pci_dev *dev = udev->pdev;
>  	int err;
>  
> -	pci_reset_function(dev);
> -
>  	/* set bus master, which was cleared by the reset function */
>  	pci_set_master(dev);
>  
> 

Re: [PATCH] igb_uio: remove device reset in open

[Gregory Etelson]

On Friday, 20 October 2017 19:57:38 IDT Ferruh Yigit wrote:
> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
> > Remove device reset during application start, the reset for application
> > exit still there.
> > 
> > Reset in open removed because of following comments:
> > 1- Device reset not completed when VF driver loaded, which cause VF PMD
> > 
> >    initialization error.
> >    Adding delay can solve the issue but will increase driver load time.
> > 
> > 2- Reset will be issues all devices unconditionally, not very efficient
> > 
> >    way.
> > 
> > Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of device
> > file") Cc: stable@dpdk.org
> > 
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Hi Jingjing, Shijith, Gregory, Harish,
> 
> Can you please test this on top of current master (which has already
> Jingjin's fix) ?
> 
> Thanks,
> ferruh
> 

sure.

> > ---
> > Cc: Jianfeng Tan <jianfeng.tan@intel.com>
> > Cc: Jingjing Wu <jingjing.wu@intel.com>
> > Cc: Shijith Thotton <shijith.thotton@caviumnetworks.com>
> > Cc: Gregory Etelson <gregory@weka.io>
> > Cc: Harish Patil <harish.patil@cavium.com>
> > Cc: George Prekas <george.prekas@epfl.ch>
> > Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > ---
> > 
> >  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c index f7ef82554..fd320d87d
> > 100644
> > --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > @@ -336,8 +336,6 @@ igbuio_pci_open(struct uio_info *info, struct inode
> > *inode)> 
> >  	struct pci_dev *dev = udev->pdev;
> >  	int err;
> > 
> > -	pci_reset_function(dev);
> > -
> > 
> >  	/* set bus master, which was cleared by the reset function */
> >  	pci_set_master(dev);

Re: [PATCH] igb_uio: remove device reset in open

[Patil, Harish]

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com>
Date: Friday, October 20, 2017 at 9:57 AM
To: Jingjing Wu <jingjing.wu@intel.com>, "Thotton, Shijith"
<Shijith.Thotton@cavium.com>, Gregory Etelson <gregory@weka.io>, Harish
Patil <Harish.Patil@cavium.com>
Cc: Thomas Monjalon <thomas@monjalon.net>, "dev@dpdk.org" <dev@dpdk.org>,
"stable@dpdk.org" <stable@dpdk.org>, Jianfeng Tan
<jianfeng.tan@intel.com>, George Prekas <george.prekas@epfl.ch>, Sergio
Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Subject: Re: [PATCH] igb_uio: remove device reset in open

>On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
>> Remove device reset during application start, the reset for application
>> exit still there.
>> 
>> Reset in open removed because of following comments:
>> 1- Device reset not completed when VF driver loaded, which cause VF PMD
>>    initialization error.
>>    Adding delay can solve the issue but will increase driver load time.
>> 
>> 2- Reset will be issues all devices unconditionally, not very efficient
>>    way.
>> 
>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of
>>device file")
>> Cc: stable@dpdk.org
>> 
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>
>Hi Jingjing, Shijith, Gregory, Harish,
>
>Can you please test this on top of current master (which has already
>Jingjin's
>fix) ?
>
>Thanks,
>Ferruh


Ferruh,
Sure, will try and get back to you.
Thanks.
>
>> ---
>> Cc: Jianfeng Tan <jianfeng.tan@intel.com>
>> Cc: Jingjing Wu <jingjing.wu@intel.com>
>> Cc: Shijith Thotton <shijith.thotton@caviumnetworks.com>
>> Cc: Gregory Etelson <gregory@weka.io>
>> Cc: Harish Patil <harish.patil@cavium.com>
>> Cc: George Prekas <george.prekas@epfl.ch>
>> Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>> ---
>>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 2 --
>>  1 file changed, 2 deletions(-)
>> 
>> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>> index f7ef82554..fd320d87d 100644
>> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>> @@ -336,8 +336,6 @@ igbuio_pci_open(struct uio_info *info, struct inode
>>*inode)
>>  	struct pci_dev *dev = udev->pdev;
>>  	int err;
>>  
>> -	pci_reset_function(dev);
>> -
>>  	/* set bus master, which was cleared by the reset function */
>>  	pci_set_master(dev);
>>  
>> 
>

Re: [PATCH] igb_uio: remove device reset in open

[Shijith Thotton]

On Fri, Oct 20, 2017 at 09:57:38AM -0700, Ferruh Yigit wrote:
> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
> > Remove device reset during application start, the reset for application
> > exit still there.
> > 
> > Reset in open removed because of following comments:
> > 1- Device reset not completed when VF driver loaded, which cause VF PMD
> >    initialization error.
> >    Adding delay can solve the issue but will increase driver load time.
> > 
> > 2- Reset will be issues all devices unconditionally, not very efficient
> >    way.
> > 
> > Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of device file")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Hi Jingjing, Shijith, Gregory, Harish,
> 
> Can you please test this on top of current master (which has already Jingjin's
> fix) ?

Hi Ferruh,

Tested for LiquidIO cards and it works.

Thanks,
Shijith

Re: [PATCH] igb_uio: remove device reset in open

[Ferruh Yigit]

On 10/23/2017 5:28 AM, Shijith Thotton wrote:
> On Fri, Oct 20, 2017 at 09:57:38AM -0700, Ferruh Yigit wrote:
>> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
>>> Remove device reset during application start, the reset for application
>>> exit still there.
>>>
>>> Reset in open removed because of following comments:
>>> 1- Device reset not completed when VF driver loaded, which cause VF PMD
>>>    initialization error.
>>>    Adding delay can solve the issue but will increase driver load time.
>>>
>>> 2- Reset will be issues all devices unconditionally, not very efficient
>>>    way.
>>>
>>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of device file")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>> Hi Jingjing, Shijith, Gregory, Harish,
>>
>> Can you please test this on top of current master (which has already Jingjin's
>> fix) ?
> 
> Hi Ferruh,
> 
> Tested for LiquidIO cards and it works.

Thanks Shijith,

Is following commit needs to be reverted after pci_reset removed from open():

Commit: 9ed3f38770c6 ("net/liquidio: remove FLR request to PF driver")

Thanks,
ferruh

> 
> Thanks,
> Shijith
> 

Re: [PATCH] igb_uio: remove device reset in open

[Shijith Thotton]

On Mon, Oct 23, 2017 at 09:36:38AM -0700, Ferruh Yigit wrote:
> On 10/23/2017 5:28 AM, Shijith Thotton wrote:
> > On Fri, Oct 20, 2017 at 09:57:38AM -0700, Ferruh Yigit wrote:
> >> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
> >>> Remove device reset during application start, the reset for application
> >>> exit still there.
> >>>
> >>> Reset in open removed because of following comments:
> >>> 1- Device reset not completed when VF driver loaded, which cause VF PMD
> >>>    initialization error.
> >>>    Adding delay can solve the issue but will increase driver load time.
> >>>
> >>> 2- Reset will be issues all devices unconditionally, not very efficient
> >>>    way.
> >>>
> >>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of device file")
> >>> Cc: stable@dpdk.org
> >>>
> >>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>
> >> Hi Jingjing, Shijith, Gregory, Harish,
> >>
> >> Can you please test this on top of current master (which has already Jingjin's
> >> fix) ?
> > 
> > Hi Ferruh,
> > 
> > Tested for LiquidIO cards and it works.
> 
> Thanks Shijith,
> 
> Is following commit needs to be reverted after pci_reset removed from open():
> 
> Commit: 9ed3f38770c6 ("net/liquidio: remove FLR request to PF driver")

Not required. The commit can stay.

Thanks,
Shijith

Re: [PATCH] igb_uio: remove device reset in open

[Wu, Jingjing]

> -----Original Message-----
> From: Shijith Thotton [mailto:shijith.thotton@caviumnetworks.com]
> Sent: Monday, October 23, 2017 8:28 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Gregory Etelson <gregory@weka.io>;
> Harish Patil <harish.patil@cavium.com>; Thomas Monjalon
> <thomas@monjalon.net>; dev@dpdk.org; stable@dpdk.org; Tan, Jianfeng
> <jianfeng.tan@intel.com>; George Prekas <george.prekas@epfl.ch>; Gonzalez
> Monroy, Sergio <sergio.gonzalez.monroy@intel.com>
> Subject: Re: [PATCH] igb_uio: remove device reset in open
> 
> On Fri, Oct 20, 2017 at 09:57:38AM -0700, Ferruh Yigit wrote:
> > On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
> > > Remove device reset during application start, the reset for
> > > application exit still there.
> > >
> > > Reset in open removed because of following comments:
> > > 1- Device reset not completed when VF driver loaded, which cause VF PMD
> > >    initialization error.
> > >    Adding delay can solve the issue but will increase driver load time.
> > >
> > > 2- Reset will be issues all devices unconditionally, not very efficient
> > >    way.
> > >
> > > Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of
> > > device file")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >
> > Hi Jingjing, Shijith, Gregory, Harish,
> >
> > Can you please test this on top of current master (which has already
> > Jingjin's
> > fix) ?
> 
> Hi Ferruh,
> 
> Tested for LiquidIO cards and it works.
> 
> Thanks,
> Shijith

Hi, Ferruh

For i40e, it's OK too.

Thanks
Jingjing

Re: [PATCH] igb_uio: remove device reset in open

[Mody, Rasesh]

Hi Ferruh,

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Friday, October 20, 2017 9:58 AM
> 
> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
> > Remove device reset during application start, the reset for
> > application exit still there.
> >
> > Reset in open removed because of following comments:
> > 1- Device reset not completed when VF driver loaded, which cause VF PMD
> >    initialization error.
> >    Adding delay can solve the issue but will increase driver load time.
> >
> > 2- Reset will be issues all devices unconditionally, not very efficient
> >    way.
> >
> > Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of
> > device file")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Hi Jingjing, Shijith, Gregory, Harish,
> 
> Can you please test this on top of current master (which has already Jingjin's
> fix) ?

The original FLR change during igb_uio open()/release() in DPDK17.08 also impacts BNX2X PMD and it exhibits the issues with bare metal testing.
 
Now, we tested this change for BNX2X PMD using latest dpdk, which has this fix where FLR is invoked only in the release(). However, we ran into an issue when trying to reload the testpmd application in quick succession. The pci reset, called during the igb_uio release() operation, is taking longer time and adapter is still doing the FLR when we relaunch the application. We see this behavior with bare metal testing.

Thanks!
-Rasesh

> 
> Thanks,
> ferruh
> 
> > ---
> > Cc: Jianfeng Tan <jianfeng.tan@intel.com>
> > Cc: Jingjing Wu <jingjing.wu@intel.com>
> > Cc: Shijith Thotton <shijith.thotton@caviumnetworks.com>
> > Cc: Gregory Etelson <gregory@weka.io>
> > Cc: Harish Patil <harish.patil@cavium.com>
> > Cc: George Prekas <george.prekas@epfl.ch>
> > Cc: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > ---
> >  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > index f7ef82554..fd320d87d 100644
> > --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > @@ -336,8 +336,6 @@ igbuio_pci_open(struct uio_info *info, struct inode
> *inode)
> >  	struct pci_dev *dev = udev->pdev;
> >  	int err;
> >
> > -	pci_reset_function(dev);
> > -
> >  	/* set bus master, which was cleared by the reset function */
> >  	pci_set_master(dev);
> >
> >

Re: [PATCH] igb_uio: remove device reset in open

[Tan, Jianfeng]

Hi Rasesh,


On 10/26/2017 7:43 AM, Mody, Rasesh wrote:
> Hi Ferruh,
>
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
>> Sent: Friday, October 20, 2017 9:58 AM
>>
>> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
>>> Remove device reset during application start, the reset for
>>> application exit still there.
>>>
>>> Reset in open removed because of following comments:
>>> 1- Device reset not completed when VF driver loaded, which cause VF PMD
>>>     initialization error.
>>>     Adding delay can solve the issue but will increase driver load time.
>>>
>>> 2- Reset will be issues all devices unconditionally, not very efficient
>>>     way.
>>>
>>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of
>>> device file")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> Hi Jingjing, Shijith, Gregory, Harish,
>>
>> Can you please test this on top of current master (which has already Jingjin's
>> fix) ?
> The original FLR change during igb_uio open()/release() in DPDK17.08 also impacts BNX2X PMD and it exhibits the issues with bare metal testing.
>   
> Now, we tested this change for BNX2X PMD using latest dpdk, which has this fix where FLR is invoked only in the release(). However, we ran into an issue when trying to reload the testpmd application in quick succession. The pci reset, called during the igb_uio release() operation, is taking longer time and adapter is still doing the FLR when we relaunch the application. We see this behavior with bare metal testing.

If we don't reset that device, it will continue working which is a more 
serious issue IMO.

How long does it take to reset BTW?

Thanks,
Jianfeng

Re: [PATCH] igb_uio: remove device reset in open

[Ferruh Yigit]

On 10/26/2017 2:28 AM, Tan, Jianfeng wrote:
> Hi Rasesh,
> 
> 
> On 10/26/2017 7:43 AM, Mody, Rasesh wrote:
>> Hi Ferruh,
>>
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
>>> Sent: Friday, October 20, 2017 9:58 AM
>>>
>>> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
>>>> Remove device reset during application start, the reset for
>>>> application exit still there.
>>>>
>>>> Reset in open removed because of following comments:
>>>> 1- Device reset not completed when VF driver loaded, which cause VF PMD
>>>>     initialization error.
>>>>     Adding delay can solve the issue but will increase driver load time.
>>>>
>>>> 2- Reset will be issues all devices unconditionally, not very efficient
>>>>     way.
>>>>
>>>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of
>>>> device file")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>> Hi Jingjing, Shijith, Gregory, Harish,
>>>
>>> Can you please test this on top of current master (which has already Jingjin's
>>> fix) ?
>> The original FLR change during igb_uio open()/release() in DPDK17.08 also impacts BNX2X PMD and it exhibits the issues with bare metal testing.
>>   
>> Now, we tested this change for BNX2X PMD using latest dpdk, which has this fix where FLR is invoked only in the release(). 

Good to hear this fixed the problem.

>> However, we ran into an issue when trying to reload the testpmd application in quick succession. The pci reset, called during the igb_uio release() operation, is taking longer time and adapter is still doing the FLR when we relaunch the application. We see this behavior with bare metal testing.
> 
> If we don't reset that device, it will continue working which is a more 
> serious issue IMO.

+1

> How long does it take to reset BTW?

I was wondering same thing.

> 
> Thanks,
> Jianfeng
> 

Re: [PATCH] igb_uio: remove device reset in open

[Mody, Rasesh]

Hi Jianfeng and Ferruh,

> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Thursday, October 26, 2017 5:50 PM
> 
> On 10/26/2017 2:28 AM, Tan, Jianfeng wrote:
> > Hi Rasesh,
> >
> >
> > On 10/26/2017 7:43 AM, Mody, Rasesh wrote:
> >> Hi Ferruh,
> >>
> >>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> >>> Sent: Friday, October 20, 2017 9:58 AM
> >>>
> >>> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
> >>>> Remove device reset during application start, the reset for
> >>>> application exit still there.
> >>>>
> >>>> Reset in open removed because of following comments:
> >>>> 1- Device reset not completed when VF driver loaded, which cause VF
> PMD
> >>>>     initialization error.
> >>>>     Adding delay can solve the issue but will increase driver load time.
> >>>>
> >>>> 2- Reset will be issues all devices unconditionally, not very efficient
> >>>>     way.
> >>>>
> >>>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of
> >>>> device file")
> >>>> Cc: stable@dpdk.org
> >>>>
> >>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>> Hi Jingjing, Shijith, Gregory, Harish,
> >>>
> >>> Can you please test this on top of current master (which has already
> >>> Jingjin's
> >>> fix) ?
> >> The original FLR change during igb_uio open()/release() in DPDK17.08 also
> impacts BNX2X PMD and it exhibits the issues with bare metal testing.
> >>
> >> Now, we tested this change for BNX2X PMD using latest dpdk, which has
> this fix where FLR is invoked only in the release().
> 
> Good to hear this fixed the problem.

Yes, it fixed the issue caused by pci reset during application start.

> 
> >> However, we ran into an issue when trying to reload the testpmd
> application in quick succession. The pci reset, called during the igb_uio
> release() operation, is taking longer time and adapter is still doing the FLR
> when we relaunch the application. We see this behavior with bare metal
> testing.
> >
> > If we don't reset that device, it will continue working which is a
> > more serious issue IMO.
> 
> +1

I think, it would better for the individual PMDs to take care of the reset during the application exit.

> > How long does it take to reset BTW?
> 
> I was wondering same thing.

A five minutes delay was introduced for the reload of the application, however, we continue to see the issue with FLR during the pci release() operation.

Thanks!
-Rasesh

> 
> >
> > Thanks,
> > Jianfeng
> >

Re: [PATCH] igb_uio: remove device reset in open

[Stephen Hemminger]

On Wed, 1 Nov 2017 06:58:53 +0000
"Mody, Rasesh" <Rasesh.Mody@cavium.com> wrote:

> Hi Jianfeng and Ferruh,
> 
> > From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> > Sent: Thursday, October 26, 2017 5:50 PM
> > 
> > On 10/26/2017 2:28 AM, Tan, Jianfeng wrote:  
> > > Hi Rasesh,
> > >
> > >
> > > On 10/26/2017 7:43 AM, Mody, Rasesh wrote:  
> > >> Hi Ferruh,
> > >>  
> > >>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> > >>> Sent: Friday, October 20, 2017 9:58 AM
> > >>>
> > >>> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:  
> > >>>> Remove device reset during application start, the reset for
> > >>>> application exit still there.
> > >>>>
> > >>>> Reset in open removed because of following comments:
> > >>>> 1- Device reset not completed when VF driver loaded, which cause VF  
> > PMD  
> > >>>>     initialization error.
> > >>>>     Adding delay can solve the issue but will increase driver load time.
> > >>>>
> > >>>> 2- Reset will be issues all devices unconditionally, not very efficient
> > >>>>     way.
> > >>>>
> > >>>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of
> > >>>> device file")
> > >>>> Cc: stable@dpdk.org
> > >>>>
> > >>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>  
> > >>> Hi Jingjing, Shijith, Gregory, Harish,
> > >>>
> > >>> Can you please test this on top of current master (which has already
> > >>> Jingjin's
> > >>> fix) ?  
> > >> The original FLR change during igb_uio open()/release() in DPDK17.08 also  
> > impacts BNX2X PMD and it exhibits the issues with bare metal testing.  
> > >>
> > >> Now, we tested this change for BNX2X PMD using latest dpdk, which has  
> > this fix where FLR is invoked only in the release().
> > 
> > Good to hear this fixed the problem.  
> 
> Yes, it fixed the issue caused by pci reset during application start.
> 
> >   
> > >> However, we ran into an issue when trying to reload the testpmd  
> > application in quick succession. The pci reset, called during the igb_uio
> > release() operation, is taking longer time and adapter is still doing the FLR
> > when we relaunch the application. We see this behavior with bare metal
> > testing.  
> > >
> > > If we don't reset that device, it will continue working which is a
> > > more serious issue IMO.  
> > 
> > +1  
> 
> I think, it would better for the individual PMDs to take care of the reset during the application exit.

That will never be possible. Poll Mode Drivers are userspace entities and part of the application. If application crashes, there is no way for PMD to do cleanup, it must
be handled by kernel.

Re: [PATCH] igb_uio: remove device reset in open

[Mody, Rasesh]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, November 01, 2017 7:12 AM
> 
> On Wed, 1 Nov 2017 06:58:53 +0000
> "Mody, Rasesh" <Rasesh.Mody@cavium.com> wrote:
> 
> > Hi Jianfeng and Ferruh,
> >
> > > From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> > > Sent: Thursday, October 26, 2017 5:50 PM
> > >
> > > On 10/26/2017 2:28 AM, Tan, Jianfeng wrote:
> > > > Hi Rasesh,
> > > >
> > > >
> > > > On 10/26/2017 7:43 AM, Mody, Rasesh wrote:
> > > >> Hi Ferruh,
> > > >>
> > > >>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh
> > > >>> Yigit
> > > >>> Sent: Friday, October 20, 2017 9:58 AM
> > > >>>
> > > >>> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
> > > >>>> Remove device reset during application start, the reset for
> > > >>>> application exit still there.
> > > >>>>
> > > >>>> Reset in open removed because of following comments:
> > > >>>> 1- Device reset not completed when VF driver loaded, which
> > > >>>> cause VF
> > > PMD
> > > >>>>     initialization error.
> > > >>>>     Adding delay can solve the issue but will increase driver load time.
> > > >>>>
> > > >>>> 2- Reset will be issues all devices unconditionally, not very efficient
> > > >>>>     way.
> > > >>>>
> > > >>>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and
> > > >>>> release of device file")
> > > >>>> Cc: stable@dpdk.org
> > > >>>>
> > > >>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > > >>> Hi Jingjing, Shijith, Gregory, Harish,
> > > >>>
> > > >>> Can you please test this on top of current master (which has
> > > >>> already Jingjin's
> > > >>> fix) ?
> > > >> The original FLR change during igb_uio open()/release() in
> > > >> DPDK17.08 also
> > > impacts BNX2X PMD and it exhibits the issues with bare metal testing.
> > > >>
> > > >> Now, we tested this change for BNX2X PMD using latest dpdk, which
> > > >> has
> > > this fix where FLR is invoked only in the release().
> > >
> > > Good to hear this fixed the problem.
> >
> > Yes, it fixed the issue caused by pci reset during application start.
> >
> > >
> > > >> However, we ran into an issue when trying to reload the testpmd
> > > application in quick succession. The pci reset, called during the
> > > igb_uio
> > > release() operation, is taking longer time and adapter is still
> > > doing the FLR when we relaunch the application. We see this behavior
> > > with bare metal testing.
> > > >
> > > > If we don't reset that device, it will continue working which is a
> > > > more serious issue IMO.
> > >
> > > +1
> >
> > I think, it would better for the individual PMDs to take care of the reset
> during the application exit.
> 
> That will never be possible. Poll Mode Drivers are userspace entities and part
> of the application. If application crashes, there is no way for PMD to do
> cleanup, it must be handled by kernel.

The pci reset in release is breaking the BNX2X PMD. Could we revert this reset and get it included with a solution that works for all in the next release?

Thanks!
-Rasesh

Re: [PATCH] igb_uio: remove device reset in open

[Ferruh Yigit]

On 11/2/2017 1:03 AM, Mody, Rasesh wrote:
>> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>> Sent: Wednesday, November 01, 2017 7:12 AM
>>
>> On Wed, 1 Nov 2017 06:58:53 +0000
>> "Mody, Rasesh" <Rasesh.Mody@cavium.com> wrote:
>>
>>> Hi Jianfeng and Ferruh,
>>>
>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>> Sent: Thursday, October 26, 2017 5:50 PM
>>>>
>>>> On 10/26/2017 2:28 AM, Tan, Jianfeng wrote:
>>>>> Hi Rasesh,
>>>>>
>>>>>
>>>>> On 10/26/2017 7:43 AM, Mody, Rasesh wrote:
>>>>>> Hi Ferruh,
>>>>>>
>>>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh
>>>>>>> Yigit
>>>>>>> Sent: Friday, October 20, 2017 9:58 AM
>>>>>>>
>>>>>>> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
>>>>>>>> Remove device reset during application start, the reset for
>>>>>>>> application exit still there.
>>>>>>>>
>>>>>>>> Reset in open removed because of following comments:
>>>>>>>> 1- Device reset not completed when VF driver loaded, which
>>>>>>>> cause VF
>>>> PMD
>>>>>>>>     initialization error.
>>>>>>>>     Adding delay can solve the issue but will increase driver load time.
>>>>>>>>
>>>>>>>> 2- Reset will be issues all devices unconditionally, not very efficient
>>>>>>>>     way.
>>>>>>>>
>>>>>>>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and
>>>>>>>> release of device file")
>>>>>>>> Cc: stable@dpdk.org
>>>>>>>>
>>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>>> Hi Jingjing, Shijith, Gregory, Harish,
>>>>>>>
>>>>>>> Can you please test this on top of current master (which has
>>>>>>> already Jingjin's
>>>>>>> fix) ?
>>>>>> The original FLR change during igb_uio open()/release() in
>>>>>> DPDK17.08 also
>>>> impacts BNX2X PMD and it exhibits the issues with bare metal testing.
>>>>>>
>>>>>> Now, we tested this change for BNX2X PMD using latest dpdk, which
>>>>>> has
>>>> this fix where FLR is invoked only in the release().
>>>>
>>>> Good to hear this fixed the problem.
>>>
>>> Yes, it fixed the issue caused by pci reset during application start.
>>>
>>>>
>>>>>> However, we ran into an issue when trying to reload the testpmd
>>>> application in quick succession. The pci reset, called during the
>>>> igb_uio
>>>> release() operation, is taking longer time and adapter is still
>>>> doing the FLR when we relaunch the application. We see this behavior
>>>> with bare metal testing.
>>>>>
>>>>> If we don't reset that device, it will continue working which is a
>>>>> more serious issue IMO.
>>>>
>>>> +1
>>>
>>> I think, it would better for the individual PMDs to take care of the reset
>> during the application exit.
>>
>> That will never be possible. Poll Mode Drivers are userspace entities and part
>> of the application. If application crashes, there is no way for PMD to do
>> cleanup, it must be handled by kernel.
> 
> The pci reset in release is breaking the BNX2X PMD. Could we revert this reset and get it included with a solution that works for all in the next release?

Hi Rasesh,

I am not sure if there is more to do for solution for next releases, and related
to your case, indeed I wasn't expecting a device reset will take more than five
minutes...

Would you be OK to control the reset via a compile time config option, which is
enabled by default. So you will need to disable it to prevent the reset?

> 
> Thanks!
> -Rasesh
> 

Re: [PATCH] igb_uio: remove device reset in open

[Mody, Rasesh]

> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Thursday, November 02, 2017 1:55 AM
> 
> On 11/2/2017 1:03 AM, Mody, Rasesh wrote:
> >> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> >> Sent: Wednesday, November 01, 2017 7:12 AM
> >>
> >> On Wed, 1 Nov 2017 06:58:53 +0000
> >> "Mody, Rasesh" <Rasesh.Mody@cavium.com> wrote:
> >>
> >>> Hi Jianfeng and Ferruh,
> >>>
> >>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> >>>> Sent: Thursday, October 26, 2017 5:50 PM
> >>>>
> >>>> On 10/26/2017 2:28 AM, Tan, Jianfeng wrote:
> >>>>> Hi Rasesh,
> >>>>>
> >>>>>
> >>>>> On 10/26/2017 7:43 AM, Mody, Rasesh wrote:
> >>>>>> Hi Ferruh,
> >>>>>>
> >>>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh
> >>>>>>> Yigit
> >>>>>>> Sent: Friday, October 20, 2017 9:58 AM
> >>>>>>>
> >>>>>>> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
> >>>>>>>> Remove device reset during application start, the reset for
> >>>>>>>> application exit still there.
> >>>>>>>>
> >>>>>>>> Reset in open removed because of following comments:
> >>>>>>>> 1- Device reset not completed when VF driver loaded, which
> >>>>>>>> cause VF
> >>>> PMD
> >>>>>>>>     initialization error.
> >>>>>>>>     Adding delay can solve the issue but will increase driver load
> time.
> >>>>>>>>
> >>>>>>>> 2- Reset will be issues all devices unconditionally, not very
> efficient
> >>>>>>>>     way.
> >>>>>>>>
> >>>>>>>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and
> >>>>>>>> release of device file")
> >>>>>>>> Cc: stable@dpdk.org
> >>>>>>>>
> >>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>>>>> Hi Jingjing, Shijith, Gregory, Harish,
> >>>>>>>
> >>>>>>> Can you please test this on top of current master (which has
> >>>>>>> already Jingjin's
> >>>>>>> fix) ?
> >>>>>> The original FLR change during igb_uio open()/release() in
> >>>>>> DPDK17.08 also
> >>>> impacts BNX2X PMD and it exhibits the issues with bare metal testing.
> >>>>>>
> >>>>>> Now, we tested this change for BNX2X PMD using latest dpdk, which
> >>>>>> has
> >>>> this fix where FLR is invoked only in the release().
> >>>>
> >>>> Good to hear this fixed the problem.
> >>>
> >>> Yes, it fixed the issue caused by pci reset during application start.
> >>>
> >>>>
> >>>>>> However, we ran into an issue when trying to reload the testpmd
> >>>> application in quick succession. The pci reset, called during the
> >>>> igb_uio
> >>>> release() operation, is taking longer time and adapter is still
> >>>> doing the FLR when we relaunch the application. We see this
> >>>> behavior with bare metal testing.
> >>>>>
> >>>>> If we don't reset that device, it will continue working which is a
> >>>>> more serious issue IMO.
> >>>>
> >>>> +1
> >>>
> >>> I think, it would better for the individual PMDs to take care of the
> >>> reset
> >> during the application exit.
> >>
> >> That will never be possible. Poll Mode Drivers are userspace entities
> >> and part of the application. If application crashes, there is no way
> >> for PMD to do cleanup, it must be handled by kernel.
> >
> > The pci reset in release is breaking the BNX2X PMD. Could we revert this
> reset and get it included with a solution that works for all in the next release?
> 
> Hi Rasesh,
> 
> I am not sure if there is more to do for solution for next releases, and related
> to your case, indeed I wasn't expecting a device reset will take more than
> five minutes...
> 
> Would you be OK to control the reset via a compile time config option, which
> is enabled by default. So you will need to disable it to prevent the reset?

Hi Ferruh,

As I understand, we will have a compile time config option, enabled by default, to guard the pci_reset_function() in the igbuio_pci_release(). We will disable this config option to prevent the reset when using BNX2X.
The controlled reset should work for us.

Thanks!
-Rasesh

Re: [PATCH] igb_uio: remove device reset in open

[Ferruh Yigit]

On 11/2/2017 10:34 AM, Mody, Rasesh wrote:
>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>> Sent: Thursday, November 02, 2017 1:55 AM
>>
>> On 11/2/2017 1:03 AM, Mody, Rasesh wrote:
>>>> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>>>> Sent: Wednesday, November 01, 2017 7:12 AM
>>>>
>>>> On Wed, 1 Nov 2017 06:58:53 +0000
>>>> "Mody, Rasesh" <Rasesh.Mody@cavium.com> wrote:
>>>>
>>>>> Hi Jianfeng and Ferruh,
>>>>>
>>>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>>>> Sent: Thursday, October 26, 2017 5:50 PM
>>>>>>
>>>>>> On 10/26/2017 2:28 AM, Tan, Jianfeng wrote:
>>>>>>> Hi Rasesh,
>>>>>>>
>>>>>>>
>>>>>>> On 10/26/2017 7:43 AM, Mody, Rasesh wrote:
>>>>>>>> Hi Ferruh,
>>>>>>>>
>>>>>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh
>>>>>>>>> Yigit
>>>>>>>>> Sent: Friday, October 20, 2017 9:58 AM
>>>>>>>>>
>>>>>>>>> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
>>>>>>>>>> Remove device reset during application start, the reset for
>>>>>>>>>> application exit still there.
>>>>>>>>>>
>>>>>>>>>> Reset in open removed because of following comments:
>>>>>>>>>> 1- Device reset not completed when VF driver loaded, which
>>>>>>>>>> cause VF
>>>>>> PMD
>>>>>>>>>>     initialization error.
>>>>>>>>>>     Adding delay can solve the issue but will increase driver load
>> time.
>>>>>>>>>>
>>>>>>>>>> 2- Reset will be issues all devices unconditionally, not very
>> efficient
>>>>>>>>>>     way.
>>>>>>>>>>
>>>>>>>>>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and
>>>>>>>>>> release of device file")
>>>>>>>>>> Cc: stable@dpdk.org
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>>>>> Hi Jingjing, Shijith, Gregory, Harish,
>>>>>>>>>
>>>>>>>>> Can you please test this on top of current master (which has
>>>>>>>>> already Jingjin's
>>>>>>>>> fix) ?
>>>>>>>> The original FLR change during igb_uio open()/release() in
>>>>>>>> DPDK17.08 also
>>>>>> impacts BNX2X PMD and it exhibits the issues with bare metal testing.
>>>>>>>>
>>>>>>>> Now, we tested this change for BNX2X PMD using latest dpdk, which
>>>>>>>> has
>>>>>> this fix where FLR is invoked only in the release().
>>>>>>
>>>>>> Good to hear this fixed the problem.
>>>>>
>>>>> Yes, it fixed the issue caused by pci reset during application start.
>>>>>
>>>>>>
>>>>>>>> However, we ran into an issue when trying to reload the testpmd
>>>>>> application in quick succession. The pci reset, called during the
>>>>>> igb_uio
>>>>>> release() operation, is taking longer time and adapter is still
>>>>>> doing the FLR when we relaunch the application. We see this
>>>>>> behavior with bare metal testing.
>>>>>>>
>>>>>>> If we don't reset that device, it will continue working which is a
>>>>>>> more serious issue IMO.
>>>>>>
>>>>>> +1
>>>>>
>>>>> I think, it would better for the individual PMDs to take care of the
>>>>> reset
>>>> during the application exit.
>>>>
>>>> That will never be possible. Poll Mode Drivers are userspace entities
>>>> and part of the application. If application crashes, there is no way
>>>> for PMD to do cleanup, it must be handled by kernel.
>>>
>>> The pci reset in release is breaking the BNX2X PMD. Could we revert this
>> reset and get it included with a solution that works for all in the next release?
>>
>> Hi Rasesh,
>>
>> I am not sure if there is more to do for solution for next releases, and related
>> to your case, indeed I wasn't expecting a device reset will take more than
>> five minutes...
>>
>> Would you be OK to control the reset via a compile time config option, which
>> is enabled by default. So you will need to disable it to prevent the reset?
> 
> Hi Ferruh,
> 
> As I understand, we will have a compile time config option, enabled by default, to guard the pci_reset_function() in the igbuio_pci_release(). We will disable this config option to prevent the reset when using BNX2X.

Yep, this is the idea.

> The controlled reset should work for us.

If there is no objection, I can send a patch for this.

> 
> Thanks!
> -Rasesh
> 

Re: [PATCH] igb_uio: remove device reset in open

[Mody, Rasesh]

> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Thursday, November 02, 2017 11:10 AM
> 
> On 11/2/2017 10:34 AM, Mody, Rasesh wrote:
> >> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> >> Sent: Thursday, November 02, 2017 1:55 AM
> >>
> >> On 11/2/2017 1:03 AM, Mody, Rasesh wrote:
> >>>> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> >>>> Sent: Wednesday, November 01, 2017 7:12 AM
> >>>>
> >>>> On Wed, 1 Nov 2017 06:58:53 +0000
> >>>> "Mody, Rasesh" <Rasesh.Mody@cavium.com> wrote:
> >>>>
> >>>>> Hi Jianfeng and Ferruh,
> >>>>>
> >>>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> >>>>>> Sent: Thursday, October 26, 2017 5:50 PM
> >>>>>>
> >>>>>> On 10/26/2017 2:28 AM, Tan, Jianfeng wrote:
> >>>>>>> Hi Rasesh,
> >>>>>>>
> >>>>>>>
> >>>>>>> On 10/26/2017 7:43 AM, Mody, Rasesh wrote:
> >>>>>>>> Hi Ferruh,
> >>>>>>>>
> >>>>>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh
> >>>>>>>>> Yigit
> >>>>>>>>> Sent: Friday, October 20, 2017 9:58 AM
> >>>>>>>>>
> >>>>>>>>> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
> >>>>>>>>>> Remove device reset during application start, the reset for
> >>>>>>>>>> application exit still there.
> >>>>>>>>>>
> >>>>>>>>>> Reset in open removed because of following comments:
> >>>>>>>>>> 1- Device reset not completed when VF driver loaded, which
> >>>>>>>>>> cause VF
> >>>>>> PMD
> >>>>>>>>>>     initialization error.
> >>>>>>>>>>     Adding delay can solve the issue but will increase driver
> >>>>>>>>>> load
> >> time.
> >>>>>>>>>>
> >>>>>>>>>> 2- Reset will be issues all devices unconditionally, not very
> >> efficient
> >>>>>>>>>>     way.
> >>>>>>>>>>
> >>>>>>>>>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and
> >>>>>>>>>> release of device file")
> >>>>>>>>>> Cc: stable@dpdk.org
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>>>>>>> Hi Jingjing, Shijith, Gregory, Harish,
> >>>>>>>>>
> >>>>>>>>> Can you please test this on top of current master (which has
> >>>>>>>>> already Jingjin's
> >>>>>>>>> fix) ?
> >>>>>>>> The original FLR change during igb_uio open()/release() in
> >>>>>>>> DPDK17.08 also
> >>>>>> impacts BNX2X PMD and it exhibits the issues with bare metal
> testing.
> >>>>>>>>
> >>>>>>>> Now, we tested this change for BNX2X PMD using latest dpdk,
> >>>>>>>> which has
> >>>>>> this fix where FLR is invoked only in the release().
> >>>>>>
> >>>>>> Good to hear this fixed the problem.
> >>>>>
> >>>>> Yes, it fixed the issue caused by pci reset during application start.
> >>>>>
> >>>>>>
> >>>>>>>> However, we ran into an issue when trying to reload the testpmd
> >>>>>> application in quick succession. The pci reset, called during the
> >>>>>> igb_uio
> >>>>>> release() operation, is taking longer time and adapter is still
> >>>>>> doing the FLR when we relaunch the application. We see this
> >>>>>> behavior with bare metal testing.
> >>>>>>>
> >>>>>>> If we don't reset that device, it will continue working which is
> >>>>>>> a more serious issue IMO.
> >>>>>>
> >>>>>> +1
> >>>>>
> >>>>> I think, it would better for the individual PMDs to take care of
> >>>>> the reset
> >>>> during the application exit.
> >>>>
> >>>> That will never be possible. Poll Mode Drivers are userspace
> >>>> entities and part of the application. If application crashes, there
> >>>> is no way for PMD to do cleanup, it must be handled by kernel.
> >>>
> >>> The pci reset in release is breaking the BNX2X PMD. Could we revert
> >>> this
> >> reset and get it included with a solution that works for all in the next
> release?
> >>
> >> Hi Rasesh,
> >>
> >> I am not sure if there is more to do for solution for next releases,
> >> and related to your case, indeed I wasn't expecting a device reset
> >> will take more than five minutes...
> >>
> >> Would you be OK to control the reset via a compile time config
> >> option, which is enabled by default. So you will need to disable it to
> prevent the reset?
> >
> > Hi Ferruh,
> >
> > As I understand, we will have a compile time config option, enabled by
> default, to guard the pci_reset_function() in the igbuio_pci_release(). We
> will disable this config option to prevent the reset when using BNX2X.
> 
> Yep, this is the idea.
> 
> > The controlled reset should work for us.
> 
> If there is no objection, I can send a patch for this.

We are ok as we have at least some way to disable the reset, please send a patch.

Thanks!
-Rasesh

Re: [PATCH] igb_uio: remove device reset in open

[Ferruh Yigit]

On 11/2/2017 11:45 AM, Mody, Rasesh wrote:
>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>> Sent: Thursday, November 02, 2017 11:10 AM
>>
>> On 11/2/2017 10:34 AM, Mody, Rasesh wrote:
>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>> Sent: Thursday, November 02, 2017 1:55 AM
>>>>
>>>> On 11/2/2017 1:03 AM, Mody, Rasesh wrote:
>>>>>> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>>>>>> Sent: Wednesday, November 01, 2017 7:12 AM
>>>>>>
>>>>>> On Wed, 1 Nov 2017 06:58:53 +0000
>>>>>> "Mody, Rasesh" <Rasesh.Mody@cavium.com> wrote:
>>>>>>
>>>>>>> Hi Jianfeng and Ferruh,
>>>>>>>
>>>>>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>>>>>> Sent: Thursday, October 26, 2017 5:50 PM
>>>>>>>>
>>>>>>>> On 10/26/2017 2:28 AM, Tan, Jianfeng wrote:
>>>>>>>>> Hi Rasesh,
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 10/26/2017 7:43 AM, Mody, Rasesh wrote:
>>>>>>>>>> Hi Ferruh,
>>>>>>>>>>
>>>>>>>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh
>>>>>>>>>>> Yigit
>>>>>>>>>>> Sent: Friday, October 20, 2017 9:58 AM
>>>>>>>>>>>
>>>>>>>>>>> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
>>>>>>>>>>>> Remove device reset during application start, the reset for
>>>>>>>>>>>> application exit still there.
>>>>>>>>>>>>
>>>>>>>>>>>> Reset in open removed because of following comments:
>>>>>>>>>>>> 1- Device reset not completed when VF driver loaded, which
>>>>>>>>>>>> cause VF
>>>>>>>> PMD
>>>>>>>>>>>>     initialization error.
>>>>>>>>>>>>     Adding delay can solve the issue but will increase driver
>>>>>>>>>>>> load
>>>> time.
>>>>>>>>>>>>
>>>>>>>>>>>> 2- Reset will be issues all devices unconditionally, not very
>>>> efficient
>>>>>>>>>>>>     way.
>>>>>>>>>>>>
>>>>>>>>>>>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and
>>>>>>>>>>>> release of device file")
>>>>>>>>>>>> Cc: stable@dpdk.org
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>>>>>>> Hi Jingjing, Shijith, Gregory, Harish,
>>>>>>>>>>>
>>>>>>>>>>> Can you please test this on top of current master (which has
>>>>>>>>>>> already Jingjin's
>>>>>>>>>>> fix) ?
>>>>>>>>>> The original FLR change during igb_uio open()/release() in
>>>>>>>>>> DPDK17.08 also
>>>>>>>> impacts BNX2X PMD and it exhibits the issues with bare metal
>> testing.
>>>>>>>>>>
>>>>>>>>>> Now, we tested this change for BNX2X PMD using latest dpdk,
>>>>>>>>>> which has
>>>>>>>> this fix where FLR is invoked only in the release().
>>>>>>>>
>>>>>>>> Good to hear this fixed the problem.
>>>>>>>
>>>>>>> Yes, it fixed the issue caused by pci reset during application start.
>>>>>>>
>>>>>>>>
>>>>>>>>>> However, we ran into an issue when trying to reload the testpmd
>>>>>>>> application in quick succession. The pci reset, called during the
>>>>>>>> igb_uio
>>>>>>>> release() operation, is taking longer time and adapter is still
>>>>>>>> doing the FLR when we relaunch the application. We see this
>>>>>>>> behavior with bare metal testing.
>>>>>>>>>
>>>>>>>>> If we don't reset that device, it will continue working which is
>>>>>>>>> a more serious issue IMO.
>>>>>>>>
>>>>>>>> +1
>>>>>>>
>>>>>>> I think, it would better for the individual PMDs to take care of
>>>>>>> the reset
>>>>>> during the application exit.
>>>>>>
>>>>>> That will never be possible. Poll Mode Drivers are userspace
>>>>>> entities and part of the application. If application crashes, there
>>>>>> is no way for PMD to do cleanup, it must be handled by kernel.
>>>>>
>>>>> The pci reset in release is breaking the BNX2X PMD. Could we revert
>>>>> this
>>>> reset and get it included with a solution that works for all in the next
>> release?
>>>>
>>>> Hi Rasesh,
>>>>
>>>> I am not sure if there is more to do for solution for next releases,
>>>> and related to your case, indeed I wasn't expecting a device reset
>>>> will take more than five minutes...
>>>>
>>>> Would you be OK to control the reset via a compile time config
>>>> option, which is enabled by default. So you will need to disable it to
>> prevent the reset?
>>>
>>> Hi Ferruh,
>>>
>>> As I understand, we will have a compile time config option, enabled by
>> default, to guard the pci_reset_function() in the igbuio_pci_release(). We
>> will disable this config option to prevent the reset when using BNX2X.
>>
>> Yep, this is the idea.
>>
>>> The controlled reset should work for us.
>>
>> If there is no objection, I can send a patch for this.
> 
> We are ok as we have at least some way to disable the reset, please send a patch.

Sent http://dpdk.org/dev/patchwork/patch/31143/, can you please test?

> 
> Thanks!
> -Rasesh
> 

Re: [PATCH] igb_uio: remove device reset in open

[Mody, Rasesh]

> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Thursday, November 02, 2017 5:32 PM
> 
> On 11/2/2017 11:45 AM, Mody, Rasesh wrote:
> >> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> >> Sent: Thursday, November 02, 2017 11:10 AM
> >>
> >> On 11/2/2017 10:34 AM, Mody, Rasesh wrote:
> >>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> >>>> Sent: Thursday, November 02, 2017 1:55 AM
> >>>>
> >>>> On 11/2/2017 1:03 AM, Mody, Rasesh wrote:
> >>>>>> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> >>>>>> Sent: Wednesday, November 01, 2017 7:12 AM
> >>>>>>
> >>>>>> On Wed, 1 Nov 2017 06:58:53 +0000 "Mody, Rasesh"
> >>>>>> <Rasesh.Mody@cavium.com> wrote:
> >>>>>>
> >>>>>>> Hi Jianfeng and Ferruh,
> >>>>>>>
> >>>>>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> >>>>>>>> Sent: Thursday, October 26, 2017 5:50 PM
> >>>>>>>>
> >>>>>>>> On 10/26/2017 2:28 AM, Tan, Jianfeng wrote:
> >>>>>>>>> Hi Rasesh,
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> On 10/26/2017 7:43 AM, Mody, Rasesh wrote:
> >>>>>>>>>> Hi Ferruh,
> >>>>>>>>>>
> >>>>>>>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of
> Ferruh
> >>>>>>>>>>> Yigit
> >>>>>>>>>>> Sent: Friday, October 20, 2017 9:58 AM
> >>>>>>>>>>>
> >>>>>>>>>>> On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
> >>>>>>>>>>>> Remove device reset during application start, the reset for
> >>>>>>>>>>>> application exit still there.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Reset in open removed because of following comments:
> >>>>>>>>>>>> 1- Device reset not completed when VF driver loaded, which
> >>>>>>>>>>>> cause VF
> >>>>>>>> PMD
> >>>>>>>>>>>>     initialization error.
> >>>>>>>>>>>>     Adding delay can solve the issue but will increase
> >>>>>>>>>>>> driver load
> >>>> time.
> >>>>>>>>>>>>
> >>>>>>>>>>>> 2- Reset will be issues all devices unconditionally, not
> >>>>>>>>>>>> very
> >>>> efficient
> >>>>>>>>>>>>     way.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and
> >>>>>>>>>>>> release of device file")
> >>>>>>>>>>>> Cc: stable@dpdk.org
> >>>>>>>>>>>>
> >>>>>>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>>>>>>>>> Hi Jingjing, Shijith, Gregory, Harish,
> >>>>>>>>>>>
> >>>>>>>>>>> Can you please test this on top of current master (which has
> >>>>>>>>>>> already Jingjin's
> >>>>>>>>>>> fix) ?
> >>>>>>>>>> The original FLR change during igb_uio open()/release() in
> >>>>>>>>>> DPDK17.08 also
> >>>>>>>> impacts BNX2X PMD and it exhibits the issues with bare metal
> >> testing.
> >>>>>>>>>>
> >>>>>>>>>> Now, we tested this change for BNX2X PMD using latest dpdk,
> >>>>>>>>>> which has
> >>>>>>>> this fix where FLR is invoked only in the release().
> >>>>>>>>
> >>>>>>>> Good to hear this fixed the problem.
> >>>>>>>
> >>>>>>> Yes, it fixed the issue caused by pci reset during application start.
> >>>>>>>
> >>>>>>>>
> >>>>>>>>>> However, we ran into an issue when trying to reload the
> >>>>>>>>>> testpmd
> >>>>>>>> application in quick succession. The pci reset, called during
> >>>>>>>> the igb_uio
> >>>>>>>> release() operation, is taking longer time and adapter is still
> >>>>>>>> doing the FLR when we relaunch the application. We see this
> >>>>>>>> behavior with bare metal testing.
> >>>>>>>>>
> >>>>>>>>> If we don't reset that device, it will continue working which
> >>>>>>>>> is a more serious issue IMO.
> >>>>>>>>
> >>>>>>>> +1
> >>>>>>>
> >>>>>>> I think, it would better for the individual PMDs to take care of
> >>>>>>> the reset
> >>>>>> during the application exit.
> >>>>>>
> >>>>>> That will never be possible. Poll Mode Drivers are userspace
> >>>>>> entities and part of the application. If application crashes,
> >>>>>> there is no way for PMD to do cleanup, it must be handled by kernel.
> >>>>>
> >>>>> The pci reset in release is breaking the BNX2X PMD. Could we
> >>>>> revert this
> >>>> reset and get it included with a solution that works for all in the
> >>>> next
> >> release?
> >>>>
> >>>> Hi Rasesh,
> >>>>
> >>>> I am not sure if there is more to do for solution for next
> >>>> releases, and related to your case, indeed I wasn't expecting a
> >>>> device reset will take more than five minutes...
> >>>>
> >>>> Would you be OK to control the reset via a compile time config
> >>>> option, which is enabled by default. So you will need to disable it
> >>>> to
> >> prevent the reset?
> >>>
> >>> Hi Ferruh,
> >>>
> >>> As I understand, we will have a compile time config option, enabled
> >>> by
> >> default, to guard the pci_reset_function() in the
> >> igbuio_pci_release(). We will disable this config option to prevent the
> reset when using BNX2X.
> >>
> >> Yep, this is the idea.
> >>
> >>> The controlled reset should work for us.
> >>
> >> If there is no objection, I can send a patch for this.
> >
> > We are ok as we have at least some way to disable the reset, please send a
> patch.
> 
> Sent http://dpdk.org/dev/patchwork/patch/31143/, can you please test?

The testing of BNX2X looks OK with this patch. However, the solution has following drawbacks:
 - an application will need to be recompiled to have the igb_uio kernel module rebuilt to support bnx2x devices
 - this will break pre-compiled solutions that are provided with an OS such as RHOSP or as part of a pre-compiled VNF

We can live with this temporary solution for now. In the long term, we may have to revisit this.
We are also looking at why bnx2x FLR is taking this long.

Thanks!
-Rasesh

Re: [PATCH] igb_uio: remove device reset in open

[Thomas Monjalon]

03/11/2017 20:18, Mody, Rasesh:
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> > On 11/2/2017 11:45 AM, Mody, Rasesh wrote:
> > > From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> > >> On 11/2/2017 10:34 AM, Mody, Rasesh wrote:
> > > We are ok as we have at least some way to disable the reset, please send a
> > patch.
> > 
> > Sent http://dpdk.org/dev/patchwork/patch/31143/, can you please test?
> 
> The testing of BNX2X looks OK with this patch. However, the solution has following drawbacks:
>  - an application will need to be recompiled to have the igb_uio kernel module rebuilt to support bnx2x devices
>  - this will break pre-compiled solutions that are provided with an OS such as RHOSP or as part of a pre-compiled VNF
> 
> We can live with this temporary solution for now. In the long term, we may have to revisit this.
> We are also looking at why bnx2x FLR is taking this long.

Yes, I think the long term solution should be to fix your PMD
or your firmware.
Please keep us posted when you find the root cause with your device.
Thanks

Re: [PATCH] igb_uio: remove device reset in open

[Ferruh Yigit]

On 11/3/2017 12:24 PM, Thomas Monjalon wrote:
> 03/11/2017 20:18, Mody, Rasesh:
>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>> On 11/2/2017 11:45 AM, Mody, Rasesh wrote:
>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>>> On 11/2/2017 10:34 AM, Mody, Rasesh wrote:
>>>> We are ok as we have at least some way to disable the reset, please send a
>>> patch.
>>>
>>> Sent http://dpdk.org/dev/patchwork/patch/31143/, can you please test?
>>
>> The testing of BNX2X looks OK with this patch. However, the solution has following drawbacks:
>>  - an application will need to be recompiled to have the igb_uio kernel module rebuilt to support bnx2x devices
>>  - this will break pre-compiled solutions that are provided with an OS such as RHOSP or as part of a pre-compiled VNF
>>
>> We can live with this temporary solution for now. In the long term, we may have to revisit this.
>> We are also looking at why bnx2x FLR is taking this long.
> 
> Yes, I think the long term solution should be to fix your PMD
> or your firmware.
> Please keep us posted when you find the root cause with your device.
> Thanks

Lee Roberts suggested doing device specific action instead of build time option.

I send following:
http://dpdk.org/dev/patchwork/patch/31168/

What do you think about this approach?

Meanwhile I will send a more proper patch to discuss on.


Thanks,
ferruh

Re: [PATCH] igb_uio: remove device reset in open

[Ferruh Yigit]

On 11/3/2017 3:20 PM, Ferruh Yigit wrote:
> On 11/3/2017 12:24 PM, Thomas Monjalon wrote:
>> 03/11/2017 20:18, Mody, Rasesh:
>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>> On 11/2/2017 11:45 AM, Mody, Rasesh wrote:
>>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>>>> On 11/2/2017 10:34 AM, Mody, Rasesh wrote:
>>>>> We are ok as we have at least some way to disable the reset, please send a
>>>> patch.
>>>>
>>>> Sent http://dpdk.org/dev/patchwork/patch/31143/, can you please test?
>>>
>>> The testing of BNX2X looks OK with this patch. However, the solution has following drawbacks:
>>>  - an application will need to be recompiled to have the igb_uio kernel module rebuilt to support bnx2x devices
>>>  - this will break pre-compiled solutions that are provided with an OS such as RHOSP or as part of a pre-compiled VNF
>>>
>>> We can live with this temporary solution for now. In the long term, we may have to revisit this.
>>> We are also looking at why bnx2x FLR is taking this long.
>>
>> Yes, I think the long term solution should be to fix your PMD
>> or your firmware.
>> Please keep us posted when you find the root cause with your device.
>> Thanks
> 
> Lee Roberts suggested doing device specific action instead of build time option.
> 
> I send following:
> http://dpdk.org/dev/patchwork/patch/31168/
> 
> What do you think about this approach?
> 
> Meanwhile I will send a more proper patch to discuss on.

http://dpdk.org/dev/patchwork/patch/31169/

Re: [PATCH] igb_uio: remove device reset in open

[Ferruh Yigit]

On 10/20/2017 9:55 AM, Ferruh Yigit wrote:
> Remove device reset during application start, the reset for application
> exit still there.
> 
> Reset in open removed because of following comments:
> 1- Device reset not completed when VF driver loaded, which cause VF PMD
>    initialization error.
>    Adding delay can solve the issue but will increase driver load time.
> 
> 2- Reset will be issues all devices unconditionally, not very efficient
>    way.
> 
> Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of device file")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Tested-by: Harish Patil <harish.patil@cavium.com>
Tested-by: Shijith Thotton <shijith.thotton@caviumnetworks.com>
Tested-by: Jingjing Wu <jingjing.wu@intel.com>

Applied to dpdk/master, thanks.