diff mbox series

[iptables,2/2] Revert "ebtables: use extrapositioned negation consistently"

Message ID 20190121164335.25025-3-phil@nwl.cc
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series ebtables-nft output fixes | expand

Commit Message

Phil Sutter Jan. 21, 2019, 4:43 p.m. UTC 
This reverts commit 5f508b76a0cebaf91965ffa678089222e2d47964.

While attempts at unifying syntax between arp-, eb- and iptables-nft
increase the opportunity for more code-sharing, they are problematic
when it comes to compatibility. Accepting the old syntax on input helps,
but due to the fact that neither arptables nor ebtables support --check
command we must expect for users to test existence of a rule by
comparing input with output. If that happens in a script, deviating from
the old syntax in output has a high chance of breaking it.

Therefore revert Florian's patch changing inversion character position
in output and review the old code for consistency - the only thing
changed on top of the actual revert is ebtables' own copy of
print_iface() to make it adhere to the intrapositioned negation scheme
used throughout ebtables.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libebt_802_3.c    |  4 ++--
 extensions/libebt_802_3.t    |  2 +-
 extensions/libebt_arp.c      | 14 +++++++-------
 extensions/libebt_arp.t      |  9 ++++-----
 extensions/libebt_ip.c       | 16 ++++++++--------
 extensions/libebt_ip.t       |  8 +++-----
 extensions/libebt_ip6.c      | 14 +++++++-------
 extensions/libebt_ip6.t      |  8 +++-----
 extensions/libebt_mark_m.c   |  2 +-
 extensions/libebt_mark_m.t   |  4 ++--
 extensions/libebt_pkttype.c  |  5 +----
 extensions/libebt_pkttype.t  |  8 ++------
 extensions/libebt_standard.t |  4 ----
 extensions/libebt_stp.c      |  5 ++---
 extensions/libebt_vlan.c     | 13 ++++---------
 extensions/libebt_vlan.t     | 12 ++----------
 iptables/nft-bridge.c        |  6 +++---
 17 files changed, 52 insertions(+), 82 deletions(-)

Comments

Florian Westphal Jan. 21, 2019, 5:23 p.m. UTC | #1 
Phil Sutter <phil@nwl.cc> wrote:
> This reverts commit 5f508b76a0cebaf91965ffa678089222e2d47964.
> 
> While attempts at unifying syntax between arp-, eb- and iptables-nft
> increase the opportunity for more code-sharing, they are problematic
> when it comes to compatibility. Accepting the old syntax on input helps,
> but due to the fact that neither arptables nor ebtables support --check
> command we must expect for users to test existence of a rule by
> comparing input with output. If that happens in a script, deviating from
> the old syntax in output has a high chance of breaking it.

Is there a known script that is affected?

We broke this in iptables in even worse way, as we even do not support
-i ! "foo" anymore (you get a syntax error).

Do you think adding a warning on -i ! "foo" would help?

The many syntax deviations between the flavours is not nice at all,
making this more consistent would be a nice thing imo.
Phil Sutter Jan. 21, 2019, 5:51 p.m. UTC | #2 
On Mon, Jan 21, 2019 at 06:23:42PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > This reverts commit 5f508b76a0cebaf91965ffa678089222e2d47964.
> > 
> > While attempts at unifying syntax between arp-, eb- and iptables-nft
> > increase the opportunity for more code-sharing, they are problematic
> > when it comes to compatibility. Accepting the old syntax on input helps,
> > but due to the fact that neither arptables nor ebtables support --check
> > command we must expect for users to test existence of a rule by
> > comparing input with output. If that happens in a script, deviating from
> > the old syntax in output has a high chance of breaking it.
> 
> Is there a known script that is affected?

I guess some CI test script is since that's where the ticket came from.
;)

> We broke this in iptables in even worse way, as we even do not support
> -i ! "foo" anymore (you get a syntax error).

Well, the relevant difference here is that with iptables, you may use
'-C' to check for your rule but have to parse regular list output with
arptables/ebtables instead. So output format is a tad more important
with those tools.

> Do you think adding a warning on -i ! "foo" would help?

Well, downstream we would rather make use of release notes to inform
users I guess.

> The many syntax deviations between the flavours is not nice at all,
> making this more consistent would be a nice thing imo.

The bright side here is that at least for now no shared code is
affected. So we may stick with the quirky ebtables syntax without cost
at this point.

BTW: What about changing legacy ebtables code to align its syntax more
with iptables one? I know that "thou shall not touch the legacy". Though
deviating ebtables-nft from ebtables-legacy means users would have to
adapt - although we seem to pretend they can't when it comes to changing
legacy code. Don't get me wrong, I'm open for anything but appreciate if
things are done consistently.

Cheers, Phil
Florian Westphal Jan. 21, 2019, 11:50 p.m. UTC | #3 
Phil Sutter <phil@nwl.cc> wrote:
> The bright side here is that at least for now no shared code is
> affected. So we may stick with the quirky ebtables syntax without cost
> at this point.

Oh well, I guess we should revert then and just give up on this.
diff mbox series

Patch

diff --git a/extensions/libebt_802_3.c b/extensions/libebt_802_3.c
index 9e91d05262591..f05d02ead5a4a 100644
--- a/extensions/libebt_802_3.c
+++ b/extensions/libebt_802_3.c
@@ -98,15 +98,15 @@  static void br802_3_print(const void *ip, const struct xt_entry_match *match,
 	struct ebt_802_3_info *info = (struct ebt_802_3_info *)match->data;
 
 	if (info->bitmask & EBT_802_3_SAP) {
+		printf("--802_3-sap ");
 		if (info->invflags & EBT_802_3_SAP)
 			printf("! ");
-		printf("--802_3-sap ");
 		printf("0x%.2x ", info->sap);
 	}
 	if (info->bitmask & EBT_802_3_TYPE) {
+		printf("--802_3-type ");
 		if (info->invflags & EBT_802_3_TYPE)
 			printf("! ");
-		printf("--802_3-type ");
 		printf("0x%.4x ", ntohs(info->type));
 	}
 }
diff --git a/extensions/libebt_802_3.t b/extensions/libebt_802_3.t
index 61081bd6983a8..ddfb2f0a72baf 100644
--- a/extensions/libebt_802_3.t
+++ b/extensions/libebt_802_3.t
@@ -1,3 +1,3 @@ 
 :INPUT,FORWARD,OUTPUT
-! --802_3-sap 0x0a -j CONTINUE;=;OK
+--802_3-sap ! 0x0a -j CONTINUE;=;OK
 --802_3-type 0x000a -j RETURN;=;OK
diff --git a/extensions/libebt_arp.c b/extensions/libebt_arp.c
index c1b0ab1db0cf1..a062b7e7e5864 100644
--- a/extensions/libebt_arp.c
+++ b/extensions/libebt_arp.c
@@ -338,51 +338,51 @@  static void brarp_print(const void *ip, const struct xt_entry_match *match, int
 
 	if (arpinfo->bitmask & EBT_ARP_OPCODE) {
 		int opcode = ntohs(arpinfo->opcode);
+		printf("--arp-op ");
 		if (arpinfo->invflags & EBT_ARP_OPCODE)
 			printf("! ");
-		printf("--arp-op ");
 		if (opcode > 0 && opcode <= ARRAY_SIZE(opcodes))
 			printf("%s ", opcodes[opcode - 1]);
 		else
 			printf("%d ", opcode);
 	}
 	if (arpinfo->bitmask & EBT_ARP_HTYPE) {
+		printf("--arp-htype ");
 		if (arpinfo->invflags & EBT_ARP_HTYPE)
 			printf("! ");
-		printf("--arp-htype ");
 		printf("%d ", ntohs(arpinfo->htype));
 	}
 	if (arpinfo->bitmask & EBT_ARP_PTYPE) {
+		printf("--arp-ptype ");
 		if (arpinfo->invflags & EBT_ARP_PTYPE)
 			printf("! ");
-		printf("--arp-ptype ");
 		printf("0x%x ", ntohs(arpinfo->ptype));
 	}
 	if (arpinfo->bitmask & EBT_ARP_SRC_IP) {
+		printf("--arp-ip-src ");
 		if (arpinfo->invflags & EBT_ARP_SRC_IP)
 			printf("! ");
-		printf("--arp-ip-src ");
 		printf("%s%s ", xtables_ipaddr_to_numeric((const struct in_addr*) &arpinfo->saddr),
 		       xtables_ipmask_to_numeric((const struct in_addr*)&arpinfo->smsk));
 	}
 	if (arpinfo->bitmask & EBT_ARP_DST_IP) {
+		printf("--arp-ip-dst ");
 		if (arpinfo->invflags & EBT_ARP_DST_IP)
 			printf("! ");
-		printf("--arp-ip-dst ");
 		printf("%s%s ", xtables_ipaddr_to_numeric((const struct in_addr*) &arpinfo->daddr),
 		       xtables_ipmask_to_numeric((const struct in_addr*)&arpinfo->dmsk));
 	}
 	if (arpinfo->bitmask & EBT_ARP_SRC_MAC) {
+		printf("--arp-mac-src ");
 		if (arpinfo->invflags & EBT_ARP_SRC_MAC)
 			printf("! ");
-		printf("--arp-mac-src ");
 		xtables_print_mac_and_mask(arpinfo->smaddr, arpinfo->smmsk);
 		printf(" ");
 	}
 	if (arpinfo->bitmask & EBT_ARP_DST_MAC) {
+		printf("--arp-mac-dst ");
 		if (arpinfo->invflags & EBT_ARP_DST_MAC)
 			printf("! ");
-		printf("--arp-mac-dst ");
 		xtables_print_mac_and_mask(arpinfo->dmaddr, arpinfo->dmmsk);
 		printf(" ");
 	}
diff --git a/extensions/libebt_arp.t b/extensions/libebt_arp.t
index a05ab12dc566f..2b064c4bd2066 100644
--- a/extensions/libebt_arp.t
+++ b/extensions/libebt_arp.t
@@ -1,12 +1,11 @@ 
 :INPUT,FORWARD,OUTPUT
 -p ARP --arp-op Request;=;OK
--p ARP ! --arp-htype 1;=;OK
+-p ARP --arp-htype ! 1;=;OK
 -p ARP --arp-ptype 0x2;=;OK
 -p ARP --arp-ip-src 1.2.3.4;=;OK
--p ARP ! --arp-ip-dst 1.2.3.4;=;OK
--p ARP ! --arp-ip-src 0.0.0.0;=;OK
--p ARP ! --arp-ip-dst 0.0.0.0/8;=;OK
+-p ARP ! --arp-ip-dst 1.2.3.4;-p ARP --arp-ip-dst ! 1.2.3.4 -j CONTINUE;OK
+-p ARP --arp-ip-src ! 0.0.0.0;=;OK
+-p ARP --arp-ip-dst ! 0.0.0.0/8;=;OK
 -p ARP --arp-mac-src 00:de:ad:be:ef:00;=;OK
 -p ARP --arp-mac-dst de:ad:be:ef:00:00/ff:ff:ff:ff:00:00;=;OK
 -p ARP --arp-gratuitous;=;OK
---arp-htype 1;=;FAIL
diff --git a/extensions/libebt_ip.c b/extensions/libebt_ip.c
index d48704fe1c802..acb9bfcdbbd9f 100644
--- a/extensions/libebt_ip.c
+++ b/extensions/libebt_ip.c
@@ -472,35 +472,35 @@  static void brip_print(const void *ip, const struct xt_entry_match *match,
 	struct in_addr *addrp, *maskp;
 
 	if (info->bitmask & EBT_IP_SOURCE) {
+		printf("--ip-src ");
 		if (info->invflags & EBT_IP_SOURCE)
 			printf("! ");
-		printf("--ip-src ");
 		addrp = (struct in_addr *)&info->saddr;
 		maskp = (struct in_addr *)&info->smsk;
 		printf("%s%s ", xtables_ipaddr_to_numeric(addrp),
 		       xtables_ipmask_to_numeric(maskp));
 	}
 	if (info->bitmask & EBT_IP_DEST) {
+		printf("--ip-dst ");
 		if (info->invflags & EBT_IP_DEST)
 			printf("! ");
-		printf("--ip-dst ");
 		addrp = (struct in_addr *)&info->daddr;
 		maskp = (struct in_addr *)&info->dmsk;
 		printf("%s%s ", xtables_ipaddr_to_numeric(addrp),
 		       xtables_ipmask_to_numeric(maskp));
 	}
 	if (info->bitmask & EBT_IP_TOS) {
+		printf("--ip-tos ");
 		if (info->invflags & EBT_IP_TOS)
 			printf("! ");
-		printf("--ip-tos ");
 		printf("0x%02X ", info->tos);
 	}
 	if (info->bitmask & EBT_IP_PROTO) {
 		struct protoent *pe;
 
+		printf("--ip-proto ");
 		if (info->invflags & EBT_IP_PROTO)
 			printf("! ");
-		printf("--ip-proto ");
 		pe = getprotobynumber(info->protocol);
 		if (pe == NULL) {
 			printf("%d ", info->protocol);
@@ -509,28 +509,28 @@  static void brip_print(const void *ip, const struct xt_entry_match *match,
 		}
 	}
 	if (info->bitmask & EBT_IP_SPORT) {
+		printf("--ip-sport ");
 		if (info->invflags & EBT_IP_SPORT)
 			printf("! ");
-		printf("--ip-sport ");
 		print_port_range(info->sport);
 	}
 	if (info->bitmask & EBT_IP_DPORT) {
+		printf("--ip-dport ");
 		if (info->invflags & EBT_IP_DPORT)
 			printf("! ");
-		printf("--ip-dport ");
 		print_port_range(info->dport);
 	}
 	if (info->bitmask & EBT_IP_ICMP) {
+		printf("--ip-icmp-type ");
 		if (info->invflags & EBT_IP_ICMP)
 			printf("! ");
-		printf("--ip-icmp-type ");
 		ebt_print_icmp_type(icmp_codes, ARRAY_SIZE(icmp_codes),
 				    info->icmp_type, info->icmp_code);
 	}
 	if (info->bitmask & EBT_IP_IGMP) {
+		printf("--ip-igmp-type ");
 		if (info->invflags & EBT_IP_IGMP)
 			printf("! ");
-		printf("--ip-igmp-type ");
 		ebt_print_icmp_type(igmp_types, ARRAY_SIZE(igmp_types),
 				    info->igmp_type, NULL);
 	}
diff --git a/extensions/libebt_ip.t b/extensions/libebt_ip.t
index 01a91a7385fcc..87602367182a6 100644
--- a/extensions/libebt_ip.t
+++ b/extensions/libebt_ip.t
@@ -1,13 +1,11 @@ 
 :INPUT,FORWARD,OUTPUT
--p ip --ip-src ! 192.168.0.0/24 -j ACCEPT;-p IPv4 ! --ip-src 192.168.0.0/24 -j ACCEPT;OK
+-p ip --ip-src ! 192.168.0.0/24 -j ACCEPT;-p IPv4 --ip-src ! 192.168.0.0/24 -j ACCEPT;OK
 -p IPv4 --ip-dst 10.0.0.1;=;OK
 -p IPv4 --ip-tos 0xFF;=;OK
--p IPv4 ! --ip-tos 0xFF;=;OK
+-p IPv4 --ip-tos ! 0xFF;=;OK
 -p IPv4 --ip-proto tcp --ip-dport 22;=;OK
 -p IPv4 --ip-proto udp --ip-sport 1024:65535;=;OK
 -p IPv4 --ip-proto 253;=;OK
 -p IPv4 --ip-proto icmp --ip-icmp-type echo-request;=;OK
 -p IPv4 --ip-proto icmp --ip-icmp-type 1/1;=;OK
--p ip --ip-protocol icmp --ip-icmp-type ! 1:10;-p IPv4 --ip-proto icmp ! --ip-icmp-type 1:10/0:255 -j CONTINUE;OK
---ip-proto icmp --ip-icmp-type 1/1;=;FAIL
-! -p ip --ip-proto icmp --ip-icmp-type 1/1;=;FAIL
+-p ip --ip-protocol icmp --ip-icmp-type ! 1:10;-p IPv4 --ip-proto icmp --ip-icmp-type ! 1:10/0:255 -j CONTINUE;OK
diff --git a/extensions/libebt_ip6.c b/extensions/libebt_ip6.c
index b727764903ffa..b8a5a5d8c3a92 100644
--- a/extensions/libebt_ip6.c
+++ b/extensions/libebt_ip6.c
@@ -399,31 +399,31 @@  static void brip6_print(const void *ip, const struct xt_entry_match *match,
 	struct ebt_ip6_info *ipinfo = (struct ebt_ip6_info *)match->data;
 
 	if (ipinfo->bitmask & EBT_IP6_SOURCE) {
+		printf("--ip6-src ");
 		if (ipinfo->invflags & EBT_IP6_SOURCE)
 			printf("! ");
-		printf("--ip6-src ");
 		printf("%s", xtables_ip6addr_to_numeric(&ipinfo->saddr));
 		printf("%s ", xtables_ip6mask_to_numeric(&ipinfo->smsk));
 	}
 	if (ipinfo->bitmask & EBT_IP6_DEST) {
+		printf("--ip6-dst ");
 		if (ipinfo->invflags & EBT_IP6_DEST)
 			printf("! ");
-		printf("--ip6-dst ");
 		printf("%s", xtables_ip6addr_to_numeric(&ipinfo->daddr));
 		printf("%s ", xtables_ip6mask_to_numeric(&ipinfo->dmsk));
 	}
 	if (ipinfo->bitmask & EBT_IP6_TCLASS) {
+		printf("--ip6-tclass ");
 		if (ipinfo->invflags & EBT_IP6_TCLASS)
 			printf("! ");
-		printf("--ip6-tclass ");
 		printf("0x%02X ", ipinfo->tclass);
 	}
 	if (ipinfo->bitmask & EBT_IP6_PROTO) {
 		struct protoent *pe;
 
+		printf("--ip6-proto ");
 		if (ipinfo->invflags & EBT_IP6_PROTO)
 			printf("! ");
-		printf("--ip6-proto ");
 		pe = getprotobynumber(ipinfo->protocol);
 		if (pe == NULL) {
 			printf("%d ", ipinfo->protocol);
@@ -432,21 +432,21 @@  static void brip6_print(const void *ip, const struct xt_entry_match *match,
 		}
 	}
 	if (ipinfo->bitmask & EBT_IP6_SPORT) {
+		printf("--ip6-sport ");
 		if (ipinfo->invflags & EBT_IP6_SPORT)
 			printf("! ");
-		printf("--ip6-sport ");
 		print_port_range(ipinfo->sport);
 	}
 	if (ipinfo->bitmask & EBT_IP6_DPORT) {
+		printf("--ip6-dport ");
 		if (ipinfo->invflags & EBT_IP6_DPORT)
 			printf("! ");
-		printf("--ip6-dport ");
 		print_port_range(ipinfo->dport);
 	}
 	if (ipinfo->bitmask & EBT_IP6_ICMP6) {
+		printf("--ip6-icmp-type ");
 		if (ipinfo->invflags & EBT_IP6_ICMP6)
 			printf("! ");
-		printf("--ip6-icmp-type ");
 		print_icmp_type(ipinfo->icmpv6_type, ipinfo->icmpv6_code);
 	}
 }
diff --git a/extensions/libebt_ip6.t b/extensions/libebt_ip6.t
index 6b3221ea58f62..9d4effdc20e5a 100644
--- a/extensions/libebt_ip6.t
+++ b/extensions/libebt_ip6.t
@@ -1,15 +1,13 @@ 
 :INPUT,FORWARD,OUTPUT
--p ip6 ! --ip6-src dead::beef/64 -j ACCEPT;-p IPv6 ! --ip6-src dead::/64 -j ACCEPT;OK
+-p ip6 --ip6-src ! dead::beef/64 -j ACCEPT;-p IPv6 --ip6-src ! dead::/64 -j ACCEPT;OK
 -p IPv6 --ip6-dst dead:beef::/64 -j ACCEPT;=;OK
 -p IPv6 --ip6-dst f00:ba::;=;OK
 -p IPv6 --ip6-tclass 0xFF;=;OK
 -p IPv6 --ip6-proto tcp --ip6-dport 22;=;OK
--p IPv6 --ip6-proto tcp ! --ip6-dport 22;=;OK
+-p IPv6 --ip6-proto tcp --ip6-dport ! 22;=;OK
 -p IPv6 --ip6-proto udp --ip6-sport 1024:65535;=;OK
 -p IPv6 --ip6-proto 253;=;OK
 -p IPv6 --ip6-proto ipv6-icmp --ip6-icmp-type echo-request -j CONTINUE;=;OK
 -p IPv6 --ip6-proto ipv6-icmp --ip6-icmp-type echo-request;=;OK
 -p ip6 --ip6-protocol icmpv6 --ip6-icmp-type 1/1;-p IPv6 --ip6-proto ipv6-icmp --ip6-icmp-type communication-prohibited -j CONTINUE;OK
--p IPv6 --ip6-proto ipv6-icmp ! --ip6-icmp-type 1:10/0:255;=;OK
---ip6-proto ipv6-icmp ! --ip6-icmp-type 1:10/0:255;=;FAIL
-! -p IPv6 --ip6-proto ipv6-icmp ! --ip6-icmp-type 1:10/0:255;=;FAIL
+-p IPv6 --ip6-proto ipv6-icmp --ip6-icmp-type ! 1:10/0:255;=;OK
diff --git a/extensions/libebt_mark_m.c b/extensions/libebt_mark_m.c
index 64ad926f19959..2462d0af7d0bc 100644
--- a/extensions/libebt_mark_m.c
+++ b/extensions/libebt_mark_m.c
@@ -86,9 +86,9 @@  static void brmark_m_print(const void *ip, const struct xt_entry_match *match,
 {
 	struct ebt_mark_m_info *info = (struct ebt_mark_m_info *)match->data;
 
+	printf("--mark ");
 	if (info->invert)
 		printf("! ");
-	printf("--mark ");
 	if (info->bitmask == EBT_MARK_OR)
 		printf("/0x%lx ", info->mask);
 	else if (info->mask != 0xffffffff)
diff --git a/extensions/libebt_mark_m.t b/extensions/libebt_mark_m.t
index 9ad41704fbb82..00035427f8b6e 100644
--- a/extensions/libebt_mark_m.t
+++ b/extensions/libebt_mark_m.t
@@ -1,6 +1,6 @@ 
 :INPUT,FORWARD,OUTPUT
 --mark 42;--mark 0x2a;OK
---mark ! 42;! --mark 0x2a;OK
+--mark ! 42;--mark ! 0x2a;OK
 --mark 42/0xff;--mark 0x2a/0xff;OK
-! --mark 0x1/0xff;=;OK
+--mark ! 0x1/0xff;=;OK
 --mark /0x2;=;OK
diff --git a/extensions/libebt_pkttype.c b/extensions/libebt_pkttype.c
index 265674d19bde6..4e2d19de7983b 100644
--- a/extensions/libebt_pkttype.c
+++ b/extensions/libebt_pkttype.c
@@ -75,10 +75,7 @@  static void brpkttype_print(const void *ip, const struct xt_entry_match *match,
 {
 	struct ebt_pkttype_info *pt = (struct ebt_pkttype_info *)match->data;
 
-	if (pt->invert)
-		printf("! ");
-
-	printf("--pkttype-type ");
+	printf("--pkttype-type %s", pt->invert ? "! " : "");
 
 	if (pt->pkt_type < ARRAY_SIZE(classes))
 		printf("%s ", classes[pt->pkt_type]);
diff --git a/extensions/libebt_pkttype.t b/extensions/libebt_pkttype.t
index f870f5c7f05c4..f5f76aaaebfdc 100644
--- a/extensions/libebt_pkttype.t
+++ b/extensions/libebt_pkttype.t
@@ -1,13 +1,9 @@ 
 :INPUT,FORWARD,OUTPUT
+! --pkttype-type host;--pkttype-type ! host -j CONTINUE;OK
 --pkttype-type host;=;OK
-! --pkttype-type host;=;OK
 --pkttype-type broadcast;=;OK
-! --pkttype-type broadcast;=;OK
+--pkttype-type ! multicast;=;OK
 --pkttype-type multicast;=;OK
-! --pkttype-type multicast;=;OK
 --pkttype-type otherhost;=;OK
-! --pkttype-type otherhost;=;OK
 --pkttype-type outgoing;=;OK
-! --pkttype-type outgoing;=;OK
 --pkttype-type loopback;=;OK
-! --pkttype-type loopback;=;OK
diff --git a/extensions/libebt_standard.t b/extensions/libebt_standard.t
index 72081fd6575a0..c0b87e128d518 100644
--- a/extensions/libebt_standard.t
+++ b/extensions/libebt_standard.t
@@ -5,7 +5,3 @@ 
 -s de:ad:be:ef:0:00 -j RETURN;-s de:ad:be:ef:00:00 -j RETURN;OK
 -d de:ad:be:ef:00:00 -j CONTINUE;=;OK
 -d de:ad:be:ef:0:00/ff:ff:ff:ff:0:0 -j DROP;-d de:ad:be:ef:00:00/ff:ff:ff:ff:00:00 -j DROP;OK
--p ARP -j ACCEPT;=;OK
-! -p ARP -j ACCEPT;=;OK
--p 0 -j ACCEPT;=;FAIL
-! -p 0 -j ACCEPT;=;FAIL
diff --git a/extensions/libebt_stp.c b/extensions/libebt_stp.c
index 33e4c8d9c615d..06cf93b8d8449 100644
--- a/extensions/libebt_stp.c
+++ b/extensions/libebt_stp.c
@@ -307,9 +307,8 @@  static void brstp_print(const void *ip, const struct xt_entry_match *match,
 	for (i = 0; i < STP_NUMOPS; i++) {
 		if (!(stpinfo->bitmask & (1 << i)))
 			continue;
-		if (stpinfo->invflags & (1 << i))
-			printf("! ");
-		printf("--%s ", brstp_opts[i].name);
+		printf("--%s %s", brstp_opts[i].name,
+		       (stpinfo->invflags & (1 << i)) ? "! " : "");
 		if (EBT_STP_TYPE == (1 << i)) {
 			if (stpinfo->type == BPDU_TYPE_CONFIG)
 				printf("%s", BPDU_TYPE_CONFIG_STRING);
diff --git a/extensions/libebt_vlan.c b/extensions/libebt_vlan.c
index be269c6cdb4c8..fa697921068dc 100644
--- a/extensions/libebt_vlan.c
+++ b/extensions/libebt_vlan.c
@@ -109,19 +109,14 @@  static void brvlan_print(const void *ip, const struct xt_entry_match *match,
 	struct ebt_vlan_info *vlaninfo = (struct ebt_vlan_info *) match->data;
 
 	if (vlaninfo->bitmask & EBT_VLAN_ID) {
-		if (vlaninfo->invflags & EBT_VLAN_ID)
-			printf("! ");
-		printf("--vlan-id %d ", vlaninfo->id);
+		printf("--vlan-id %s%d ", (vlaninfo->invflags & EBT_VLAN_ID) ? "! " : "", vlaninfo->id);
 	}
 	if (vlaninfo->bitmask & EBT_VLAN_PRIO) {
-		if (vlaninfo->invflags & EBT_VLAN_PRIO)
-			printf("! ");
-		printf("--vlan-prio %d ", vlaninfo->prio);
+		printf("--vlan-prio %s%d ", (vlaninfo->invflags & EBT_VLAN_PRIO) ? "! " : "", vlaninfo->prio);
 	}
 	if (vlaninfo->bitmask & EBT_VLAN_ENCAP) {
-		if (vlaninfo->invflags & EBT_VLAN_ENCAP)
-			printf("! ");
-		printf("--vlan-encap %4.4X ", ntohs(vlaninfo->encap));
+		printf("--vlan-encap %s", (vlaninfo->invflags & EBT_VLAN_ENCAP) ? "! " : "");
+		printf("%4.4X ", ntohs(vlaninfo->encap));
 	}
 }
 
diff --git a/extensions/libebt_vlan.t b/extensions/libebt_vlan.t
index 106374cd9cb80..58471caa2343d 100644
--- a/extensions/libebt_vlan.t
+++ b/extensions/libebt_vlan.t
@@ -1,13 +1,5 @@ 
 :INPUT,FORWARD,OUTPUT
 -p 802_1Q --vlan-id 42;=;OK
--p 802_1Q ! --vlan-id 42;=;OK
--p 802_1Q --vlan-prio 1;=;OK
--p 802_1Q ! --vlan-prio 1;=;OK
+-p 802_1Q --vlan-prio ! 1;=;OK
 -p 802_1Q --vlan-encap ip;-p 802_1Q --vlan-encap 0800 -j CONTINUE;OK
--p 802_1Q --vlan-encap 0800 ;=;OK
--p 802_1Q ! --vlan-encap 0800 ;=;OK
--p 802_1Q --vlan-encap IPv6 ! --vlan-id 1;-p 802_1Q ! --vlan-id 1 --vlan-encap 86DD -j CONTINUE;OK
--p 802_1Q ! --vlan-id 1 --vlan-encap 86DD;=;OK
---vlan-encap ip;=;FAIL
---vlan-id 2;=;FAIL
---vlan-prio 1;=;FAIL
+-p 802_1Q --vlan-encap IPv6 ! --vlan-id 1;-p 802_1Q --vlan-id ! 1 --vlan-encap 86DD -j CONTINUE;OK
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 61c82f72c7740..a7708139324da 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -334,7 +334,7 @@  static void nft_rule_to_ebtables_command_state(const struct nftnl_rule *r,
 static void print_iface(const char *option, const char *name, bool invert)
 {
 	if (*name)
-		printf("%s%s %s ", invert ? "! " : "", option, name);
+		printf("%s%s %s ", option, invert ? " !" : "", name);
 }
 
 static void nft_bridge_print_table_header(const char *tablename)
@@ -379,9 +379,9 @@  static void print_mac(char option, const unsigned char *mac,
 		      const unsigned char *mask,
 		      bool invert)
 {
+	printf("-%c ", option);
 	if (invert)
 		printf("! ");
-	printf("-%c ", option);
 	ebt_print_mac_and_mask(mac, mask);
 	printf(" ");
 }
@@ -396,9 +396,9 @@  static void print_protocol(uint16_t ethproto, bool invert, unsigned int bitmask)
 	if (bitmask & EBT_NOPROTO)
 		return;
 
+	printf("-p ");
 	if (invert)
 		printf("! ");
-	printf("-p ");
 
 	if (bitmask & EBT_802_3) {
 		printf("length ");