On Thu, Jan 10, 2019 at 11:32:27AM +0100, Phil Sutter wrote: > On Thu, Jan 10, 2019 at 01:28:22AM +0100, Pablo Neira Ayuso wrote: > > On Sun, Dec 30, 2018 at 08:06:12PM +0100, Phil Sutter wrote: > > > diff --git a/iptables/xtables.c b/iptables/xtables.c > > > index da11e8cc159a0..28223e8edc799 100644 > > > --- a/iptables/xtables.c > > > +++ b/iptables/xtables.c > > > @@ -1139,6 +1139,8 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table, > > > cs.options & OPT_NUMERIC, > > > cs.options & OPT_EXPANDED, > > > cs.options & OPT_LINENUMBERS); > > > + if (p.command == CMD_LIST) > > > + nft_abort(h); > > > > Goal of this patch is to reset the batch of any pending object to be > > added before -L call? > > The goal is to prevent the unconditional commit (e.g. in xtables_main()) > from adding builtin tables/chains when listing only. Ah I see, thanks for explaining. > In the past, we had to commit early after creating builtin tables/chains > to make them appear in ruleset listing. A problem we faced with multiple > times was that 'nft flush ruleset ; iptables-nft -L' did not print > anything. Fix was to commit early (commit a4e78370af849) and flush cache > so it's refreshed (commit 206033ede9461). > > Since builtin tables/chains are manually added to cache, the early > commit and cache flush is no longer required. But after listing the > empty ruleset, the final commit still adds the builtin tables/chains > although not required for list command. This patch eliminates those > pointless batch jobs. I think we need to add the built-in chains when listing if we want to emulate the iptables-legacy behaviour. Listing via -L implies table autoload, ie. # iptables-legacy -L -t raw pulls in the raw table and its chains. For iptables-save, behaviour is different since we do _not_ pull in tables and built-in chains. Anyway, if we need disable the nft_commit() call, I would prefer if we pass a parameter to do_commandx(..., &commit); so we just set this commit to false from within do_commandx() depending on the command type, rather than calling nft_abort(). Thanks.
