From dac904bdcd9a18aabafee7275ccf0c2bd53800f3 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Mon, 5 Oct 2020 16:06:49 +0200 Subject: nft: Fix for concurrent noflush restore calls Transaction refresh was broken with regards to nft_chain_restore(): It created a rule flush batch object only if the chain was found in cache and a chain add object only if the chain was not found. Yet with concurrent ruleset updates, one has to expect both situations: * If a chain vanishes, the rule flush job must be skipped and instead the chain add job become active. * If a chain appears, the chain add job must be skipped and instead rules flushed. Change the code accordingly: Create both batch objects and set their 'skip' field depending on the situation in cache and adjust both in nft_refresh_transaction(). As a side-effect, the implicit rule flush becomes explicit and all handling of implicit batch jobs is dropped along with the related field indicating such. Reuse the 'implicit' parameter of __nft_rule_flush() to control the initial 'skip' field value instead. A subtle caveat is vanishing of existing chains: Creating the chain add job based on the chain in cache causes a netlink message containing that chain's handle which the kernel dislikes. Therefore unset the chain's handle in that case. Fixes: 58d7de0181f61 ("xtables: handle concurrent ruleset modifications") Signed-off-by: Phil Sutter --- .../ipt-restore/0016-concurrent-restores_0 | 53 ++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100755 iptables/tests/shell/testcases/ipt-restore/0016-concurrent-restores_0 (limited to 'iptables/tests/shell/testcases') diff --git a/iptables/tests/shell/testcases/ipt-restore/0016-concurrent-restores_0 b/iptables/tests/shell/testcases/ipt-restore/0016-concurrent-restores_0 new file mode 100755 index 00000000..53ec12fa --- /dev/null +++ b/iptables/tests/shell/testcases/ipt-restore/0016-concurrent-restores_0 @@ -0,0 +1,53 @@ +#!/bin/bash + +set -e + +RS="*filter +:INPUT ACCEPT [12024:3123388] +:FORWARD ACCEPT [0:0] +:OUTPUT ACCEPT [12840:2144421] +:FOO - [0:0] +:BAR0 - [0:0] +:BAR1 - [0:0] +:BAR2 - [0:0] +:BAR3 - [0:0] +:BAR4 - [0:0] +:BAR5 - [0:0] +:BAR6 - [0:0] +:BAR7 - [0:0] +:BAR8 - [0:0] +:BAR9 - [0:0] +" + +RS1="$RS +-X BAR3 +-X BAR6 +-X BAR9 +-A FOO -s 9.9.0.1/32 -j BAR1 +-A FOO -s 9.9.0.2/32 -j BAR2 +-A FOO -s 9.9.0.4/32 -j BAR4 +-A FOO -s 9.9.0.5/32 -j BAR5 +-A FOO -s 9.9.0.7/32 -j BAR7 +-A FOO -s 9.9.0.8/32 -j BAR8 +COMMIT +" + +RS2="$RS +-X BAR2 +-X BAR5 +-X BAR7 +-A FOO -s 9.9.0.1/32 -j BAR1 +-A FOO -s 9.9.0.3/32 -j BAR3 +-A FOO -s 9.9.0.4/32 -j BAR4 +-A FOO -s 9.9.0.6/32 -j BAR6 +-A FOO -s 9.9.0.8/32 -j BAR8 +-A FOO -s 9.9.0.9/32 -j BAR9 +COMMIT +" + +for n in $(seq 1 10); do + $XT_MULTI iptables-restore --noflush -w <<< "$RS1" & + $XT_MULTI iptables-restore --noflush -w <<< "$RS2" & + wait -n + wait -n +done -- cgit v1.2.3