netdev - Re: [PATCH net] iavf: fix hang on reboot with ice

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 13 Mar 2023 15:52:22 +0100
From:   Stefan Assmann <sassmann@...nic.de>
To:     Michal Kubiak <michal.kubiak@...el.com>
Cc:     intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
        anthony.l.nguyen@...el.com, patryk.piotrowski@...el.com,
        slawomirx.laba@...el.com
Subject: Re: [PATCH net] iavf: fix hang on reboot with ice

On 13.03.23 10:15, Michal Kubiak wrote:
> On Mon, Mar 13, 2023 at 07:48:52AM +0100, Stefan Assmann wrote:
>>
>> Hi Michal,
>>
>> is it okay to add this change in a follow-up patch? I'd like to get this
>> patch merged quickly since we have a customer being blocked by this
>> issue.
>>
>> Thanks!
>>
>>   Stefan
> 
> 
> Hi Stefan,
> 
> I think it is OK.
> Moreover, I consulted that idea with Slawomir Laba - the author of that loop.
> It seems adding a timeout needs a further investigation. Slawomir claims
> that adding that timeout may cause some side effects in other corner cases,
> so for now let's leave your patch as it is.
> Thank you for fixing it!

Hi Michal, Slawomir,

I looked at this once more and noticed that iavf_remove() had the
following code removed in a8417330f8a5.
       /* When reboot/shutdown is in progress no need to do anything
        * as the adapter is already REMOVE state that was set during
        * iavf_shutdown() callback.
        */
       if (adapter->state == __IAVF_REMOVE)
               return;

So instead of breaking out of the while(1) loop, in iavf_remove(), it
might be better to simply return and avoid going through the whole of
iavf_remove(). That's more in line with the behaviour before
a8417330f8a5.
Otherwise pci_disable_device() might be called twice and give a warning,

  Stefan

> 
> Thanks,
> Michal
> 
> Reviewed-by: Michal Kubiak <michal.kubiak@...el.com>
>