Could not process rule: Cannot allocate memory

Could not process rule: Cannot allocate memory

[Sven Auhagen]

Hi,

I am using nftables with geoip sets.
When I have larger sets in my ruleset and I want to atomically update the entire ruleset, I start with
destroy table inet filter and then continue with my new ruleset.

When the sets are larger I now always get an error:
./main.nft:13:1-26: Error: Could not process rule: Cannot allocate memory
destroy table inet filter
^^^^^^^^^^^^^^^^^^^^^^^^^^

along with the kernel message
percpu: allocation failed, size=16 align=8 atomic=1, atomic alloc failed, no space left

This also happens when I use delete instead of destroy.

This seems to be an issue with allocating atomic memory in the netfilter kernel code.
Does anyone have a hint what is going on and how to debug it or maybe a suggestion
for a patch?

Best and thanks
Sven

Re: Could not process rule: Cannot allocate memory

[Florian Westphal]

Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> I am using nftables with geoip sets.
> When I have larger sets in my ruleset and I want to atomically update the entire ruleset, I start with
> destroy table inet filter and then continue with my new ruleset.
> 
> When the sets are larger I now always get an error:
> ./main.nft:13:1-26: Error: Could not process rule: Cannot allocate memory
> destroy table inet filter
> ^^^^^^^^^^^^^^^^^^^^^^^^^^

> along with the kernel message
> percpu: allocation failed, size=16 align=8 atomic=1, atomic alloc failed, no space left

Are you using 'counter' extension on the set definition?

Could yo usahre a minimal reproducer? You can omit the actual
elements, its easy to autogen that.

[PATCH nf-next] netfilter: nf_tables: allow clone callbacks to sleep

[Florian Westphal]

Sven Auhagen reports transaction failures with following error:
  ./main.nft:13:1-26: Error: Could not process rule: Cannot allocate memory
  percpu: allocation failed, size=16 align=8 atomic=1, atomic alloc failed, no space left

This points to failing pcpu allocation with GFP_ATOMIC flag.
However, transactions happen from user context and are allowed to sleep.

One case where we can call into percpu allocator with GFP_ATOMIC is
nft_counter expression.

Normally this happens from control plane, so this could use GFP_KERNEL
instead.  But one use case, element insertion from packet path,
needs to use GFP_ATOMIC allocations (nft_dynset expression).

At this time, .clone callbacks always use GFP_ATOMIC for this reason.

Add gfp_t argument to the .clone function and pass GFP_KERNEL or
GFP_ATOMIC flag depending on context, this allows all clone memory
allocations to sleep for the normal (transaction) case.

Cc: Sven Auhagen <sven.auhagen@voleatech.de>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/netfilter/nf_tables.h |  4 ++--
 net/netfilter/nf_tables_api.c     |  8 ++++----
 net/netfilter/nft_connlimit.c     |  4 ++--
 net/netfilter/nft_counter.c       |  4 ++--
 net/netfilter/nft_dynset.c        |  2 +-
 net/netfilter/nft_last.c          |  4 ++--
 net/netfilter/nft_limit.c         | 14 ++++++++------
 net/netfilter/nft_quota.c         |  4 ++--
 8 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 3f1ed467f951..2796153b03da 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -416,7 +416,7 @@ struct nft_expr_info;
 
 int nft_expr_inner_parse(const struct nft_ctx *ctx, const struct nlattr *nla,
 			 struct nft_expr_info *info);
-int nft_expr_clone(struct nft_expr *dst, struct nft_expr *src);
+int nft_expr_clone(struct nft_expr *dst, struct nft_expr *src, gfp_t gfp);
 void nft_expr_destroy(const struct nft_ctx *ctx, struct nft_expr *expr);
 int nft_expr_dump(struct sk_buff *skb, unsigned int attr,
 		  const struct nft_expr *expr, bool reset);
@@ -935,7 +935,7 @@ struct nft_expr_ops {
 						struct nft_regs *regs,
 						const struct nft_pktinfo *pkt);
 	int				(*clone)(struct nft_expr *dst,
-						 const struct nft_expr *src);
+						 const struct nft_expr *src, gfp_t gfp);
 	unsigned int			size;
 
 	int				(*init)(const struct nft_ctx *ctx,
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 167074283ea9..c0a4813c80c9 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3333,7 +3333,7 @@ static struct nft_expr *nft_expr_init(const struct nft_ctx *ctx,
 	return ERR_PTR(err);
 }
 
-int nft_expr_clone(struct nft_expr *dst, struct nft_expr *src)
+int nft_expr_clone(struct nft_expr *dst, struct nft_expr *src, gfp_t gfp)
 {
 	int err;
 
@@ -3341,7 +3341,7 @@ int nft_expr_clone(struct nft_expr *dst, struct nft_expr *src)
 		return -EINVAL;
 
 	dst->ops = src->ops;
-	err = src->ops->clone(dst, src);
+	err = src->ops->clone(dst, src, gfp);
 	if (err < 0)
 		return err;
 
@@ -6525,7 +6525,7 @@ int nft_set_elem_expr_clone(const struct nft_ctx *ctx, struct nft_set *set,
 		if (!expr)
 			goto err_expr;
 
-		err = nft_expr_clone(expr, set->exprs[i]);
+		err = nft_expr_clone(expr, set->exprs[i], GFP_KERNEL_ACCOUNT);
 		if (err < 0) {
 			kfree(expr);
 			goto err_expr;
@@ -6564,7 +6564,7 @@ static int nft_set_elem_expr_setup(struct nft_ctx *ctx,
 
 	for (i = 0; i < num_exprs; i++) {
 		expr = nft_setelem_expr_at(elem_expr, elem_expr->size);
-		err = nft_expr_clone(expr, expr_array[i]);
+		err = nft_expr_clone(expr, expr_array[i], GFP_KERNEL_ACCOUNT);
 		if (err < 0)
 			goto err_elem_expr_setup;
 
diff --git a/net/netfilter/nft_connlimit.c b/net/netfilter/nft_connlimit.c
index de9d1980df69..92b984fa8175 100644
--- a/net/netfilter/nft_connlimit.c
+++ b/net/netfilter/nft_connlimit.c
@@ -210,12 +210,12 @@ static void nft_connlimit_destroy(const struct nft_ctx *ctx,
 	nft_connlimit_do_destroy(ctx, priv);
 }
 
-static int nft_connlimit_clone(struct nft_expr *dst, const struct nft_expr *src)
+static int nft_connlimit_clone(struct nft_expr *dst, const struct nft_expr *src, gfp_t gfp)
 {
 	struct nft_connlimit *priv_dst = nft_expr_priv(dst);
 	struct nft_connlimit *priv_src = nft_expr_priv(src);
 
-	priv_dst->list = kmalloc(sizeof(*priv_dst->list), GFP_ATOMIC);
+	priv_dst->list = kmalloc(sizeof(*priv_dst->list), gfp);
 	if (!priv_dst->list)
 		return -ENOMEM;
 
diff --git a/net/netfilter/nft_counter.c b/net/netfilter/nft_counter.c
index dccc68a5135a..291ed2026367 100644
--- a/net/netfilter/nft_counter.c
+++ b/net/netfilter/nft_counter.c
@@ -226,7 +226,7 @@ static void nft_counter_destroy(const struct nft_ctx *ctx,
 	nft_counter_do_destroy(priv);
 }
 
-static int nft_counter_clone(struct nft_expr *dst, const struct nft_expr *src)
+static int nft_counter_clone(struct nft_expr *dst, const struct nft_expr *src, gfp_t gfp)
 {
 	struct nft_counter_percpu_priv *priv = nft_expr_priv(src);
 	struct nft_counter_percpu_priv *priv_clone = nft_expr_priv(dst);
@@ -236,7 +236,7 @@ static int nft_counter_clone(struct nft_expr *dst, const struct nft_expr *src)
 
 	nft_counter_fetch(priv, &total);
 
-	cpu_stats = alloc_percpu_gfp(struct nft_counter, GFP_ATOMIC);
+	cpu_stats = alloc_percpu_gfp(struct nft_counter, gfp);
 	if (cpu_stats == NULL)
 		return -ENOMEM;
 
diff --git a/net/netfilter/nft_dynset.c b/net/netfilter/nft_dynset.c
index c09dba57354c..b4ada3ab2167 100644
--- a/net/netfilter/nft_dynset.c
+++ b/net/netfilter/nft_dynset.c
@@ -35,7 +35,7 @@ static int nft_dynset_expr_setup(const struct nft_dynset *priv,
 
 	for (i = 0; i < priv->num_exprs; i++) {
 		expr = nft_setelem_expr_at(elem_expr, elem_expr->size);
-		if (nft_expr_clone(expr, priv->expr_array[i]) < 0)
+		if (nft_expr_clone(expr, priv->expr_array[i], GFP_ATOMIC) < 0)
 			return -1;
 
 		elem_expr->size += priv->expr_array[i]->ops->size;
diff --git a/net/netfilter/nft_last.c b/net/netfilter/nft_last.c
index 8e6d7eaf9dc8..de1b6066bfa8 100644
--- a/net/netfilter/nft_last.c
+++ b/net/netfilter/nft_last.c
@@ -102,12 +102,12 @@ static void nft_last_destroy(const struct nft_ctx *ctx,
 	kfree(priv->last);
 }
 
-static int nft_last_clone(struct nft_expr *dst, const struct nft_expr *src)
+static int nft_last_clone(struct nft_expr *dst, const struct nft_expr *src, gfp_t gfp)
 {
 	struct nft_last_priv *priv_dst = nft_expr_priv(dst);
 	struct nft_last_priv *priv_src = nft_expr_priv(src);
 
-	priv_dst->last = kzalloc(sizeof(*priv_dst->last), GFP_ATOMIC);
+	priv_dst->last = kzalloc(sizeof(*priv_dst->last), gfp);
 	if (!priv_dst->last)
 		return -ENOMEM;
 
diff --git a/net/netfilter/nft_limit.c b/net/netfilter/nft_limit.c
index cefa25e0dbb0..21d26b79b460 100644
--- a/net/netfilter/nft_limit.c
+++ b/net/netfilter/nft_limit.c
@@ -150,7 +150,7 @@ static void nft_limit_destroy(const struct nft_ctx *ctx,
 }
 
 static int nft_limit_clone(struct nft_limit_priv *priv_dst,
-			   const struct nft_limit_priv *priv_src)
+			   const struct nft_limit_priv *priv_src, gfp_t gfp)
 {
 	priv_dst->tokens_max = priv_src->tokens_max;
 	priv_dst->rate = priv_src->rate;
@@ -158,7 +158,7 @@ static int nft_limit_clone(struct nft_limit_priv *priv_dst,
 	priv_dst->burst = priv_src->burst;
 	priv_dst->invert = priv_src->invert;
 
-	priv_dst->limit = kmalloc(sizeof(*priv_dst->limit), GFP_ATOMIC);
+	priv_dst->limit = kmalloc(sizeof(*priv_dst->limit), gfp);
 	if (!priv_dst->limit)
 		return -ENOMEM;
 
@@ -223,14 +223,15 @@ static void nft_limit_pkts_destroy(const struct nft_ctx *ctx,
 	nft_limit_destroy(ctx, &priv->limit);
 }
 
-static int nft_limit_pkts_clone(struct nft_expr *dst, const struct nft_expr *src)
+static int nft_limit_pkts_clone(struct nft_expr *dst, const struct nft_expr *src,
+				gfp_t gfp)
 {
 	struct nft_limit_priv_pkts *priv_dst = nft_expr_priv(dst);
 	struct nft_limit_priv_pkts *priv_src = nft_expr_priv(src);
 
 	priv_dst->cost = priv_src->cost;
 
-	return nft_limit_clone(&priv_dst->limit, &priv_src->limit);
+	return nft_limit_clone(&priv_dst->limit, &priv_src->limit, gfp);
 }
 
 static struct nft_expr_type nft_limit_type;
@@ -281,12 +282,13 @@ static void nft_limit_bytes_destroy(const struct nft_ctx *ctx,
 	nft_limit_destroy(ctx, priv);
 }
 
-static int nft_limit_bytes_clone(struct nft_expr *dst, const struct nft_expr *src)
+static int nft_limit_bytes_clone(struct nft_expr *dst, const struct nft_expr *src,
+				 gfp_t gfp)
 {
 	struct nft_limit_priv *priv_dst = nft_expr_priv(dst);
 	struct nft_limit_priv *priv_src = nft_expr_priv(src);
 
-	return nft_limit_clone(priv_dst, priv_src);
+	return nft_limit_clone(priv_dst, priv_src, gfp);
 }
 
 static const struct nft_expr_ops nft_limit_bytes_ops = {
diff --git a/net/netfilter/nft_quota.c b/net/netfilter/nft_quota.c
index 3ba12a7471b0..9b2d7463d3d3 100644
--- a/net/netfilter/nft_quota.c
+++ b/net/netfilter/nft_quota.c
@@ -233,7 +233,7 @@ static void nft_quota_destroy(const struct nft_ctx *ctx,
 	return nft_quota_do_destroy(ctx, priv);
 }
 
-static int nft_quota_clone(struct nft_expr *dst, const struct nft_expr *src)
+static int nft_quota_clone(struct nft_expr *dst, const struct nft_expr *src, gfp_t gfp)
 {
 	struct nft_quota *priv_dst = nft_expr_priv(dst);
 	struct nft_quota *priv_src = nft_expr_priv(src);
@@ -241,7 +241,7 @@ static int nft_quota_clone(struct nft_expr *dst, const struct nft_expr *src)
 	priv_dst->quota = priv_src->quota;
 	priv_dst->flags = priv_src->flags;
 
-	priv_dst->consumed = kmalloc(sizeof(*priv_dst->consumed), GFP_ATOMIC);
+	priv_dst->consumed = kmalloc(sizeof(*priv_dst->consumed), gfp);
 	if (!priv_dst->consumed)
 		return -ENOMEM;
 
-- 
2.43.2

Re: Could not process rule: Cannot allocate memory

[Sven Auhagen]

On Wed, May 08, 2024 at 02:15:26PM +0200, Florian Westphal wrote:
> Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> > I am using nftables with geoip sets.
> > When I have larger sets in my ruleset and I want to atomically update the entire ruleset, I start with
> > destroy table inet filter and then continue with my new ruleset.
> > 
> > When the sets are larger I now always get an error:
> > ./main.nft:13:1-26: Error: Could not process rule: Cannot allocate memory
> > destroy table inet filter
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> > along with the kernel message
> > percpu: allocation failed, size=16 align=8 atomic=1, atomic alloc failed, no space left
> 
> Are you using 'counter' extension on the set definition?

Yes I do and I just tested it, when I remove the counter it works without issues.

> 
> Could yo usahre a minimal reproducer? You can omit the actual
> elements, its easy to autogen that.

I just saw your patch, do you still want me to send a reproducer?

> 

Re: Could not process rule: Cannot allocate memory

[Florian Westphal]

Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> When the sets are larger I now always get an error:
> ./main.nft:13:1-26: Error: Could not process rule: Cannot allocate memory
> destroy table inet filter
> ^^^^^^^^^^^^^^^^^^^^^^^^^^
> along with the kernel message
> percpu: allocation failed, size=16 align=8 atomic=1, atomic alloc failed, no space left

This specific pcpu allocation failure aside, I think we need to reduce
memory waste with flush op.

Flushing a set with 1m elements will need >100Mbyte worth of memory for
the delsetelem transactional log.

The ratio of preamble to set_elem isn't great, we need 88 bytes for the
nft_trans struct and 24 bytes to store one set elem, i.e. 112 bytes per
to-be-deleted element.

I'd say we should look into adding a del_setelem_many struct that stores
e.g. up to 20 elem_priv pointers.  With such a ratio we could probably
get memory waste down to ~20 Mbytes for 1m element sets.

Re: Could not process rule: Cannot allocate memory

[Florian Westphal]

Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> On Wed, May 08, 2024 at 02:15:26PM +0200, Florian Westphal wrote:
> > Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> > > I am using nftables with geoip sets.
> > > When I have larger sets in my ruleset and I want to atomically update the entire ruleset, I start with
> > > destroy table inet filter and then continue with my new ruleset.
> > > 
> > > When the sets are larger I now always get an error:
> > > ./main.nft:13:1-26: Error: Could not process rule: Cannot allocate memory
> > > destroy table inet filter
> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > 
> > > along with the kernel message
> > > percpu: allocation failed, size=16 align=8 atomic=1, atomic alloc failed, no space left
> > 
> > Are you using 'counter' extension on the set definition?
> 
> Yes I do and I just tested it, when I remove the counter it works without issues.
> 
> > 
> > Could yo usahre a minimal reproducer? You can omit the actual
> > elements, its easy to autogen that.
> 
> I just saw your patch, do you still want me to send a reproducer?

In that case I guess the patch will help as the pcpu area
should grow.

But I think it might still make sense, could probably extend on of
the test cases we have with a huge-set+counter+flush op.

Re: Could not process rule: Cannot allocate memory

[Jan Engelhardt]

On Wednesday 2024-05-08 16:08, Florian Westphal wrote:
>Sven Auhagen <sven.auhagen@voleatech.de> wrote:
>> When the sets are larger I now always get an error:
>> ./main.nft:13:1-26: Error: Could not process rule: Cannot allocate memory
>> destroy table inet filter
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^
>> along with the kernel message
>> percpu: allocation failed, size=16 align=8 atomic=1, atomic alloc failed, no space left
>
>This specific pcpu allocation failure aside, I think we need to reduce
>memory waste with flush op.
>
>Flushing a set with 1m elements will need >100Mbyte worth of memory for
>the delsetelem transactional log.

Whoa. Isn't there a way to just switch out the set/ruleset
and then forget the old set as a whole?

(I'm thinking of something in the sense of `btrfs sub del /subvol` vs.
the-slow-way `rm -Rf /subvol`)

Re: Could not process rule: Cannot allocate memory

[Sven Auhagen]

On Wed, May 08, 2024 at 04:08:20PM +0200, Florian Westphal wrote:
> Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> > When the sets are larger I now always get an error:
> > ./main.nft:13:1-26: Error: Could not process rule: Cannot allocate memory
> > destroy table inet filter
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > along with the kernel message
> > percpu: allocation failed, size=16 align=8 atomic=1, atomic alloc failed, no space left
> 
> This specific pcpu allocation failure aside, I think we need to reduce
> memory waste with flush op.
> 
> Flushing a set with 1m elements will need >100Mbyte worth of memory for
> the delsetelem transactional log.
> 
> The ratio of preamble to set_elem isn't great, we need 88 bytes for the
> nft_trans struct and 24 bytes to store one set elem, i.e. 112 bytes per
> to-be-deleted element.
> 
> I'd say we should look into adding a del_setelem_many struct that stores
> e.g. up to 20 elem_priv pointers.  With such a ratio we could probably
> get memory waste down to ~20 Mbytes for 1m element sets.

You are right and also loading a large set like this takes a lot of
time as well besides the memory.

Re: Could not process rule: Cannot allocate memory

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> > When the sets are larger I now always get an error:
> > ./main.nft:13:1-26: Error: Could not process rule: Cannot allocate memory
> > destroy table inet filter
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > along with the kernel message
> > percpu: allocation failed, size=16 align=8 atomic=1, atomic alloc failed, no space left
> 
> This specific pcpu allocation failure aside, I think we need to reduce
> memory waste with flush op.

Plan is:

1. Get rid of ->data[] in struct nft_trans.
   All nft_trans_xxx will add struct nft_trans as first
   member instead.

2. Add nft_trans_binding.  Move binding_list head from
   nft_trans to nft_trans_binding.
   nft_trans_set and nft_trans_chain use nft_trans_binding
   as first member.
   This gets rid of struct list_head for all other types.

3. Get rid of struct nft_ctx from nft_trans.
   As far as I can see a lot of data here is redundant,
   We can likely stash only struct net, u16 flags,
   bool report.
   nft_chain can be moved to the appropriate sub-trans type
   struct.

Re: Could not process rule: Cannot allocate memory

[Sven Auhagen]

On Fri, May 10, 2024 at 11:06:29AM +0200, Florian Westphal wrote:
> Florian Westphal <fw@strlen.de> wrote:
> > Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> > > When the sets are larger I now always get an error:
> > > ./main.nft:13:1-26: Error: Could not process rule: Cannot allocate memory
> > > destroy table inet filter
> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > along with the kernel message
> > > percpu: allocation failed, size=16 align=8 atomic=1, atomic alloc failed, no space left
> > 
> > This specific pcpu allocation failure aside, I think we need to reduce
> > memory waste with flush op.
> 
> Plan is:
> 
> 1. Get rid of ->data[] in struct nft_trans.
>    All nft_trans_xxx will add struct nft_trans as first
>    member instead.
> 
> 2. Add nft_trans_binding.  Move binding_list head from
>    nft_trans to nft_trans_binding.
>    nft_trans_set and nft_trans_chain use nft_trans_binding
>    as first member.
>    This gets rid of struct list_head for all other types.
> 
> 3. Get rid of struct nft_ctx from nft_trans.
>    As far as I can see a lot of data here is redundant,
>    We can likely stash only struct net, u16 flags,
>    bool report.
>    nft_chain can be moved to the appropriate sub-trans type
>    struct.

Here is also a minimal example to trigger the problem.
I left out the ip addresses:

destroy table inet filter

table inet filter {

    set SET1_FW_V4 {
        type ipv4_addr;
        flags interval;
        counter;
        elements = { }
    }

    set SET2_FW_V4 {
        type ipv4_addr;
        flags interval;
        counter;
        elements = { }
    }

    set SET3_FW_V4 {
        type ipv4_addr;
        flags interval;
        counter;
        elements = { }
    }

    set SET4_FW_V4 {
        type ipv4_addr;
        flags interval;
        counter;
        elements = { }
    }

    chain input {
        type filter hook input priority 0;
        policy accept;

        ip saddr @SET1_FW_V4 drop
        ip saddr @SET2_FW_V4 drop
        ip saddr @SET3_FW_V4 drop
        ip saddr @SET4_FW_V4 drop
    }
}

Re: Could not process rule: Cannot allocate memory

[Pablo Neira Ayuso]

On Fri, May 10, 2024 at 12:45:15PM +0200, Sven Auhagen wrote:
> On Fri, May 10, 2024 at 11:06:29AM +0200, Florian Westphal wrote:
> > Florian Westphal <fw@strlen.de> wrote:
> > > Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> > > > When the sets are larger I now always get an error:
> > > > ./main.nft:13:1-26: Error: Could not process rule: Cannot allocate memory
> > > > destroy table inet filter
> > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > > along with the kernel message
> > > > percpu: allocation failed, size=16 align=8 atomic=1, atomic alloc failed, no space left
> > > 
> > > This specific pcpu allocation failure aside, I think we need to reduce
> > > memory waste with flush op.

Agreed.

One more question below.

> > Plan is:
> > 
> > 1. Get rid of ->data[] in struct nft_trans.
> >    All nft_trans_xxx will add struct nft_trans as first
> >    member instead.
> > 
> > 2. Add nft_trans_binding.  Move binding_list head from
> >    nft_trans to nft_trans_binding.
> >    nft_trans_set and nft_trans_chain use nft_trans_binding
> >    as first member.
> >    This gets rid of struct list_head for all other types.
> > 
> > 3. Get rid of struct nft_ctx from nft_trans.
> >    As far as I can see a lot of data here is redundant,
> >    We can likely stash only struct net, u16 flags,
> >    bool report.
> >    nft_chain can be moved to the appropriate sub-trans type
> >    struct.
> 
> Here is also a minimal example to trigger the problem.

Can you still see this after Florian's patch?

> I left out the ip addresses:
> 
> destroy table inet filter
> 
> table inet filter {
> 
>     set SET1_FW_V4 {
>         type ipv4_addr;
>         flags interval;
>         counter;
>         elements = { }
>     }
> 
>     set SET2_FW_V4 {
>         type ipv4_addr;
>         flags interval;
>         counter;
>         elements = { }
>     }
> 
>     set SET3_FW_V4 {
>         type ipv4_addr;
>         flags interval;
>         counter;
>         elements = { }
>     }
> 
>     set SET4_FW_V4 {
>         type ipv4_addr;
>         flags interval;
>         counter;
>         elements = { }
>     }
> 
>     chain input {
>         type filter hook input priority 0;
>         policy accept;
> 
>         ip saddr @SET1_FW_V4 drop
>         ip saddr @SET2_FW_V4 drop
>         ip saddr @SET3_FW_V4 drop
>         ip saddr @SET4_FW_V4 drop
>     }
> }
> 

Re: Could not process rule: Cannot allocate memory

[Florian Westphal]

Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> destroy table inet filter
> 
> table inet filter {
> 
>     set SET1_FW_V4 {
>         type ipv4_addr;
>         flags interval;
>         counter;
>         elements = { }
>     }

Thanks, so this happens even with simple set like rbtree.

Re: Could not process rule: Cannot allocate memory

[Sven Auhagen]

On Fri, May 10, 2024 at 12:51:53PM +0200, Pablo Neira Ayuso wrote:
> On Fri, May 10, 2024 at 12:45:15PM +0200, Sven Auhagen wrote:
> > On Fri, May 10, 2024 at 11:06:29AM +0200, Florian Westphal wrote:
> > > Florian Westphal <fw@strlen.de> wrote:
> > > > Sven Auhagen <sven.auhagen@voleatech.de> wrote:
> > > > > When the sets are larger I now always get an error:
> > > > > ./main.nft:13:1-26: Error: Could not process rule: Cannot allocate memory
> > > > > destroy table inet filter
> > > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > > > along with the kernel message
> > > > > percpu: allocation failed, size=16 align=8 atomic=1, atomic alloc failed, no space left
> > > > 
> > > > This specific pcpu allocation failure aside, I think we need to reduce
> > > > memory waste with flush op.
> 
> Agreed.
> 
> One more question below.
> 
> > > Plan is:
> > > 
> > > 1. Get rid of ->data[] in struct nft_trans.
> > >    All nft_trans_xxx will add struct nft_trans as first
> > >    member instead.
> > > 
> > > 2. Add nft_trans_binding.  Move binding_list head from
> > >    nft_trans to nft_trans_binding.
> > >    nft_trans_set and nft_trans_chain use nft_trans_binding
> > >    as first member.
> > >    This gets rid of struct list_head for all other types.
> > > 
> > > 3. Get rid of struct nft_ctx from nft_trans.
> > >    As far as I can see a lot of data here is redundant,
> > >    We can likely stash only struct net, u16 flags,
> > >    bool report.
> > >    nft_chain can be moved to the appropriate sub-trans type
> > >    struct.
> > 
> > Here is also a minimal example to trigger the problem.
> 
> Can you still see this after Florian's patch?

I double checked and it works fine now with Florian's Patch.
Also removing the counter is mitigating the issue as well.

> 
> > I left out the ip addresses:
> > 
> > destroy table inet filter
> > 
> > table inet filter {
> > 
> >     set SET1_FW_V4 {
> >         type ipv4_addr;
> >         flags interval;
> >         counter;
> >         elements = { }
> >     }
> > 
> >     set SET2_FW_V4 {
> >         type ipv4_addr;
> >         flags interval;
> >         counter;
> >         elements = { }
> >     }
> > 
> >     set SET3_FW_V4 {
> >         type ipv4_addr;
> >         flags interval;
> >         counter;
> >         elements = { }
> >     }
> > 
> >     set SET4_FW_V4 {
> >         type ipv4_addr;
> >         flags interval;
> >         counter;
> >         elements = { }
> >     }
> > 
> >     chain input {
> >         type filter hook input priority 0;
> >         policy accept;
> > 
> >         ip saddr @SET1_FW_V4 drop
> >         ip saddr @SET2_FW_V4 drop
> >         ip saddr @SET3_FW_V4 drop
> >         ip saddr @SET4_FW_V4 drop
> >     }
> > }
> >