| Message ID | 20190720163026.15410-10-phil@nwl.cc |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | Larger xtables-save review | expand |
Phil Sutter <phil@nwl.cc> wrote: > Change xtables_save_main to support optional printing of COMMIT line as > it is not used in arp- or ebtables. Why? Is this so ebt-save dumps are compatible with the old ebt-restore?
On Sat, Jul 20, 2019 at 09:29:18PM +0200, Florian Westphal wrote: > Phil Sutter <phil@nwl.cc> wrote: > > Change xtables_save_main to support optional printing of COMMIT line as > > it is not used in arp- or ebtables. > > Why? Is this so ebt-save dumps are compatible with the > old ebt-restore? Hmm. I haven't considered deviating in ebtables-nft-save format from legacy one yet, but it may actually be no problem given that people shouldn't switch between both. The only bummer might be if someone has a custom script which then trips over the unknown non-comment line. I'm undecided, but not against "breaking" with legacy save dumps per se. If you think it's not a problem, we could just simplify things. OTOH this is not a big patch, either so maybe not even worth the risk. :) Thanks, Phil
Phil Sutter <phil@nwl.cc> wrote: > On Sat, Jul 20, 2019 at 09:29:18PM +0200, Florian Westphal wrote: > > Phil Sutter <phil@nwl.cc> wrote: > > > Change xtables_save_main to support optional printing of COMMIT line as > > > it is not used in arp- or ebtables. > > > > Why? Is this so ebt-save dumps are compatible with the > > old ebt-restore? > > Hmm. I haven't considered deviating in ebtables-nft-save format from > legacy one yet, but it may actually be no problem given that people > shouldn't switch between both. The only bummer might be if someone has a > custom script which then trips over the unknown non-comment line. > > I'm undecided, but not against "breaking" with legacy save dumps per se. > If you think it's not a problem, we could just simplify things. OTOH > this is not a big patch, either so maybe not even worth the risk. :) I'm fine with this, I was just wondering because the changelog explains whats happening, not why.
On Sat, Jul 20, 2019 at 10:13:10PM +0200, Florian Westphal wrote: > Phil Sutter <phil@nwl.cc> wrote: > > On Sat, Jul 20, 2019 at 09:29:18PM +0200, Florian Westphal wrote: > > > Phil Sutter <phil@nwl.cc> wrote: > > > > Change xtables_save_main to support optional printing of COMMIT line as > > > > it is not used in arp- or ebtables. > > > > > > Why? Is this so ebt-save dumps are compatible with the > > > old ebt-restore? > > > > Hmm. I haven't considered deviating in ebtables-nft-save format from > > legacy one yet, but it may actually be no problem given that people > > shouldn't switch between both. The only bummer might be if someone has a > > custom script which then trips over the unknown non-comment line. > > > > I'm undecided, but not against "breaking" with legacy save dumps per se. > > If you think it's not a problem, we could just simplify things. OTOH > > this is not a big patch, either so maybe not even worth the risk. :) > > I'm fine with this, I was just wondering because the changelog explains > whats happening, not why. Yes, typical pitfall for me. Maybe I should put a sticky note somewhere. :) Thanks, Phil
diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c index b4d14b5bcd016..249b396091af4 100644 --- a/iptables/xtables-save.c +++ b/iptables/xtables-save.c @@ -67,6 +67,7 @@ static bool ebt_legacy_counter_format; struct do_output_data { bool counters; + bool commit; }; static int @@ -98,7 +99,8 @@ __do_output(struct nft_handle *h, const char *tablename, void *data) * thereby preventing dependency conflicts */ nft_chain_save(h, chain_list); nft_rule_save(h, tablename, d->counters ? 0 : FMT_NOCOUNTS); - printf("COMMIT\n"); + if (d->commit) + printf("COMMIT\n"); now = time(NULL); printf("# Completed on %s", ctime(&now)); @@ -219,6 +221,7 @@ xtables_save_main(int family, int argc, char *argv[], init_extensions4(); #endif tables = xtables_ipv4; + d.commit = true; break; case NFPROTO_ARP: tables = xtables_arp;
Change xtables_save_main to support optional printing of COMMIT line as it is not used in arp- or ebtables. Signed-off-by: Phil Sutter <phil@nwl.cc> --- iptables/xtables-save.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)