[nf] netfilter: nf_tables: meter: pick a set backend that supports updates

Message ID 20180314123758.2583-1-fw@strlen.de
State Accepted
Delegated to: Pablo Neira
Headers show
Series
  • [nf] netfilter: nf_tables: meter: pick a set backend that supports updates
Related show

Commit Message

Florian Westphal March 14, 2018, 12:37 p.m.
in nftables, 'meter' can be used to instantiate a hash-table at run
time:

rule add filter forward iif "internal" meter hostacct { ip saddr counter}
nft list meter ip filter hostacct
table ip filter {
  meter hostacct {
    type ipv4_addr
    elements = { 192.168.0.1 : counter packets 8 bytes 2672, ..

because elemets get added on the fly, the kernel must chose a set
backend type that implements the ->update() function, otherwise
rule insertion fails with EOPNOTSUPP.

Therefore, skip set types that lack ->update, and also
make sure we do not discard a (bad) candidate when we did yet
find any candidate at all.  This could happen when userspace prefers
low memory footprint -- the set implementation currently checked might
not be a fit at all.  Make sure we pick it anyway (!bops).  In
case next candidate is a better fix, it will be chosen instead.

But in case nothing else is found we at least have a non-ideal
match rather than no match at all.

Fixes: 6c03ae210ce3 ("netfilter: nft_set_hash: add non-resizable hashtable implementation")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_tables_api.c | 5 ++++-
 net/netfilter/nft_set_hash.c  | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

Comments

Pablo Neira Ayuso March 20, 2018, 12:53 p.m. | #1
On Wed, Mar 14, 2018 at 01:37:58PM +0100, Florian Westphal wrote:
> in nftables, 'meter' can be used to instantiate a hash-table at run
> time:
> 
> rule add filter forward iif "internal" meter hostacct { ip saddr counter}
> nft list meter ip filter hostacct
> table ip filter {
>   meter hostacct {
>     type ipv4_addr
>     elements = { 192.168.0.1 : counter packets 8 bytes 2672, ..
> 
> because elemets get added on the fly, the kernel must chose a set
> backend type that implements the ->update() function, otherwise
> rule insertion fails with EOPNOTSUPP.
> 
> Therefore, skip set types that lack ->update, and also
> make sure we do not discard a (bad) candidate when we did yet
> find any candidate at all.  This could happen when userspace prefers
> low memory footprint -- the set implementation currently checked might
> not be a fit at all.  Make sure we pick it anyway (!bops).  In
> case next candidate is a better fix, it will be chosen instead.
> 
> But in case nothing else is found we at least have a non-ideal
> match rather than no match at all.

Applied, thanks Florian.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c4acc7340eb1..36f69acaf51f 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2446,6 +2446,9 @@  EXPORT_SYMBOL_GPL(nft_unregister_set);
 
 static bool nft_set_ops_candidate(const struct nft_set_ops *ops, u32 flags)
 {
+	if ((flags & NFT_SET_EVAL) && !ops->update)
+		return false;
+
 	return (flags & ops->features) == (flags & NFT_SET_FEATURES);
 }
 
@@ -2510,7 +2513,7 @@  nft_select_set_ops(const struct nft_ctx *ctx,
 				if (est.space == best.space &&
 				    est.lookup < best.lookup)
 					break;
-			} else if (est.size < best.size) {
+			} else if (est.size < best.size || !bops) {
 				break;
 			}
 			continue;
diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c
index d40591fe1b2f..fc9c6d5d64cd 100644
--- a/net/netfilter/nft_set_hash.c
+++ b/net/netfilter/nft_set_hash.c
@@ -674,7 +674,7 @@  static const struct nft_set_ops *
 nft_hash_select_ops(const struct nft_ctx *ctx, const struct nft_set_desc *desc,
 		    u32 flags)
 {
-	if (desc->size && !(flags & NFT_SET_TIMEOUT)) {
+	if (desc->size && !(flags & (NFT_SET_EVAL | NFT_SET_TIMEOUT))) {
 		switch (desc->klen) {
 		case 4:
 			return &nft_hash_fast_ops;