* [PATCH nf] netfilter: nft_flow_offload: fix unnecessary use counter decrease in destory
@ 2019-12-20 9:08 wenxu
2019-12-30 13:25 ` wenxu
0 siblings, 1 reply; 7+ messages in thread
From: wenxu @ 2019-12-20 9:08 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
From: wenxu <wenxu@ucloud.cn>
create a flowtable:
nft add table firewall
nft add flowtable firewall fb1 { hook ingress priority 2 \; devices = { tun1,
mlx_pf0vf0 } \; }
nft add chain firewall ftb-all {type filter hook forward priority 0 \; policy
accept \; }
nft add rule firewall ftb-all ct zone 1 ip protocol tcp flow offload @fb1
nft add rule firewall ftb-all ct zone 1 ip protocol udp flow offload @fb1
delete the related rule:
nft delete chain firewall ftb-all
The flowtable can be deleted
nft delete flowtable firewall fb1
But failed with: Device is busy
The nf_flowtable->use is not zero and overflow for unnecessary use counter
decrease..
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
net/netfilter/nft_flow_offload.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index dd82ff2..b70b489 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -200,9 +200,6 @@ static void nft_flow_offload_activate(const struct nft_ctx *ctx,
static void nft_flow_offload_destroy(const struct nft_ctx *ctx,
const struct nft_expr *expr)
{
- struct nft_flow_offload *priv = nft_expr_priv(expr);
-
- priv->flowtable->use--;
nf_ct_netns_put(ctx->net, ctx->family);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH nf] netfilter: nft_flow_offload: fix unnecessary use counter decrease in destory
2019-12-20 9:08 [PATCH nf] netfilter: nft_flow_offload: fix unnecessary use counter decrease in destory wenxu
@ 2019-12-30 13:25 ` wenxu
2019-12-30 20:02 ` Pablo Neira Ayuso
0 siblings, 1 reply; 7+ messages in thread
From: wenxu @ 2019-12-30 13:25 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
Hi pablo,
How about this patch?
BR
wenxu
在 2019/12/20 17:08, wenxu@ucloud.cn 写道:
> From: wenxu <wenxu@ucloud.cn>
>
> create a flowtable:
> nft add table firewall
> nft add flowtable firewall fb1 { hook ingress priority 2 \; devices = { tun1,
> mlx_pf0vf0 } \; }
> nft add chain firewall ftb-all {type filter hook forward priority 0 \; policy
> accept \; }
> nft add rule firewall ftb-all ct zone 1 ip protocol tcp flow offload @fb1
> nft add rule firewall ftb-all ct zone 1 ip protocol udp flow offload @fb1
>
> delete the related rule:
> nft delete chain firewall ftb-all
>
> The flowtable can be deleted
> nft delete flowtable firewall fb1
>
> But failed with: Device is busy
>
> The nf_flowtable->use is not zero and overflow for unnecessary use counter
> decrease..
>
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
> net/netfilter/nft_flow_offload.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
> index dd82ff2..b70b489 100644
> --- a/net/netfilter/nft_flow_offload.c
> +++ b/net/netfilter/nft_flow_offload.c
> @@ -200,9 +200,6 @@ static void nft_flow_offload_activate(const struct nft_ctx *ctx,
> static void nft_flow_offload_destroy(const struct nft_ctx *ctx,
> const struct nft_expr *expr)
> {
> - struct nft_flow_offload *priv = nft_expr_priv(expr);
> -
> - priv->flowtable->use--;
> nf_ct_netns_put(ctx->net, ctx->family);
> }
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nf] netfilter: nft_flow_offload: fix unnecessary use counter decrease in destory
2019-12-30 13:25 ` wenxu
@ 2019-12-30 20:02 ` Pablo Neira Ayuso
2019-12-31 0:45 ` wenxu
0 siblings, 1 reply; 7+ messages in thread
From: Pablo Neira Ayuso @ 2019-12-30 20:02 UTC (permalink / raw)
To: wenxu; +Cc: netfilter-devel
On Mon, Dec 30, 2019 at 09:25:36PM +0800, wenxu wrote:
> Hi pablo,
>
> How about this patch?
This test still fails after a second run with this patch:
./run-tests.sh testcases/flowtable/0009deleteafterflush_0
I: using nft binary ./../../src/nft
W: [FAILED] testcases/flowtable/0009deleteafterflush_0: got 1
Error: Could not process rule: Device or resource busy
delete flowtable x f
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nf] netfilter: nft_flow_offload: fix unnecessary use counter decrease in destory
2019-12-30 20:02 ` Pablo Neira Ayuso
@ 2019-12-31 0:45 ` wenxu
2019-12-31 5:12 ` wenxu
2020-01-03 17:12 ` Pablo Neira Ayuso
0 siblings, 2 replies; 7+ messages in thread
From: wenxu @ 2019-12-31 0:45 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
在 2019/12/31 4:02, Pablo Neira Ayuso 写道:
> On Mon, Dec 30, 2019 at 09:25:36PM +0800, wenxu wrote:
>> Hi pablo,
>>
>> How about this patch?
> This test still fails after a second run with this patch:
>
> ./run-tests.sh testcases/flowtable/0009deleteafterflush_0
> I: using nft binary ./../../src/nft
>
> W: [FAILED] testcases/flowtable/0009deleteafterflush_0: got 1
> Error: Could not process rule: Device or resource busy
> delete flowtable x f
Hi pablo,
I did the same test for testcase 0009deleteafterflush_0, It is okay even
there is no this patch in my tree.
++ which nft
+ NFT=/usr/sbin/nft
+ /usr/sbin/nft add table x
+ /usr/sbin/nft add chain x y
+ /usr/sbin/nft add flowtable x f '{' hook ingress priority '0;' devices
= '{' lo '};}'
+ /usr/sbin/nft add rule x y flow add @f
+ /usr/sbin/nft flush chain x y
+ /usr/sbin/nft delete flowtable x f
This patch fix the problem that there are nft_flow_offload rules, when
flush the rules or chain which will lead the use counter double decrease
and overflow.
nft add rule firewall ftb-all ct zone 1 ip protocol tcp flow offload @f
nft add rule firewall ftb-all ct zone 1 ip protocol udp flow offload @f
This testcase does not have any nft_flow_offload rules. So this testcase
don't cover the problem I want to fixes.
Ps:
I test the nf-next tree, this testcase have the problem, I think it
should be another new problem. I will check it.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nf] netfilter: nft_flow_offload: fix unnecessary use counter decrease in destory
2019-12-31 0:45 ` wenxu
@ 2019-12-31 5:12 ` wenxu
2020-01-03 17:12 ` Pablo Neira Ayuso
1 sibling, 0 replies; 7+ messages in thread
From: wenxu @ 2019-12-31 5:12 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Hi Pablo,
I found this testcase already contain the rules.
I can reproduce it with your nf branch.
# uname -r
5.5.0-rc2+
# cat testcases/flowtable/0009deleteafterflush_0:
#!/bin/bash
set -e
$NFT add table x
$NFT add chain x y
$NFT add flowtable x f { hook ingress priority 0\; devices = { lo }\;}
$NFT add rule x y flow add @f
$NFT flush chain x y
sleep 1
$NFT delete flowtable x f
It is not easy to reproduce it without the sleep 1 between flush chain and delete the flowtable.
The flowtable delete maybe early than rule destroy because the operation is the flush chain
but not delete rule directly .
Without my patch:
# ./run-tests.sh testcases/flowtable/0009deleteafterflush_0
I: using nft binary ./../../src/nft
W: [FAILED] testcases/flowtable/0009deleteafterflush_0: got 1
Error: Could not process rule: Device or resource busy
delete flowtable x f
^^^^^^^^^^^^^^^^^^^^^
I: results: [OK] 0 [FAILED] 1 [TOTAL] 1
Add with my fixes patch: I run this 50 times all result is OK
# ./run-tests.sh testcases/flowtable/0009deleteafterflush_0
I: using nft binary ./../../src/nft
I: [OK] testcases/flowtable/0009deleteafterflush_0
I: results: [OK] 1 [FAILED] 0 [TOTAL] 1
So which branch did you test with this patch?
BR
wenxu
On 12/31/2019 8:45 AM, wenxu wrote:
>
> 在 2019/12/31 4:02, Pablo Neira Ayuso 写道:
>> On Mon, Dec 30, 2019 at 09:25:36PM +0800, wenxu wrote:
>>> Hi pablo,
>>>
>>> How about this patch?
>> This test still fails after a second run with this patch:
>>
>> ./run-tests.sh testcases/flowtable/0009deleteafterflush_0
>> I: using nft binary ./../../src/nft
>>
>> W: [FAILED] testcases/flowtable/0009deleteafterflush_0: got 1
>> Error: Could not process rule: Device or resource busy
>> delete flowtable x f
>
> Hi pablo,
>
>
> I did the same test for testcase 0009deleteafterflush_0, It is okay even there is no this patch in my tree.
>
> ++ which nft
> + NFT=/usr/sbin/nft
> + /usr/sbin/nft add table x
> + /usr/sbin/nft add chain x y
> + /usr/sbin/nft add flowtable x f '{' hook ingress priority '0;' devices = '{' lo '};}'
> + /usr/sbin/nft add rule x y flow add @f
> + /usr/sbin/nft flush chain x y
>
> + /usr/sbin/nft delete flowtable x f
>
>
> This patch fix the problem that there are nft_flow_offload rules, when flush the rules or chain which will lead the use counter double decrease and overflow.
>
> nft add rule firewall ftb-all ct zone 1 ip protocol tcp flow offload @f
> nft add rule firewall ftb-all ct zone 1 ip protocol udp flow offload @f
>
> This testcase does not have any nft_flow_offload rules. So this testcase don't cover the problem I want to fixes.
>
>
> Ps:
>
> I test the nf-next tree, this testcase have the problem, I think it should be another new problem. I will check it.
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nf] netfilter: nft_flow_offload: fix unnecessary use counter decrease in destory
2019-12-31 0:45 ` wenxu
2019-12-31 5:12 ` wenxu
@ 2020-01-03 17:12 ` Pablo Neira Ayuso
2020-01-04 8:28 ` wenxu
1 sibling, 1 reply; 7+ messages in thread
From: Pablo Neira Ayuso @ 2020-01-03 17:12 UTC (permalink / raw)
To: wenxu; +Cc: netfilter-devel
[-- Attachment #1: Type: text/plain, Size: 747 bytes --]
On Tue, Dec 31, 2019 at 08:45:27AM +0800, wenxu wrote:
>
> 在 2019/12/31 4:02, Pablo Neira Ayuso 写道:
> > On Mon, Dec 30, 2019 at 09:25:36PM +0800, wenxu wrote:
> > > Hi pablo,
> > >
> > > How about this patch?
> > This test still fails after a second run with this patch:
> >
> > ./run-tests.sh testcases/flowtable/0009deleteafterflush_0
> > I: using nft binary ./../../src/nft
> >
> > W: [FAILED] testcases/flowtable/0009deleteafterflush_0: got 1
> > Error: Could not process rule: Device or resource busy
> > delete flowtable x f
>
> Hi pablo,
>
> I did the same test for testcase 0009deleteafterflush_0, It is okay even
> there is no this patch in my tree.
Thanks, I'm going to apply the patch that I'm attaching to this email.
[-- Attachment #2: 0001-netfilter-nft_flow_offload-fix-underflow-in-flowtabl.patch --]
[-- Type: text/x-diff, Size: 1152 bytes --]
From 7c79a32304ccd7ee33f672a3b543ef62a664ce5d Mon Sep 17 00:00:00 2001
From: wenxu <wenxu@ucloud.cn>
Date: Fri, 20 Dec 2019 17:08:46 +0800
Subject: [PATCH] netfilter: nft_flow_offload: fix underflow in flowtable
reference counter
The .deactivate and .activate interfaces already deal with the reference
counter. Otherwise, this results in spurious "Device is busy" errors.
Fixes: a3c90f7a2323 ("netfilter: nf_tables: flow offload expression")
Signed-off-by: wenxu <wenxu@ucloud.cn>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nft_flow_offload.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index dd82ff2ee19f..b70b48996801 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -200,9 +200,6 @@ static void nft_flow_offload_activate(const struct nft_ctx *ctx,
static void nft_flow_offload_destroy(const struct nft_ctx *ctx,
const struct nft_expr *expr)
{
- struct nft_flow_offload *priv = nft_expr_priv(expr);
-
- priv->flowtable->use--;
nf_ct_netns_put(ctx->net, ctx->family);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH nf] netfilter: nft_flow_offload: fix unnecessary use counter decrease in destory
2020-01-03 17:12 ` Pablo Neira Ayuso
@ 2020-01-04 8:28 ` wenxu
0 siblings, 0 replies; 7+ messages in thread
From: wenxu @ 2020-01-04 8:28 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
在 2020/1/4 1:12, Pablo Neira Ayuso 写道:
> On Tue, Dec 31, 2019 at 08:45:27AM +0800, wenxu wrote:
>> 在 2019/12/31 4:02, Pablo Neira Ayuso 写道:
>>> On Mon, Dec 30, 2019 at 09:25:36PM +0800, wenxu wrote:
>>>> Hi pablo,
>>>>
>>>> How about this patch?
>>> This test still fails after a second run with this patch:
>>>
>>> ./run-tests.sh testcases/flowtable/0009deleteafterflush_0
>>> I: using nft binary ./../../src/nft
>>>
>>> W: [FAILED] testcases/flowtable/0009deleteafterflush_0: got 1
>>> Error: Could not process rule: Device or resource busy
>>> delete flowtable x f
>> Hi pablo,
>>
>> I did the same test for testcase 0009deleteafterflush_0, It is okay even
>> there is no this patch in my tree.
> Thanks, I'm going to apply the patch that I'm attaching to this email.
Thanks, It's ok for me.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-01-04 8:29 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-20 9:08 [PATCH nf] netfilter: nft_flow_offload: fix unnecessary use counter decrease in destory wenxu
2019-12-30 13:25 ` wenxu
2019-12-30 20:02 ` Pablo Neira Ayuso
2019-12-31 0:45 ` wenxu
2019-12-31 5:12 ` wenxu
2020-01-03 17:12 ` Pablo Neira Ayuso
2020-01-04 8:28 ` wenxu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.