* [PATCH nf-next 1/3] netfilter: nf_tables_offload: remove reference to flow rule from deletion path
@ 2019-11-14 13:24 Pablo Neira Ayuso
2019-11-14 13:24 ` [PATCH nf-next 2/3] netfilter: nf_tables_offload: release flow_rule on error from commit path Pablo Neira Ayuso
2019-11-14 13:24 ` [PATCH nf-next 3/3] netfilter: nf_tables_offload: undo updates if transaction fails Pablo Neira Ayuso
0 siblings, 2 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2019-11-14 13:24 UTC (permalink / raw)
To: netfilter-devel
The cookie is sufficient to delete the rule from the hardware.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_tables_offload.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 741045eb530e..528886bb3481 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -437,8 +437,7 @@ int nft_flow_rule_offload_commit(struct net *net)
err = nft_flow_offload_rule(trans->ctx.chain,
nft_trans_rule(trans),
- nft_trans_flow_rule(trans),
- FLOW_CLS_DESTROY);
+ NULL, FLOW_CLS_DESTROY);
break;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH nf-next 2/3] netfilter: nf_tables_offload: release flow_rule on error from commit path
2019-11-14 13:24 [PATCH nf-next 1/3] netfilter: nf_tables_offload: remove reference to flow rule from deletion path Pablo Neira Ayuso
@ 2019-11-14 13:24 ` Pablo Neira Ayuso
2019-11-14 13:24 ` [PATCH nf-next 3/3] netfilter: nf_tables_offload: undo updates if transaction fails Pablo Neira Ayuso
1 sibling, 0 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2019-11-14 13:24 UTC (permalink / raw)
To: netfilter-devel
If hardware offload commit path fails, release all flow_rule object.
Fixes: c9626a2cbdb2 ("netfilter: nf_tables: add hardware offload support")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_tables_offload.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 528886bb3481..6d5f3cd7f1b7 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -422,14 +422,14 @@ int nft_flow_rule_offload_commit(struct net *net)
continue;
if (trans->ctx.flags & NLM_F_REPLACE ||
- !(trans->ctx.flags & NLM_F_APPEND))
- return -EOPNOTSUPP;
-
+ !(trans->ctx.flags & NLM_F_APPEND)) {
+ err = -EOPNOTSUPP;
+ break;
+ }
err = nft_flow_offload_rule(trans->ctx.chain,
nft_trans_rule(trans),
nft_trans_flow_rule(trans),
FLOW_CLS_REPLACE);
- nft_flow_rule_destroy(nft_trans_flow_rule(trans));
break;
case NFT_MSG_DELRULE:
if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD))
@@ -442,7 +442,23 @@ int nft_flow_rule_offload_commit(struct net *net)
}
if (err)
- return err;
+ break;
+ }
+
+ list_for_each_entry(trans, &net->nft.commit_list, list) {
+ if (trans->ctx.family != NFPROTO_NETDEV)
+ continue;
+
+ switch (trans->msg_type) {
+ case NFT_MSG_NEWRULE:
+ if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD))
+ continue;
+
+ nft_flow_rule_destroy(nft_trans_flow_rule(trans));
+ break;
+ default:
+ break;
+ }
}
return err;
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH nf-next 3/3] netfilter: nf_tables_offload: undo updates if transaction fails
2019-11-14 13:24 [PATCH nf-next 1/3] netfilter: nf_tables_offload: remove reference to flow rule from deletion path Pablo Neira Ayuso
2019-11-14 13:24 ` [PATCH nf-next 2/3] netfilter: nf_tables_offload: release flow_rule on error from commit path Pablo Neira Ayuso
@ 2019-11-14 13:24 ` Pablo Neira Ayuso
1 sibling, 0 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2019-11-14 13:24 UTC (permalink / raw)
To: netfilter-devel
The nft_flow_rule_offload_commit() might fail after several successful
commands leaving the hardware filtering policy in inconsistent state.
This patch adds nft_flow_rule_offload_abort() which undoes the updates
that have been already processed if one command of this transaction
fails. Hence, the hardware ruleset is left as is it was before the
update.
Thee deletion path needs to create the flow_rule object too, in case
that an existing rule needs to be re-added from the abort path.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_tables_api.c | 11 ++++++++
net/netfilter/nf_tables_offload.c | 54 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 1dcab99cb947..575791a36d59 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -361,6 +361,7 @@ static struct nft_trans *nft_trans_rule_add(struct nft_ctx *ctx, int msg_type,
static int nft_delrule(struct nft_ctx *ctx, struct nft_rule *rule)
{
+ struct nft_flow_rule *flow;
struct nft_trans *trans;
int err;
@@ -368,6 +369,16 @@ static int nft_delrule(struct nft_ctx *ctx, struct nft_rule *rule)
if (trans == NULL)
return -ENOMEM;
+ if (ctx->chain->flags & NFT_CHAIN_HW_OFFLOAD) {
+ flow = nft_flow_rule_create(ctx->net, rule);
+ if (IS_ERR(flow)) {
+ nft_trans_destroy(trans);
+ return PTR_ERR(flow);
+ }
+
+ nft_trans_flow_rule(trans) = flow;
+ }
+
err = nf_tables_delrule_deactivate(ctx, rule);
if (err < 0) {
nft_trans_destroy(trans);
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 6d5f3cd7f1b7..68f17a6921d8 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -389,6 +389,55 @@ static int nft_flow_offload_chain(struct nft_chain *chain, u8 *ppolicy,
return nft_flow_block_chain(basechain, NULL, cmd);
}
+static void nft_flow_rule_offload_abort(struct net *net,
+ struct nft_trans *trans)
+{
+ int err = 0;
+
+ list_for_each_entry_continue_reverse(trans, &net->nft.commit_list, list) {
+ if (trans->ctx.family != NFPROTO_NETDEV)
+ continue;
+
+ switch (trans->msg_type) {
+ case NFT_MSG_NEWCHAIN:
+ if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD) ||
+ nft_trans_chain_update(trans))
+ continue;
+
+ err = nft_flow_offload_chain(trans->ctx.chain, NULL,
+ FLOW_BLOCK_UNBIND);
+ break;
+ case NFT_MSG_DELCHAIN:
+ if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD))
+ continue;
+
+ err = nft_flow_offload_chain(trans->ctx.chain, NULL,
+ FLOW_BLOCK_BIND);
+ break;
+ case NFT_MSG_NEWRULE:
+ if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD))
+ continue;
+
+ err = nft_flow_offload_rule(trans->ctx.chain,
+ nft_trans_rule(trans),
+ NULL, FLOW_CLS_DESTROY);
+ break;
+ case NFT_MSG_DELRULE:
+ if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD))
+ continue;
+
+ err = nft_flow_offload_rule(trans->ctx.chain,
+ nft_trans_rule(trans),
+ nft_trans_flow_rule(trans),
+ FLOW_CLS_REPLACE);
+ break;
+ }
+
+ if (WARN_ON_ONCE(err))
+ break;
+ }
+}
+
int nft_flow_rule_offload_commit(struct net *net)
{
struct nft_trans *trans;
@@ -441,8 +490,10 @@ int nft_flow_rule_offload_commit(struct net *net)
break;
}
- if (err)
+ if (err) {
+ nft_flow_rule_offload_abort(net, trans);
break;
+ }
}
list_for_each_entry(trans, &net->nft.commit_list, list) {
@@ -451,6 +502,7 @@ int nft_flow_rule_offload_commit(struct net *net)
switch (trans->msg_type) {
case NFT_MSG_NEWRULE:
+ case NFT_MSG_DELRULE:
if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD))
continue;
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-11-14 13:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-11-14 13:24 [PATCH nf-next 1/3] netfilter: nf_tables_offload: remove reference to flow rule from deletion path Pablo Neira Ayuso
2019-11-14 13:24 ` [PATCH nf-next 2/3] netfilter: nf_tables_offload: release flow_rule on error from commit path Pablo Neira Ayuso
2019-11-14 13:24 ` [PATCH nf-next 3/3] netfilter: nf_tables_offload: undo updates if transaction fails Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).