[PATCH 0/3 nftables RFC] set infrastructure updates

[PATCH 0/3 nftables RFC] set infrastructure updates

[Pablo Neira Ayuso]

Hi,

The following patchset contains three updates for the set
infrastructure in nf_tables, they are:

1) Fix suboptimal set selection. In my testbed, all anonymous/constant sets
   are being created via the rb-tree set type, which is not optimal. To
   resolve this problem I have added a priority field, so in case that we
   find several set types that are suitable for the features that are
   requested. As a result, the hash is prefered to the rb-tree set type
   whenever possible.

2) Limit maximum number of elements per sets: Currently we have no way to set
   this limit. This adds two new netlink attributes, one to set the maximum
   number of elements and another to return the current number of elements
   per sets. The default maximum size is set to 1024, thus, the hashtable
   array consumes 16 KBytes in x86_64.

3) Calculate the number of buckets in the hash set based on the maximum
   number of elements to achieve a load factor of .75. For anonymous/constant
   sets, the maximum element number that nft specifies is exactly the amount
   of elements that the set contains to optimize memory consumption.

In this direction, by adding more specific set descriptions (via a new netlink
attributes that contains configuration information specified by the user) it
should be possible to allow space-time trade-offs for named sets.

Pablo Neira Ayuso (3):
  netfilter: nf_tables: fix suboptimal set selection
  netfilter: nf_tables: limit maximum number of elements
  netfilter: nft_hash: use set->maxelems to calculate number of buckets

 include/net/netfilter/nf_tables.h        |   17 +++++++++++++++++
 include/uapi/linux/netfilter/nf_tables.h |    4 ++++
 net/netfilter/nf_tables_api.c            |   25 ++++++++++++++++++++-----
 net/netfilter/nft_hash.c                 |    5 ++---
 net/netfilter/nft_rbtree.c               |    1 +
 5 files changed, 44 insertions(+), 8 deletions(-)

-- 
1.7.10.4

[PATCH 1/3] netfilter: nf_tables: fix suboptimal set selection

[Pablo Neira Ayuso]

The rb-tree is currently used for simple sets and maps with no
intervals which is suboptimal. Fix it by adding the weight field
to each existing set implementation, this value allows to select
the best candidate in case that several set types can be used.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables.h |   13 +++++++++++++
 net/netfilter/nf_tables_api.c     |   10 +++++-----
 net/netfilter/nft_hash.c          |    1 +
 net/netfilter/nft_rbtree.c        |    1 +
 4 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 5a91abf..82920e8 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -143,6 +143,17 @@ struct nft_set_iter {
 };
 
 /**
+ *	enum nft_set_prio - nf_tables set priority
+ *
+ *	This is used to set preference in case that all set types provide the
+ *	same features.
+ */
+enum nft_set_prio {
+	NFT_SET_PRIO_HASH	= 0,
+	NFT_SET_PRIO_RBTREE,
+};
+
+/**
  *	struct nft_set_ops - nf_tables set operations
  *
  *	@lookup: look up an element within the set
@@ -155,6 +166,7 @@ struct nft_set_iter {
  *	@list: nf_tables_set_ops list node
  *	@owner: module reference
  *	@features: features supported by the implementation
+ *	@priority: priority of this set type
  */
 struct nft_set_ops {
 	bool				(*lookup)(const struct nft_set *set,
@@ -178,6 +190,7 @@ struct nft_set_ops {
 	struct list_head		list;
 	struct module			*owner;
 	u32				features;
+	u32				priority;
 };
 
 int nft_register_set(struct nft_set_ops *ops);
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 0d4b42d..60efb61 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1857,7 +1857,7 @@ EXPORT_SYMBOL_GPL(nft_unregister_set);
 
 static const struct nft_set_ops *nft_select_set_ops(const struct nlattr * const nla[])
 {
-	const struct nft_set_ops *ops;
+	const struct nft_set_ops *ops, *cand = NULL;
 	u32 features;
 
 #ifdef CONFIG_MODULES
@@ -1875,14 +1875,14 @@ static const struct nft_set_ops *nft_select_set_ops(const struct nlattr * const
 		features &= NFT_SET_INTERVAL | NFT_SET_MAP;
 	}
 
-	// FIXME: implement selection properly
 	list_for_each_entry(ops, &nf_tables_set_ops, list) {
 		if ((ops->features & features) != features)
 			continue;
-		if (!try_module_get(ops->owner))
-			continue;
-		return ops;
+		if (!cand || cand->priority > ops->priority)
+			cand = ops;
 	}
+	if (cand && try_module_get(cand->owner))
+		return 0;
 
 	return ERR_PTR(-EOPNOTSUPP);
 }
diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index 3d3f8fc..f640c1c 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -210,6 +210,7 @@ static struct nft_set_ops nft_hash_ops __read_mostly = {
 	.lookup		= nft_hash_lookup,
 	.walk		= nft_hash_walk,
 	.features	= NFT_SET_MAP,
+	.priority	= NFT_SET_PRIO_HASH,
 	.owner		= THIS_MODULE,
 };
 
diff --git a/net/netfilter/nft_rbtree.c b/net/netfilter/nft_rbtree.c
index ca0c1b2..acddc64 100644
--- a/net/netfilter/nft_rbtree.c
+++ b/net/netfilter/nft_rbtree.c
@@ -226,6 +226,7 @@ static struct nft_set_ops nft_rbtree_ops __read_mostly = {
 	.lookup		= nft_rbtree_lookup,
 	.walk		= nft_rbtree_walk,
 	.features	= NFT_SET_INTERVAL | NFT_SET_MAP,
+	.priority	= NFT_SET_PRIO_RBTREE,
 	.owner		= THIS_MODULE,
 };
 
-- 
1.7.10.4

[PATCH 2/3] netfilter: nf_tables: limit maximum number of elements

[Pablo Neira Ayuso]

This patch adds a limit to the maximum number of elements that
belong to a set. It also adds a new field to count the current
number of elements, so in case that limit is reached, we hit
-ENOSPC.

This patch also adds two new attributes: NFTA_SET_MAXELEMS to
set and to indicate the current limit and NFTA_SET_NUMELEMS
to export the current number of set elements.

If not specified, it defaults to 1024 that results in a hashtable
of 16 KBytes in x86_64.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables.h        |    4 ++++
 include/uapi/linux/netfilter/nf_tables.h |    4 ++++
 net/netfilter/nf_tables_api.c            |   15 +++++++++++++++
 3 files changed, 23 insertions(+)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 82920e8..bb51649 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -208,6 +208,8 @@ void nft_unregister_set(struct nft_set_ops *ops);
  * 	@flags: set flags
  * 	@klen: key length
  * 	@dlen: data length
+ *	@maxelems: maximum number of elemens (default to 65535)
+ *	@numelems: current number of elemens
  * 	@data: private set data
  */
 struct nft_set {
@@ -221,6 +223,8 @@ struct nft_set {
 	u16				flags;
 	u8				klen;
 	u8				dlen;
+	u32				maxelems;
+	u32				numelems;
 	unsigned char			data[]
 		__attribute__((aligned(__alignof__(u64))));
 };
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index aa86a152..aea7589 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -218,6 +218,8 @@ enum nft_set_flags {
  * @NFTA_SET_KEY_LEN: key data length (NLA_U32)
  * @NFTA_SET_DATA_TYPE: mapping data type (NLA_U32)
  * @NFTA_SET_DATA_LEN: mapping data length (NLA_U32)
+ * @NFTA_SET_MAXELEMS: maximum number of elements (NLA_U32)
+ * @NFTA_SET_NUMELEMS: current number of elements (NLA_U32)
  */
 enum nft_set_attributes {
 	NFTA_SET_UNSPEC,
@@ -228,6 +230,8 @@ enum nft_set_attributes {
 	NFTA_SET_KEY_LEN,
 	NFTA_SET_DATA_TYPE,
 	NFTA_SET_DATA_LEN,
+	NFTA_SET_MAXELEMS,
+	NFTA_SET_NUMELEMS,
 	__NFTA_SET_MAX
 };
 #define NFTA_SET_MAX		(__NFTA_SET_MAX - 1)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 60efb61..488277d 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1895,6 +1895,7 @@ static const struct nla_policy nft_set_policy[NFTA_SET_MAX + 1] = {
 	[NFTA_SET_KEY_LEN]		= { .type = NLA_U32 },
 	[NFTA_SET_DATA_TYPE]		= { .type = NLA_U32 },
 	[NFTA_SET_DATA_LEN]		= { .type = NLA_U32 },
+	[NFTA_SET_MAXELEMS]		= { .type = NLA_U32 },
 };
 
 static int nft_ctx_init_from_setattr(struct nft_ctx *ctx,
@@ -2015,6 +2016,10 @@ static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,
 		if (nla_put_be32(skb, NFTA_SET_DATA_LEN, htonl(set->dlen)))
 			goto nla_put_failure;
 	}
+	if (nla_put_be32(skb, NFTA_SET_MAXELEMS, htonl(set->maxelems)))
+		goto nla_put_failure;
+	if (nla_put_be32(skb, NFTA_SET_NUMELEMS, htonl(set->numelems)))
+		goto nla_put_failure;
 
 	return nlmsg_end(skb, nlh);
 
@@ -2360,6 +2365,11 @@ static int nf_tables_newset(struct sock *nlsk, struct sk_buff *skb,
 	set->dlen  = dlen;
 	set->flags = flags;
 
+	if (nla[NFTA_SET_MAXELEMS])
+		set->maxelems = ntohl(nla_get_be32(nla[NFTA_SET_MAXELEMS]));
+	else
+		set->maxelems = 1024;
+
 	err = ops->init(set, nla);
 	if (err < 0)
 		goto err2;
@@ -2669,6 +2679,9 @@ static int nft_add_set_elem(const struct nft_ctx *ctx, struct nft_set *set,
 	enum nft_registers dreg;
 	int err;
 
+	if (set->numelems >= set->maxelems)
+		return -ENOSPC;
+
 	err = nla_parse_nested(nla, NFTA_SET_ELEM_MAX, attr,
 			       nft_set_elem_policy);
 	if (err < 0)
@@ -2732,6 +2745,7 @@ static int nft_add_set_elem(const struct nft_ctx *ctx, struct nft_set *set,
 	if (err < 0)
 		goto err3;
 
+	set->numelems++;
 	return 0;
 
 err3:
@@ -2805,6 +2819,7 @@ static int nft_del_setelem(const struct nft_ctx *ctx, struct nft_set *set,
 	if (set->flags & NFT_SET_MAP)
 		nft_data_uninit(&elem.data, set->dtype);
 
+	set->numelems--;
 err2:
 	nft_data_uninit(&elem.key, desc.type);
 err1:
-- 
1.7.10.4

[PATCH 3/3] netfilter: nft_hash: use set->maxelems to calculate number of buckets

[Pablo Neira Ayuso]

Use set->maxelems to calculate the number of buckets that holds a
load factor of 0.75.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_hash.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index f640c1c..99e4742 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -169,9 +169,7 @@ static int nft_hash_init(const struct nft_set *set,
 	}
 
 	/* Aim for a load factor of 0.75 */
-	// FIXME: temporarily broken until we have set descriptions
-	cnt = 100;
-	cnt = cnt * 4 / 3;
+	cnt = set->maxelems * 4 / 3;
 
 	priv->hash = kcalloc(cnt, sizeof(struct hlist_head), GFP_KERNEL);
 	if (priv->hash == NULL)
-- 
1.7.10.4

Re: [PATCH 1/3] netfilter: nf_tables: fix suboptimal set selection

[Patrick McHardy]

On Sun, Jan 05, 2014 at 10:18:46PM +0100, Pablo Neira Ayuso wrote:
> The rb-tree is currently used for simple sets and maps with no
> intervals which is suboptimal. Fix it by adding the weight field
> to each existing set implementation, this value allows to select
> the best candidate in case that several set types can be used.

Ohh, lets do this properly. This is one point why I was opposed to a
merge at this stage, lets not paper over this but fix this how it
was intended. I'll work on that after finishing the inet family.
Until then no harm is done, just some memory waste.

Re: [PATCH 1/3] netfilter: nf_tables: fix suboptimal set selection

[Pablo Neira Ayuso]

On Sun, Jan 05, 2014 at 09:28:35PM +0000, Patrick McHardy wrote:
> On Sun, Jan 05, 2014 at 10:18:46PM +0100, Pablo Neira Ayuso wrote:
> > The rb-tree is currently used for simple sets and maps with no
> > intervals which is suboptimal. Fix it by adding the weight field
> > to each existing set implementation, this value allows to select
> > the best candidate in case that several set types can be used.
> 
> Ohh, lets do this properly. This is one point why I was opposed to a
> merge at this stage, lets not paper over this but fix this how it
> was intended. I'll work on that after finishing the inet family.
> Until then no harm is done, just some memory waste.

Please, elaborate your plans on this.

Re: [PATCH 1/3] netfilter: nf_tables: fix suboptimal set selection

[Patrick McHardy]

On Sun, Jan 05, 2014 at 10:34:03PM +0100, Pablo Neira Ayuso wrote:
> On Sun, Jan 05, 2014 at 09:28:35PM +0000, Patrick McHardy wrote:
> > On Sun, Jan 05, 2014 at 10:18:46PM +0100, Pablo Neira Ayuso wrote:
> > > The rb-tree is currently used for simple sets and maps with no
> > > intervals which is suboptimal. Fix it by adding the weight field
> > > to each existing set implementation, this value allows to select
> > > the best candidate in case that several set types can be used.
> > 
> > Ohh, lets do this properly. This is one point why I was opposed to a
> > merge at this stage, lets not paper over this but fix this how it
> > was intended. I'll work on that after finishing the inet family.
> > Until then no harm is done, just some memory waste.
> 
> Please, elaborate your plans on this.

Well, the selection should happen based on the set contents, not on
some static weight. Basically we have memory usage and performance
as measurable weights, and both depend on the contents of the set,
so we need an abstrace description of those.

I have another set type coming up, static weights will not help to
make a good decision long term. This won't be easy to get right,
but I really don't want to paper over this since this just allows
to delay to proper fix that will be needed at some point anyway.

Re: [PATCH 3/3] netfilter: nft_hash: use set->maxelems to calculate number of buckets

[Patrick McHardy]

On Sun, Jan 05, 2014 at 10:18:48PM +0100, Pablo Neira Ayuso wrote:
> Use set->maxelems to calculate the number of buckets that holds a
> load factor of 0.75.

This patch is perfectly fine of course.

> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  net/netfilter/nft_hash.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
> index f640c1c..99e4742 100644
> --- a/net/netfilter/nft_hash.c
> +++ b/net/netfilter/nft_hash.c
> @@ -169,9 +169,7 @@ static int nft_hash_init(const struct nft_set *set,
>  	}
>  
>  	/* Aim for a load factor of 0.75 */
> -	// FIXME: temporarily broken until we have set descriptions
> -	cnt = 100;
> -	cnt = cnt * 4 / 3;
> +	cnt = set->maxelems * 4 / 3;
>  
>  	priv->hash = kcalloc(cnt, sizeof(struct hlist_head), GFP_KERNEL);
>  	if (priv->hash == NULL)
> -- 
> 1.7.10.4

Re: [PATCH 2/3] netfilter: nf_tables: limit maximum number of elements

[Patrick McHardy]

On Sun, Jan 05, 2014 at 10:18:47PM +0100, Pablo Neira Ayuso wrote:
> This patch adds a limit to the maximum number of elements that
> belong to a set. It also adds a new field to count the current
> number of elements, so in case that limit is reached, we hit
> -ENOSPC.
> 
> This patch also adds two new attributes: NFTA_SET_MAXELEMS to
> set and to indicate the current limit and NFTA_SET_NUMELEMS
> to export the current number of set elements.

This affects the API, I'm really opposed to make any temporary
fixes, but any temporary fix affecting the API is a complete
no go IMO.

Re: [PATCH 1/3] netfilter: nf_tables: fix suboptimal set selection

[Pablo Neira Ayuso]

On Sun, Jan 05, 2014 at 09:45:31PM +0000, Patrick McHardy wrote:
> On Sun, Jan 05, 2014 at 10:34:03PM +0100, Pablo Neira Ayuso wrote:
> > On Sun, Jan 05, 2014 at 09:28:35PM +0000, Patrick McHardy wrote:
> > > On Sun, Jan 05, 2014 at 10:18:46PM +0100, Pablo Neira Ayuso wrote:
> > > > The rb-tree is currently used for simple sets and maps with no
> > > > intervals which is suboptimal. Fix it by adding the weight field
> > > > to each existing set implementation, this value allows to select
> > > > the best candidate in case that several set types can be used.
> > > 
> > > Ohh, lets do this properly. This is one point why I was opposed to a
> > > merge at this stage, lets not paper over this but fix this how it
> > > was intended. I'll work on that after finishing the inet family.
> > > Until then no harm is done, just some memory waste.
> > 
> > Please, elaborate your plans on this.
> 
> Well, the selection should happen based on the set contents, not on
> some static weight. Basically we have memory usage and performance
> as measurable weights, and both depend on the contents of the set,
> so we need an abstrace description of those.

I agree on that the more information we provide to the kernel, the
best decision regarding the set type and its configuration will be
made.

This simple weight thing that this patch adds is just a way to
break a tie in case two sets provides the features that are needed
(this is the only description information that we have at this moment
from userspace). I think for simple sets (with no intervals) hash
should be prefered to rb-trees.

But if bitmap sets come into place, the selection between hash and
bitmaps cannot be done by this weight this patch adds, as we need more
information on the set elements to make the right choice. That I can
think, we need the distance between the two elements that are further
from each other to calculate the memory consumption at least.

Re: [PATCH 3/3] netfilter: nft_hash: use set->maxelems to calculate number of buckets

[Pablo Neira Ayuso]

On Sun, Jan 05, 2014 at 09:47:44PM +0000, Patrick McHardy wrote:
> On Sun, Jan 05, 2014 at 10:18:48PM +0100, Pablo Neira Ayuso wrote:
> > Use set->maxelems to calculate the number of buckets that holds a
> > load factor of 0.75.
> 
> This patch is perfectly fine of course.

Thanks. So I guess you're fine with 2/3 as well which is needed by
this patch.

Re: [PATCH 2/3] netfilter: nf_tables: limit maximum number of elements

[Pablo Neira Ayuso]

On Sun, Jan 05, 2014 at 09:51:02PM +0000, Patrick McHardy wrote:
> On Sun, Jan 05, 2014 at 10:18:47PM +0100, Pablo Neira Ayuso wrote:
> > This patch adds a limit to the maximum number of elements that
> > belong to a set. It also adds a new field to count the current
> > number of elements, so in case that limit is reached, we hit
> > -ENOSPC.
> > 
> > This patch also adds two new attributes: NFTA_SET_MAXELEMS to
> > set and to indicate the current limit and NFTA_SET_NUMELEMS
> > to export the current number of set elements.
> 
> This affects the API, I'm really opposed to make any temporary
> fixes, but any temporary fix affecting the API is a complete
> no go IMO.

We need to limit elements in a set. I think it's not good that you can
add an unlimited number of sets, right?

This patch is independent of 1/3, which can be discarded. But 3/3
relies on it.

Re: [PATCH 2/3] netfilter: nf_tables: limit maximum number of elements

[Pablo Neira Ayuso]

On Sun, Jan 05, 2014 at 11:14:04PM +0100, Pablo Neira Ayuso wrote:
> On Sun, Jan 05, 2014 at 09:51:02PM +0000, Patrick McHardy wrote:
> > On Sun, Jan 05, 2014 at 10:18:47PM +0100, Pablo Neira Ayuso wrote:
> > > This patch adds a limit to the maximum number of elements that
> > > belong to a set. It also adds a new field to count the current
> > > number of elements, so in case that limit is reached, we hit
> > > -ENOSPC.
> > > 
> > > This patch also adds two new attributes: NFTA_SET_MAXELEMS to
> > > set and to indicate the current limit and NFTA_SET_NUMELEMS
> > > to export the current number of set elements.
> > 
> > This affects the API, I'm really opposed to make any temporary
> > fixes, but any temporary fix affecting the API is a complete
> > no go IMO.
> 
> We need to limit elements in a set. I think it's not good that you can
> add an unlimited number of sets, right?

(I meant to say) ... an unlimited number of elements

> This patch is independent of 1/3, which can be discarded. But 3/3
> relies on it.
> --
> 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

Re: [PATCH 1/3] netfilter: nf_tables: fix suboptimal set selection

[Patrick McHardy]

On Sun, Jan 05, 2014 at 11:11:05PM +0100, Pablo Neira Ayuso wrote:
> On Sun, Jan 05, 2014 at 09:45:31PM +0000, Patrick McHardy wrote:
> > On Sun, Jan 05, 2014 at 10:34:03PM +0100, Pablo Neira Ayuso wrote:
> > > On Sun, Jan 05, 2014 at 09:28:35PM +0000, Patrick McHardy wrote:
> > > > On Sun, Jan 05, 2014 at 10:18:46PM +0100, Pablo Neira Ayuso wrote:
> > > > > The rb-tree is currently used for simple sets and maps with no
> > > > > intervals which is suboptimal. Fix it by adding the weight field
> > > > > to each existing set implementation, this value allows to select
> > > > > the best candidate in case that several set types can be used.
> > > > 
> > > > Ohh, lets do this properly. This is one point why I was opposed to a
> > > > merge at this stage, lets not paper over this but fix this how it
> > > > was intended. I'll work on that after finishing the inet family.
> > > > Until then no harm is done, just some memory waste.
> > > 
> > > Please, elaborate your plans on this.
> > 
> > Well, the selection should happen based on the set contents, not on
> > some static weight. Basically we have memory usage and performance
> > as measurable weights, and both depend on the contents of the set,
> > so we need an abstrace description of those.
> 
> I agree on that the more information we provide to the kernel, the
> best decision regarding the set type and its configuration will be
> made.
> 
> This simple weight thing that this patch adds is just a way to
> break a tie in case two sets provides the features that are needed
> (this is the only description information that we have at this moment
> from userspace). I think for simple sets (with no intervals) hash
> should be prefered to rb-trees.

Indeed, but not based on the fact that its a hash, but based on
performance and memory usage.

> But if bitmap sets come into place, the selection between hash and
> bitmaps cannot be done by this weight this patch adds, as we need more
> information on the set elements to make the right choice. That I can
> think, we need the distance between the two elements that are further
> from each other to calculate the memory consumption at least.

Correct. I would really prefer to keep the waste to keep the incentive
to fix this properly up. Its definitely doable, lets keep the pain
up to fix this properly, nobody is using this in production anyway.

Re: [PATCH 2/3] netfilter: nf_tables: limit maximum number of elements

[Patrick McHardy]

On Sun, Jan 05, 2014 at 11:14:04PM +0100, Pablo Neira Ayuso wrote:
> On Sun, Jan 05, 2014 at 09:51:02PM +0000, Patrick McHardy wrote:
> > On Sun, Jan 05, 2014 at 10:18:47PM +0100, Pablo Neira Ayuso wrote:
> > > This patch adds a limit to the maximum number of elements that
> > > belong to a set. It also adds a new field to count the current
> > > number of elements, so in case that limit is reached, we hit
> > > -ENOSPC.
> > > 
> > > This patch also adds two new attributes: NFTA_SET_MAXELEMS to
> > > set and to indicate the current limit and NFTA_SET_NUMELEMS
> > > to export the current number of set elements.
> > 
> > This affects the API, I'm really opposed to make any temporary
> > fixes, but any temporary fix affecting the API is a complete
> > no go IMO.
> 
> We need to limit elements in a set. I think it's not good that you can
> add an unlimited number of sets, right?
> 
> This patch is independent of 1/3, which can be discarded. But 3/3
> relies on it.

In that case we should add rehashing. But adding arbitrary limits and
even encoding them in the API is wrong on so many levels. We really
need to abstract this stuff. Constant sets can't grow. If we're using
a hash that's not constant, we can easily either make a resizeable
hash or add a second implementation that's resizable and improve
selection. But the point is, this should be (apart from being constant)
transparent to userspace. The kernel is to provide the implementation,
not the policy.