public inbox for audit@vger.kernel.org
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Florian Westphal <fw@strlen.de>
Cc: netfilter-devel@vger.kernel.org, paul@paul-moore.com,
	rgb@redhat.com, audit@vger.kernel.org
Subject: Re: [PATCH nf-next v3 0/5] netfilter: nf_tables: reduce set element transaction size
Date: Thu, 17 Oct 2024 18:23:59 +0200	[thread overview]
Message-ID: <ZxE6H03jhdp3gONB@calendula> (raw)
In-Reply-To: <20241016161044.GC6576@breakpoint.cc>

Cc'ing audit ML.

On Wed, Oct 16, 2024 at 06:10:44PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > This is bad, but I do not know if we can change things to make
> > > nft_audit NOT do that.  Hence add a new workaround patch that
> > > inflates the length based on the number of set elements in the
> > > container structure.
> > 
> > It actually shows the number of entries that have been updated, right?
> > 
> > Before this series, there was a 1:1 mapping between transaction and
> > objects so it was easier to infer it from the number of transaction
> > objects.
> 
> Yes, but... for element add (but not create), we used to not do anything
> (no-op), so we did not allocate a new transaction and pretend request
> did not exist.

You refer to element updates above, those used to be elided, yes. Now
they are shown. I think that is correct.

> Now we can enter update path, so we do allocate a transaction, hence,
> audit record changes.
>
> What if we add an internal special-case 'flush' op in the future?

You mean, if 'flush' does not get expanded to one delete transaction
for each element. Yes, that would require to look at ->nelems as in
this patch.

> It will break, and the workaround added in this series needs to be
> extended.
> 
> Same for an other change that could elide a transaction request, or,
> add expand something to multiple ones (as flush currently does).
> 
> Its doesn't *break* audit, but it changes the output.

My understanding is that audit is exposing the entries that have been
added/updated/deleted, which is already sparse: Note that nftables
audit works at 'table' granurality.

IIRC, one of the audit maintainers mentioned it should be possible to
add chain and set to the audit logs in the future, such change would
necessarily change the audit logs output.

Thanks.

       reply	other threads:[~2024-10-17 16:24 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20241016131917.17193-1-fw@strlen.de>
     [not found] ` <Zw_PY7MXqNDOWE71@calendula>
     [not found]   ` <20241016161044.GC6576@breakpoint.cc>
2024-10-17 16:23     ` Pablo Neira Ayuso [this message]
2024-10-17 19:33       ` [PATCH nf-next v3 0/5] netfilter: nf_tables: reduce set element transaction size Paul Moore
2024-10-17 22:53         ` Pablo Neira Ayuso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZxE6H03jhdp3gONB@calendula \
    --to=pablo@netfilter.org \
    --cc=audit@vger.kernel.org \
    --cc=fw@strlen.de \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=rgb@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox