[iptables,09/12] xtables-save: Make COMMIT line optional
diff mbox series

Message ID 20190720163026.15410-10-phil@nwl.cc
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series
  • Larger xtables-save review
Related show

Commit Message

Phil Sutter July 20, 2019, 4:30 p.m. UTC
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(-)

Comments

Florian Westphal July 20, 2019, 7:29 p.m. UTC | #1
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?
Phil Sutter July 20, 2019, 8:11 p.m. UTC | #2
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
Florian Westphal July 20, 2019, 8:13 p.m. UTC | #3
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.
Phil Sutter July 20, 2019, 8:15 p.m. UTC | #4
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

Patch
diff mbox series

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;