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]

Message-ID: <YNStvWD3gd3E5fgv@us.ibm.com>
Date:   Thu, 24 Jun 2021 09:07:25 -0700
From:   Sukadev Bhattiprolu <sukadev@...ux.ibm.com>
To:     Lijun Pan <lijunp213@...il.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"

Lijun Pan [lijunp213@...il.com] wrote:
> 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.

All the more reason to be careful about ripping out code.

>If you believe the verbose hcall is really necessary,
> you'd better document that in the function/source code.

It was necessary and present until you removed it. I am reverting it
after lot of debugging and with sufficient explanation. Feel free to
submit a new patch.

>This patch may be reverted again
> some time later when the verbose calling isn't needed.

Hopefully not without sufficient testing.

Sukadev

Sukadev