[nft PATCH v2 0/5] Reduce cache overhead a bit

[nft PATCH v2 0/5] Reduce cache overhead a bit

[Phil Sutter]

Second try after a quick review and some testing:

- Tested with stable kernels v4.4.293 and v4.9.291: This series does not
  change any of the shell tests' results. The changes are supposedly
  bug- and feature-compatible.

- Upon error return from kernel, check errno to make sure it is really
  ENOENT which is expected instead of ignoring any error with non-dump
  requests.

Phil Sutter (5):
  cache: Filter tables on kernel side
  cache: Filter rule list on kernel side
  cache: Filter chain list on kernel side
  cache: Filter set list on server side
  cache: Support filtering for a specific flowtable

 include/cache.h                               |   1 +
 include/mnl.h                                 |  14 +-
 include/netlink.h                             |   3 +-
 src/cache.c                                   | 188 ++++++++++--------
 src/mnl.c                                     |  93 +++++++--
 src/netlink.c                                 |  15 +-
 tests/shell/testcases/listing/0020flowtable_0 |  51 ++++-
 7 files changed, 248 insertions(+), 117 deletions(-)

-- 
2.33.0

[nft PATCH v2 1/5] cache: Filter tables on kernel side

[Phil Sutter]

Instead of requesting a dump of all tables and filtering the data in
user space, construct a non-dump request if filter contains a table so
kernel returns only that single table.

This should improve nft performance in rulesets with many tables
present.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Expect possible ENOENT reply from kernel, it is not an error in this
  context.
---
 include/mnl.h     |  2 +-
 include/netlink.h |  3 ++-
 src/cache.c       |  9 +--------
 src/mnl.c         | 22 +++++++++++++++++++---
 src/netlink.c     | 12 ++++++++++--
 5 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/include/mnl.h b/include/mnl.h
index 68ec80cd22821..344030f306940 100644
--- a/include/mnl.h
+++ b/include/mnl.h
@@ -50,7 +50,7 @@ int mnl_nft_table_add(struct netlink_ctx *ctx, struct cmd *cmd,
 int mnl_nft_table_del(struct netlink_ctx *ctx, struct cmd *cmd);
 
 struct nftnl_table_list *mnl_nft_table_dump(struct netlink_ctx *ctx,
-					    int family);
+					    int family, const char *table);
 
 int mnl_nft_set_add(struct netlink_ctx *ctx, struct cmd *cmd,
 		    unsigned int flags);
diff --git a/include/netlink.h b/include/netlink.h
index a692edcdb5bf2..0e439061e3800 100644
--- a/include/netlink.h
+++ b/include/netlink.h
@@ -135,7 +135,8 @@ extern int netlink_list_chains(struct netlink_ctx *ctx, const struct handle *h);
 extern struct chain *netlink_delinearize_chain(struct netlink_ctx *ctx,
 					       const struct nftnl_chain *nlc);
 
-extern int netlink_list_tables(struct netlink_ctx *ctx, const struct handle *h);
+extern int netlink_list_tables(struct netlink_ctx *ctx, const struct handle *h,
+			       const struct nft_cache_filter *filter);
 extern struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
 					       const struct nftnl_table *nlt);
 
diff --git a/src/cache.c b/src/cache.c
index 6d20716d73110..66da2b3475732 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -772,19 +772,12 @@ static int cache_init_tables(struct netlink_ctx *ctx, struct handle *h,
 	struct table *table, *next;
 	int ret;
 
-	ret = netlink_list_tables(ctx, h);
+	ret = netlink_list_tables(ctx, h, filter);
 	if (ret < 0)
 		return -1;
 
 	list_for_each_entry_safe(table, next, &ctx->list, list) {
 		list_del(&table->list);
-
-		if (filter && filter->list.table &&
-		    (filter->list.family != table->handle.family ||
-		     strcmp(filter->list.table, table->handle.table.name))) {
-			table_free(table);
-			continue;
-		}
 		table_cache_add(table, cache);
 	}
 
diff --git a/src/mnl.c b/src/mnl.c
index 23348e1393bce..21b98e34ed176 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -1016,10 +1016,12 @@ err_free:
 }
 
 struct nftnl_table_list *mnl_nft_table_dump(struct netlink_ctx *ctx,
-					    int family)
+					    int family, const char *table)
 {
 	char buf[MNL_SOCKET_BUFFER_SIZE];
 	struct nftnl_table_list *nlt_list;
+	struct nftnl_table *nlt = NULL;
+	int flags = NLM_F_DUMP;
 	struct nlmsghdr *nlh;
 	int ret;
 
@@ -1027,11 +1029,25 @@ struct nftnl_table_list *mnl_nft_table_dump(struct netlink_ctx *ctx,
 	if (nlt_list == NULL)
 		return NULL;
 
+	if (table) {
+		nlt = nftnl_table_alloc();
+		if (!nlt)
+			memory_allocation_error();
+
+		nftnl_table_set_u32(nlt, NFTNL_TABLE_FAMILY, family);
+		nftnl_table_set_str(nlt, NFTNL_TABLE_NAME, table);
+		flags = NLM_F_ACK;
+	}
+
 	nlh = nftnl_nlmsg_build_hdr(buf, NFT_MSG_GETTABLE, family,
-				    NLM_F_DUMP, ctx->seqnum);
+				    flags, ctx->seqnum);
+	if (nlt) {
+		nftnl_table_nlmsg_build_payload(nlh, nlt);
+		nftnl_table_free(nlt);
+	}
 
 	ret = nft_mnl_talk(ctx, nlh, nlh->nlmsg_len, table_cb, nlt_list);
-	if (ret < 0)
+	if (ret < 0 && errno != ENOENT)
 		goto err;
 
 	return nlt_list;
diff --git a/src/netlink.c b/src/netlink.c
index ab90d0c05acaf..f74c0383a0db3 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -664,11 +664,19 @@ static int list_table_cb(struct nftnl_table *nlt, void *arg)
 	return 0;
 }
 
-int netlink_list_tables(struct netlink_ctx *ctx, const struct handle *h)
+int netlink_list_tables(struct netlink_ctx *ctx, const struct handle *h,
+			const struct nft_cache_filter *filter)
 {
 	struct nftnl_table_list *table_cache;
+	uint32_t family = h->family;
+	const char *table = NULL;
 
-	table_cache = mnl_nft_table_dump(ctx, h->family);
+	if (filter) {
+		family = filter->list.family;
+		table = filter->list.table;
+	}
+
+	table_cache = mnl_nft_table_dump(ctx, family, table);
 	if (table_cache == NULL) {
 		if (errno == EINTR)
 			return -1;
-- 
2.33.0

[nft PATCH v2 2/5] cache: Filter rule list on kernel side

[Phil Sutter]

Instead of fetching all existing rules in kernel's ruleset and filtering
in user space, add payload to the dump request specifying the table and
chain to filter for.

Since list_rule_cb() no longer needs the filter, pass only netlink_ctx
to the callback and drop struct rule_cache_dump_ctx.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/mnl.h |  4 ++--
 src/cache.c   | 23 +++--------------------
 src/mnl.c     | 21 +++++++++++++++++++--
 3 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/include/mnl.h b/include/mnl.h
index 344030f306940..19faa651fdb91 100644
--- a/include/mnl.h
+++ b/include/mnl.h
@@ -33,8 +33,8 @@ int mnl_nft_rule_add(struct netlink_ctx *ctx, struct cmd *cmd,
 int mnl_nft_rule_del(struct netlink_ctx *ctx, struct cmd *cmd);
 int mnl_nft_rule_replace(struct netlink_ctx *ctx, struct cmd *cmd);
 
-struct nftnl_rule_list *mnl_nft_rule_dump(struct netlink_ctx *ctx,
-					  int family);
+struct nftnl_rule_list *mnl_nft_rule_dump(struct netlink_ctx *ctx, int family,
+					  const struct nft_cache_filter *filter);
 
 int mnl_nft_chain_add(struct netlink_ctx *ctx, struct cmd *cmd,
 		      unsigned int flags);
diff --git a/src/cache.c b/src/cache.c
index 66da2b3475732..484efdb93862b 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -441,16 +441,9 @@ struct chain *chain_cache_find(const struct table *table, const char *name)
 	return NULL;
 }
 
-struct rule_cache_dump_ctx {
-	struct netlink_ctx	*nlctx;
-	const struct nft_cache_filter *filter;
-};
-
 static int list_rule_cb(struct nftnl_rule *nlr, void *data)
 {
-	struct rule_cache_dump_ctx *rule_ctx = data;
-	const struct nft_cache_filter *filter = rule_ctx->filter;
-	struct netlink_ctx *ctx = rule_ctx->nlctx;
+	struct netlink_ctx *ctx = data;
 	const struct handle *h = ctx->data;
 	const char *table, *chain;
 	struct rule *rule;
@@ -465,12 +458,6 @@ static int list_rule_cb(struct nftnl_rule *nlr, void *data)
 	    (h->chain.name && strcmp(chain, h->chain.name) != 0))
 		return 0;
 
-	if (filter && filter->list.table && filter->list.chain &&
-	    (filter->list.family != family ||
-	     strcmp(filter->list.table, table) ||
-	     strcmp(filter->list.chain, chain)))
-		return 0;
-
 	netlink_dump_rule(nlr, ctx);
 	rule = netlink_delinearize_rule(ctx, nlr);
 	list_add_tail(&rule->list, &ctx->list);
@@ -481,13 +468,9 @@ static int list_rule_cb(struct nftnl_rule *nlr, void *data)
 static int rule_cache_init(struct netlink_ctx *ctx, const struct handle *h,
 			   const struct nft_cache_filter *filter)
 {
-	struct rule_cache_dump_ctx rule_ctx = {
-		.nlctx = ctx,
-		.filter = filter,
-	};
 	struct nftnl_rule_list *rule_cache;
 
-	rule_cache = mnl_nft_rule_dump(ctx, h->family);
+	rule_cache = mnl_nft_rule_dump(ctx, h->family, filter);
 	if (rule_cache == NULL) {
 		if (errno == EINTR)
 			return -1;
@@ -496,7 +479,7 @@ static int rule_cache_init(struct netlink_ctx *ctx, const struct handle *h,
 	}
 
 	ctx->data = h;
-	nftnl_rule_list_foreach(rule_cache, list_rule_cb, &rule_ctx);
+	nftnl_rule_list_foreach(rule_cache, list_rule_cb, ctx);
 	nftnl_rule_list_free(rule_cache);
 	return 0;
 }
diff --git a/src/mnl.c b/src/mnl.c
index 21b98e34ed176..26f643fb282ea 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -653,20 +653,37 @@ err_free:
 	return MNL_CB_OK;
 }
 
-struct nftnl_rule_list *mnl_nft_rule_dump(struct netlink_ctx *ctx,
-					  int family)
+struct nftnl_rule_list *mnl_nft_rule_dump(struct netlink_ctx *ctx, int family,
+					  const struct nft_cache_filter *filter)
 {
 	char buf[MNL_SOCKET_BUFFER_SIZE];
 	struct nftnl_rule_list *nlr_list;
+	struct nftnl_rule *nlr = NULL;
 	struct nlmsghdr *nlh;
 	int ret;
 
+	if (filter && filter->list.table) {
+		nlr = nftnl_rule_alloc();
+		if (!nlr)
+			memory_allocation_error();
+
+		nftnl_rule_set_str(nlr, NFTNL_RULE_TABLE,
+				   filter->list.table);
+		if (filter->list.chain)
+			nftnl_rule_set_str(nlr, NFTNL_RULE_CHAIN,
+					   filter->list.chain);
+	}
+
 	nlr_list = nftnl_rule_list_alloc();
 	if (nlr_list == NULL)
 		memory_allocation_error();
 
 	nlh = nftnl_nlmsg_build_hdr(buf, NFT_MSG_GETRULE, family,
 				    NLM_F_DUMP, ctx->seqnum);
+	if (nlr) {
+		nftnl_rule_nlmsg_build_payload(nlh, nlr);
+		nftnl_rule_free(nlr);
+	}
 
 	ret = nft_mnl_talk(ctx, nlh, nlh->nlmsg_len, rule_cb, nlr_list);
 	if (ret < 0)
-- 
2.33.0

[nft PATCH v2 3/5] cache: Filter chain list on kernel side

[Phil Sutter]

When operating on a specific chain, add payload to NFT_MSG_GETCHAIN so
kernel returns only relevant data. Since ENOENT is an expected return
code, do not treat this as error.

While being at it, improve code in chain_cache_cb() a bit:
- Check chain's family first, it is a less expensive check than
  comparing table names.
- Do not extract chain name of uninteresting chains.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Catch ENOENT instead of ignoring all errors for non-dump requests.
- Use NFPROTO_UNSPEC instead of AF_UNSPEC for default family value. No
  functional change, merely conformity.
---
 include/mnl.h |  3 ++-
 src/cache.c   | 38 +++++++++++++++++++-------------------
 src/mnl.c     | 21 ++++++++++++++++++---
 3 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/include/mnl.h b/include/mnl.h
index 19faa651fdb91..9d54aac876dc1 100644
--- a/include/mnl.h
+++ b/include/mnl.h
@@ -43,7 +43,8 @@ int mnl_nft_chain_rename(struct netlink_ctx *ctx, const struct cmd *cmd,
 			 const struct chain *chain);
 
 struct nftnl_chain_list *mnl_nft_chain_dump(struct netlink_ctx *ctx,
-					    int family);
+					    int family, const char *table,
+					    const char *chain);
 
 int mnl_nft_table_add(struct netlink_ctx *ctx, struct cmd *cmd,
 		      unsigned int flags);
diff --git a/src/cache.c b/src/cache.c
index 484efdb93862b..1e98e6cf7cc17 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -342,31 +342,23 @@ struct table *table_cache_find(const struct cache *cache,
 struct chain_cache_dump_ctx {
 	struct netlink_ctx	*nlctx;
 	struct table		*table;
-	const struct nft_cache_filter *filter;
 };
 
 static int chain_cache_cb(struct nftnl_chain *nlc, void *arg)
 {
 	struct chain_cache_dump_ctx *ctx = arg;
-	const struct nft_cache_filter *filter = ctx->filter;
 	const char *chain_name, *table_name;
 	uint32_t hash, family;
 	struct chain *chain;
 
 	table_name = nftnl_chain_get_str(nlc, NFTNL_CHAIN_TABLE);
-	chain_name = nftnl_chain_get_str(nlc, NFTNL_CHAIN_NAME);
 	family = nftnl_chain_get_u32(nlc, NFTNL_CHAIN_FAMILY);
 
-	if (strcmp(table_name, ctx->table->handle.table.name) ||
-	    family != ctx->table->handle.family)
-		return 0;
-
-	if (filter && filter->list.table && filter->list.chain &&
-	    (filter->list.family != family ||
-	     strcmp(filter->list.table, table_name) ||
-	     strcmp(filter->list.chain, chain_name)))
+	if (family != ctx->table->handle.family ||
+	    strcmp(table_name, ctx->table->handle.table.name))
 		return 0;
 
+	chain_name = nftnl_chain_get_str(nlc, NFTNL_CHAIN_NAME);
 	hash = djb_hash(chain_name) % NFT_CACHE_HSIZE;
 	chain = netlink_delinearize_chain(ctx->nlctx, nlc);
 
@@ -383,25 +375,33 @@ static int chain_cache_cb(struct nftnl_chain *nlc, void *arg)
 }
 
 static int chain_cache_init(struct netlink_ctx *ctx, struct table *table,
-			    struct nftnl_chain_list *chain_list,
-			    const struct nft_cache_filter *filter)
+			    struct nftnl_chain_list *chain_list)
 {
 	struct chain_cache_dump_ctx dump_ctx = {
 		.nlctx	= ctx,
 		.table	= table,
-		.filter	= filter,
 	};
 	nftnl_chain_list_foreach(chain_list, chain_cache_cb, &dump_ctx);
 
 	return 0;
 }
 
-static struct nftnl_chain_list *chain_cache_dump(struct netlink_ctx *ctx,
-						 int *err)
+static struct nftnl_chain_list *
+chain_cache_dump(struct netlink_ctx *ctx,
+		 const struct nft_cache_filter *filter, int *err)
 {
 	struct nftnl_chain_list *chain_list;
+	const char *table = NULL;
+	const char *chain = NULL;
+	int family = NFPROTO_UNSPEC;
+
+	if (filter && filter->list.table && filter->list.chain) {
+		family = filter->list.family;
+		table = filter->list.table;
+		chain = filter->list.chain;
+	}
 
-	chain_list = mnl_nft_chain_dump(ctx, AF_UNSPEC);
+	chain_list = mnl_nft_chain_dump(ctx, family, table, chain);
 	if (chain_list == NULL) {
 		if (errno == EINTR) {
 			*err = -1;
@@ -781,7 +781,7 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 	int ret = 0;
 
 	if (flags & NFT_CACHE_CHAIN_BIT) {
-		chain_list = chain_cache_dump(ctx, &ret);
+		chain_list = chain_cache_dump(ctx, filter, &ret);
 		if (!chain_list)
 			return -1;
 	}
@@ -834,7 +834,7 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 			}
 		}
 		if (flags & NFT_CACHE_CHAIN_BIT) {
-			ret = chain_cache_init(ctx, table, chain_list, filter);
+			ret = chain_cache_init(ctx, table, chain_list);
 			if (ret < 0) {
 				ret = -1;
 				goto cache_fails;
diff --git a/src/mnl.c b/src/mnl.c
index 26f643fb282ea..109c3dd81e578 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -906,10 +906,12 @@ err_free:
 }
 
 struct nftnl_chain_list *mnl_nft_chain_dump(struct netlink_ctx *ctx,
-					    int family)
+					    int family, const char *table,
+					    const char *chain)
 {
 	char buf[MNL_SOCKET_BUFFER_SIZE];
 	struct nftnl_chain_list *nlc_list;
+	struct nftnl_chain *nlc = NULL;
 	struct nlmsghdr *nlh;
 	int ret;
 
@@ -917,11 +919,24 @@ struct nftnl_chain_list *mnl_nft_chain_dump(struct netlink_ctx *ctx,
 	if (nlc_list == NULL)
 		memory_allocation_error();
 
+	if (table && chain) {
+		nlc = nftnl_chain_alloc();
+		if (!nlc)
+			memory_allocation_error();
+
+		nftnl_chain_set_str(nlc, NFTNL_CHAIN_TABLE, table);
+		nftnl_chain_set_str(nlc, NFTNL_CHAIN_NAME, chain);
+	}
+
 	nlh = nftnl_nlmsg_build_hdr(buf, NFT_MSG_GETCHAIN, family,
-				    NLM_F_DUMP, ctx->seqnum);
+				    nlc ? NLM_F_ACK : NLM_F_DUMP, ctx->seqnum);
+	if (nlc) {
+		nftnl_chain_nlmsg_build_payload(nlh, nlc);
+		nftnl_chain_free(nlc);
+	}
 
 	ret = nft_mnl_talk(ctx, nlh, nlh->nlmsg_len, chain_cb, nlc_list);
-	if (ret < 0)
+	if (ret < 0 && errno != ENOENT)
 		goto err;
 
 	return nlc_list;
-- 
2.33.0

[nft PATCH v2 4/5] cache: Filter set list on server side

[Phil Sutter]

Fetch either all tables' sets at once, a specific table's sets or even a
specific set if needed instead of iterating over the list of previously
fetched tables and fetching for each, then ignoring anything returned
that doesn't match the filter.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Catch ENOENT instead of speculating about the error cause.
---
 include/mnl.h |  2 +-
 src/cache.c   | 63 ++++++++++++++++++++++++++++++---------------------
 src/mnl.c     | 15 ++++++++----
 3 files changed, 49 insertions(+), 31 deletions(-)

diff --git a/include/mnl.h b/include/mnl.h
index 9d54aac876dc1..8659879c17f44 100644
--- a/include/mnl.h
+++ b/include/mnl.h
@@ -58,7 +58,7 @@ int mnl_nft_set_add(struct netlink_ctx *ctx, struct cmd *cmd,
 int mnl_nft_set_del(struct netlink_ctx *ctx, struct cmd *cmd);
 
 struct nftnl_set_list *mnl_nft_set_dump(struct netlink_ctx *ctx, int family,
-					const char *table);
+					const char *table, const char *set);
 
 int mnl_nft_setelem_add(struct netlink_ctx *ctx, const struct set *set,
 			const struct expr *expr, unsigned int flags);
diff --git a/src/cache.c b/src/cache.c
index 1e98e6cf7cc17..38a34ee3c406f 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -487,57 +487,66 @@ static int rule_cache_init(struct netlink_ctx *ctx, const struct handle *h,
 struct set_cache_dump_ctx {
 	struct netlink_ctx	*nlctx;
 	struct table		*table;
-	const struct nft_cache_filter *filter;
 };
 
 static int set_cache_cb(struct nftnl_set *nls, void *arg)
 {
 	struct set_cache_dump_ctx *ctx = arg;
+	const char *set_table;
 	const char *set_name;
+	uint32_t set_family;
 	struct set *set;
 	uint32_t hash;
 
+	set_table = nftnl_set_get_str(nls, NFTNL_SET_TABLE);
+	set_family = nftnl_set_get_u32(nls, NFTNL_SET_FAMILY);
+
+	if (set_family != ctx->table->handle.family ||
+	    strcmp(set_table, ctx->table->handle.table.name))
+		return 0;
+
 	set = netlink_delinearize_set(ctx->nlctx, nls);
 	if (!set)
 		return -1;
 
-	if (ctx->filter && ctx->filter->list.set &&
-	    (ctx->filter->list.family != set->handle.family ||
-	     strcmp(ctx->filter->list.table, set->handle.table.name) ||
-	     strcmp(ctx->filter->list.set, set->handle.set.name))) {
-		set_free(set);
-		return 0;
-	}
-
 	set_name = nftnl_set_get_str(nls, NFTNL_SET_NAME);
 	hash = djb_hash(set_name) % NFT_CACHE_HSIZE;
 	cache_add(&set->cache, &ctx->table->set_cache, hash);
 
+	nftnl_set_list_del(nls);
+	nftnl_set_free(nls);
 	return 0;
 }
 
 static int set_cache_init(struct netlink_ctx *ctx, struct table *table,
-			  struct nftnl_set_list *set_list,
-			  const struct nft_cache_filter *filter)
+			  struct nftnl_set_list *set_list)
 {
 	struct set_cache_dump_ctx dump_ctx = {
 		.nlctx	= ctx,
 		.table	= table,
-		.filter = filter,
 	};
+
 	nftnl_set_list_foreach(set_list, set_cache_cb, &dump_ctx);
 
 	return 0;
 }
 
-static struct nftnl_set_list *set_cache_dump(struct netlink_ctx *ctx,
-					     const struct table *table,
-					     int *err)
+static struct nftnl_set_list *
+set_cache_dump(struct netlink_ctx *ctx,
+	       const struct nft_cache_filter *filter, int *err)
 {
 	struct nftnl_set_list *set_list;
+	int family = NFPROTO_UNSPEC;
+	const char *table = NULL;
+	const char *set = NULL;
+
+	if (filter) {
+		family = filter->list.family;
+		table = filter->list.table;
+		set = filter->list.set;
+	}
 
-	set_list = mnl_nft_set_dump(ctx, table->handle.family,
-				    table->handle.table.name);
+	set_list = mnl_nft_set_dump(ctx, family, table, set);
 	if (!set_list) {
                 if (errno == EINTR) {
 			*err = -1;
@@ -785,18 +794,17 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 		if (!chain_list)
 			return -1;
 	}
+	if (flags & NFT_CACHE_SET_BIT) {
+		set_list = set_cache_dump(ctx, filter, &ret);
+		if (!set_list) {
+			ret = -1;
+			goto cache_fails;
+		}
+	}
 
 	list_for_each_entry(table, &ctx->nft->cache.table_cache.list, cache.list) {
 		if (flags & NFT_CACHE_SET_BIT) {
-			set_list = set_cache_dump(ctx, table, &ret);
-			if (!set_list) {
-				ret = -1;
-				goto cache_fails;
-			}
-			ret = set_cache_init(ctx, table, set_list, filter);
-
-			nftnl_set_list_free(set_list);
-
+			ret = set_cache_init(ctx, table, set_list);
 			if (ret < 0) {
 				ret = -1;
 				goto cache_fails;
@@ -893,6 +901,9 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 	}
 
 cache_fails:
+	if (set_list)
+		nftnl_set_list_free(set_list);
+
 	if (flags & NFT_CACHE_CHAIN_BIT)
 		nftnl_chain_list_free(chain_list);
 
diff --git a/src/mnl.c b/src/mnl.c
index 109c3dd81e578..47b3ca613e419 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -1300,10 +1300,12 @@ err_free:
 }
 
 struct nftnl_set_list *
-mnl_nft_set_dump(struct netlink_ctx *ctx, int family, const char *table)
+mnl_nft_set_dump(struct netlink_ctx *ctx, int family,
+		 const char *table, const char *set)
 {
 	char buf[MNL_SOCKET_BUFFER_SIZE];
 	struct nftnl_set_list *nls_list;
+	int flags = NLM_F_DUMP;
 	struct nlmsghdr *nlh;
 	struct nftnl_set *s;
 	int ret;
@@ -1312,10 +1314,15 @@ mnl_nft_set_dump(struct netlink_ctx *ctx, int family, const char *table)
 	if (s == NULL)
 		memory_allocation_error();
 
-	nlh = nftnl_nlmsg_build_hdr(buf, NFT_MSG_GETSET, family,
-				    NLM_F_DUMP, ctx->seqnum);
 	if (table != NULL)
 		nftnl_set_set_str(s, NFTNL_SET_TABLE, table);
+	if (set) {
+		nftnl_set_set_str(s, NFTNL_SET_NAME, set);
+		flags = NLM_F_ACK;
+	}
+
+	nlh = nftnl_nlmsg_build_hdr(buf, NFT_MSG_GETSET, family,
+				    flags, ctx->seqnum);
 	nftnl_set_nlmsg_build_payload(nlh, s);
 	nftnl_set_free(s);
 
@@ -1324,7 +1331,7 @@ mnl_nft_set_dump(struct netlink_ctx *ctx, int family, const char *table)
 		memory_allocation_error();
 
 	ret = nft_mnl_talk(ctx, nlh, nlh->nlmsg_len, set_cb, nls_list);
-	if (ret < 0)
+	if (ret < 0 && errno != ENOENT)
 		goto err;
 
 	return nls_list;
-- 
2.33.0

[nft PATCH v2 5/5] cache: Support filtering for a specific flowtable

[Phil Sutter]

Extend nft_cache_filter to hold a flowtable name so 'list flowtable'
command causes fetching the requested flowtable only.

Dump flowtables just once instead of for each table, merely assign
fetched data to tables inside the loop.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Catch ENOENT instead of speculating about error cause.
---
 include/cache.h                               |  1 +
 include/mnl.h                                 |  3 +-
 src/cache.c                                   | 55 ++++++++++++++-----
 src/mnl.c                                     | 14 +++--
 src/netlink.c                                 |  3 +-
 tests/shell/testcases/listing/0020flowtable_0 | 51 +++++++++++++++--
 6 files changed, 103 insertions(+), 24 deletions(-)

diff --git a/include/cache.h b/include/cache.h
index 3a9a5e8197114..d185f3cfeda06 100644
--- a/include/cache.h
+++ b/include/cache.h
@@ -54,6 +54,7 @@ struct nft_cache_filter {
 		const char	*table;
 		const char	*chain;
 		const char	*set;
+		const char	*ft;
 	} list;
 
 	struct {
diff --git a/include/mnl.h b/include/mnl.h
index 8659879c17f44..b006192cf7b23 100644
--- a/include/mnl.h
+++ b/include/mnl.h
@@ -77,7 +77,8 @@ int mnl_nft_obj_add(struct netlink_ctx *ctx, struct cmd *cmd,
 int mnl_nft_obj_del(struct netlink_ctx *ctx, struct cmd *cmd, int type);
 
 struct nftnl_flowtable_list *
-mnl_nft_flowtable_dump(struct netlink_ctx *ctx, int family, const char *table);
+mnl_nft_flowtable_dump(struct netlink_ctx *ctx, int family,
+		       const char *table, const char *ft);
 
 int mnl_nft_flowtable_add(struct netlink_ctx *ctx, struct cmd *cmd,
 			  unsigned int flags);
diff --git a/src/cache.c b/src/cache.c
index 38a34ee3c406f..6494e4743f8dd 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -229,6 +229,15 @@ static unsigned int evaluate_cache_list(struct nft_ctx *nft, struct cmd *cmd,
 	case CMD_OBJ_MAPS:
 		flags |= NFT_CACHE_TABLE | NFT_CACHE_SET;
 		break;
+	case CMD_OBJ_FLOWTABLE:
+		if (filter &&
+		    cmd->handle.table.name &&
+		    cmd->handle.flowtable.name) {
+			filter->list.family = cmd->handle.family;
+			filter->list.table = cmd->handle.table.name;
+			filter->list.ft = cmd->handle.flowtable.name;
+		}
+		/* fall through */
 	case CMD_OBJ_FLOWTABLES:
 		flags |= NFT_CACHE_TABLE | NFT_CACHE_FLOWTABLE;
 		break;
@@ -681,10 +690,19 @@ struct ft_cache_dump_ctx {
 static int ft_cache_cb(struct nftnl_flowtable *nlf, void *arg)
 {
 	struct ft_cache_dump_ctx *ctx = arg;
-	const char *ft_name;
 	struct flowtable *ft;
+	const char *ft_table;
+	const char *ft_name;
+	uint32_t ft_family;
 	uint32_t hash;
 
+	ft_family = nftnl_flowtable_get_u32(nlf, NFTNL_FLOWTABLE_FAMILY);
+	ft_table = nftnl_flowtable_get_str(nlf, NFTNL_FLOWTABLE_TABLE);
+
+	if (ft_family != ctx->table->handle.family ||
+	    strcmp(ft_table, ctx->table->handle.table.name))
+		return 0;
+
 	ft = netlink_delinearize_flowtable(ctx->nlctx, nlf);
 	if (!ft)
 		return -1;
@@ -693,6 +711,8 @@ static int ft_cache_cb(struct nftnl_flowtable *nlf, void *arg)
 	hash = djb_hash(ft_name) % NFT_CACHE_HSIZE;
 	cache_add(&ft->cache, &ctx->table->ft_cache, hash);
 
+	nftnl_flowtable_list_del(nlf);
+	nftnl_flowtable_free(nlf);
 	return 0;
 }
 
@@ -708,13 +728,21 @@ static int ft_cache_init(struct netlink_ctx *ctx, struct table *table,
 	return 0;
 }
 
-static struct nftnl_flowtable_list *ft_cache_dump(struct netlink_ctx *ctx,
-						  const struct table *table)
+static struct nftnl_flowtable_list *
+ft_cache_dump(struct netlink_ctx *ctx, const struct nft_cache_filter *filter)
 {
 	struct nftnl_flowtable_list *ft_list;
+	int family = NFPROTO_UNSPEC;
+	const char *table = NULL;
+	const char *ft = NULL;
 
-	ft_list = mnl_nft_flowtable_dump(ctx, table->handle.family,
-					 table->handle.table.name);
+	if (filter) {
+		family = filter->list.family;
+		table = filter->list.table;
+		ft = filter->list.ft;
+	}
+
+	ft_list = mnl_nft_flowtable_dump(ctx, family, table, ft);
 	if (!ft_list) {
                 if (errno == EINTR)
 			return NULL;
@@ -801,6 +829,13 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 			goto cache_fails;
 		}
 	}
+	if (flags & NFT_CACHE_FLOWTABLE_BIT) {
+		ft_list = ft_cache_dump(ctx, filter);
+		if (!ft_list) {
+			ret = -1;
+			goto cache_fails;
+		}
+	}
 
 	list_for_each_entry(table, &ctx->nft->cache.table_cache.list, cache.list) {
 		if (flags & NFT_CACHE_SET_BIT) {
@@ -849,15 +884,7 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 			}
 		}
 		if (flags & NFT_CACHE_FLOWTABLE_BIT) {
-			ft_list = ft_cache_dump(ctx, table);
-			if (!ft_list) {
-				ret = -1;
-				goto cache_fails;
-			}
 			ret = ft_cache_init(ctx, table, ft_list);
-
-			nftnl_flowtable_list_free(ft_list);
-
 			if (ret < 0) {
 				ret = -1;
 				goto cache_fails;
@@ -903,6 +930,8 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 cache_fails:
 	if (set_list)
 		nftnl_set_list_free(set_list);
+	if (ft_list)
+		nftnl_flowtable_list_free(ft_list);
 
 	if (flags & NFT_CACHE_CHAIN_BIT)
 		nftnl_chain_list_free(chain_list);
diff --git a/src/mnl.c b/src/mnl.c
index 47b3ca613e419..5413f8658f9b3 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -1826,11 +1826,13 @@ err_free:
 }
 
 struct nftnl_flowtable_list *
-mnl_nft_flowtable_dump(struct netlink_ctx *ctx, int family, const char *table)
+mnl_nft_flowtable_dump(struct netlink_ctx *ctx, int family,
+		       const char *table, const char *ft)
 {
 	struct nftnl_flowtable_list *nln_list;
 	char buf[MNL_SOCKET_BUFFER_SIZE];
 	struct nftnl_flowtable *n;
+	int flags = NLM_F_DUMP;
 	struct nlmsghdr *nlh;
 	int ret;
 
@@ -1838,10 +1840,14 @@ mnl_nft_flowtable_dump(struct netlink_ctx *ctx, int family, const char *table)
 	if (n == NULL)
 		memory_allocation_error();
 
-	nlh = nftnl_nlmsg_build_hdr(buf, NFT_MSG_GETFLOWTABLE, family,
-				    NLM_F_DUMP, ctx->seqnum);
 	if (table != NULL)
 		nftnl_flowtable_set_str(n, NFTNL_FLOWTABLE_TABLE, table);
+	if (ft) {
+		nftnl_flowtable_set_str(n, NFTNL_FLOWTABLE_NAME, ft);
+		flags = NLM_F_ACK;
+	}
+	nlh = nftnl_nlmsg_build_hdr(buf, NFT_MSG_GETFLOWTABLE, family,
+				    flags, ctx->seqnum);
 	nftnl_flowtable_nlmsg_build_payload(nlh, n);
 	nftnl_flowtable_free(n);
 
@@ -1850,7 +1856,7 @@ mnl_nft_flowtable_dump(struct netlink_ctx *ctx, int family, const char *table)
 		memory_allocation_error();
 
 	ret = nft_mnl_talk(ctx, nlh, nlh->nlmsg_len, flowtable_cb, nln_list);
-	if (ret < 0)
+	if (ret < 0 && errno != ENOENT)
 		goto err;
 
 	return nln_list;
diff --git a/src/netlink.c b/src/netlink.c
index f74c0383a0db3..359d801c29d35 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1721,7 +1721,8 @@ int netlink_list_flowtables(struct netlink_ctx *ctx, const struct handle *h)
 	struct nftnl_flowtable_list *flowtable_cache;
 	int err;
 
-	flowtable_cache = mnl_nft_flowtable_dump(ctx, h->family, h->table.name);
+	flowtable_cache = mnl_nft_flowtable_dump(ctx, h->family,
+						 h->table.name, NULL);
 	if (flowtable_cache == NULL) {
 		if (errno == EINTR)
 			return -1;
diff --git a/tests/shell/testcases/listing/0020flowtable_0 b/tests/shell/testcases/listing/0020flowtable_0
index 2f0a98d16fd38..47488d8ea92a4 100755
--- a/tests/shell/testcases/listing/0020flowtable_0
+++ b/tests/shell/testcases/listing/0020flowtable_0
@@ -2,19 +2,60 @@
 
 # list only the flowtable asked for with table
 
+FLOWTABLES="flowtable f {
+	hook ingress priority filter
+	devices = { lo }
+}
+flowtable f2 {
+	hook ingress priority filter
+	devices = { d0 }
+}"
+
+RULESET="table inet filter {
+	$FLOWTABLES
+}
+table ip filter {
+	$FLOWTABLES
+}"
+
 EXPECTED="table inet filter {
 	flowtable f {
 		hook ingress priority filter
 		devices = { lo }
 	}
 }"
+EXPECTED2="table ip filter {
+	flowtable f2 {
+		hook ingress priority filter
+		devices = { d0 }
+	}
+}"
+EXPECTED3="table ip filter {
+	flowtable f {
+		hook ingress priority filter
+		devices = { lo }
+	}
+	flowtable f2 {
+		hook ingress priority filter
+		devices = { d0 }
+	}
+}"
+
+ip link add d0 type dummy || {
+	echo "Skipping, no dummy interface available"
+	exit 0
+}
+trap "ip link del d0" EXIT
 
 set -e
 
-$NFT -f - <<< "$EXPECTED"
+$NFT -f - <<< "$RULESET"
 
 GET="$($NFT list flowtable inet filter f)"
-if [ "$EXPECTED" != "$GET" ] ; then
-	$DIFF -u <(echo "$EXPECTED") <(echo "$GET")
-	exit 1
-fi
+$DIFF -u <(echo "$EXPECTED") <(echo "$GET")
+
+GET="$($NFT list flowtable ip filter f2)"
+$DIFF -u <(echo "$EXPECTED2") <(echo "$GET")
+
+GET="$($NFT list flowtables ip)"
+$DIFF -u <(echo "$EXPECTED3") <(echo "$GET")
-- 
2.33.0

Re: [nft PATCH v2 0/5] Reduce cache overhead a bit

[Pablo Neira Ayuso]

On Thu, Dec 02, 2021 at 02:11:31PM +0100, Phil Sutter wrote:
> Second try after a quick review and some testing:
> 
> - Tested with stable kernels v4.4.293 and v4.9.291: This series does not
>   change any of the shell tests' results. The changes are supposedly
>   bug- and feature-compatible.

LGTM, thanks