Hi again,
On Fri, Feb 15, 2019 at 10:39:38AM +0100, Pablo Neira Ayuso wrote:
> Hi Phil,
>
> On Thu, Feb 14, 2019 at 06:21:04PM +0100, Phil Sutter wrote:
> > Hi Pablo,
> >
> > Originating from a problem with ebtables-nft user-defined chain
> > policies, I made up the following use-case:
> >
> > | # iptables-nft -A FORWARD -j ACCEPT
> > | # iptables-nft-restore --noflush <<EOF
> > | *filter
> > | -D FORWARD -j ACCEPT
> > | -F
> > | COMMIT
> > | EOF
> > | iptables-restore v1.8.2 (nf_tables):
> > | line 3: RULE_FLUSH failed (No such file or directory): rule in chain FORWARD
> >
> > In case anyone reading this is not aware of it: In nftables, flushing a
> > chain works by sending NFT_MSG_DELRULE message with just table and chain
> > defined, no rule handle or position.
> >
> > The problem is that delete command in batch removes the rule, flush
> > command then tries to delete the same rule again. Kernel returns -ENOENT
> > in nf_tables_delrule_deactivate().
> >
> > The above use-case works with legacy iptables.
> >
> > Question is if I have to work around this in userspace or if we should
> > make nf_tables_delrule_deactivate() return 0 even if given rule is not
> > active? Downside is that second option would cause double deletion of
> > same rule within a single batch to succeed.
>
> From the flush path, we can skip requests for deletion of rules that
> have been already deleted, ie. we add no transaction since there is
> already one in place.
nft_delrule_by_chain() is always called from flush path. We can apply
this smaller fix I think.
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 5a92f23f179f..4893f248dfdc 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -313,6 +313,9 @@ static int nft_delrule_by_chain(struct nft_ctx *ctx)
int err;
list_for_each_entry(rule, &ctx->chain->rules, list) {
+ if (!nft_is_active_next(ctx->net, rule))
+ continue;
+
err = nft_delrule(ctx, rule);
if (err < 0)
return err;
