Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c7e4a416-9da4-7ff2-2223-589fd66f557d@intel.com>
Date:   Thu, 6 Jul 2023 13:39:07 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [EXTERNAL] Re: [PATCH V4 net] net: mana: Fix MANA VF unload when
 host is unresponsive
Content-Language: en-US
To:     Souradeep Chakrabarti <schakrabarti@microsoft.com>,
        souradeep chakrabarti <schakrabarti@linux.microsoft.com>
CC:     KY Srinivasan <kys@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>,
        "wei.liu@kernel.org" <wei.liu@kernel.org>,
        Dexuan Cui <decui@microsoft.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        Long Li <longli@microsoft.com>,
        Ajay Sharma <sharmaajay@microsoft.com>,
        "leon@kernel.org" <leon@kernel.org>,
        "cai.huoqing@linux.dev" <cai.huoqing@linux.dev>,
        "ssengar@linux.microsoft.com" <ssengar@linux.microsoft.com>,
        "vkuznets@redhat.com" <vkuznets@redhat.com>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>
References: <1688374171-10534-1-git-send-email-schakrabarti@linux.microsoft.com>
 <83ef6401-8736-8416-c898-2fbbb786726e@intel.com>
 <PUZP153MB07880E6D692FD5D13C508694CC29A@PUZP153MB0788.APCP153.PROD.OUTLOOK.COM>
 <7e316b51-be46-96db-84cb-addd28d90b0f@intel.com>
 <PUZP153MB0788A5F92E65AC9A98AF03AFCC2CA@PUZP153MB0788.APCP153.PROD.OUTLOOK.COM>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <PUZP153MB0788A5F92E65AC9A98AF03AFCC2CA@PUZP153MB0788.APCP153.PROD.OUTLOOK.COM>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UTBsT3FmUVRLaFRyaGdDaWh3bS9yN2JDQjlrdWw3YUNjeVY1VlJRY3luM05X?=
 =?utf-8?B?U0pjTGY5TkdmaFo2ZEhRRHZQUGJNZWtsSTExb0publRWWldTN3ByN05uM2hN?=
 =?utf-8?B?Z3pmbUdxSnZTclhwQU1UTkx3L2xtYUlhR2ZJVEtNdnlQVDF3c01vbDFxd0lr?=
 =?utf-8?B?QXgvUEgwVVBrdTdISmdjU3lEUWlCMnpPOEJTbEE4ZWZvSHdyUGNhemxYZTI4?=
 =?utf-8?B?NEdKSk5uS3lTNkZHM3dRYlQvemF6UVdhVlpQTVc4ZGtLM0JJelRnSlczZmtV?=
 =?utf-8?B?Q1ZNYm5zK3VYemhxcm1hNWFWUVp0VGVIUG1EZGlGbURwK0cvVDQvUVI3VjB3?=
 =?utf-8?B?c2VZMml5MDRqalR6bVo3dEpKSTV6Y2ZNR2RGaS9kTytrVjZUNDZ5Z0Z6Z1lE?=
 =?utf-8?B?NTNFRnNjaWVGUkVTWEMrL2RibmVhU3g5WURSZ2taZ2poRmJpQTlYaXlpbHpj?=
 =?utf-8?B?WWIxb2F0TkUzdVYyU2tHM2g4aWhvOXZITTlidlRqNVEyay9USUh2MEhrRDRx?=
 =?utf-8?B?Nmd2bGxldE1ibksvdFVJWWxJZWh0UDRvN0JETXljVWN6UTZrQk1VOWkzM05o?=
 =?utf-8?B?cWVzbVkwL1BRUkRIeVA1eUlDTkZzMG1RYXVOdHAxbWxFR2VvbUFiUU1Zd1Mx?=
 =?utf-8?B?VkJUQkhrZkNxOTdMQjU3Ni92UHlWTVJyOUpJN2VVNjdhZkZucTVFck5rcSsr?=
 =?utf-8?B?b1B3RE0rRW8vL1hpb3R2aGtTb2dOUzdubE1Vc2VUOWlUQ2pNM0pIeEtNTTlF?=
 =?utf-8?B?dEpqbjE5UHJMV2g1a3NSbDk3dFBMaGZwT0laYTBRamVFb3Q1RUxWZWZQRkRh?=
 =?utf-8?B?QkMxUjlxQlBNajlDSWdhOStXbUhrNDFOV2kxaVVETUxHcll4by9yN3hjdTlp?=
 =?utf-8?B?MHlTWjY1SEZpbFRydWpSMGE4TmJxT2pCVHFVZVBuZHo4QjUwMFAyaWVlMGlY?=
 =?utf-8?B?YzZaeVdmbDcrSENXR2d3Z2tnaFdGR2pzcGZYNk1NMzM2RFJTUFVxbHMxWmpU?=
 =?utf-8?B?ZDdWM1pmN3UrejB1QklWcU5DWXEraVkvQTJuQ0JSN1pJRjg0ci9NM056SzJq?=
 =?utf-8?B?b0Q2d1pkVmdUL0Yvd2RlN21IL2hBVkloNlJpc212MEhhVjJ4SkE1dmlFS2dj?=
 =?utf-8?B?M0pjU01CWlBOQnNVUkM2UTZMTFhWcjd6djBETmRjQ0dySkx2RUh4bElaK0h3?=
 =?utf-8?B?cjU3TXprcmhBblJFT1hRcEk5Z1RNclRVcGluZFhINUF6SFZocFk3bnkyV29Y?=
 =?utf-8?B?T3NlRXV3Mi9qSkN1L3o3T2h5NlJsbHFWTUptcGMzL3IyMjVVVGZ1WWNxcUR0?=
 =?utf-8?B?MTZCbnpLcnVWR1NLVThnbFkvcDZaeC9SL3gxVzBPaHFrcmkwQ0h6SmNsTVFB?=
 =?utf-8?B?dmtjTFl2SzN3ejR0OEYxY1hnVTN0VXo2REhFR24xckJhVmt6VjZJVGZpMUNE?=
 =?utf-8?B?VTJZeDBtMlJNTkUySnRvNi9KSGo5MDhBQ0FGckpLVDd4LzE0SlY2NW5mRHN0?=
 =?utf-8?B?M0Rhd3dEMnNaSWs5ZUdiY2hUUGIrb3BERjJPRVlSM2diL3g0eThDRm8zNkpk?=
 =?utf-8?B?SmZGaGNGaG9MbmdYdXpwcERnQUFXRUdiaE8wY1NkTUFlUW9XOFgvZkhyendN?=
 =?utf-8?B?ZE54VlZiZE5UTTJjMG5KellmMGF0ckZvQjBNekF6L05xQy9nQThySDB1WnNF?=
 =?utf-8?B?aEkwam5pU1BreDh6T2l4T0ZYNStnanZodFYzT3J2U0JnYXkvN0hrQkV6Q2Vs?=
 =?utf-8?B?VTFCbnBzZmcvS1RoNmpVckoyUDV3Wk5UVktLQnAzbmhkUlRSbzFGK0s2RkF1?=
 =?utf-8?B?RTNubXpHTWFHTlZ3Mll1WjZUM2pGV01heEJicWtkejRHZTkzdGVudzFjaXlr?=
 =?utf-8?B?c2RLTC94c2JPdVRZY3VTZG5FUVhIbTlyVTFCR1hHcjd0RmhzN2d4a2NTMkpI?=
 =?utf-8?B?YU9MQnVPY1BhaHB0TkxDL2VPYm5Zdm4yblFUaVpiN3dUcEJlaUZ0N2NpdFh1?=
 =?utf-8?B?U09ydjBQVWplL0NtTkxqU3hZWTVnWFVTb2FNK2tVSTltSHZOYjFaUUVsaVlE?=
 =?utf-8?B?MHBXckRGQ0FDMmh2K3JxVldyaFlwd3JobUFnd2NWTFBCRmh1WUIyRWdudDJN?=
 =?utf-8?B?cHBYL0NnTlFvazk4UTBpREFNQ09iaHdianIyOGRYUjZuQ1l1UUxadlRXeHRj?=
 =?utf-8?B?M0E9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b7954c41-4f5f-4462-5ed2-08db7e15d022
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jul 2023 11:40:35.6668
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 5GXgTBPUEmvhe4S8vs1G5SmWza3miZ3J9RFrXdkD75kidOw58WUulkpHIX0dsGG4bfjD5QJz+uvO5Lucrsd3jqcfSaqTNKVw5lZ66pMfR84=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5482
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Souradeep Chakrabarti <schakrabarti@microsoft.com>
Date: Thu, 6 Jul 2023 10:41:03 +0000

> 
> 
>> -----Original Message-----
>> From: Alexander Lobakin <aleksander.lobakin@intel.com>
>> Sent: Wednesday, July 5, 2023 8:06 PM

[...]

>>>> 120 seconds by 2 msec step is 60000 iterations, by 1 msec is 120000
>>>> iterations. I know usleep_range() often is much less precise, but still.
>>>> Do you really need that much time? Has this been measured during the
>>>> tests that it can take up to 120 seconds or is it just some random
>>>> value that "should be enough"?
>>>> If you really need 120 seconds, I'd suggest using a timer / delayed
>>>> work instead of wasting resources.
>>> Here the intent is not waiting for 120 seconds, rather than avoid
>>> continue checking the pending_sends  of each tx queues for an indefinite time,
>> before freeing sk_buffs.
>>> The pending_sends can only get decreased for a tx queue,  if
>>> mana_poll_tx_cq() gets called for a completion notification and increased by
>> xmit.
>>>
>>> In this particular bug, apc->port_is_up is not set to false, causing
>>> xmit to keep increasing the pending_sends for the queue and
>>> mana_poll_tx_cq() not getting called for the queue.
>>>
>>> If we see the comment in the function mana_dealloc_queues(), it mentions it :
>>>
>>> 2346     /* No packet can be transmitted now since apc->port_is_up is false.
>>> 2347      * There is still a tiny chance that mana_poll_tx_cq() can re-enable
>>> 2348      * a txq because it may not timely see apc->port_is_up being cleared
>>> 2349      * to false, but it doesn't matter since mana_start_xmit() drops any
>>> 2350      * new packets due to apc->port_is_up being false.
>>>
>>> The value 120 seconds has been decided here based on maximum number of
>>> queues
>>
>> This is quite opposite to what you're saying above. How should I connect these
>> two:
>>
>> Here the intent is not waiting for 120 seconds
>>
>> +
>>
>> The value 120 seconds has been decided here based on maximum number of
>> queues
>>
>> ?
>> Can cleaning the Tx queues really last for 120 seconds?
>> My understanding is that timeouts need to be sensible and not go to the outer
>> space. What is the medium value you got during the tests?
>>
> For each queue each takes few milli second, in a normal condition. So
> based on maximum number of allowed queues for our h/w it won't
> go beyond a sec. 
> The 120s only happens rarely during some NIC HW issue -unexpected.
> So this timeout will only trigger in a very rare scenario.

So set the timeout to 2 seconds if it makes no difference?

>>> are allowed in this specific hardware, it is a safe assumption.
>>>>
>>>>>
>>>>> +   for (i = 0; i < apc->num_queues; i++) {
>>>>> +           txq = &apc->tx_qp[i].txq;
>>>>> +           cq = &apc->tx_qp[i].tx_cq;
[...]

Thanks,
Olek