[EXT] Re: [PATCH v2] lib/cryptodev: fix assertion to remove GCC compilation warning

Akhil Goyal gakhil at marvell.com
Wed May 24 09:21:34 CEST 2023


> On Tue, 23 May 2023 08:12:28 +0000
> Akhil Goyal <gakhil at marvell.com> wrote:
> 
> > >
> > > This could happen if the passed in length to this routine was larger than
> > > the amount of data in the mbuf. Should the function check and return an
> error?
> > >
> > > Panic should only be reserved for seriously corrupted input (like invalid
> mbuf).
> > >
> > > Also, this is a big enough function that it really should not be inlined.
> >
> > This is a datapath API. RTE_ASSERT is normally not enabled in release build.
> > So, this assert is not doing any check for normal scenario.
> > We normally avoid these type of error checks in the datapath.
> > And while building in debug mode, we need these asserts to give a backtrace
> also
> > To debug the rootcause of the issue.
> >
> > I would suggest fixing the assert itself instead of adding a check.
> > Current patch will affect performance.
> >
> > Agreed, that the function is big for being an inline function,
> > but that is what all the datapath APIs are and
> > we keep them inline to improve the performance.
> 
> Inline is not a magic go fast switch. Turns out that the compilers and cpu's
> already do good job with functions.  Using LTO helps too.

Inline perform better in cases of small functions.
For big functions like the above one, it may not be much beneficial,
but it won't cause any harm as well.
Yes, LTO can be explored. Thanks for the suggestion.



More information about the dev mailing list