netdev - Re: [PATCH net 1/7] Revert "ibmvnic: simplify reset_long_term

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 24 Jun 2021 01:07:00 -0500
From:   Lijun Pan <lijunp213@...il.com>
To:     Sukadev Bhattiprolu <sukadev@...ux.ibm.com>
Cc:     Networking <netdev@...r.kernel.org>,
        Dany Madden <drt@...ux.ibm.com>,
        Rick Lindsley <ricklind@...ux.ibm.com>,
        Brian King <brking@...ux.ibm.com>,
        Cristobal Forno <cforno12@...ux.ibm.com>
Subject: Re: [PATCH net 1/7] Revert "ibmvnic: simplify reset_long_term_buff function"

On Wed, Jun 23, 2021 at 11:16 PM Sukadev Bhattiprolu
<sukadev@...ux.ibm.com> wrote:
>
> This reverts commit 1c7d45e7b2c29080bf6c8cd0e213cc3cbb62a054.
>
> We tried to optimize the number of hcalls we send and skipped sending
> the REQUEST_MAP calls for some maps. However during resets, we need to
> resend all the maps to the VIOS since the VIOS does not remember the
> old values. In fact we may have failed over to a new VIOS which will
> not have any of the mappings.
>
> When we send packets with map ids the VIOS does not know about, it
> triggers a FATAL reset. While the client does recover from the FATAL
> error reset, we are seeing a large number of such resets. Handling
> FATAL resets is lot more unnecessary work than issuing a few more
> hcalls so revert the commit and resend the maps to the VIOS.
>

This was not an issue when the original optimization code was committed.
VIOS changes over time and it is proprietary code, so people don't really know
what it changes every time. If you believe the verbose hcall is really
necessary,
you'd better document that in the function/source code. This patch may
be reverted again
some time later when the verbose calling isn't needed.