|
|
bacbc8 |
From 97ce308f4fe922009df5ca2ea84d7c32cdab2532 Mon Sep 17 00:00:00 2001
|
|
|
bacbc8 |
From: Eric Garver <eric@garver.life>
|
|
|
bacbc8 |
Date: Wed, 22 May 2019 21:44:04 +0200
|
|
|
bacbc8 |
Subject: [PATCH] src: update cache if cmd is more specific
|
|
|
bacbc8 |
|
|
|
bacbc8 |
If we've done a partial fetch of the cache and the genid is the same the
|
|
|
bacbc8 |
cache update will be skipped without fetching the needed items. This
|
|
|
bacbc8 |
change flushes the cache if the new request is more specific than the
|
|
|
bacbc8 |
current cache - forcing a cache update which includes the needed items.
|
|
|
bacbc8 |
|
|
|
bacbc8 |
Introduces a simple scoring system which reflects how
|
|
|
bacbc8 |
cache_init_objects() looks at the current command to decide if it is
|
|
|
bacbc8 |
finished already or not. Then use that in cache_needs_more(): If current
|
|
|
bacbc8 |
command's score is higher than old command's, cache needs an update.
|
|
|
bacbc8 |
|
|
|
bacbc8 |
Fixes: 816d8c7659c1 ("Support 'add/insert rule index <IDX>'")
|
|
|
bacbc8 |
Signed-off-by: Eric Garver <eric@garver.life>
|
|
|
bacbc8 |
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
bacbc8 |
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
bacbc8 |
(cherry picked from commit eeda228c2d1719f5b6276b40ad14a5b3c3e88536)
|
|
|
bacbc8 |
|
|
|
bacbc8 |
Conflicts:
|
|
|
bacbc8 |
src/rule.c
|
|
|
bacbc8 |
-> Context change due to missing commit 0562beb6544d3
|
|
|
bacbc8 |
("src: get rid of netlink_genid_get()").
|
|
|
bacbc8 |
|
|
|
bacbc8 |
Signed-off-by: Phil Sutter <psutter@redhat.com>
|
|
|
bacbc8 |
---
|
|
|
bacbc8 |
include/nftables.h | 1 +
|
|
|
bacbc8 |
src/rule.c | 20 +++++++++++++++++++
|
|
|
bacbc8 |
.../shell/testcases/cache/0003_cache_update_0 | 14 +++++++++++++
|
|
|
bacbc8 |
3 files changed, 35 insertions(+)
|
|
|
bacbc8 |
|
|
|
bacbc8 |
diff --git a/include/nftables.h b/include/nftables.h
|
|
|
bacbc8 |
index 5e209b417d5a5..e9425d1fc8fb3 100644
|
|
|
bacbc8 |
--- a/include/nftables.h
|
|
|
bacbc8 |
+++ b/include/nftables.h
|
|
|
bacbc8 |
@@ -36,6 +36,7 @@ struct nft_cache {
|
|
|
bacbc8 |
uint16_t genid;
|
|
|
bacbc8 |
struct list_head list;
|
|
|
bacbc8 |
uint32_t seqnum;
|
|
|
bacbc8 |
+ uint32_t cmd;
|
|
|
bacbc8 |
};
|
|
|
bacbc8 |
|
|
|
bacbc8 |
struct mnl_socket;
|
|
|
bacbc8 |
diff --git a/src/rule.c b/src/rule.c
|
|
|
bacbc8 |
index 850b00cfc9874..a03abe1bf0c47 100644
|
|
|
bacbc8 |
--- a/src/rule.c
|
|
|
bacbc8 |
+++ b/src/rule.c
|
|
|
bacbc8 |
@@ -151,6 +151,23 @@ static int cache_init(struct netlink_ctx *ctx, enum cmd_ops cmd)
|
|
|
bacbc8 |
return 0;
|
|
|
bacbc8 |
}
|
|
|
bacbc8 |
|
|
|
bacbc8 |
+/* Return a "score" of how complete local cache will be if
|
|
|
bacbc8 |
+ * cache_init_objects() ran for given cmd. Higher value
|
|
|
bacbc8 |
+ * means more complete. */
|
|
|
bacbc8 |
+static int cache_completeness(enum cmd_ops cmd)
|
|
|
bacbc8 |
+{
|
|
|
bacbc8 |
+ if (cmd == CMD_LIST)
|
|
|
bacbc8 |
+ return 3;
|
|
|
bacbc8 |
+ if (cmd != CMD_RESET)
|
|
|
bacbc8 |
+ return 2;
|
|
|
bacbc8 |
+ return 1;
|
|
|
bacbc8 |
+}
|
|
|
bacbc8 |
+
|
|
|
bacbc8 |
+static bool cache_needs_more(enum cmd_ops old_cmd, enum cmd_ops cmd)
|
|
|
bacbc8 |
+{
|
|
|
bacbc8 |
+ return cache_completeness(old_cmd) < cache_completeness(cmd);
|
|
|
bacbc8 |
+}
|
|
|
bacbc8 |
+
|
|
|
bacbc8 |
int cache_update(struct nft_ctx *nft, enum cmd_ops cmd, struct list_head *msgs)
|
|
|
bacbc8 |
{
|
|
|
bacbc8 |
uint16_t genid;
|
|
|
bacbc8 |
@@ -166,6 +183,8 @@ int cache_update(struct nft_ctx *nft, enum cmd_ops cmd, struct list_head *msgs)
|
|
|
bacbc8 |
replay:
|
|
|
bacbc8 |
ctx.seqnum = cache->seqnum++;
|
|
|
bacbc8 |
genid = netlink_genid_get(&ctx;;
|
|
|
bacbc8 |
+ if (cache->genid && cache_needs_more(cache->cmd, cmd))
|
|
|
bacbc8 |
+ cache_release(cache);
|
|
|
bacbc8 |
if (genid && genid == cache->genid)
|
|
|
bacbc8 |
return 0;
|
|
|
bacbc8 |
if (cache->genid)
|
|
|
bacbc8 |
@@ -181,6 +200,7 @@ replay:
|
|
|
bacbc8 |
return -1;
|
|
|
bacbc8 |
}
|
|
|
bacbc8 |
cache->genid = genid;
|
|
|
bacbc8 |
+ cache->cmd = cmd;
|
|
|
bacbc8 |
return 0;
|
|
|
bacbc8 |
}
|
|
|
bacbc8 |
|
|
|
bacbc8 |
diff --git a/tests/shell/testcases/cache/0003_cache_update_0 b/tests/shell/testcases/cache/0003_cache_update_0
|
|
|
bacbc8 |
index deb45db2c43be..fa9b5df380a41 100755
|
|
|
bacbc8 |
--- a/tests/shell/testcases/cache/0003_cache_update_0
|
|
|
bacbc8 |
+++ b/tests/shell/testcases/cache/0003_cache_update_0
|
|
|
bacbc8 |
@@ -27,3 +27,17 @@ EOF
|
|
|
bacbc8 |
$NFT -i >/dev/null <
|
|
|
bacbc8 |
add table ip t3; add chain ip t c
|
|
|
bacbc8 |
EOF
|
|
|
bacbc8 |
+
|
|
|
bacbc8 |
+# The following test exposes a problem with incremental cache update when
|
|
|
bacbc8 |
+# reading commands from a file that add a rule using the "index" keyword.
|
|
|
bacbc8 |
+#
|
|
|
bacbc8 |
+# add rule ip t4 c meta l4proto icmp accept -> rule to reference in next step
|
|
|
bacbc8 |
+# add rule ip t4 c index 0 drop -> index 0 is not found due to rule cache not
|
|
|
bacbc8 |
+# being updated
|
|
|
bacbc8 |
+$NFT -i >/dev/null <
|
|
|
bacbc8 |
+add table ip t4; add chain ip t4 c
|
|
|
bacbc8 |
+add rule ip t4 c meta l4proto icmp accept
|
|
|
bacbc8 |
+EOF
|
|
|
bacbc8 |
+$NFT -f - >/dev/null <
|
|
|
bacbc8 |
+add rule ip t4 c index 0 drop
|
|
|
bacbc8 |
+EOF
|
|
|
bacbc8 |
--
|
|
|
bacbc8 |
2.22.0
|
|
|
bacbc8 |
|