libnftables: Get rid of explicit cache flushes

In the past, CLI as a potentially long running process had to make sure
it kept it's cache up to date with kernel's rule set. A simple test case
is this:

| shell a		|	shell b
|			|	# nft -i
| # nft add table ip t	|
|			|	nft> list ruleset
|			|	table ip t {
|			|	}
| # nft flush ruleset	|
|			|	nft> list ruleset
|			|	nft>

In order to make sure interactive CLI wouldn't incorrectly list the
table again in the second 'list' command, it immediately flushed it's
cache after every command execution.

This patch eliminates the need for that by making cache updates depend
on kernel's generation ID: A cache update stores the current rule set's
ID in struct nft_cache, consecutive calls to cache_update() compare that
stored value to the current generation ID received from kernel - if the
stored value is zero (i.e. no previous cache update did happen) or if it
doesn't match the kernel's value (i.e. cache is outdated) the cache is
flushed and fully initialized again.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/mnl.h b/include/mnl.h
index 3df71467c108006c2e9837212fd7dd5c3fab8c71..84c362a23bd791d3e5fc2846ab0bee8c0d9f9cc7 100644 (file)
--- a/include/mnl.h
+++ b/include/mnl.h
@@ -15,7 +15,7 @@ struct mnl_socket *netlink_open_sock(void);
 void netlink_close_sock(struct mnl_socket *nf_sock);
 
 uint32_t mnl_seqnum_alloc(uint32_t *seqnum);
-void mnl_genid_get(struct mnl_socket *nf_sock, uint32_t seqnum);
+uint16_t mnl_genid_get(struct mnl_socket *nf_sock, uint32_t seqnum);
 
 struct mnl_err {
        struct list_head        head;

diff --git a/include/netlink.h b/include/netlink.h
index 2ca6f345b6ac9d702813aeb0b3f0ec09daf1abee..b30c05f8335543bc243f280aee8c7467be177bb9 100644 (file)
--- a/include/netlink.h
+++ b/include/netlink.h
@@ -191,7 +191,7 @@ extern void netlink_dump_obj(struct nftnl_obj *nlo, struct netlink_ctx *ctx);
 
 extern int netlink_batch_send(struct netlink_ctx *ctx, struct list_head *err_list);
 
-extern void netlink_genid_get(struct mnl_socket *nf_sock, uint32_t seqnum);
+extern uint16_t netlink_genid_get(struct mnl_socket *nf_sock, uint32_t seqnum);
 extern void netlink_restart(struct mnl_socket *nf_sock);
 #define netlink_abi_error()    \
        __netlink_abi_error(__FILE__, __LINE__, strerror(errno));

diff --git a/include/nftables.h b/include/nftables.h
index 97a0436693cfa6838801ed86e9936d6f849bf4d7..d69079fe03e3a4eadd03e297e3861df7fdf67f2d 100644 (file)
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -16,7 +16,7 @@ struct output_ctx {
 };
 
 struct nft_cache {
-       bool                    initialized;
+       uint16_t                genid;
        struct list_head        list;
        uint32_t                seqnum;
 };

diff --git a/include/nftables/nftables.h b/include/nftables/nftables.h
index 449f9e4ee879c61bc53c73ce1ce704474426a884..4211be76e6e4004eb3dc3a9d8582cd600b618f46 100644 (file)
--- a/include/nftables/nftables.h
+++ b/include/nftables/nftables.h
@@ -70,8 +70,6 @@ FILE *nft_ctx_set_output(struct nft_ctx *ctx, FILE *fp);
 int nft_ctx_add_include_path(struct nft_ctx *ctx, const char *path);
 void nft_ctx_clear_include_paths(struct nft_ctx *ctx);
 
-void nft_ctx_flush_cache(struct nft_ctx *ctx);
-
 int nft_run_cmd_from_buffer(struct nft_ctx *nft, char *buf, size_t buflen);
 int nft_run_cmd_from_filename(struct nft_ctx *nft, const char *filename);
 

diff --git a/src/cli.c b/src/cli.c
index 37351f2f8b04f4acab8b0573682f1c2004fd1821..ec4d2a6f2046d5bd42f576af665960ca27923820 100644 (file)
--- a/src/cli.c
+++ b/src/cli.c
@@ -128,7 +128,6 @@ static void cli_complete(char *line)
 
        nft_run_cmd_from_buffer(cli_nft, line, len + 2);
        xfree(line);
-       nft_ctx_flush_cache(cli_nft);
 }
 
 static char **cli_completion(const char *text, int start, int end)

diff --git a/src/libnftables.c b/src/libnftables.c
index 5ef5532c7f0759ef9e7de61bf940b50543eedabb..0d04ec21d57d3fde12169ac9b96736549dc9e038 100644 (file)
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -174,18 +174,13 @@ struct nft_ctx *nft_ctx_new(uint32_t flags)
        return ctx;
 }
 
-void nft_ctx_flush_cache(struct nft_ctx *ctx)
-{
-       iface_cache_release();
-       cache_release(&ctx->cache);
-}
-
 void nft_ctx_free(struct nft_ctx *ctx)
 {
        if (ctx->nf_sock)
                netlink_close_sock(ctx->nf_sock);
 
-       nft_ctx_flush_cache(ctx);
+       iface_cache_release();
+       cache_release(&ctx->cache);
        nft_ctx_clear_include_paths(ctx);
        xfree(ctx);
        nft_exit();

diff --git a/src/mnl.c b/src/mnl.c
index 8db2a1847ec51942f038c37bfad9e4e8aa9082e2..3be6ebaf1f6823680aa287f35fbcb683919edd5c 100644 (file)
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -94,7 +94,7 @@ static int genid_cb(const struct nlmsghdr *nlh, void *data)
        return MNL_CB_OK;
 }
 
-void mnl_genid_get(struct mnl_socket *nf_sock, uint32_t seqnum)
+uint16_t mnl_genid_get(struct mnl_socket *nf_sock, uint32_t seqnum)
 {
        char buf[MNL_SOCKET_BUFFER_SIZE];
        struct mnl_ctx ctx = {
@@ -106,6 +106,8 @@ void mnl_genid_get(struct mnl_socket *nf_sock, uint32_t seqnum)
        nlh = nftnl_nlmsg_build_hdr(buf, NFT_MSG_GETGEN, AF_UNSPEC, 0, seqnum);
        /* Skip error checking, old kernels sets res_id field to zero. */
        nft_mnl_talk(&ctx, nlh, nlh->nlmsg_len, genid_cb, NULL);
+
+       return nft_genid;
 }
 
 static int check_genid(const struct nlmsghdr *nlh)

diff --git a/src/netlink.c b/src/netlink.c
index abc22504f877a5edc616f31a45390dff9ea3cdfd..845eeeffd73871bc5ea3177ef9a2393f16368281 100644 (file)
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -75,9 +75,9 @@ void netlink_restart(struct mnl_socket *nf_sock)
        nf_sock = netlink_open_sock();
 }
 
-void netlink_genid_get(struct mnl_socket *nf_sock, uint32_t seqnum)
+uint16_t netlink_genid_get(struct mnl_socket *nf_sock, uint32_t seqnum)
 {
-       mnl_genid_get(nf_sock, seqnum);
+       return mnl_genid_get(nf_sock, seqnum);
 }
 
 void __noreturn __netlink_abi_error(const char *file, int line,

diff --git a/src/rule.c b/src/rule.c
index 948478c96bda2bee6f77f3353ff8e1fb848ae7aa..6a322167b8265f751eafdb28a174d757f8b3e917 100644 (file)
--- a/src/rule.c
+++ b/src/rule.c
@@ -153,12 +153,14 @@ int cache_update(struct mnl_socket *nf_sock, struct nft_cache *cache,
                 enum cmd_ops cmd, struct list_head *msgs, bool debug,
                 struct output_ctx *octx)
 {
+       uint16_t genid;
        int ret;
 
-       if (cache->initialized)
-               return 0;
 replay:
-       netlink_genid_get(nf_sock, cache->seqnum++);
+       genid = netlink_genid_get(nf_sock, cache->seqnum++);
+       if (genid && genid == cache->genid)
+               return 0;
+       cache_release(cache);
        ret = cache_init(nf_sock, cache, cmd, msgs, debug, octx);
        if (ret < 0) {
                cache_release(cache);
@@ -168,7 +170,7 @@ replay:
                }
                return -1;
        }
-       cache->initialized = true;
+       cache->genid = genid;
        return 0;
 }
 
@@ -185,7 +187,7 @@ void cache_flush(struct list_head *table_list)
 void cache_release(struct nft_cache *cache)
 {
        cache_flush(&cache->list);
-       cache->initialized = false;
+       cache->genid = 0;
 }
 
 /* internal ID to uniquely identify a set in the batch */