Re: [PATCH net-next v2 2/2] net: ipa: fix IPA validation

From: Leon Romanovsky
Date: Mon Mar 22 2021 - 10:18:20 EST


On Mon, Mar 22, 2021 at 08:17:59AM -0500, Alex Elder wrote:
> On 3/22/21 1:40 AM, Leon Romanovsky wrote:
> > > I'd like to suggest a plan so I can begin to make progress,
> > > but do so in a way you/others think is satisfactory.
> > > - I would first like to fix the existing bugs, namely that
> > > if IPA_VALIDATION is defined there are build errors, and
> > > that IPA_VALIDATION is not consistently used. That is
> > > this 2-patch series.
> > The thing is that IPA_VALIDATION is not defined in the upstream kernel.
> > There is nothing to be fixed in netdev repository
> >
> > > - I assure you that my goal is to simplify the code that
> > > does this sort of checking. So here are some specific
> > > things I can implement in the coming weeks toward that:
> > > - Anything that can be checked at build time, will
> > > be checked with BUILD_BUG_ON().
> > +1
> >
> > > - Anything checked with BUILD_BUG_ON() will*not*
> > > be conditional. I.e. it won't be inside an
> > > #ifdef IPA_VALIDATION block.
> > > - I will review all remaining VALIDATION code (which
> > > can't--or can't always--be checked at build time),
> > > If it looks prudent to make it*always* be checked,
> > > I will make it always be checked (not conditional
> > > on IPA_VALIDATION).
> > +1
> >
> > > The result should clearly separate checks that can be done
> > > at build time from those that can't.
> > >
> > > And with what's left (especially on that third sub-bullet)
> > > I might have some better examples with which to argue
> > > for something different. Or I might just concede that
> > > you were right all along.
> > I hope so.
>
> I came up with a solution last night that I'm going to try
> to implement. I will still do the things I mention above.
>
> The solution is to create a user space tool inside the
> drivers/net/ipa directory that will link with the kernel
> source files and will perform all the basic one-time checks
> I want to make.

We are not talking about putting this tool in upstream repo, right?
If yes, please get buy-in from David/Jakub first.

Thanks