Re: [Intel-wired-lan] [PATCH iwl-next v3] ice: Reset VF on Tx MDD event

[Marcin Szycik <marcin.szycik@linux.intel.com>]

On 29.03.2024 12:31, Marcin Szycik wrote:
> 
> 
> On 28.03.2024 18:34, Simon Horman wrote:
>> On Tue, Mar 26, 2024 at 05:44:55PM +0100, Marcin Szycik wrote:
>>> In cases when VF sends malformed packets that are classified as malicious,
>>> sometimes it causes Tx queue to freeze. This frozen queue can be stuck
>>> for several minutes being unusable. This behavior can be reproduced with
>>> a faulty userspace app running on VF.
>>>
>>> When Malicious Driver Detection event occurs and the mdd-auto-reset-vf
>>> private flag is set, perform a graceful VF reset to quickly bring VF back
>>> to operational state. Add a log message to notify about the cause of
>>> the reset. Add a helper for this to be reused for both TX and RX events.
>>>
>>> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
>>> Co-developed-by: Liang-Min Wang <liang-min.wang@intel.com>
>>> Signed-off-by: Liang-Min Wang <liang-min.wang@intel.com>
>>> Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com>
>>
>> Hi Marcin,
>>
>> If I read this correctly then a reset may be performed for several
>> different conditions - values of different registers - for a VF
>> as checked in a for loop.
>>
>> I am wondering if multiple resets could occur for the same VF within
>> an iteration of the for loop - because more than one of the conditions is
>> met. And, if so, is this ok?
> 
> Hi Simon,
> 
> Good point. Nothing too bad should happen, as ice_reset_vf() acquires mutex lock

Sorry, that mutex doesn't matter much here, as we'd call another ice_reset_vf()
after previous one is done anyway.

> (in fact two locks), so several resets would just happen in sequence. However,
> it doesn't make much sense to reset VF multiple times, so maybe instead of issuing
> reset on each condition, I'll set some flag, and after checking all registers I'll
> trigger reset if that flag is set. What do you think?
> 
> Thanks,
> Marcin