| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
|
|
|
|
|
|
|
|
| |
This allows to call nft_table_builtin_find() and hence removes the only
real user of __nft_table_builtin_find(). Consequently remove the latter
by integrating it into its sole caller.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Unless --noflush was given, xtables-restore merely needs the list of
tables to decide whether to delete it or not. Introduce nft_fake_cache()
function which populates table list, initializes chain lists (so
nft_chain_list_get() returns an empty list instead of NULL) and sets
'have_cache' to turn any later calls to nft_build_cache() into nops.
If --noflush was given, call nft_build_cache() just once instead of for
each table line in input.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
| |
No need for a full cache to serve the list of tables.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Another corner-case found when extending restore ordering test: If a
delete command in a dump referenced a rule added earlier within the same
dump, kernel would reject the resulting NFT_MSG_DELRULE command.
Catch this by assigning the rule to delete a RULE_ID value if it doesn't
have a handle yet. Since __nft_rule_del() does not duplicate the
nftnl_rule object when creating the NFT_COMPAT_RULE_DELETE command, this
RULE_ID value is added to both NEWRULE and DELRULE commands - exactly
what is needed to establish the reference.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Instead simply use ARRAY_SIZE() macro to not overstep supported_exprs
array.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
It's a define which resolves into a callback which in turn is declared
with noreturn attribute. It will never return, therefore drop all
explicit exit() calls or other dead code immediately following it.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This improves cache population quite a bit and therefore helps when
dealing with large rulesets. A simple hard to improve use-case is
listing the last rule in a large chain. These are the average program
run times depending on number of rules:
rule count | legacy | nft old | nft new
---------------------------------------------------------
50,000 | .052s | .611s | .406s
100,000 | .115s | 2.12s | 1.24s
150,000 | .265s | 7.63s | 4.14s
200,000 | .411s | 21.0s | 10.6s
So while legacy iptables is still magnitudes faster, this simple change
doubles iptables-nft performance in ideal cases.
Note that using a larger buffer than 32KB doesn't further improve
performance since linux kernel won't transmit more data at once. This
limit was set (actually extended from 16KB) in kernel commit
d35c99ff77ecb ("netlink: do not enter direct reclaim from
netlink_dump()").
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
No need to check family value from nft_commit() if we can have a
dedicated callback for bridge family.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
| |
Although it doesn't make a difference in practice, they are the correct
API functions to use when assigning string attributes.
While doing so, also drop the needless casts to non-const.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
This is a leftover, the file does not exist in fresh clones.
Fixes: 06fd5e46d46f7 ("xtables: Drop support for /etc/xtables.conf")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
As decided upon at NFWS2019, drop support for configurable nftables base
chains to use with iptables-nft.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
| |
When trying to flush a non-existent chain, errno gets set in
nft_xtables_config_load(). That is an unintended side-effect and when
support for xtables.conf is later removed, iptables-nft will emit the
generic "Incompatible with this kernel." error message instead of "No
chain/target/match by that name." as it should.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
Support passing arbitrary data (via void pointer) to the callback.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
| |
When running iptables -nL as non-root user, iptables would loop indefinitely.
With this change, it will fail with
iptables v1.8.3 (nf_tables): Could not fetch rule set generation id: Permission denied (you must be root)
Reported-by: Amish <anon.amish@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
Store them next to the mnl_socket pointer. While being at it, add a
comment to mnl_set_rcvbuffer() explaining why the buffer size is
changed.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
>From there, pass it along to mnl_nft_socket_sendmsg() and further down
to mnl_set_{snd,rcv}buffer(). This prepares the code path for keeping
stored socket buffer sizes in struct nft_handle.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When trying to delete user-defined chains in a large ruleset,
iptables-nft aborts with "No buffer space available". This can be
reproduced using the following script:
| #! /bin/bash
| iptables-nft-restore <(
|
| echo "*filter"
| for i in $(seq 0 200000);do
| printf ":chain_%06x - [0:0]\n" $i
| done
| for i in $(seq 0 200000);do
| printf -- "-A INPUT -j chain_%06x\n" $i
| printf -- "-A INPUT -j chain_%06x\n" $i
| done
| echo COMMIT
|
| )
| iptables-nft -X
The problem seems to be the sheer amount of netlink error messages sent
back to user space (one EBUSY for each chain). To solve this, set
receive buffer size depending on number of commands sent to kernel.
Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
Otherwise, mnl_set_sndbuffer() skips the buffer update after socket
restart. Then, sendmsg() fails with EMSGSIZE later on when sending the
batch to the kernel.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
Patch ab1cd3b510fa ("nft: ensure cache consistency") already handles
consistency via generation ID.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
We need to re-evalute based on the existing cache generation.
Fixes: 58d7de0181f6 ("xtables: handle concurrent ruleset modifications")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
I don't find a scenario that trigger this case.
Fixes: 58d7de0181f6 ("xtables: handle concurrent ruleset modifications")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
| |
The commit this fixes added a new parameter to __nft_rule_flush() to
mark a rule flush job as implicit or not. Yet the code added to that
function ignores the parameter and instead always sets batch job's
'implicit' flag to 1.
Fixes: 77e6a93d5c9dc ("xtables: add and set "implict" flag on transaction objects")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Phil Sutter says:
"The problem is that data in h->obj_list potentially sits in cache, too.
At least rules have to be there so insert with index works correctly. If
the cache is flushed before regenerating the batch, use-after-free
occurs which crashes the program."
This patch keeps around the original cache until we have refreshed the
batch.
Fixes: 862818ac3a0de ("xtables: add and use nft_build_cache")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
Check for generation ID before and after fetching the cache to ensure
consistency.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
nft_table_find() uses the table list cache to look up for existing
tables.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
This new function takes a struct nft_cache as parameter.
This patch also introduces __nft_table_builtin_find() which is required
to look up for built-in tables without the nft_handle structure.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
| |
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
| |
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
| |
Add new structure that encloses the cache and update the code to use it.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
If batch_rule_add() fails, this function leaked the rule iterator
object.
Fixes: 4c54c892443c2 ("xtables: Catch errors when zeroing rule rounters")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We currently race when several xtables-nft-restore processes attempt to
handle rules in parallel. For instance, when no rules are present at
all, then
iptables-nft-restore < X & iptables-nft-restore < X
... can cause rules to be restored twice.
Reason is that both processes might detect 'no rules exist', so
neither issues a flush operation.
We can't unconditionally issue the flush, because it would
cause kernel to fail with -ENOENT unless the to-be-flushed table
exists.
This change passes the generation id that was used to build
the transaction to the kernel.
In case another process changed *any* rule somewhere, the transaction
will now fail with -ERESTART.
We then flush the cache, re-fetch the ruleset and refresh
our transaction.
For example, in the above 'parallel restore' case, the iptables-restore
instance that lost the race would detect that the table has been created
already, and would add the needed flush.
In a similar vein, in case --noflush is used, we will add the flush
op for user-defined chains that were created in the mean-time.
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Its used to flag the rule flushes that get added in user-defined-chains
that get redefined with --noflush.
IOW, those objects that are added not by explicit instruction but
to keep semantics.
With --noflush, iptables-legacy-restore will behave as if
-F USERCHAIN was given, in case USERCHAIN exists and USERCHAIN gets
redefined, i.e.:
iptables-save v1.8.2 on Thu Apr 18 17:11:05 2019
*filter
:USERCHAIN - [0:0]
COMMIT
... will remove all existing rules from USERCHAIN.
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
| |
Will be used with the "generation id" infrastructure.
When we're told that the commit failed because someone else made
changes, we can use this to re-initialize the cache and then
revalidate the transaction list (e.g. to detect that we now have
to flush the user-defined chain 'foo' that we wanted to create, but
was added just now by someone else).
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
| |
This will be used to skip transaction objects when committing to
kernel. This is needed for example when we restore a table that
doesn't exist yet. In such a case we would already build a flush
operation so we can just enable it when we hit problem with the
generation id and we find that the table/chain was already created
in the mean time.
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
| |
The idea here is to move the 'flush' decision into the core, rather than
have the decision in the frontend.
This will be required later when "generation id" is passed to kernel.
In this case, we might have to add the flush when re-trying the
transaction.
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
If the new name already exists, legacy iptables prints "File exists.".
This is a bit exotic, but more appropriate than "No chain/target/match
by that name." printed by iptables-nft without this patch.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
| |
Use E2BIG if rule identified by given number is not found. ENOENT is
used if referenced chain is not found. Without this, a command
specifying a non-existing chain in combination with a rule number like
e.g.: 'iptables-nft -I nonexist 23 -j ACCEPT' returns "Index of
insertion too big." instead of "No chain/target/match by that name."
like legacy iptables does.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
Instead of checking chain existence in xtables.c, do it in
nft_chain_user_add() and reuse predefined error message.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously, error message was a bit misleading:
| # iptables-nft -Z noexist
| iptables: Incompatible with this kernel.
Set errno value so that the typical "No chain/target/match by that
name." is printed instead.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
| |
If passed a rulenum > 0, the function uses nftnl_rule_lookup_byindex()
and returns early. Negative rulenum values are not supposed to happen,
so the remaining code which iterates over the full list of rules does
not need to respect rulenum anymore.
Fixes: 039b048965210 ("nft: Make use of nftnl_rule_lookup_byindex()")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Legacy ebtables supports policies for user-defined chains - and what's
worse, they default to ACCEPT unlike anywhere else. So lack of support
for this braindead feature in ebtables-nft is actually a change of
behaviour which very likely affects all ebtables users out there.
The solution implemented here uses an implicit (and transparent) last
rule in all user-defined ebtables-nft chains with policy other than
RETURN. This rule is identified by an nft comment
"XTABLES_EB_INTERNAL_POLICY_RULE" (since commit ccf154d7420c0 ("xtables:
Don't use native nftables comments") nft comments are not used
otherwise).
To minimize interference with existing code, this policy rule is removed
from chains during cache population and the policy is saved in
NFTNL_CHAIN_POLICY attribute. When committing changes to the kernel,
nft_commit() traverses through the list of chains and (re-)creates
policy rules if required.
In ebtables-nft-restore, table flushes are problematic. To avoid weird
kernel error responses, introduce a custom 'table_flush' callback which
removes any pending policy rule add/remove jobs prior to creating the
NFT_COMPAT_TABLE_FLUSH one.
I've hidden all this mess behind checks for h->family, so hopefully
impact on {ip,ip6,arp}tables-nft should be negligible.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
| |
This will be used later to identify ebtables user-defined chain policy
rules.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
| |
Covscan complained about call to batch_rule_add() not being checked.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
Kernel prefers to identify chain by handle if it was given which causes
manual traversal of the chain list. In contrast, chain lookup by name in
kernel makes use of a hash table so is considerably faster. Force this
code path by removing the cached chain's handle when removing it.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
iptables-restore allows to insert rules at a certain position which is
problematic for iptables-nft to realize since rule position is not
determined by number but handle of previous or following rule and in
case the rules surrounding the new one are new as well, they don't have
a handle to refer to yet.
Fix this by making use of NFTNL_RULE_POSITION_ID attribute: When
inserting before a rule which does not have a handle, refer to it using
its NFTNL_RULE_ID value. If the latter doesn't exist either, assign a
new one to it.
The last used rule ID value is tracked in a new field of struct
nft_handle which is incremented before each use.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
| |
When replacing a rule, the replacement was simply appended to the
chain's rule list. Instead, insert it where the rule it replaces was.
This also fixes for zero counters command to remove the old rule from
cache.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
| |
Newly created builtin chains missing from cache was the sole reason for
the immediate calls to nft_commit(). With nft_chain_builtin_add()
inserting the new chain into the table's chain list, this is not needed
anymore. Just make sure batch_obj_del() doesn't free the payload of
NFT_COMPAT_CHAIN_ADD jobs since it contains the new chain which has
been added to cache.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
| |
With this, the explicit check for chain existence can be removed from
xtables.c since all related commands do this now.
Note that this effectively changes the error message printed by
iptables-nft when given a non-existing chain, but the new error
message(s) conform with those printed by legacy iptables.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
| |
With all the checks for 'tablename' being non-NULL, this code was rather
stupid and really hard to read. And the fix is indeed quite simple: If a
table name was given, use nft_table_builtin_find() and just flush its
chain cache. Otherwise iterate over all builtin tables without any
conditionals for 'tablename'.
Fixes: d4b0d248cc057 ("nft: Reduce indenting level in flush_chain_cache()")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
| |
Make use of nft_{table,chain}_builtin_find() instead of open-coding the
list traversal. Since code is pretty obvious now, drop the comments
added earlier.
Fixes: e774b15299c27 ("nft: Review is_*_compatible() routines")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|