netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Phil Sutter <phil@nwl.cc>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netfilter-devel@vger.kernel.org,
	Serhey Popovych <serhe.popovych@gmail.com>
Subject: [iptables PATCH 0/3] libxtables: Fix for pointless socket() calls
Date: Wed, 23 Sep 2020 00:53:38 +0200	[thread overview]
Message-ID: <20200922225341.8976-1-phil@nwl.cc> (raw)

The motivation for this series was a bug report claiming a near 100%
slowdown of iptables-restore when passed a large number of rules
containing conntrack match between two kernel versions. Turns out the
curlprit kernel change was within SELinux and in fact a performance
optimization, namely an introduced hash table mapping from security
context string to SID. This hash table insert, which happened for each
new socket, slowed iptables-restore down considerably.

The actual problem exposed by the above was that iptables-restore opens
a surprisingly large number of sockets when restoring said ruleset. This
stems from bugs in extension compatibility checks done during extension
registration (actually, "full registration").

One of the problems was that incompatible older revsions of an extension
never were never dropped from the pending list, and thus retried for
each rule using the extension. Coincidently, conntrack revision 0
matches this criteria.

Another problem was a (likely) accidental recursion of
xtables_fully_register_pending_*() via xtables_find_*(). In combination
with incompatible match revisions stuck in pending list, this caused
even more extra compatibility checks.

Solve all these problems by making pending extension lists sorted by
(descending) revision number. If at least one revision was compatible
with the kernel, any following incompatible ones may safely be dropped.
This should on one hand get rid of the repeated compatibility checks
while on the other maintain the presumptions stated in commit
3b2530ce7a0d6 ("xtables: Do not register matches/targets with
incompatible revision").

Patch 1 establishes the needed sorting in pending extension lists,
patch 2 then simplifies xtables_fully_register_pending_*() functions.
Patch 3 is strictly speaking not necessary but nice to have as it
streamlines array-based extension registrators with the extension
sorting.

Phil Sutter (3):
  libxtables: Make sure extensions register in revision order
  libxtables: Simplify pending extension registration
  libxtables: Register multiple extensions in ascending order

 libxtables/xtables.c | 206 +++++++++++++++++++++----------------------
 1 file changed, 99 insertions(+), 107 deletions(-)

-- 
2.28.0


             reply	other threads:[~2020-09-22 22:42 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-22 22:53 Phil Sutter [this message]
2020-09-22 22:53 ` [iptables PATCH 1/3] libxtables: Make sure extensions register in revision order Phil Sutter
2020-10-03 11:17   ` Pablo Neira Ayuso
2020-10-04 14:53     ` Phil Sutter
2020-10-05 22:42       ` Pablo Neira Ayuso
2020-10-06  9:27         ` Phil Sutter
2020-10-06  9:50           ` Pablo Neira Ayuso
2020-10-06 10:13             ` Phil Sutter
2020-10-06 10:48               ` Pablo Neira Ayuso
2020-10-06 12:07   ` [iptables PATCH v2] " Phil Sutter
2020-10-06 23:59     ` Pablo Neira Ayuso
2020-09-22 22:53 ` [iptables PATCH 2/3] libxtables: Simplify pending extension registration Phil Sutter
2020-10-05 23:08   ` Pablo Neira Ayuso
2020-09-22 22:53 ` [iptables PATCH 3/3] libxtables: Register multiple extensions in ascending order Phil Sutter
2020-10-05 23:41   ` Pablo Neira Ayuso
2020-10-06  9:29     ` Phil Sutter
2020-09-23 11:45 ` [iptables PATCH 0/3] libxtables: Fix for pointless socket() calls Pablo Neira Ayuso
2020-09-23 14:30   ` Phil Sutter
2020-10-07  0:02 ` 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=20200922225341.8976-1-phil@nwl.cc \
    --to=phil@nwl.cc \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=serhe.popovych@gmail.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).