Re: [PATCH net 1/7] Revert "ibmvnic: simplify reset_long_term_buff function"

[Sukadev Bhattiprolu <sukadev@linux.ibm.com>]

Lijun Pan [lijunp213@gmail.com] wrote:
> On Wed, Jun 23, 2021 at 11:16 PM Sukadev Bhattiprolu
> <sukadev@linux.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