On Thu, Dec 27, 2018 at 07:55:40PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Dec 20, 2018 at 04:09:04PM +0100, Phil Sutter wrote:
> > There is no need to "delete" (actually, remove from cache) a chain if
> > noflush wasn't given: While handling the corresponding table line,
> > 'table_flush' callback has already taken care of that.
> >
> > Streamlining the code further, move syntax checks to the top. If these
> > concede, there are three cases to distinguish:
> >
> > A) Given chain name matches a builtin one in current table, so assume it
> > exists already and just set policy and counters.
> >
> > B) Noflush was given and the (custom) chain exists already, flush it.
> >
> > C) Custom chain was either flushed (noflush not given) or didn't exist
> > before, create it.
> >
> > Signed-off-by: Phil Sutter <phil@xxxxxx>
> > ---
> > iptables/nft-shared.h | 2 --
> > iptables/xtables-restore.c | 68 +++++++++++---------------------------
> > 2 files changed, 19 insertions(+), 51 deletions(-)
> >
> > diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
> > index 388abb97303ab..019c1f20e2939 100644
> > --- a/iptables/nft-shared.h
> > +++ b/iptables/nft-shared.h
> > @@ -245,8 +245,6 @@ struct nft_xt_restore_cb {
> > void (*table_new)(struct nft_handle *h, const char *table);
> > struct nftnl_chain_list *(*chain_list)(struct nft_handle *h,
> > const char *table);
> > - void (*chain_del)(struct nftnl_chain_list *clist, const char *curtable,
> > - const char *chain);
>
> I added to this patch description that chain_del is basically dead
> code since d1eb4d587297.
Ah, yes. But still, I had problems getting my automatic rule adding from
cache approach working with it because there was some ordering issue
with implicit base chain creation and this removal from cache thing.
> Thanks for disentangling this part of the code, looks better now.
Yes, apart from it getting into my way whatever simplifies those restore
parsers is a good thing I guess. :)
Thanks, Phil
