Date:   Fri, 2 Jun 2023 16:15:03 +0100
From:   Ignat Korchagin <ignat@...udflare.com>
To:     Paul Moore <paul@...l-moore.com>
Cc:     Ivan Babrou <ivan@...udflare.com>, audit@...r.kernel.org,
        linux-kernel@...r.kernel.org, kernel-team@...udflare.com,
        Eric Paris <eparis@...hat.com>
Subject: Re: [PATCH] audit: check syscall bitmap on entry to avoid extra work

On Fri, Jun 2, 2023 at 3:56 PM Paul Moore <paul@...l-moore.com> wrote:
>
> On Fri, Jun 2, 2023 at 7:08 AM Ignat Korchagin <ignat@...udflare.com> wrote:
> > On Thu, May 25, 2023 at 3:15 AM Paul Moore <paul@...l-moore.com> wrote:
> > > On Wed, May 24, 2023 at 2:05 PM Ivan Babrou <ivan@...udflare.com> wrote:
> > > > On Tue, May 23, 2023 at 7:03 PM Paul Moore <paul@...l-moore.com> wrote:
> > > > > > Could you elaborate on what exactly you would like to see added? It's
> > > > > > not clear to me what is missing.
> > > > >
> > > > > I should have been more clear, let me try again ...
> > > > >
> > > > > From my perspective, this patch adds code and complexity to deal with
> > > > > the performance impact of auditing.  In some cases that is the right
> > > > > thing to do, but I would much rather see a more in-depth analysis of
> > > > > where the audit hot spots are in this benchmark, and some thoughts on
> > > > > how we might improve that.  In other words, don't just add additional
> > > > > processing to bypass (slower, more involved) processing; look at the
> > > > > processing that is currently being done and see if you can find a way
> > > > > to make it faster.  It will likely take longer, but the results will
> > > > > be much more useful.
> > > >
> > > > The fastest way to do something is to not do it to begin with.
> > >
> > > While you are not wrong, I believe you and I are focusing on different
> > > things.  From my perspective, you appear primarily concerned with
> > > improving performance by reducing the overhead of auditing.  I too am
> > > interested in reducing the audit overhead, but I also place a very
> > > high value on maintainable code, perhaps more than performance simply
> > > because the current audit code quality is so very poor.
> > > Unfortunately, the patch you posted appears to me as yet another
> > > bolt-on performance tweak that doesn't make an attempt at analyzing
> > > the current hot spots of syscall auditing, and ideally offering
> > > solutions.  Perhaps ultimately this approach is the only sane thing
> > > that can be done, but I'd like to see some analysis first of the
> > > syscall auditing path.
> >
> > Ivan is out of office, but I would like to keep the discussion going.
> > We do understand your position and we're actually doing a project
> > right now to investigate audit performance better ...
>
> That's great, thank you!
>
> > But the way I see it - the audit subsystem performance and the way how
> > that subsystem plugs into the rest of the kernel code are two somewhat
> > independent things with the patch proposed here addressing the latter
> > (with full understanding that the former might be improved as well) ...
>
> You've done a good job explaining the reasoning and motivations behind
> the patch submitted, that is good, but I'm not seeing any recognition
> or understanding about the perspective I shared with you earlier.  The
> performance of audit in general does need to be improved, I don't
> think anyone disagrees with that, but my argument is that we need to
> focus on changes which not only reduce the processing overhead, but
> *also* reduce the complexity of the code as well.

Ah, sorry. You're right - I paid too much attention to performance and
didn't quite read your concern about code complexity. But still, I
think that code complexity improvements fall onto the implementation
of the audit subsystem itself vs what we try to accomplish here is to
improve the way how that subsystem plugs into the rest of the kernel.

Ignat

Powered by blists - more mailing lists