[nft PATCH 1/4] rule: don't list anonymous sets

[nft PATCH 1/4] rule: don't list anonymous sets

[Arturo Borrero Gonzalez]

Don't list anonymous sets when listing all sets.

For example, using this ruleset:

==== 8< ====
table inet test {
	set set1 {
		type ipv4_addr
	}

	chain test {
		tcp dport { 80 } accept
	}
}
==== 8< ====

Before this patch:

% nft list sets
table inet test {
	set set0 {
		type inet_service
		flags constant
	}

	set set1 {
		type ipv4_addr
	}
}

After this patch:

% nft list sets
table inet test {
	set set1 {
		type ipv4_addr
	}
}

Fixes: 8f297010 ("rule: `list sets' only displays declaration, not definition")
Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 0 files changed

diff --git a/src/rule.c b/src/rule.c
index 5d3cd84..18ff592 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1020,6 +1020,8 @@ static int do_list_sets(struct netlink_ctx *ctx, struct cmd *cmd)
 		       table->handle.table);
 
 		list_for_each_entry(set, &table->sets, list) {
+			if (set->flags & SET_F_ANONYMOUS)
+				continue;
 			set_print_declaration(set, &opts);
 			printf("%s}%s", opts.tab, opts.nl);
 		}

[nft PATCH 2/4] rule: when listing all sets, don't print empty tables

[Arturo Borrero Gonzalez]

The table may contain sets, but they are anonymous.

For example, using this ruleset:

==== 8< ====
table arp test_arp {
	chain test {
		meta nfproto { ipv4}
	}
}
==== 8< ====

Before this patch:

% nft list sets
table arp test_arp {
}


After this patch:

% nft list sets
<no output>

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 0 files changed

diff --git a/src/rule.c b/src/rule.c
index 18ff592..c0e45aa 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1009,12 +1009,24 @@ static int do_list_sets(struct netlink_ctx *ctx, struct cmd *cmd)
 	};
 	struct table *table;
 	struct set *set;
+	bool printable_sets = false;
 
 	list_for_each_entry(table, &table_list, list) {
 		if (cmd->handle.family != NFPROTO_UNSPEC &&
 		    cmd->handle.family != table->handle.family)
 			continue;
 
+		/* if there are no printable sets, don't print empty table */
+		list_for_each_entry(set, &table->sets, list) {
+			if (!set->flags & SET_F_ANONYMOUS) {
+				printable_sets = true;
+				break;
+			}
+		}
+
+		if (!printable_sets)
+			continue;
+
 		printf("table %s %s {\n",
 		       family2str(table->handle.family),
 		       table->handle.table);
@@ -1027,6 +1039,8 @@ static int do_list_sets(struct netlink_ctx *ctx, struct cmd *cmd)
 		}
 
 		printf("}\n");
+
+		printable_sets = false;
 	}
 	return 0;
 }

[nft PATCH 3/4] rule: delete extra space in sets printing

[Arturo Borrero Gonzalez]

The extra space is printed when sets are printed in tabulated format.
However, the space is still required in printing in plain format (ie, monitor).

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 0 files changed

diff --git a/src/rule.c b/src/rule.c
index c0e45aa..ab39513 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -267,7 +267,10 @@ static void set_print_declaration(const struct set *set,
 	if (opts->table != NULL)
 		printf(" %s", opts->table);
 
-	printf(" %s { %s", set->handle.set, opts->nl);
+	printf(" %s {%s", set->handle.set, opts->nl);
+
+	if (!strcmp(opts->nl, ""))
+		printf(" ");
 
 	printf("%s%stype %s", opts->tab, opts->tab, set->keytype->name);
 	if (set->flags & SET_F_MAP)

[nft PATCH 4/4] tests/operations: add some listing tests

[Arturo Borrero Gonzalez]

Let's test what is shown with the 'list' command, for ruleset, tables and sets.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 tests/shell/testcases/listing/0001ruleset_0        |   10 ++++
 tests/shell/testcases/listing/0002ruleset_1        |   10 ++++
 tests/shell/testcases/listing/0003table_0          |   13 +++++
 tests/shell/testcases/listing/0004table_1          |   10 ++++
 tests/shell/testcases/listing/0005ruleset_ip_0     |   15 +++++
 tests/shell/testcases/listing/0006ruleset_ip6_0    |   15 +++++
 tests/shell/testcases/listing/0007ruleset_inet_0   |   15 +++++
 tests/shell/testcases/listing/0008ruleset_arp_0    |   15 +++++
 tests/shell/testcases/listing/0009ruleset_bridge_0 |   15 +++++
 tests/shell/testcases/listing/0009sets_0           |   33 ++++++++++++
 tests/shell/testcases/listing/0010sets_0           |   57 ++++++++++++++++++++
 tests/shell/testcases/listing/0011sets_0           |   26 +++++++++
 12 files changed, 234 insertions(+)
 create mode 100755 tests/shell/testcases/listing/0001ruleset_0
 create mode 100755 tests/shell/testcases/listing/0002ruleset_1
 create mode 100755 tests/shell/testcases/listing/0003table_0
 create mode 100755 tests/shell/testcases/listing/0004table_1
 create mode 100755 tests/shell/testcases/listing/0005ruleset_ip_0
 create mode 100755 tests/shell/testcases/listing/0006ruleset_ip6_0
 create mode 100755 tests/shell/testcases/listing/0007ruleset_inet_0
 create mode 100755 tests/shell/testcases/listing/0008ruleset_arp_0
 create mode 100755 tests/shell/testcases/listing/0009ruleset_bridge_0
 create mode 100755 tests/shell/testcases/listing/0009sets_0
 create mode 100755 tests/shell/testcases/listing/0010sets_0
 create mode 100755 tests/shell/testcases/listing/0011sets_0

diff --git a/tests/shell/testcases/listing/0001ruleset_0 b/tests/shell/testcases/listing/0001ruleset_0
new file mode 100755
index 0000000..662dd1a
--- /dev/null
+++ b/tests/shell/testcases/listing/0001ruleset_0
@@ -0,0 +1,10 @@
+#!/bin/bash
+
+# list ruleset shows a table
+
+EXPECTED="table ip test {
+}"
+
+$NFT add table test
+GET="$($NFT list ruleset)"
+[ "$EXPECTED" == "$GET" ] || exit 1
diff --git a/tests/shell/testcases/listing/0002ruleset_1 b/tests/shell/testcases/listing/0002ruleset_1
new file mode 100755
index 0000000..938936e
--- /dev/null
+++ b/tests/shell/testcases/listing/0002ruleset_1
@@ -0,0 +1,10 @@
+#!/bin/bash
+
+# list ruleset don't show tables that dont exist
+
+EXPECTED="table ip test {
+}"
+
+$NFT add table test2
+GET="$($NFT list ruleset)"
+[ "$EXPECTED" == "$GET" ] && exit 1
diff --git a/tests/shell/testcases/listing/0003table_0 b/tests/shell/testcases/listing/0003table_0
new file mode 100755
index 0000000..9249154
--- /dev/null
+++ b/tests/shell/testcases/listing/0003table_0
@@ -0,0 +1,13 @@
+#!/bin/bash
+
+# list table show what is expected
+
+EXPECTED="table ip test {
+}"
+
+$NFT add table test
+GET="$($NFT list table test)"
+[ "$EXPECTED" == "$GET" ] || exit 1
+
+GET="$($NFT list table ip test)"
+[ "$EXPECTED" == "$GET" ] || exit 1
diff --git a/tests/shell/testcases/listing/0004table_1 b/tests/shell/testcases/listing/0004table_1
new file mode 100755
index 0000000..617a935
--- /dev/null
+++ b/tests/shell/testcases/listing/0004table_1
@@ -0,0 +1,10 @@
+#!/bin/bash
+
+# list table can't show tables that dont exist
+
+EXPECTED="table ip test a {
+}"
+
+$NFT add table test2
+GET="$($NFT list table test)"
+[ "$EXPECTED" == "$GET" ] && exit 1
diff --git a/tests/shell/testcases/listing/0005ruleset_ip_0 b/tests/shell/testcases/listing/0005ruleset_ip_0
new file mode 100755
index 0000000..d70bc9b
--- /dev/null
+++ b/tests/shell/testcases/listing/0005ruleset_ip_0
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+# listing ruleset per family
+
+EXPECTED="table ip test {
+}"
+
+$NFT add table ip test
+$NFT add table ip6 test
+$NFT add table inet test
+$NFT add table arp test
+$NFT add table bridge test
+
+GET="$($NFT list ruleset ip)"
+[ "$EXPECTED" == "$GET" ] || exit 1
diff --git a/tests/shell/testcases/listing/0006ruleset_ip6_0 b/tests/shell/testcases/listing/0006ruleset_ip6_0
new file mode 100755
index 0000000..f9f718b
--- /dev/null
+++ b/tests/shell/testcases/listing/0006ruleset_ip6_0
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+# listing ruleset per family
+
+EXPECTED="table ip6 test {
+}"
+
+$NFT add table ip test
+$NFT add table ip6 test
+$NFT add table inet test
+$NFT add table arp test
+$NFT add table bridge test
+
+GET="$($NFT list ruleset ip6)"
+[ "$EXPECTED" == "$GET" ] || exit 1
diff --git a/tests/shell/testcases/listing/0007ruleset_inet_0 b/tests/shell/testcases/listing/0007ruleset_inet_0
new file mode 100755
index 0000000..428e9a8
--- /dev/null
+++ b/tests/shell/testcases/listing/0007ruleset_inet_0
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+# listing ruleset per family
+
+EXPECTED="table inet test {
+}"
+
+$NFT add table ip test
+$NFT add table ip6 test
+$NFT add table inet test
+$NFT add table arp test
+$NFT add table bridge test
+
+GET="$($NFT list ruleset inet)"
+[ "$EXPECTED" == "$GET" ] || exit 1
diff --git a/tests/shell/testcases/listing/0008ruleset_arp_0 b/tests/shell/testcases/listing/0008ruleset_arp_0
new file mode 100755
index 0000000..9a8b054
--- /dev/null
+++ b/tests/shell/testcases/listing/0008ruleset_arp_0
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+# listing ruleset per family
+
+EXPECTED="table arp test {
+}"
+
+$NFT add table ip test
+$NFT add table ip6 test
+$NFT add table inet test
+$NFT add table arp test
+$NFT add table bridge test
+
+GET="$($NFT list ruleset arp)"
+[ "$EXPECTED" == "$GET" ] || exit 1
diff --git a/tests/shell/testcases/listing/0009ruleset_bridge_0 b/tests/shell/testcases/listing/0009ruleset_bridge_0
new file mode 100755
index 0000000..e914aeb
--- /dev/null
+++ b/tests/shell/testcases/listing/0009ruleset_bridge_0
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+# listing ruleset per family
+
+EXPECTED="table bridge test {
+}"
+
+$NFT add table ip test
+$NFT add table ip6 test
+$NFT add table inet test
+$NFT add table arp test
+$NFT add table bridge test
+
+GET="$($NFT list ruleset bridge)"
+[ "$EXPECTED" == "$GET" ] || exit 1
diff --git a/tests/shell/testcases/listing/0009sets_0 b/tests/shell/testcases/listing/0009sets_0
new file mode 100755
index 0000000..d92c5b1
--- /dev/null
+++ b/tests/shell/testcases/listing/0009sets_0
@@ -0,0 +1,33 @@
+#!/bin/bash
+
+# listing all sets, filtering by family
+
+EXPECTED="table inet filter {
+	set set0 {
+		type inet_service
+	}
+	set set1 {
+		type inet_service
+		flags constant
+	}
+	set set2 {
+		type icmpv6_type
+	}
+}"
+
+$NFT add table ip nat
+$NFT add set ip nat ssh { type ipv4_addr \; }
+$NFT add table ip6 test
+$NFT add set ip6 test testset { type ipv6_addr \; }
+$NFT add table arp test_arp
+$NFT add set arp test_arp test_set_arp00 { type inet_service \; }
+$NFT add set arp test_arp test_set_arp01 { type inet_service \; flags constant \; }
+$NFT add table bridge test_bridge
+$NFT add set bridge test_bridge test_set_bridge { type inet_service \; }
+$NFT add table inet filter
+$NFT add set inet filter set0 { type inet_service \; }
+$NFT add set inet filter set1 { type inet_service \; flags constant \; }
+$NFT add set inet filter set2 { type icmpv6_type \; }
+
+GET="$($NFT list sets inet)"
+[ "$EXPECTED" == "$GET" ] || exit 1
diff --git a/tests/shell/testcases/listing/0010sets_0 b/tests/shell/testcases/listing/0010sets_0
new file mode 100755
index 0000000..547b8c4
--- /dev/null
+++ b/tests/shell/testcases/listing/0010sets_0
@@ -0,0 +1,57 @@
+#!/bin/bash
+
+# listing all sets
+
+EXPECTED="table ip nat {
+	set ssh {
+		type ipv4_addr
+	}
+}
+table ip6 test {
+	set testset {
+		type ipv6_addr
+	}
+}
+table inet filter {
+	set set0 {
+		type inet_service
+	}
+	set set1 {
+		type inet_service
+		flags constant
+	}
+	set set2 {
+		type icmpv6_type
+	}
+}
+table bridge test_bridge {
+	set test_set_bridge {
+		type inet_service
+	}
+}
+table arp test_arp {
+	set test_set_arp00 {
+		type inet_service
+	}
+	set test_set_arp01 {
+		type inet_service
+		flags constant
+	}
+}"
+
+$NFT add table ip nat
+$NFT add set ip nat ssh { type ipv4_addr \; }
+$NFT add table ip6 test
+$NFT add set ip6 test testset { type ipv6_addr \; }
+$NFT add table arp test_arp
+$NFT add set arp test_arp test_set_arp00 { type inet_service \; }
+$NFT add set arp test_arp test_set_arp01 { type inet_service \; flags constant \; }
+$NFT add table bridge test_bridge
+$NFT add set bridge test_bridge test_set_bridge { type inet_service \; }
+$NFT add table inet filter
+$NFT add set inet filter set0 { type inet_service \; }
+$NFT add set inet filter set1 { type inet_service \; flags constant \; }
+$NFT add set inet filter set2 { type icmpv6_type \; }
+
+GET="$($NFT list sets)"
+[ "$EXPECTED" == "$GET" ] || exit 1
diff --git a/tests/shell/testcases/listing/0011sets_0 b/tests/shell/testcases/listing/0011sets_0
new file mode 100755
index 0000000..514c6f1
--- /dev/null
+++ b/tests/shell/testcases/listing/0011sets_0
@@ -0,0 +1,26 @@
+#!/bin/bash
+
+# listing all sets, no anonymous sets allowed
+
+set -e
+$NFT add table ip nat
+$NFT add chain ip nat test
+$NFT add rule ip nat test tcp dport {123}
+
+$NFT add table ip6 test
+$NFT add chain ip6 test test
+$NFT add rule ip6 test test udp sport {123}
+
+$NFT add table arp test_arp
+$NFT add chain arp test_arp test
+$NFT add rule arp test_arp test meta nfproto {ipv4}
+
+$NFT add table bridge test_bridge
+$NFT add chain bridge test_bridge test
+$NFT add rule bridge test_bridge test ip daddr {1.1.1.1}
+
+$NFT add table inet filter
+$NFT add chain inet filter test
+$NFT add rule inet filter test tcp dport {80, 443}
+
+[ $($NFT list sets | wc -l) == 0 ] || exit 1

Re: [nft PATCH 1/4] rule: don't list anonymous sets

[Pablo Neira Ayuso]

On Mon, Jan 04, 2016 at 01:18:26PM +0100, Arturo Borrero Gonzalez wrote:
> Don't list anonymous sets when listing all sets.

Applied, thanks Arturo.

Re: [nft PATCH 2/4] rule: when listing all sets, don't print empty tables

[Pablo Neira Ayuso]

On Mon, Jan 04, 2016 at 01:18:31PM +0100, Arturo Borrero Gonzalez wrote:
> The table may contain sets, but they are anonymous.

Also applied, thanks.

Re: [nft PATCH 3/4] rule: delete extra space in sets printing

[Pablo Neira Ayuso]

On Mon, Jan 04, 2016 at 01:18:37PM +0100, Arturo Borrero Gonzalez wrote:
> The extra space is printed when sets are printed in tabulated format.
> However, the space is still required in printing in plain format (ie, monitor).
> 
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
>  0 files changed
> 
> diff --git a/src/rule.c b/src/rule.c
> index c0e45aa..ab39513 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -267,7 +267,10 @@ static void set_print_declaration(const struct set *set,
>  	if (opts->table != NULL)
>  		printf(" %s", opts->table);
>  
> -	printf(" %s { %s", set->handle.set, opts->nl);
> +	printf(" %s {%s", set->handle.set, opts->nl);
> +
> +	if (!strcmp(opts->nl, ""))

This could be replaced by:

        if (!opts->nl[0])

But could you post what output you're trying to fix?

Thanks.

> +		printf(" ");
>  
>  	printf("%s%stype %s", opts->tab, opts->tab, set->keytype->name);
>  	if (set->flags & SET_F_MAP)
> 

Re: [nft PATCH 3/4] rule: delete extra space in sets printing

[Arturo Borrero Gonzalez]

[-- Attachment #1: Type: text/plain, Size: 239 bytes --]

On 5 January 2016 at 12:28, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> But could you post what output you're trying to fix?
>

My email client would mess the text so find attached an example.

-- 
Arturo Borrero González

[-- Attachment #2: set.nft --]
[-- Type: application/octet-stream, Size: 59 bytes --]

table inet test {
	set test { 
		  ^
		type ipv4_addr
	}
}

Re: [nft PATCH 2/4] rule: when listing all sets, don't print empty tables

[Pablo Neira Ayuso]

On Mon, Jan 04, 2016 at 01:18:31PM +0100, Arturo Borrero Gonzalez wrote:
> diff --git a/src/rule.c b/src/rule.c
> index 18ff592..c0e45aa 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -1009,12 +1009,24 @@ static int do_list_sets(struct netlink_ctx *ctx, struct cmd *cmd)
>  	};
>  	struct table *table;
>  	struct set *set;
> +	bool printable_sets = false;
>  
>  	list_for_each_entry(table, &table_list, list) {
>  		if (cmd->handle.family != NFPROTO_UNSPEC &&
>  		    cmd->handle.family != table->handle.family)
>  			continue;
>  
> +		/* if there are no printable sets, don't print empty table */
> +		list_for_each_entry(set, &table->sets, list) {
> +			if (!set->flags & SET_F_ANONYMOUS) {

Wait, this should be:

        if (!(set->flags & SET_F_ANONYMOUS))

instead.

I'm fixing this here.

Re: [nft PATCH 2/4] rule: when listing all sets, don't print empty tables

[Pablo Neira Ayuso]

On Tue, Jan 05, 2016 at 12:35:44PM +0100, Pablo Neira Ayuso wrote:
> On Mon, Jan 04, 2016 at 01:18:31PM +0100, Arturo Borrero Gonzalez wrote:
> > diff --git a/src/rule.c b/src/rule.c
> > index 18ff592..c0e45aa 100644
> > --- a/src/rule.c
> > +++ b/src/rule.c
> > @@ -1009,12 +1009,24 @@ static int do_list_sets(struct netlink_ctx *ctx, struct cmd *cmd)
> >  	};
> >  	struct table *table;
> >  	struct set *set;
> > +	bool printable_sets = false;
> >  
> >  	list_for_each_entry(table, &table_list, list) {
> >  		if (cmd->handle.family != NFPROTO_UNSPEC &&
> >  		    cmd->handle.family != table->handle.family)
> >  			continue;
> >  
> > +		/* if there are no printable sets, don't print empty table */
> > +		list_for_each_entry(set, &table->sets, list) {
> > +			if (!set->flags & SET_F_ANONYMOUS) {
> 
> Wait, this should be:
> 
>         if (!(set->flags & SET_F_ANONYMOUS))
> 
> instead.
> 
> I'm fixing this here.

I'm going to keep this back.

We have to provide a consistent behaviour wrt. nft list chains, and
that is listing empty tables when it contains no chains.

I'm unsure here, I considering printing the table with no content
makes sense since the user knows no sets or chains are available
there. If we skip this, it looks like the table doesn't exists. Other
than that, the user is fully aware of having a table with no content.

Re: [nft PATCH 2/4] rule: when listing all sets, don't print empty tables

[Arturo Borrero Gonzalez]

On 5 January 2016 at 12:40, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> I'm going to keep this back.
>
> We have to provide a consistent behaviour wrt. nft list chains, and
> that is listing empty tables when it contains no chains.
>
> I'm unsure here, I considering printing the table with no content
> makes sense since the user knows no sets or chains are available
> there. If we skip this, it looks like the table doesn't exists. Other
> than that, the user is fully aware of having a table with no content.

Ok, I understand, thanks.
-- 
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nft PATCH 4/4] tests/operations: add some listing tests

[Arturo Borrero Gonzalez]

On 4 January 2016 at 13:18, Arturo Borrero Gonzalez
<arturo.borrero.glez@gmail.com> wrote:
> Let's test what is shown with the 'list' command, for ruleset, tables and sets.
>
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---

This would need some rework. I will resend.
-- 
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html