Re: [dpdk-stable] [dpdk-dev] [PATCH] igb_uio: remove device reset in open

[Ferruh Yigit <ferruh.yigit@intel.com>]

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
>