Re: [PATCH nf 2/3] netfilter: nf_tables: fix miss activate operation in the

[wenxu <wenxu@xxxxxxxxx>]

On 12/20/2019 7:55 AM, Pablo Neira Ayuso wrote:
> On Wed, Dec 18, 2019 at 10:59:12PM +0800, wenxu@xxxxxxxxx wrote:
>> From: wenxu <wenxu@xxxxxxxxx>
>>
>> nf_tables_commit for NFT_MSG_NEWRULE
>>
>> The new create rule should be activated in the nf_tables_commit.
>>
>> 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 for no activated operation.
> This is correct.
>
>> Signed-off-by: wenxu <wenxu@xxxxxxxxx>
>> ---
>>  net/netfilter/nf_tables_api.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>> index 27e6a6f..174b362 100644
>> --- a/net/netfilter/nf_tables_api.c
>> +++ b/net/netfilter/nf_tables_api.c
>> @@ -7101,6 +7101,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
>>  			nf_tables_rule_notify(&trans->ctx,
>>  					      nft_trans_rule(trans),
>>  					      NFT_MSG_NEWRULE);
>> +			nft_rule_expr_activate(&trans->ctx, nft_trans_rule(trans));
> I don't think this fix is correct, probably this patch?


Maybe your patch is also not correct.    The  nf_tables_deactivate_flowtable already ignore

the NFT_TRANS_COMMIT.

void nf_tables_deactivate_flowtable(const struct nft_ctx *ctx,
                                    struct nft_flowtable *flowtable,
                                    enum nft_trans_phase phase)
{
        switch (phase) {
        case NFT_TRANS_PREPARE:
        case NFT_TRANS_ABORT:
        case NFT_TRANS_RELEASE:
                flowtable->use--;
                /* fall through */
        default:
                return;
        }   
}


Nft_flow_offload  inc the use counter , when delete the rule and dec it in deactivate with phase NFT_TRANS_PREPARE.

So the nft_flow_offload_destroy should not dec the use?

So the patch should be as following.

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);
}


The rule should be like the following?


Create rule nft_xx_init   inc the use counter,  If the rule create failed just deactivate it

Delete the rule  deactivate dec the use counter, If the rule delete failed just activate it


BR

wenxu