Skip to content

Commit

Permalink
netfilter: nf_tables: allow to delete several objects from a batch
Browse files Browse the repository at this point in the history 
Three changes to allow the deletion of several objects with dependencies
in one transaction, they are:

1) Introduce speculative counter increment/decrement that is undone in
   the abort path if required, thus we avoid hitting -EBUSY when deleting
   the chain. The counter updates are reverted in the abort path.

2) Increment/decrement table/chain use counter for each set/rule. We need
   this to fully rely on the use counters instead of the list content,
   eg. !list_empty(&chain->rules) which evaluate true in the middle of the
   transaction.

3) Decrement table use counter when an anonymous set is bound to the
   rule in the commit path. This avoids hitting -EBUSY when deleting
   the table that contains anonymous sets. The anonymous sets are released
   in the nf_tables_rule_destroy path. This should not be a problem since
   the rule already bumped the use counter of the chain, so the bound
   anonymous set reflects dependencies through the rule object, which
   already increases the chain use counter.

So the general assumption after this patch is that the use counters are
bumped by direct object dependencies.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
  • Loading branch information
Pablo Neira Ayuso committed Jun 2, 2014
1 parent 7632667 commit 4fefee5
Showing 1 changed file with 31 additions and 9 deletions.
40 changes: 31 additions & 9 deletions net/netfilter/nf_tables_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,7 @@ static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
return PTR_ERR(table);
if (table->flags & NFT_TABLE_INACTIVE)
return -ENOENT;

if (!list_empty(&table->chains) || !list_empty(&table->sets))
if (table->use > 0)
return -EBUSY;

nft_ctx_init(&ctx, skb, nlh, afi, table, NULL, nla);
Expand All @@ -553,6 +552,8 @@ static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,

static void nf_tables_table_destroy(struct nft_ctx *ctx)
{
BUG_ON(ctx->table->use > 0);

kfree(ctx->table);
module_put(ctx->afi->owner);
}
Expand Down Expand Up @@ -1128,6 +1129,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
if (err < 0)
goto err2;

table->use++;
list_add_tail(&chain->list, &table->chains);
return 0;
err2:
Expand Down Expand Up @@ -1169,14 +1171,15 @@ static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
return PTR_ERR(chain);
if (chain->flags & NFT_CHAIN_INACTIVE)
return -ENOENT;
if (!list_empty(&chain->rules) || chain->use > 0)
if (chain->use > 0)
return -EBUSY;

nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
err = nft_trans_chain_add(&ctx, NFT_MSG_DELCHAIN);
if (err < 0)
return err;

table->use--;
list_del(&chain->list);
return 0;
}
Expand Down Expand Up @@ -1814,6 +1817,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
err = -ENOMEM;
goto err3;
}
chain->use++;
return 0;

err3:
Expand Down Expand Up @@ -1841,6 +1845,7 @@ nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule)
if (nft_trans_rule_add(ctx, NFT_MSG_DELRULE, rule) == NULL)
return -ENOMEM;
nft_rule_disactivate_next(ctx->net, rule);
ctx->chain->use--;
return 0;
}
return -ENOENT;
Expand Down Expand Up @@ -2588,6 +2593,7 @@ static int nf_tables_newset(struct sock *nlsk, struct sk_buff *skb,
goto err2;

list_add_tail(&set->list, &table->sets);
table->use++;
return 0;

err2:
Expand Down Expand Up @@ -2642,6 +2648,7 @@ static int nf_tables_delset(struct sock *nlsk, struct sk_buff *skb,
return err;

list_del(&set->list);
ctx.table->use--;
return 0;
}

Expand Down Expand Up @@ -3122,6 +3129,8 @@ static int nf_tables_newsetelem(struct sock *nlsk, struct sk_buff *skb,
err = nft_add_set_elem(&ctx, set, attr);
if (err < 0)
break;

set->nelems++;
}
return err;
}
Expand Down Expand Up @@ -3196,6 +3205,8 @@ static int nf_tables_delsetelem(struct sock *nlsk, struct sk_buff *skb,
err = nft_del_setelem(&ctx, set, attr);
if (err < 0)
break;

set->nelems--;
}
return err;
}
Expand Down Expand Up @@ -3361,15 +3372,13 @@ static int nf_tables_commit(struct sk_buff *skb)
case NFT_MSG_NEWCHAIN:
if (nft_trans_chain_update(trans))
nft_chain_commit_update(trans);
else {
else
trans->ctx.chain->flags &= ~NFT_CHAIN_INACTIVE;
trans->ctx.table->use++;
}

nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN);
nft_trans_destroy(trans);
break;
case NFT_MSG_DELCHAIN:
trans->ctx.table->use--;
nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN);
if (!(trans->ctx.table->flags & NFT_TABLE_F_DORMANT) &&
trans->ctx.chain->flags & NFT_BASE_CHAIN) {
Expand All @@ -3392,6 +3401,13 @@ static int nf_tables_commit(struct sk_buff *skb)
break;
case NFT_MSG_NEWSET:
nft_trans_set(trans)->flags &= ~NFT_SET_INACTIVE;
/* This avoids hitting -EBUSY when deleting the table
* from the transaction.
*/
if (nft_trans_set(trans)->flags & NFT_SET_ANONYMOUS &&
!list_empty(&nft_trans_set(trans)->bindings))
trans->ctx.table->use--;

nf_tables_set_notify(&trans->ctx, nft_trans_set(trans),
NFT_MSG_NEWSET);
nft_trans_destroy(trans);
Expand All @@ -3401,15 +3417,13 @@ static int nf_tables_commit(struct sk_buff *skb)
NFT_MSG_DELSET);
break;
case NFT_MSG_NEWSETELEM:
nft_trans_elem_set(trans)->nelems++;
nf_tables_setelem_notify(&trans->ctx,
nft_trans_elem_set(trans),
&nft_trans_elem(trans),
NFT_MSG_NEWSETELEM, 0);
nft_trans_destroy(trans);
break;
case NFT_MSG_DELSETELEM:
nft_trans_elem_set(trans)->nelems--;
nf_tables_setelem_notify(&trans->ctx,
nft_trans_elem_set(trans),
&nft_trans_elem(trans),
Expand Down Expand Up @@ -3487,6 +3501,7 @@ static int nf_tables_abort(struct sk_buff *skb)

nft_trans_destroy(trans);
} else {
trans->ctx.table->use--;
list_del(&trans->ctx.chain->list);
if (!(trans->ctx.table->flags & NFT_TABLE_F_DORMANT) &&
trans->ctx.chain->flags & NFT_BASE_CHAIN) {
Expand All @@ -3496,32 +3511,39 @@ static int nf_tables_abort(struct sk_buff *skb)
}
break;
case NFT_MSG_DELCHAIN:
trans->ctx.table->use++;
list_add_tail(&trans->ctx.chain->list,
&trans->ctx.table->chains);
nft_trans_destroy(trans);
break;
case NFT_MSG_NEWRULE:
trans->ctx.chain->use--;
list_del_rcu(&nft_trans_rule(trans)->list);
break;
case NFT_MSG_DELRULE:
trans->ctx.chain->use++;
nft_rule_clear(trans->ctx.net, nft_trans_rule(trans));
nft_trans_destroy(trans);
break;
case NFT_MSG_NEWSET:
trans->ctx.table->use--;
list_del(&nft_trans_set(trans)->list);
break;
case NFT_MSG_DELSET:
trans->ctx.table->use++;
list_add_tail(&nft_trans_set(trans)->list,
&trans->ctx.table->sets);
nft_trans_destroy(trans);
break;
case NFT_MSG_NEWSETELEM:
nft_trans_elem_set(trans)->nelems--;
set = nft_trans_elem_set(trans);
set->ops->get(set, &nft_trans_elem(trans));
set->ops->remove(set, &nft_trans_elem(trans));
nft_trans_destroy(trans);
break;
case NFT_MSG_DELSETELEM:
nft_trans_elem_set(trans)->nelems++;
nft_trans_destroy(trans);
break;
}
Expand Down

0 comments on commit 4fefee5

Please sign in to comment.