[nft PATCH 0/6] monitor: Some fixes and improvements

[nft PATCH 0/6] monitor: Some fixes and improvements

[Phil Sutter]

The following series happened while fixing monitor tests and contains
some changes to monitor code along the way.

Phil Sutter (6):
  tests/monitor: Ignore newgen messages in output
  monitor: Fix printing of set declarations
  tests/monitor: Simplify testcases
  monitor: Print space after semicolon
  tests/monitor: Clear ruleset after testing
  tests/monitor: Add a small README

 src/rule.c                             |  8 +++---
 tests/monitor/README                   | 48 ++++++++++++++++++++++++++++++++++
 tests/monitor/run-tests.sh             | 16 +++++++++---
 tests/monitor/testcases/set-maps.t     |  7 +++--
 tests/monitor/testcases/set-mixed.t    | 10 +++----
 tests/monitor/testcases/set-multiple.t |  7 ++---
 tests/monitor/testcases/set-simple.t   | 21 ++++++---------
 7 files changed, 80 insertions(+), 37 deletions(-)
 create mode 100644 tests/monitor/README

-- 
2.13.1

[nft PATCH 1/6] tests/monitor: Ignore newgen messages in output

[Phil Sutter]

Predicting the new ID value is not feasible and neither is implementing
support for regular expressions when matching monitor output, so simply
ignore them.

Also use diff option '-w' instead of '-Z' to ignore all whitespace, not
just at EOL.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/monitor/run-tests.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
index 7447adf1febd6..e21ed6880bc13 100755
--- a/tests/monitor/run-tests.sh
+++ b/tests/monitor/run-tests.sh
@@ -10,6 +10,9 @@ fi
 trap "rm -rf $testdir" EXIT
 
 nft=../../src/nft
+mydiff() {
+	diff -w -I '^# ' "$@"
+}
 command_file=$(mktemp -p $testdir)
 output_file=$(mktemp -p $testdir)
 
@@ -35,9 +38,9 @@ run_test() {
 	sleep 0.5
 	kill $monitor_pid
 	wait >/dev/null 2>&1
-	if ! diff -Z -q $monitor_output $output_file >/dev/null 2>&1; then
+	if ! mydiff -q $monitor_output $output_file >/dev/null 2>&1; then
 		echo "monitor output differs!"
-		diff -Z -u $output_file $monitor_output
+		mydiff -u $output_file $monitor_output
 		exit 1
 	fi
 	rm $command_file
-- 
2.13.1

[nft PATCH 2/6] monitor: Fix printing of set declarations

[Phil Sutter]

The optional attributes 'flags', 'gc-interval' and 'timeout' have to be
delimited by stmt_separator (either newline or semicolon), not 'nl'
which is set to whitespace by set_print_plain().

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/rule.c                             | 6 +++---
 tests/monitor/testcases/set-maps.t     | 2 +-
 tests/monitor/testcases/set-mixed.t    | 2 +-
 tests/monitor/testcases/set-multiple.t | 4 ++--
 tests/monitor/testcases/set-simple.t   | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/rule.c b/src/rule.c
index 7f83980c60818..5ea4fce546e30 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -335,18 +335,18 @@ static void set_print_declaration(const struct set *set,
 			printf("%stimeout", delim);
 			delim = ",";
 		}
-		printf("%s", opts->nl);
+		printf("%s", opts->stmt_separator);
 	}
 
 	if (set->timeout) {
 		printf("%s%stimeout ", opts->tab, opts->tab);
 		time_print(set->timeout / 1000);
-		printf("%s", opts->nl);
+		printf("%s", opts->stmt_separator);
 	}
 	if (set->gc_int) {
 		printf("%s%sgc-interval ", opts->tab, opts->tab);
 		time_print(set->gc_int / 1000);
-		printf("%s", opts->nl);
+		printf("%s", opts->stmt_separator);
 	}
 }
 
diff --git a/tests/monitor/testcases/set-maps.t b/tests/monitor/testcases/set-maps.t
index d94016beb0767..6ea36cb9d11d6 100644
--- a/tests/monitor/testcases/set-maps.t
+++ b/tests/monitor/testcases/set-maps.t
@@ -2,7 +2,7 @@
 I add table ip t
 O add table ip t
 I add map ip t portip { type inet_service: ipv4_addr; flags interval; }
-O add map ip t portip { type inet_service : ipv4_addr;flags interval }
+O add map ip t portip { type inet_service : ipv4_addr;flags interval; }
 
 I add element ip t portip { 80-100: 10.0.0.1 }
 O add element ip t portip { 80-100 : 10.0.0.1 }
diff --git a/tests/monitor/testcases/set-mixed.t b/tests/monitor/testcases/set-mixed.t
index c4699edacec11..2eb35b5aa1efb 100644
--- a/tests/monitor/testcases/set-mixed.t
+++ b/tests/monitor/testcases/set-mixed.t
@@ -2,7 +2,7 @@
 I add table ip t
 O add table ip t
 I add set ip t portrange { type inet_service; flags interval; }
-O add set ip t portrange { type inet_service;flags interval }
+O add set ip t portrange { type inet_service;flags interval; }
 I add set ip t ports { type inet_service; }
 O add set ip t ports { type inet_service;}
 
diff --git a/tests/monitor/testcases/set-multiple.t b/tests/monitor/testcases/set-multiple.t
index d94f941b39693..ce919125a593f 100644
--- a/tests/monitor/testcases/set-multiple.t
+++ b/tests/monitor/testcases/set-multiple.t
@@ -2,9 +2,9 @@
 I add table ip t
 O add table ip t
 I add set ip t portrange { type inet_service; flags interval; }
-O add set ip t portrange { type inet_service;flags interval }
+O add set ip t portrange { type inet_service;flags interval; }
 I add set ip t portrange2 { type inet_service; flags interval; }
-O add set ip t portrange2 { type inet_service;flags interval }
+O add set ip t portrange2 { type inet_service;flags interval; }
 
 # make sure concurrent adds work
 I add element ip t portrange { 1024-65535 }
diff --git a/tests/monitor/testcases/set-simple.t b/tests/monitor/testcases/set-simple.t
index 22f648dbfa232..e44cce08f575c 100644
--- a/tests/monitor/testcases/set-simple.t
+++ b/tests/monitor/testcases/set-simple.t
@@ -2,7 +2,7 @@
 I add table ip t
 O add table ip t
 I add set ip t portrange { type inet_service; flags interval; }
-O add set ip t portrange { type inet_service;flags interval }
+O add set ip t portrange { type inet_service;flags interval; }
 
 # adding some ranges
 I add element ip t portrange { 1-10 }
-- 
2.13.1

[nft PATCH 3/6] tests/monitor: Simplify testcases

[Phil Sutter]

By introducing 'O -' indicating that output should be identical as
input, testcases can be simplified quite a bit.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/monitor/run-tests.sh             |  4 ++++
 tests/monitor/testcases/set-maps.t     |  7 +++----
 tests/monitor/testcases/set-mixed.t    | 10 +++-------
 tests/monitor/testcases/set-multiple.t |  7 ++-----
 tests/monitor/testcases/set-simple.t   | 21 ++++++++-------------
 5 files changed, 20 insertions(+), 29 deletions(-)

diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
index e21ed6880bc13..de19462438d64 100755
--- a/tests/monitor/run-tests.sh
+++ b/tests/monitor/run-tests.sh
@@ -20,6 +20,10 @@ cmd_append() {
 	echo "$*" >>$command_file
 }
 output_append() {
+	[[ "$*" == '-' ]] && {
+		cat $command_file >>$output_file
+		return
+	}
 	echo "$*" >>$output_file
 }
 run_test() {
diff --git a/tests/monitor/testcases/set-maps.t b/tests/monitor/testcases/set-maps.t
index 6ea36cb9d11d6..3d86720ec8136 100644
--- a/tests/monitor/testcases/set-maps.t
+++ b/tests/monitor/testcases/set-maps.t
@@ -1,11 +1,10 @@
 # first the setup
 I add table ip t
-O add table ip t
 I add map ip t portip { type inet_service: ipv4_addr; flags interval; }
-O add map ip t portip { type inet_service : ipv4_addr;flags interval; }
+O -
 
 I add element ip t portip { 80-100: 10.0.0.1 }
-O add element ip t portip { 80-100 : 10.0.0.1 }
+O -
 
 I add element ip t portip { 1024-65535: 10.0.0.1 }
-O add element ip t portip { 1024-65535 : 10.0.0.1 }
+O -
diff --git a/tests/monitor/testcases/set-mixed.t b/tests/monitor/testcases/set-mixed.t
index 2eb35b5aa1efb..9c1c5323f2e4e 100644
--- a/tests/monitor/testcases/set-mixed.t
+++ b/tests/monitor/testcases/set-mixed.t
@@ -1,19 +1,15 @@
 # first the setup
 I add table ip t
-O add table ip t
 I add set ip t portrange { type inet_service; flags interval; }
-O add set ip t portrange { type inet_service;flags interval; }
 I add set ip t ports { type inet_service; }
-O add set ip t ports { type inet_service;}
+O -
 
 # make sure concurrent adds work
 I add element ip t portrange { 1024-65535 }
 I add element ip t ports { 10 }
-O add element ip t portrange { 1024-65535 }
-O add element ip t ports { 10 }
+O -
 
 # delete items again
 I delete element ip t portrange { 1024-65535 }
 I delete element ip t ports { 10 }
-O delete element ip t portrange { 1024-65535 }
-O delete element ip t ports { 10 }
+O -
diff --git a/tests/monitor/testcases/set-multiple.t b/tests/monitor/testcases/set-multiple.t
index ce919125a593f..ad91fac047fe8 100644
--- a/tests/monitor/testcases/set-multiple.t
+++ b/tests/monitor/testcases/set-multiple.t
@@ -1,13 +1,10 @@
 # first the setup
 I add table ip t
-O add table ip t
 I add set ip t portrange { type inet_service; flags interval; }
-O add set ip t portrange { type inet_service;flags interval; }
 I add set ip t portrange2 { type inet_service; flags interval; }
-O add set ip t portrange2 { type inet_service;flags interval; }
+O -
 
 # make sure concurrent adds work
 I add element ip t portrange { 1024-65535 }
 I add element ip t portrange2 { 10-20 }
-O add element ip t portrange { 1024-65535 }
-O add element ip t portrange2 { 10-20 }
+O -
diff --git a/tests/monitor/testcases/set-simple.t b/tests/monitor/testcases/set-simple.t
index e44cce08f575c..ebff7cbda0c8b 100644
--- a/tests/monitor/testcases/set-simple.t
+++ b/tests/monitor/testcases/set-simple.t
@@ -1,14 +1,13 @@
 # first the setup
 I add table ip t
-O add table ip t
 I add set ip t portrange { type inet_service; flags interval; }
-O add set ip t portrange { type inet_service;flags interval; }
+O -
 
 # adding some ranges
 I add element ip t portrange { 1-10 }
-O add element ip t portrange { 1-10 }
+O -
 I add element ip t portrange { 1024-65535 }
-O add element ip t portrange { 1024-65535 }
+O -
 I add element ip t portrange { 20-30, 40-50 }
 O add element ip t portrange { 20-30 }
 O add element ip t portrange { 40-50 }
@@ -22,26 +21,22 @@ O delete element ip t portrange { 1-10 }
 
 # make sure lower scope boundary works
 I add element ip t portrange { 0-10 }
-O add element ip t portrange { 0-10 }
+O -
 
 # make sure half open before other element works
 I add element ip t portrange { 1024-65535 }
 I add element ip t portrange { 100-200 }
-O add element ip t portrange { 1024-65535 }
-O add element ip t portrange { 100-200 }
+O -
 
 # make sure deletion of elements works
 I delete element ip t portrange { 0-10 }
-O delete element ip t portrange { 0-10 }
+O -
 I delete element ip t portrange { 100-200 }
 I delete element ip t portrange { 1024-65535 }
-O delete element ip t portrange { 100-200 }
-O delete element ip t portrange { 1024-65535 }
+O -
 
 # make sure mixed add/delete works
 I add element ip t portrange { 10-20 }
 I add element ip t portrange { 1024-65535 }
 I delete element ip t portrange { 10-20 }
-O add element ip t portrange { 10-20 }
-O add element ip t portrange { 1024-65535 }
-O delete element ip t portrange { 10-20 }
+O -
-- 
2.13.1

[nft PATCH 4/6] monitor: Print space after semicolon

[Phil Sutter]

This makes output a bit more readable.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/rule.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rule.c b/src/rule.c
index 5ea4fce546e30..12714ed3ccc70 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -381,7 +381,7 @@ void set_print_plain(const struct set *s, struct output_ctx *octx)
 		.nl		= " ",
 		.table		= s->handle.table,
 		.family		= family2str(s->handle.family),
-		.stmt_separator	= ";",
+		.stmt_separator	= "; ",
 	};
 
 	do_set_print(s, &opts, octx);
-- 
2.13.1

[nft PATCH 5/6] tests/monitor: Clear ruleset after testing

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/monitor/run-tests.sh | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
index de19462438d64..9fd0e504d08c0 100755
--- a/tests/monitor/run-tests.sh
+++ b/tests/monitor/run-tests.sh
@@ -2,17 +2,18 @@
 
 cd $(dirname $0)
 
+nft=../../src/nft
+mydiff() {
+	diff -w -I '^# ' "$@"
+}
+
 testdir=$(mktemp -d)
 if [ ! -d $testdir ]; then
 	echo "Failed to create test directory" >&2
 	exit 0
 fi
-trap "rm -rf $testdir" EXIT
+trap "rm -rf $testdir; $nft flush ruleset" EXIT
 
-nft=../../src/nft
-mydiff() {
-	diff -w -I '^# ' "$@"
-}
 command_file=$(mktemp -p $testdir)
 output_file=$(mktemp -p $testdir)
 
-- 
2.13.1

[nft PATCH 6/6] tests/monitor: Add a small README

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/monitor/README | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 tests/monitor/README

diff --git a/tests/monitor/README b/tests/monitor/README
new file mode 100644
index 0000000000000..9c5e37f5c75c9
--- /dev/null
+++ b/tests/monitor/README
@@ -0,0 +1,48 @@
+Simple NFT MONITOR Testsuite
+============================
+
+The purpose of this suite of tests is to assert correct 'nft monitor' output for
+known input. The suite consists of the single shell script 'run-tests.sh' which
+performs the tests and a number of test definition files in 'testcases/'. The
+latter have to be suffixed '.t' in order to be recognized as such.
+
+Test Case Syntax
+----------------
+
+Each testcase defines a number of commands to pass on to 'nft' binary and an
+associated 'nft monitor' output definition. Prerequisites for each command have
+to be established manually, i.e. in order to test monitor output when adding a
+chain, the table containing it has to be created first. In between each
+testcase, rule set is flushed completely.
+
+Input and output lines are prefixed by 'I' and 'O', respectively. The prefix has
+to be separated from the rest of the line by whitespace. Consecutive input lines
+are passed to 'nft' together, hence lead to a single transaction.
+
+Since in most cases output should be equal to input, there is a shortcut: If a
+line consists of 'O -' only, the test script uses all previous input lines as
+expected output directly.
+
+Empty lines and those starting with '#' are ignored.
+
+Test Script Semantics
+---------------------
+
+The script iterates over all test case files, reading them line by line. It
+assumes that sections of 'I' lines alternate with sections of 'O' lines. After
+stripping the prefix, each line is appended to a temporary file. There are
+separate files for input and output lines.
+
+If a set of input and output lines is complete (i.e. upon encountering either a
+new input line or end of file), a testrun is performed: 'nft monitor' is run in
+background, redirecting the output into a third file. The input file is passed
+to 'nft -f'. Finally 'nft monitor' is killed and it's output compared to the
+output file created earlier. If the files differ, a unified diff is printed and
+test execution aborts.
+
+After each testrun, input and output files are cleared.
+
+Note: Running 'nft monitor' in background is prone to race conditions. Hence
+an artificial delay is introduced before calling 'nft -f' to allow for 'nft
+monitor' to complete initialization and another one before comparing the output
+to allow for 'nft monitor' to process the netlink events.
-- 
2.13.1

Re: [nft PATCH 2/6] monitor: Fix printing of set declarations

[Pablo Neira Ayuso]

On Tue, Jul 25, 2017 at 04:56:25PM +0200, Phil Sutter wrote:
> diff --git a/tests/monitor/testcases/set-maps.t b/tests/monitor/testcases/set-maps.t
> index d94016beb0767..6ea36cb9d11d6 100644
> --- a/tests/monitor/testcases/set-maps.t
> +++ b/tests/monitor/testcases/set-maps.t
> @@ -2,7 +2,7 @@
>  I add table ip t
>  O add table ip t
>  I add map ip t portip { type inet_service: ipv4_addr; flags interval; }
> -O add map ip t portip { type inet_service : ipv4_addr;flags interval }
> +O add map ip t portip { type inet_service : ipv4_addr;flags interval; }

So the proposal is to remove the whitespace? I think it's more
readable the way it is already.

Unless I'm overlooking anything, please, leave it as it is. Thanks.

Re: [nft PATCH 1/6] tests/monitor: Ignore newgen messages in output

[Pablo Neira Ayuso]

On Tue, Jul 25, 2017 at 04:56:24PM +0200, Phil Sutter wrote:
> Predicting the new ID value is not feasible and neither is implementing
> support for regular expressions when matching monitor output, so simply
> ignore them.
> 
> Also use diff option '-w' instead of '-Z' to ignore all whitespace, not
> just at EOL.

Applied, thanks Phil.

Re: [nft PATCH 3/6] tests/monitor: Simplify testcases

[Pablo Neira Ayuso]

On Tue, Jul 25, 2017 at 04:56:26PM +0200, Phil Sutter wrote:
> By introducing 'O -' indicating that output should be identical as
> input, testcases can be simplified quite a bit.

I like this update.

Re: [nft PATCH 4/6] monitor: Print space after semicolon

[Pablo Neira Ayuso]

On Tue, Jul 25, 2017 at 04:56:27PM +0200, Phil Sutter wrote:
> This makes output a bit more readable.

Could you print an example in the patch description of what is being
more readable after this? Thanks!

[...]
> diff --git a/src/rule.c b/src/rule.c
> index 5ea4fce546e30..12714ed3ccc70 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -381,7 +381,7 @@ void set_print_plain(const struct set *s, struct output_ctx *octx)
>  		.nl		= " ",
>  		.table		= s->handle.table,
>  		.family		= family2str(s->handle.family),
> -		.stmt_separator	= ";",
> +		.stmt_separator	= "; ",
>  	};
>  
>  	do_set_print(s, &opts, octx);
> -- 
> 2.13.1
> 

Re: [nft PATCH 5/6] tests/monitor: Clear ruleset after testing

[Pablo Neira Ayuso]

5/6 and 6/6 also LGTM.

Re: [nft PATCH 3/6] tests/monitor: Simplify testcases

[Pablo Neira Ayuso]

On Tue, Jul 25, 2017 at 06:02:28PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Jul 25, 2017 at 04:56:26PM +0200, Phil Sutter wrote:
> > By introducing 'O -' indicating that output should be identical as
> > input, testcases can be simplified quite a bit.
> 
> I like this update.

But I can't take it, git am complains. Please, rebase and resubmit,
thanks!

Re: [nft PATCH 6/6] tests/monitor: Add a small README

[Pablo Neira Ayuso]

This applies cleanly, so I just pushed it out.

I'm taking as much as it applies cleanly via git am.

Thanks!

Re: [nft PATCH 2/6] monitor: Fix printing of set declarations

[Phil Sutter]

On Tue, Jul 25, 2017 at 05:57:41PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Jul 25, 2017 at 04:56:25PM +0200, Phil Sutter wrote:
> > diff --git a/tests/monitor/testcases/set-maps.t b/tests/monitor/testcases/set-maps.t
> > index d94016beb0767..6ea36cb9d11d6 100644
> > --- a/tests/monitor/testcases/set-maps.t
> > +++ b/tests/monitor/testcases/set-maps.t
> > @@ -2,7 +2,7 @@
> >  I add table ip t
> >  O add table ip t
> >  I add map ip t portip { type inet_service: ipv4_addr; flags interval; }
> > -O add map ip t portip { type inet_service : ipv4_addr;flags interval }
> > +O add map ip t portip { type inet_service : ipv4_addr;flags interval; }
> 
> So the proposal is to remove the whitespace? I think it's more
> readable the way it is already.

No, it is about the missing semicolon after 'flags interval' (and after
'timeout' and 'gc-interval' options if present). The output of 'nft
monitor' is not syntactically correct without this.

You are correct in that it removes the whitespace before the closing
brace, but patch 4 "fixes" that by making 'stmt_separator' consist of
semicolon + whitespace.

Is it clear now?

Thanks, Phil

Re: [nft PATCH 2/6] monitor: Fix printing of set declarations

[Pablo Neira Ayuso]

On Tue, Jul 25, 2017 at 07:09:34PM +0200, Phil Sutter wrote:
> On Tue, Jul 25, 2017 at 05:57:41PM +0200, Pablo Neira Ayuso wrote:
> > On Tue, Jul 25, 2017 at 04:56:25PM +0200, Phil Sutter wrote:
> > > diff --git a/tests/monitor/testcases/set-maps.t b/tests/monitor/testcases/set-maps.t
> > > index d94016beb0767..6ea36cb9d11d6 100644
> > > --- a/tests/monitor/testcases/set-maps.t
> > > +++ b/tests/monitor/testcases/set-maps.t
> > > @@ -2,7 +2,7 @@
> > >  I add table ip t
> > >  O add table ip t
> > >  I add map ip t portip { type inet_service: ipv4_addr; flags interval; }
> > > -O add map ip t portip { type inet_service : ipv4_addr;flags interval }
> > > +O add map ip t portip { type inet_service : ipv4_addr;flags interval; }
> > 
> > So the proposal is to remove the whitespace? I think it's more
> > readable the way it is already.
> 
> No, it is about the missing semicolon after 'flags interval' (and after
> 'timeout' and 'gc-interval' options if present). The output of 'nft
> monitor' is not syntactically correct without this.
> 
> You are correct in that it removes the whitespace before the closing
> brace, but patch 4 "fixes" that by making 'stmt_separator' consist of
> semicolon + whitespace.
> 
> Is it clear now?

Thanks for explaining, yes this clarifies. Please, add examples next
time to your commit messages.

BTW, no need to update .t files after patch 4/6 to add the space after
semicolon?

Thanks!

Re: [nft PATCH 2/6] monitor: Fix printing of set declarations

[Phil Sutter]

On Wed, Jul 26, 2017 at 01:18:03PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Jul 25, 2017 at 07:09:34PM +0200, Phil Sutter wrote:
> > On Tue, Jul 25, 2017 at 05:57:41PM +0200, Pablo Neira Ayuso wrote:
> > > On Tue, Jul 25, 2017 at 04:56:25PM +0200, Phil Sutter wrote:
> > > > diff --git a/tests/monitor/testcases/set-maps.t b/tests/monitor/testcases/set-maps.t
> > > > index d94016beb0767..6ea36cb9d11d6 100644
> > > > --- a/tests/monitor/testcases/set-maps.t
> > > > +++ b/tests/monitor/testcases/set-maps.t
> > > > @@ -2,7 +2,7 @@
> > > >  I add table ip t
> > > >  O add table ip t
> > > >  I add map ip t portip { type inet_service: ipv4_addr; flags interval; }
> > > > -O add map ip t portip { type inet_service : ipv4_addr;flags interval }
> > > > +O add map ip t portip { type inet_service : ipv4_addr;flags interval; }
> > > 
> > > So the proposal is to remove the whitespace? I think it's more
> > > readable the way it is already.
> > 
> > No, it is about the missing semicolon after 'flags interval' (and after
> > 'timeout' and 'gc-interval' options if present). The output of 'nft
> > monitor' is not syntactically correct without this.
> > 
> > You are correct in that it removes the whitespace before the closing
> > brace, but patch 4 "fixes" that by making 'stmt_separator' consist of
> > semicolon + whitespace.
> > 
> > Is it clear now?
> 
> Thanks for explaining, yes this clarifies. Please, add examples next
> time to your commit messages.

Yes, will do. And in that case I already did, please see v2 containing
the remaining patches (starting with message ID
20170725183944.9377-1-phil@nwl.cc).

> BTW, no need to update .t files after patch 4/6 to add the space after
> semicolon?

No, because I changed diff options along with the fix for NEWGEN message
to ignore all whitespace, not just at EOL.

Thanks, Phil