On 04.03, Pablo Neira Ayuso wrote:
> > > I think we can get rid of the nft_rule_clear() call from the error
> > > path of nf_tables_newrule() too.
> >
> > I don't think so, we deactivate the old rule for NLM_F_REPLACE and
> > need to undo that on error.
>
> Right.
>
> > Or are you talking about getting rid of the entire error handling
> > for NLM_F_REPLACE and have it taken care of by the abort() path?
>
> Yes, that error handling I think we can get rid of it. It's actually
> not correct because it's deleting the old rule.
It does? All I can see is reactivating it?
> In general, if a transaction object is added to the list successfully,
> we can rely on the abort path to undo what we've done. This allows us to
> simplify the error handling of the rule replacement path in
> nf_tables_newrule().
>
> This implicitly fixes an unnecessary removal of the old rule removal,
> which needs to be left in place if we fail to replace.
I agree on the simplification, but I don't see any problem with this.
> err3:
> list_del_rcu(&rule->list);
> - if (trans) {
> - list_del_rcu(&nft_trans_rule(trans)->list);
> - nft_rule_clear(net, nft_trans_rule(trans));
> - nft_trans_destroy(trans);
> - chain->use++;
> - }
> err2:
> nf_tables_rule_destroy(&ctx, rule);
> err1:
> --
> 1.7.10.4
>
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
