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: <cd36e39a-ebb9-706a-87c3-2f76de82f7ca@intel.com>
Date:   Thu, 6 Jul 2023 13:48:45 +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>
 <c7e4a416-9da4-7ff2-2223-589fd66f557d@intel.com>
 <PUZP153MB0788C7D2376F3271D77CE826CC2CA@PUZP153MB0788.APCP153.PROD.OUTLOOK.COM>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <PUZP153MB0788C7D2376F3271D77CE826CC2CA@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?NHljWngza1IvNmh5MEZkTGx2MVVsVmFnbkw4aHlQSjJEQ0JsbEZNaFluMEo2?=
 =?utf-8?B?QW5vd294U0o4RGhRMkpRek1mSDJlb3dveU8rVXM1YTdDQW4zcktwbXIrMjdP?=
 =?utf-8?B?aGdXZmR1S3pWOXMrbnU2Zmh1RjdoTzNSV3lhWVVySXJDek9XMFh3ekE0cXVt?=
 =?utf-8?B?WlV4UVc4eTR3ZWM5b0ZkVE5hM055RFNXdDlLSDAyMTBEam9HZUdyM0xldWcw?=
 =?utf-8?B?OE44Y29PUTFZWCtqekhQR3lpUVJnNkFORW9KMEdoOW1uZTB3ZzEwaHhFVytJ?=
 =?utf-8?B?RDd2bkk4aXZRNHRqaGJtczZxUXE3NUhSQkNNV2RoMlhtTS81TjNoY1RIQzQv?=
 =?utf-8?B?LzJ3Wi9RRVFOd1JIbERoODFEZlpaYUdoaml3b01hZklCaUxyWk9UK2VIVVVS?=
 =?utf-8?B?SjM0WlNBTy9qWWZTVkdpYUpnZVFYSXhxWUJEQnpVeXQydU1qcXNiUHdLeHdw?=
 =?utf-8?B?ZWw0NWZHbDJlUUhxOEl5M3M0ZjM1STE3OTFRTVBkcVNIbjZvWWY2OS91b3Vt?=
 =?utf-8?B?Z3o5VkVmT1pGcEIwZGViRW5halNTbHgzcnFQMkdlakpGK0RPMzJhU2l3VUpX?=
 =?utf-8?B?WHNwU3hROVh4OEJLQWtFdXdDOE9jKytYd3VENWtnRzZ3UnVFWXRiZjhzOENX?=
 =?utf-8?B?MzBwNWo0T0NKcm1FWFJJVEFBOTNRNGtnN0ZCZXAyMWlsRFk2ZFpwLzMvcVp6?=
 =?utf-8?B?SVRXWmxxVnNLUHRGb3grcU1tem9hSGJGZjVYWHBKQXpIejduMjdkV2J3cWpW?=
 =?utf-8?B?Y2NudW1DVDQ4TmJqODJvd1pYcXlUN3EzTTV1UnFEZFFBTG1KSndFVURpcGd4?=
 =?utf-8?B?YjRGTkhydUViSTRSSzNMdExVKzRQVElPQ05idDZTZ1JGSkIvaTNreDc4dXFr?=
 =?utf-8?B?QStlSnJQZlFKMkxVMnZiM2RKL29ScTRBWTZJTWFrRmRYcCsvbElXUkVVSDd6?=
 =?utf-8?B?c2kvQ3NLdktnZUJYRFE3MEVnWEZPZDJ2bFRJTHIyQlFhL2dERWJxV1JPMFox?=
 =?utf-8?B?K2prT3NWYlMzMDIxcGRuL3FOOVRnakVyTllkTmUyMWhVN3UzY1c2MDVPdnAr?=
 =?utf-8?B?YVJsWW1tTmRPaXliSW9wYjh5T2RIRDlGNCtxZmlMT3FMMjhDMkR0Wkc1YjNr?=
 =?utf-8?B?UURMUDkxL0JOUmk2MWlXcVhQc0VrOEdITXRDaHpLaTMvejBieWtHQW9IYVQ3?=
 =?utf-8?B?STFSTnowNnJXNCtTakFvV2gxNU9vR1FzaGxXWEt4NU9IMjh5eDJSUGVScjFY?=
 =?utf-8?B?aXBtZVJVRWZNL3Y1UGRuNUdtYlAyQnFoemZHQUQ3U2FBNDFQbVJIUTkwdTRL?=
 =?utf-8?B?VDBtVnVzWWpxUHh0dTR0VVRkV08rYjJKZXhaU3hlaXNBL2pYdjl1VnNLUTYy?=
 =?utf-8?B?K1ZWeFJSeU5oa1hrU0VyQzlWdHcxR2duc0FDWFpTS3pSaktJeXFZWW9CVFhm?=
 =?utf-8?B?V0NNQkVaYjh6cDZ0TEtRMmV3ei96VWJhZTNrMkRiKzRtWmtKSmZWQXExNmND?=
 =?utf-8?B?WU5xMmxYcXZPV0kvYk0xRG5LUEo0ZWRMRHpNWW8wcno2V0FhMUpQNk5mZUNv?=
 =?utf-8?B?YlpDUUVScm8xYlJyOTM0ZmZsTWJiY1JMQUN2MVUrMURWWDRiQUhHSWZPSjFh?=
 =?utf-8?B?aXFnUlArQlNaSDZXUjVkaG9oMUZyWEFrMTRPa2V1OSs1Tlk5RVRpUGxGaHI1?=
 =?utf-8?B?WGUrclExc0x2Q0d4bGNoUzBpTFpDTzhNU1greU1DaE9WUTRJZzZsblRJL1B4?=
 =?utf-8?B?Q1J5WnlrYUU5OUFZcGNCaW9UR0NNY3hLR2NhUXBlSUcrUG1KUjZVampiSTRY?=
 =?utf-8?B?UjVpNlNISzJ1ek90MldzK2xKYVpsYUlxMURjbXV2U0RFRXNkaVdmeTNDWXJ0?=
 =?utf-8?B?bVdwL29XclJGeTYzTTJWRy9DRmp2RmNyUkJnMzhSZldyZ01vNUt5THhiVG82?=
 =?utf-8?B?Vm5CTUxiMTQ0amJicW15aVZjQ1NvMTdPWjlGRE5oaFNYdzVTcGdJMUh4RjdI?=
 =?utf-8?B?ZkNUVDQ1Sy9SeUxQWERlaU9DTUhPNVJQZWd6dXIrV09jNTluZDhCUW9XaGZn?=
 =?utf-8?B?N2lUeGwxZStlS2dTQlEvN3NXNHEzV3d4MmxnaU40d3VrWEJ5Z0VuUmJtbldX?=
 =?utf-8?B?ZnpsU05hZnRIT3gwZG9qak5ZZHNSVWFHOXBOOExsUTVFbjR3eEVZT04vUk82?=
 =?utf-8?B?dXc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 61029a79-4dfc-475f-d1ad-08db7e1728eb
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:50:14.0191
 (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: yL93YZBaAUAsVRdh3vlnfIrl52lire1OpbLn0yut+mBzYQtzzZS25fCbS094wuZS02A9QcCrIqc+dN8PyvMM5ElJWsD8G5xdZuc/H90v+HQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN6PR11MB8101
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,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 11:43:58 +0000

> 
> 
>> -----Original Message-----
>> From: Alexander Lobakin <aleksander.lobakin@intel.com>
>> Sent: Thursday, July 6, 2023 5:09 PM
>> 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; Dexuan Cui
>> <decui@microsoft.com>; davem@davemloft.net; edumazet@google.com;
>> kuba@kernel.org; pabeni@redhat.com; Long Li <longli@microsoft.com>; Ajay
>> Sharma <sharmaajay@microsoft.com>; leon@kernel.org;
>> cai.huoqing@linux.dev; ssengar@linux.microsoft.com; vkuznets@redhat.com;
>> tglx@linutronix.de; linux-hyperv@vger.kernel.org; netdev@vger.kernel.org;
>> linux-kernel@vger.kernel.org; linux-rdma@vger.kernel.org;
>> stable@vger.kernel.org
>> Subject: Re: [EXTERNAL] Re: [PATCH V4 net] net: mana: Fix MANA VF unload
>> when host is unresponsive
>>
>> 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?
> It can go near 120 seconds in a very rare MANA h/w scenario. That normally won't happen.
> But during that scenario, we may need 120 seconds.

This waiting loop is needed to let the pending Tx packets be sent. If
they weren't sent in 1 second, it most likely makes no sense already
whether they will be sent at all or not -- the destination host won't
wait for them for so long.
You say that it may happen only in case of HW issue. If so, I assume you
need to fix it some way, e.g. do a HW reset or so? If so, why bother
waiting for Tx completions if Tx is hung? You free all skbs later either
way, so there are no leaks.

>>
>>>>> 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

Thanks,
Olek