netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Paul Moore <paul@paul-moore.com>
Cc: Florian Westphal <fw@strlen.de>,
	netfilter-devel@vger.kernel.org, rgb@redhat.com,
	audit@vger.kernel.org
Subject: Re: [PATCH nf-next v3 0/5] netfilter: nf_tables: reduce set element transaction size
Date: Fri, 18 Oct 2024 00:53:38 +0200	[thread overview]
Message-ID: <ZxGVch7AsOT5Ef_s@calendula> (raw)
In-Reply-To: <CAHC9VhQxp4_qhuuKip7qP_Jz-ysv1RZ1o83iARCRP7Psh_dBNQ@mail.gmail.com>

On Thu, Oct 17, 2024 at 03:33:14PM -0400, Paul Moore wrote:
> > On Wed, Oct 16, 2024 at 06:10:44PM +0200, Florian Westphal wrote:
> For those of us joining the conversation late, can you provide a quick
> summary of what you want to change in audit and why?

Florian said:

"I failed to realize that nft_audit leaks one implementation detail
to userspace: the length of the transaction log."

        table=t1 family=2 entries=4 op=nft_register_set

He is referring to the 'entries' key.

So far, every object gets one transaction, but now batching several
objects in one transaction is possible.

We have been discussing what the expected semantics for this audit log
key is:

- If this is the transaction log length, then the internal update of the
  transaction logic results in a smaller number of 'entries' in the
  audit log.
- If 'entries' refers to the number of "affected" objects by this
  operation, then this means we have to carry a "workaround" in
  the kernel.

This is because:

1) Element updates are now supported, this currently handles it as a
   _REGISTER operation according to enum audit_nfcfgop. This changed
   the semantics of the add command, now it is "add if it does not exist,
   otherwise update what it already exists". Before, updates where simply
   elided (not counted by 'entries' key) because they were not supported.
   That is, 'entries' now tell how many set element has been added OR
   updated. I think this is fine, this is consistent with chain updates
   where 'entries' also report added OR updated chains. The difference
   is that old kernel do not count updates (because they are elided).

2) There is ongoing work to add more internal transaction batching, ie.
   use one single transaction for several elements. This requires a
   special case to bump the 'entries' key to add the elements that the
   transaction batch contains, see:

https://patchwork.ozlabs.org/project/netfilter-devel/patch/20241016131917.17193-4-fw@strlen.de/

   There is a nft_audit.sh selftest and Florian thinks this is a
   "workaround" patch only to make this test happy, because 'entries'
   refers to the transaction log length (which is now smaller given the
   internal transaction batching approach to accumulate several elements
   is used).

      reply	other threads:[~2024-10-17 22:53 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-16 13:19 [PATCH nf-next v3 0/5] netfilter: nf_tables: reduce set element transaction size Florian Westphal
2024-10-16 13:19 ` [PATCH nf-next v3 1/5] netfilter: nf_tables: add nft_trans_commit_list_add_elem helper Florian Westphal
2024-10-16 13:19 ` [PATCH nf-next v3 2/5] netfilter: nf_tables: prepare for multiple elements in nft_trans_elem structure Florian Westphal
2024-10-16 13:19 ` [PATCH nf-next v3 3/5] netfiler: nf_tables: preemitve fix for audit failure Florian Westphal
2024-10-16 13:19 ` [PATCH nf-next v3 4/5] netfilter: nf_tables: switch trans_elem to real flex array Florian Westphal
2024-10-16 13:19 ` [PATCH nf-next v3 5/5] netfilter: nf_tables: allocate element update information dynamically Florian Westphal
2024-10-16 14:36 ` [PATCH nf-next v3 0/5] netfilter: nf_tables: reduce set element transaction size Pablo Neira Ayuso
2024-10-16 16:10   ` Florian Westphal
2024-10-17 16:23     ` Pablo Neira Ayuso
2024-10-17 19:33       ` Paul Moore
2024-10-17 22:53         ` Pablo Neira Ayuso [this message]

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=ZxGVch7AsOT5Ef_s@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;
as well as URLs for NNTP newsgroup(s).