From e07c57e94e27d2f15bfb9de4db7ca3ab9d9368ed Mon Sep 17 00:00:00 2001
From: Odin Ugedal <odin@ugedal.com>
Date: Thu, 16 Apr 2020 16:08:14 +0200
Subject: [PATCH 01/25] tc_util: detect overflow in get_size

This detects overflow during parsing of value using get_size:

eg. running:

$ tc qdisc add dev lo root cake memlimit 11gb

currently gives a memlimit of "3072Mb", while with this patch it errors
with 'illegal value for "memlimit": "11gb"', since memlinit is an
unsigned integer.

Signed-off-by: Odin Ugedal <odin@ugedal.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/tc_util.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index 5f13d729..68938fb0 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -385,6 +385,11 @@ int get_size(unsigned int *size, const char *str)
 	}
 
 	*size = sz;
+
+	/* detect if an overflow happened */
+	if (*size != floor(sz))
+		return -1;
+
 	return 0;
 }
 

From 6f883f168cf9e1f3be208a10d671a54d781e75a5 Mon Sep 17 00:00:00 2001
From: Odin Ugedal <odin@ugedal.com>
Date: Wed, 15 Apr 2020 16:39:34 +0200
Subject: [PATCH 02/25] q_cake: Make fwmark uint instead of int
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This will help avoid overflow, since setting it to 0xffffffff would
result in -1 when converted to integer, resulting in being "-1", setting
the fwmark to 0x00.

Signed-off-by: Odin Ugedal <odin@ugedal.com>
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/q_cake.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/tc/q_cake.c b/tc/q_cake.c
index 3c78b176..9ebb270c 100644
--- a/tc/q_cake.c
+++ b/tc/q_cake.c
@@ -97,6 +97,7 @@ static int cake_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	unsigned int interval = 0;
 	unsigned int diffserv = 0;
 	unsigned int memlimit = 0;
+	unsigned int fwmark = 0;
 	unsigned int target = 0;
 	__u64 bandwidth = 0;
 	int ack_filter = -1;
@@ -107,7 +108,6 @@ static int cake_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	int autorate = -1;
 	int ingress = -1;
 	int overhead = 0;
-	int fwmark = -1;
 	int wash = -1;
 	int nat = -1;
 	int atm = -1;
@@ -335,15 +335,12 @@ static int cake_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 				return -1;
 			}
 		} else if (strcmp(*argv, "fwmark") == 0) {
-			unsigned int fwm;
-
 			NEXT_ARG();
-			if (get_u32(&fwm, *argv, 0)) {
+			if (get_u32(&fwmark, *argv, 0)) {
 				fprintf(stderr,
 					"Illegal value for \"fwmark\": \"%s\"\n", *argv);
 				return -1;
 			}
-			fwmark = fwm;
 		} else if (strcmp(*argv, "help") == 0) {
 			explain();
 			return -1;
@@ -388,7 +385,7 @@ static int cake_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	if (memlimit)
 		addattr_l(n, 1024, TCA_CAKE_MEMORY, &memlimit,
 			  sizeof(memlimit));
-	if (fwmark != -1)
+	if (fwmark)
 		addattr_l(n, 1024, TCA_CAKE_FWMARK, &fwmark,
 			  sizeof(fwmark));
 	if (nat != -1)

From 14d2df887481dd2130c6ae5d023325262429ce3c Mon Sep 17 00:00:00 2001
From: Odin Ugedal <odin@ugedal.com>
Date: Wed, 15 Apr 2020 16:39:35 +0200
Subject: [PATCH 03/25] q_cake: properly print memlimit
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Load memlimit so that it will be printed if it isn't set to zero.

Also add a space to properly print it.

Signed-off-by: Odin Ugedal <odin@ugedal.com>
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/q_cake.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tc/q_cake.c b/tc/q_cake.c
index 9ebb270c..bf116e80 100644
--- a/tc/q_cake.c
+++ b/tc/q_cake.c
@@ -520,6 +520,10 @@ static int cake_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 	    RTA_PAYLOAD(tb[TCA_CAKE_RTT]) >= sizeof(__u32)) {
 		interval = rta_getattr_u32(tb[TCA_CAKE_RTT]);
 	}
+	if (tb[TCA_CAKE_MEMORY] &&
+		RTA_PAYLOAD(tb[TCA_CAKE_MEMORY]) >= sizeof(__u32)) {
+		memlimit = rta_getattr_u32(tb[TCA_CAKE_MEMORY]);
+	}
 	if (tb[TCA_CAKE_FWMARK] &&
 	    RTA_PAYLOAD(tb[TCA_CAKE_FWMARK]) >= sizeof(__u32)) {
 		fwmark = rta_getattr_u32(tb[TCA_CAKE_FWMARK]);
@@ -572,7 +576,7 @@ static int cake_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 
 	if (memlimit) {
 		print_uint(PRINT_JSON, "memlimit", NULL, memlimit);
-		print_string(PRINT_FP, NULL, "memlimit %s",
+		print_string(PRINT_FP, NULL, "memlimit %s ",
 			     sprint_size(memlimit, b1));
 	}
 

From f03ad792f3cdef5ade92392db6710441b35b47dd Mon Sep 17 00:00:00 2001
From: Benjamin Lee <ben@b1c1l1.com>
Date: Tue, 14 Apr 2020 21:11:12 -0700
Subject: [PATCH 04/25] tc: fq_codel: fix class stat deficit is signed int

The fq_codel class stat deficit is a signed int.  This is a regression
from when JSON output was added.

Fixes: 997f2dc19378 ("tc: Add JSON output of fq_codel stats")
Signed-off-by: Benjamin Lee <ben@b1c1l1.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/q_fq_codel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/q_fq_codel.c b/tc/q_fq_codel.c
index efed4d28..1c6cf1e0 100644
--- a/tc/q_fq_codel.c
+++ b/tc/q_fq_codel.c
@@ -264,7 +264,7 @@ static int fq_codel_print_xstats(struct qdisc_util *qu, FILE *f,
 			st->qdisc_stats.old_flows_len);
 	}
 	if (st->type == TCA_FQ_CODEL_XSTATS_CLASS) {
-		print_uint(PRINT_ANY, "deficit", "  deficit %u",
+		print_int(PRINT_ANY, "deficit", "  deficit %d",
 			st->class_stats.deficit);
 		print_uint(PRINT_ANY, "count", " count %u",
 			st->class_stats.count);

From 48e05899d0b73429ceca05c08e8ae3eac8fd1eba Mon Sep 17 00:00:00 2001
From: Brian Norris <briannorris@chromium.org>
Date: Tue, 7 Apr 2020 10:43:05 -0700
Subject: [PATCH 05/25] man: add ip-netns(8) as generation target

Prepare for adding new variable substitutions. Unify the sed rules while
we're at it, since there's no need to write this out 4 times.

Signed-off-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 man/man8/.gitignore                    |  1 +
 man/man8/Makefile                      | 10 ++--------
 man/man8/{ip-netns.8 => ip-netns.8.in} |  0
 3 files changed, 3 insertions(+), 8 deletions(-)
 rename man/man8/{ip-netns.8 => ip-netns.8.in} (100%)

diff --git a/man/man8/.gitignore b/man/man8/.gitignore
index 0c3d1504..7b08e911 100644
--- a/man/man8/.gitignore
+++ b/man/man8/.gitignore
@@ -1,4 +1,5 @@
 # these pages are built
 ip-address.8
 ip-link.8
+ip-netns.8
 ip-route.8
diff --git a/man/man8/Makefile b/man/man8/Makefile
index 0269e174..9c623123 100644
--- a/man/man8/Makefile
+++ b/man/man8/Makefile
@@ -1,17 +1,11 @@
 # SPDX-License-Identifier: GPL-2.0
-TARGETS = ip-address.8 ip-link.8 ip-route.8
+TARGETS = ip-address.8 ip-link.8 ip-netns.8 ip-route.8
 
 MAN8PAGES = $(TARGETS) $(filter-out $(TARGETS),$(wildcard *.8))
 
 all: $(TARGETS)
 
-ip-address.8: ip-address.8.in
-	sed "s|@SYSCONFDIR@|$(CONFDIR)|g" $< > $@
-
-ip-link.8: ip-link.8.in
-	sed "s|@SYSCONFDIR@|$(CONFDIR)|g" $< > $@
-
-ip-route.8: ip-route.8.in
+%: %.in
 	sed "s|@SYSCONFDIR@|$(CONFDIR)|g" $< > $@
 
 distclean: clean
diff --git a/man/man8/ip-netns.8 b/man/man8/ip-netns.8.in
similarity index 100%
rename from man/man8/ip-netns.8
rename to man/man8/ip-netns.8.in

From 8b9d5728c1d6919f68f9386845ae37575dbdf627 Mon Sep 17 00:00:00 2001
From: Brian Norris <briannorris@chromium.org>
Date: Tue, 7 Apr 2020 10:43:06 -0700
Subject: [PATCH 06/25] man: replace $(NETNS_ETC_DIR) and $(NETNS_RUN_DIR) in
 ip-netns(8)

These can be configured to different paths. Reflect that in the
generated documentation.

Signed-off-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 man/man8/Makefile      |  6 +++++-
 man/man8/ip-netns.8.in | 20 ++++++++++----------
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/man/man8/Makefile b/man/man8/Makefile
index 9c623123..b1fd87bd 100644
--- a/man/man8/Makefile
+++ b/man/man8/Makefile
@@ -6,7 +6,11 @@ MAN8PAGES = $(TARGETS) $(filter-out $(TARGETS),$(wildcard *.8))
 all: $(TARGETS)
 
 %: %.in
-	sed "s|@SYSCONFDIR@|$(CONFDIR)|g" $< > $@
+	sed \
+		-e "s|@NETNS_ETC_DIR@|$(NETNS_ETC_DIR)|g" \
+		-e "s|@NETNS_RUN_DIR@|$(NETNS_RUN_DIR)|g" \
+		-e "s|@SYSCONFDIR@|$(CONFDIR)|g" \
+		$< > $@
 
 distclean: clean
 
diff --git a/man/man8/ip-netns.8.in b/man/man8/ip-netns.8.in
index c75917da..2911bdd3 100644
--- a/man/man8/ip-netns.8.in
+++ b/man/man8/ip-netns.8.in
@@ -61,9 +61,9 @@ By default a process inherits its network namespace from its parent. Initially a
 the processes share the same default network namespace from the init process.
 
 By convention a named network namespace is an object at
-.BR "/var/run/netns/" NAME
+.BR "@NETNS_RUN_DIR@/" NAME
 that can be opened. The file descriptor resulting from opening
-.BR "/var/run/netns/" NAME
+.BR "@NETNS_RUN_DIR@/" NAME
 refers to the specified network namespace. Holding that file
 descriptor open keeps the network namespace alive. The file
 descriptor can be used with the
@@ -72,13 +72,13 @@ system call to change the network namespace associated with a task.
 
 For applications that are aware of network namespaces, the convention
 is to look for global network configuration files first in
-.BR "/etc/netns/" NAME "/"
+.BR "@NETNS_ETC_DIR@/" NAME "/"
 then in
 .BR "/etc/".
 For example, if you want a different version of
 .BR /etc/resolv.conf
 for a network namespace used to isolate your vpn you would name it
-.BR /etc/netns/myvpn/resolv.conf.
+.BR @NETNS_ETC_DIR@/myvpn/resolv.conf.
 
 .B ip netns exec
 automates handling of this configuration, file convention for network
@@ -89,24 +89,24 @@ their traditional location in /etc.
 .TP
 .B ip netns list - show all of the named network namespaces
 .sp
-This command displays all of the network namespaces in /var/run/netns
+This command displays all of the network namespaces in @NETNS_RUN_DIR@
 
 .TP
 .B ip netns add NAME - create a new named network namespace
 .sp
-If NAME is available in /var/run/netns/ this command creates a new
+If NAME is available in @NETNS_RUN_DIR@ this command creates a new
 network namespace and assigns NAME.
 
 .TP
 .B ip netns attach NAME PID - create a new named network namespace
 .sp
-If NAME is available in /var/run/netns/ this command attaches the network
+If NAME is available in @NETNS_RUN_DIR@ this command attaches the network
 namespace of the process PID to NAME as if it were created with ip netns.
 
 .TP
 .B ip [-all] netns delete [ NAME ] - delete the name of a network namespace(s)
 .sp
-If NAME is present in /var/run/netns it is umounted and the mount
+If NAME is present in @NETNS_RUN_DIR@ it is umounted and the mount
 point is removed. If this is the last user of the network namespace the
 network namespace will be freed and all physical devices will be moved to the
 default one, otherwise the network namespace persists until it has no more
@@ -160,7 +160,7 @@ Once it is assigned, it's not possible to change it.
 .TP
 .B ip netns identify [PID] - Report network namespaces names for process
 .sp
-This command walks through /var/run/netns and finds all the network
+This command walks through @NETNS_RUN_DIR@ and finds all the network
 namespace names for network namespace of the specified process, if PID is
 not specified then the current process will be used.
 
@@ -201,7 +201,7 @@ and prints a line for each event it sees.
 .sp
 Network namespace ids are used to identify a peer network namespace. This
 command displays nsids of the current network namespace and provides the
-corresponding iproute2 netns name (from /var/run/netns) if any.
+corresponding iproute2 netns name (from @NETNS_RUN_DIR@) if any.
 
 The
 .B target-nsid

From 706f7d35e2cb9cdc935db3b7785b305029ffd736 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastien=20Roucari=C3=A8s?= <rouca@debian.org>
Date: Mon, 13 Apr 2020 01:50:33 +0200
Subject: [PATCH 07/25] Better documentation of mcast_to_unicast option
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This option is useful for Wifi bridge but need some tweak.

Document it from kernel patches documentation

Signed-off-by: Bastien Roucariès <rouca@debian.org>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 man/man8/bridge.8 | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index b9bd6bc5..ff6f6f37 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -383,6 +383,32 @@ there is no MDB entry. By default this flag is on.
 Controls whether a given port will replicate packets using unicast
 instead of multicast. By default this flag is off.
 
+This is done by copying the packet per host and
+changing the multicast destination MAC to a unicast one accordingly.
+
+.BR mcast_to_unicast
+works on top of the multicast snooping feature of
+the bridge. Which means unicast copies are only delivered to hosts which
+are interested in it and signalized this via IGMP/MLD reports
+previously.
+
+This feature is intended for interface types which have a more reliable
+and/or efficient way to deliver unicast packets than broadcast ones
+(e.g. WiFi).
+
+However, it should only be enabled on interfaces where no IGMPv2/MLDv1
+report suppression takes place. IGMP/MLD report suppression issue is usually
+overcome by the network daemon (supplicant) enabling AP isolation and
+by that separating all STAs.
+
+Delivery of STA-to-STA IP mulitcast is made possible again by
+enabling and utilizing the bridge hairpin mode, which considers the
+incoming port as a potential outgoing port, too (see
+.B hairpin
+option).
+Hairpin mode is performed after multicast snooping, therefore leading to
+only deliver reports to STAs running a multicast router.
+
 .TP
 .BR "neigh_suppress on " or " neigh_suppress off "
 Controls whether neigh discovery (arp and nd) proxy and suppression is

From 1cad8f8d78fd10d4bf86962ceb3b1e092fe9ef15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastien=20Roucari=C3=A8s?= <rouca@debian.org>
Date: Mon, 13 Apr 2020 01:50:34 +0200
Subject: [PATCH 08/25] Improve hairpin mode description
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Mention VEPA and reflective relay.

Signed-off-by: Bastien Roucariès <rouca@debian.org>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 man/man8/bridge.8 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index ff6f6f37..584324b5 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -332,7 +332,9 @@ cause the port to stop processing STP BPDUs.
 .TP
 .BR "hairpin on " or " hairpin off "
 Controls whether traffic may be send back out of the port on which it was
-received. By default, this flag is turned off and the bridge will not forward
+received. This option is also called reflective relay mode, and is used to support
+basic VEPA (Virtual Ethernet Port Aggregator) capabilities.
+By default, this flag is turned off and the bridge will not forward
 traffic back out of the receiving port.
 
 .TP

From 420febf9611d5cb4061055913041be00859210da Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastien=20Roucari=C3=A8s?= <rouca@debian.org>
Date: Mon, 13 Apr 2020 01:50:35 +0200
Subject: [PATCH 09/25] Document BPDU filter option

Disabled state is also BPDU filter

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 man/man8/bridge.8 | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index 584324b5..bd33635a 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -293,32 +293,45 @@ droot port selectio algorithms.
 
 .TP
 .BI state " STATE "
-the operation state of the port. This is primarily used by user space STP/RSTP
+the operation state of the port. Except state 0 (disabled),
+this is primarily used by user space STP/RSTP
 implementation. One may enter a lowercased port state name, or one of the
 numbers below. Negative inputs are ignored, and unrecognized names return an
 error.
 
 .B 0
-- port is DISABLED. Make this port completely inactive.
+- port is in
+.B DISABLED
+state. Make this port completely inactive. This is also called
+BPDU filter and could be used to disable STP on an untrusted port, like
+a leaf virtual devices.
 .sp
 
 .B 1
-- STP LISTENING state. Only valid if STP is enabled on the bridge. In this
+- STP
+.B LISTENING
+state. Only valid if STP is enabled on the bridge. In this
 state the port listens for STP BPDUs and drops all other traffic frames.
 .sp
 
 .B 2
-- STP LEARNING state. Only valid if STP is enabled on the bridge. In this
+- STP
+.B LEARNING
+state. Only valid if STP is enabled on the bridge. In this
 state the port will accept traffic only for the purpose of updating MAC
 address tables.
 .sp
 
 .B 3
-- STP FORWARDING state. Port is fully active.
+- STP
+.B FORWARDING
+state. Port is fully active.
 .sp
 
 .B 4
-- STP BLOCKING state. Only valid if STP is enabled on the bridge. This state
+- STP
+.B BLOCKING
+state. Only valid if STP is enabled on the bridge. This state
 is used during the STP election process. In this state, port will only process
 STP BPDUs.
 .sp

From 19bbebc4590ef0322b8358d63957b32faa9345d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastien=20Roucari=C3=A8s?= <rouca@debian.org>
Date: Mon, 13 Apr 2020 01:50:36 +0200
Subject: [PATCH 10/25] Better documentation of BDPU guard
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Document that guard disable the port and how to reenable it

Signed-off-by: Bastien Roucariès <rouca@debian.org>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 man/man8/bridge.8 | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index bd33635a..9bfd942f 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -340,7 +340,18 @@ STP BPDUs.
 .BR "guard on " or " guard off "
 Controls whether STP BPDUs will be processed by the bridge port. By default,
 the flag is turned off allowed BPDU processing. Turning this flag on will
-cause the port to stop processing STP BPDUs.
+disables
+the bridge port if a STP BPDU packet is received.
+
+If running Spanning Tree on bridge, hostile devices on the network
+may send BPDU on a port and cause network failure. Setting
+.B guard on
+will detect and stop this by disabling the port.
+The port will be restarted if link is brought down, or
+removed and reattached.  For example if guard is enable on
+eth0:
+
+.B ip link set dev eth0 down; ip link set dev eth0 up
 
 .TP
 .BR "hairpin on " or " hairpin off "

From 498883a00f522f812cb9d38802c145a5828d7df7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastien=20Roucari=C3=A8s?= <rouca@debian.org>
Date: Mon, 13 Apr 2020 01:50:37 +0200
Subject: [PATCH 11/25] Document root_block option
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Root_block is also called root port guard, document it.

Signed-off-by: Bastien Roucariès <rouca@debian.org>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 man/man8/bridge.8 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index 9bfd942f..ff6a5cc9 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -372,6 +372,11 @@ enabled on the bridge. By default the flag is off.
 Controls whether a given port is allowed to become root port or not. Only used
 when STP is enabled on the bridge. By default the flag is off.
 
+This feature is also called root port guard.
+If BPDU is received from a leaf (edge) port, it should not
+be elected as root port. This could be used if using STP on a bridge and the downstream bridges are not fully
+trusted; this prevents a hostile guest from rerouting traffic.
+
 .TP
 .BR "learning on " or " learning off "
 Controls whether a given port will learn MAC addresses from received traffic or

From 8d5d91fd5874744abb685a7cd980a4f0b01be9af Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastien=20Roucari=C3=A8s?= <rouca@debian.org>
Date: Mon, 13 Apr 2020 01:50:38 +0200
Subject: [PATCH 12/25] State of bridge STP port are now case insensitive
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Improve use experience

Signed-off-by: Bastien Roucariès <rouca@debian.org>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 bridge/link.c     |  2 +-
 man/man8/bridge.8 | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/bridge/link.c b/bridge/link.c
index 074edf00..3bc7af20 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -378,7 +378,7 @@ static int brlink_modify(int argc, char **argv)
 			state = strtol(*argv, &endptr, 10);
 			if (!(**argv != '\0' && *endptr == '\0')) {
 				for (state = 0; state < nstates; state++)
-					if (strcmp(port_states[state], *argv) == 0)
+					if (strcasecmp(port_states[state], *argv) == 0)
 						break;
 				if (state == nstates) {
 					fprintf(stderr,
diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index ff6a5cc9..5efbd466 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -293,29 +293,29 @@ droot port selectio algorithms.
 
 .TP
 .BI state " STATE "
-the operation state of the port. Except state 0 (disabled),
+the operation state of the port. Except state 0 (disable STP or BPDU filter feature),
 this is primarily used by user space STP/RSTP
-implementation. One may enter a lowercased port state name, or one of the
+implementation. One may enter port state name (case insensitive), or one of the
 numbers below. Negative inputs are ignored, and unrecognized names return an
 error.
 
 .B 0
-- port is in
+- port is in STP
 .B DISABLED
-state. Make this port completely inactive. This is also called
+state. Make this port completely inactive for STP. This is also called
 BPDU filter and could be used to disable STP on an untrusted port, like
 a leaf virtual devices.
 .sp
 
 .B 1
-- STP
+- port is in STP
 .B LISTENING
 state. Only valid if STP is enabled on the bridge. In this
 state the port listens for STP BPDUs and drops all other traffic frames.
 .sp
 
 .B 2
-- STP
+- port is in STP
 .B LEARNING
 state. Only valid if STP is enabled on the bridge. In this
 state the port will accept traffic only for the purpose of updating MAC
@@ -323,13 +323,13 @@ address tables.
 .sp
 
 .B 3
-- STP
+- port is in STP
 .B FORWARDING
 state. Port is fully active.
 .sp
 
 .B 4
-- STP
+- port is in STP
 .B BLOCKING
 state. Only valid if STP is enabled on the bridge. This state
 is used during the STP election process. In this state, port will only process

From b831c5ffcc78a518dcd267717a299ce158b0475c Mon Sep 17 00:00:00 2001
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 20 Apr 2020 09:48:57 -0700
Subject: [PATCH 13/25] bridge: man page spelling fixes

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 man/man8/bridge.8 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index 5efbd466..71f2e890 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -289,7 +289,7 @@ the STP path cost of the specified port.
 .BI priority " PRIO "
 the STP port priority. The priority value is an unsigned 8-bit quantity
 (number between 0 and 255). This metric is used in the designated port an
-droot port selectio algorithms.
+droot port selection algorithms.
 
 .TP
 .BI state " STATE "
@@ -432,7 +432,7 @@ report suppression takes place. IGMP/MLD report suppression issue is usually
 overcome by the network daemon (supplicant) enabling AP isolation and
 by that separating all STAs.
 
-Delivery of STA-to-STA IP mulitcast is made possible again by
+Delivery of STA-to-STA IP multicast is made possible again by
 enabling and utilizing the bridge hairpin mode, which considers the
 incoming port as a potential outgoing port, too (see
 .B hairpin
@@ -529,7 +529,7 @@ the interface to which this address is associated.
 .B router
 - the destination address is associated with a router.
 Valid if the referenced device is a VXLAN type device and has
-route shortcircuit enabled.
+route short circuit enabled.
 .sp
 
 .B use

From 0149dabf2a1bad2f210ca2d987b29083247b7bd0 Mon Sep 17 00:00:00 2001
From: Jiri Pirko <jiri@mellanox.com>
Date: Mon, 27 Apr 2020 08:10:55 +0200
Subject: [PATCH 14/25] tc: m_action: check cookie hex string len

Check the cookie hex string len is dividable by 2 as the valid hex
string always should be.

Reported-by: Alex Kushnarov <alexanderk@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/m_action.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tc/m_action.c b/tc/m_action.c
index 108329db..b41782de 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -291,7 +291,8 @@ done0:
 					invarg(cookie_err_m, *argv);
 				}
 
-				if (hex2mem(*argv, act_ck, slen / 2) < 0)
+				if (slen % 2 ||
+				    hex2mem(*argv, act_ck, slen / 2) < 0)
 					invarg("cookie must be a hex string\n",
 					       *argv);
 

From d27fc6390ce32ecdba6324e22b1c341791c5c63f Mon Sep 17 00:00:00 2001
From: Xin Long <lucien.xin@gmail.com>
Date: Mon, 27 Apr 2020 15:14:24 +0800
Subject: [PATCH 15/25] xfrm: also check for ipv6 state in xfrm_state_keep

As commit f9d696cf414c ("xfrm: not try to delete ipcomp states when using
deleteall") does, this patch is to fix the same issue for ip6 state where
xsinfo->id.proto == IPPROTO_IPV6.

  # ip xfrm state add src 2000::1 dst 2000::2 spi 0x1000 \
    proto comp comp deflate mode tunnel sel src 2000::1 dst \
    2000::2 proto gre
  # ip xfrm sta deleteall
  Failed to send delete-all request
  : Operation not permitted

Note that the xsinfo->proto in common states can never be IPPROTO_IPV6.

Fixes: f9d696cf414c ("xfrm: not try to delete ipcomp states when using deleteall")
Reported-by: Xiumei Mu <xmu@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Andrea Claudi <aclaudi@redhat.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 ip/xfrm_state.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
index d68f600a..f4bf3356 100644
--- a/ip/xfrm_state.c
+++ b/ip/xfrm_state.c
@@ -1131,7 +1131,8 @@ static int xfrm_state_keep(struct nlmsghdr *n, void *arg)
 	if (!xfrm_state_filter_match(xsinfo))
 		return 0;
 
-	if (xsinfo->id.proto == IPPROTO_IPIP)
+	if (xsinfo->id.proto == IPPROTO_IPIP ||
+	    xsinfo->id.proto == IPPROTO_IPV6)
 		return 0;
 
 	if (xb->offset > xb->size) {

From 7868f802e2d93a9f6e2d64a6fc608445a33c8a8d Mon Sep 17 00:00:00 2001
From: Eric Dumazet <edumazet@google.com>
Date: Mon, 27 Apr 2020 10:51:55 -0700
Subject: [PATCH 16/25] tc: fq_codel: add drop_batch parameter

Commit 9d18562a2278 ("fq_codel: add batch ability to fq_codel_drop()")
added the new TCA_FQ_CODEL_DROP_BATCH_SIZE parameter, set by default to 64.

Add to tc command the ability to get/set the drop_batch

Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/q_fq_codel.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tc/q_fq_codel.c b/tc/q_fq_codel.c
index 1c6cf1e0..1a51302e 100644
--- a/tc/q_fq_codel.c
+++ b/tc/q_fq_codel.c
@@ -54,12 +54,14 @@ static void explain(void)
 					"[ memory_limit BYTES ]\n"
 					"[ target TIME ] [ interval TIME ]\n"
 					"[ quantum BYTES ] [ [no]ecn ]\n"
-					"[ ce_threshold TIME ]\n");
+					"[ ce_threshold TIME ]\n"
+					"[ drop_batch SIZE ]\n");
 }
 
 static int fq_codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 			      struct nlmsghdr *n, const char *dev)
 {
+	unsigned int drop_batch = 0;
 	unsigned int limit = 0;
 	unsigned int flows = 0;
 	unsigned int target = 0;
@@ -89,6 +91,12 @@ static int fq_codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 				fprintf(stderr, "Illegal \"quantum\"\n");
 				return -1;
 			}
+		} else if (strcmp(*argv, "drop_batch") == 0) {
+			NEXT_ARG();
+			if (get_unsigned(&drop_batch, *argv, 0)) {
+				fprintf(stderr, "Illegal \"drop_batch\"\n");
+				return -1;
+			}
 		} else if (strcmp(*argv, "target") == 0) {
 			NEXT_ARG();
 			if (get_time(&target, *argv)) {
@@ -147,6 +155,8 @@ static int fq_codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	if (memory != ~0U)
 		addattr_l(n, 1024, TCA_FQ_CODEL_MEMORY_LIMIT,
 			  &memory, sizeof(memory));
+	if (drop_batch)
+		addattr_l(n, 1024, TCA_FQ_CODEL_DROP_BATCH_SIZE, &drop_batch, sizeof(drop_batch));
 
 	addattr_nest_end(n, tail);
 	return 0;
@@ -163,6 +173,7 @@ static int fq_codel_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt
 	unsigned int quantum;
 	unsigned int ce_threshold;
 	unsigned int memory_limit;
+	unsigned int drop_batch;
 
 	SPRINT_BUF(b1);
 
@@ -220,6 +231,12 @@ static int fq_codel_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt
 		if (ecn)
 			print_bool(PRINT_ANY, "ecn", "ecn ", true);
 	}
+	if (tb[TCA_FQ_CODEL_DROP_BATCH_SIZE] &&
+	    RTA_PAYLOAD(tb[TCA_FQ_CODEL_DROP_BATCH_SIZE]) >= sizeof(__u32)) {
+		drop_batch = rta_getattr_u32(tb[TCA_FQ_CODEL_DROP_BATCH_SIZE]);
+		if (drop_batch)
+			print_uint(PRINT_ANY, "drop_batch", "drop_batch %u ", drop_batch);
+	}
 
 	return 0;
 }

From be9ca9d541232112ed3a878ddfe0a5829bf0e887 Mon Sep 17 00:00:00 2001
From: Eric Dumazet <edumazet@google.com>
Date: Mon, 27 Apr 2020 11:05:43 -0700
Subject: [PATCH 17/25] tc: fq: add timer_slack parameter

Commit 583396f4ca4d ("net_sched: sch_fq: enable use of hrtimer slack")
added TCA_FQ_TIMER_SLACK parameter, with a default value of 10 usec.

Add the corresponding tc support to get/set this tunable.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/q_fq.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tc/q_fq.c b/tc/q_fq.c
index 44d8a7e0..ffae0523 100644
--- a/tc/q_fq.c
+++ b/tc/q_fq.c
@@ -57,6 +57,7 @@ static void explain(void)
 		"		[ [no]pacing ] [ refill_delay TIME ]\n"
 		"		[ low_rate_threshold RATE ]\n"
 		"		[ orphan_mask MASK]\n"
+		"		[ timer_slack TIME]\n"
 		"		[ ce_threshold TIME ]\n");
 }
 
@@ -86,6 +87,7 @@ static int fq_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	unsigned int refill_delay;
 	unsigned int orphan_mask;
 	unsigned int ce_threshold;
+	unsigned int timer_slack;
 	bool set_plimit = false;
 	bool set_flow_plimit = false;
 	bool set_quantum = false;
@@ -96,6 +98,7 @@ static int fq_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	bool set_orphan_mask = false;
 	bool set_low_rate_threshold = false;
 	bool set_ce_threshold = false;
+	bool set_timer_slack = false;
 	int pacing = -1;
 	struct rtattr *tail;
 
@@ -146,6 +149,20 @@ static int fq_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 				return -1;
 			}
 			set_ce_threshold = true;
+		} else if (strcmp(*argv, "timer_slack") == 0) {
+			__s64 t64;
+
+			NEXT_ARG();
+			if (get_time64(&t64, *argv)) {
+				fprintf(stderr, "Illegal \"timer_slack\"\n");
+				return -1;
+			}
+			timer_slack = t64;
+			if (timer_slack != t64) {
+				fprintf(stderr, "Illegal (out of range) \"timer_slack\"\n");
+				return -1;
+			}
+			set_timer_slack = true;
 		} else if (strcmp(*argv, "defrate") == 0) {
 			NEXT_ARG();
 			if (strchr(*argv, '%')) {
@@ -240,6 +257,9 @@ static int fq_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	if (set_ce_threshold)
 		addattr_l(n, 1024, TCA_FQ_CE_THRESHOLD,
 			  &ce_threshold, sizeof(ce_threshold));
+    if (set_timer_slack)
+		addattr_l(n, 1024, TCA_FQ_TIMER_SLACK,
+			  &timer_slack, sizeof(timer_slack));
 	addattr_nest_end(n, tail);
 	return 0;
 }
@@ -254,6 +274,7 @@ static int fq_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 	unsigned int refill_delay;
 	unsigned int orphan_mask;
 	unsigned int ce_threshold;
+	unsigned int timer_slack;
 
 	SPRINT_BUF(b1);
 
@@ -355,6 +376,12 @@ static int fq_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 		}
 	}
 
+	if (tb[TCA_FQ_TIMER_SLACK] &&
+	    RTA_PAYLOAD(tb[TCA_FQ_TIMER_SLACK]) >= sizeof(__u32)) {
+		timer_slack = rta_getattr_u32(tb[TCA_FQ_TIMER_SLACK]);
+		fprintf(f, "timer_slack %s ", sprint_time64(timer_slack, b1));
+	}
+
 	return 0;
 }
 

From 2b93f66863a91251e6e974de05d7ea5d7fba8949 Mon Sep 17 00:00:00 2001
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Wed, 29 Apr 2020 22:30:48 -0700
Subject: [PATCH 18/25] uapi: update bpf.h

Minor spelling in comment
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 include/uapi/linux/bpf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index c7b2ffb2..60684b7b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1642,7 +1642,7 @@ union bpf_attr {
  * 		ifindex, but doesn't require a map to do so.
  * 	Return
  * 		**XDP_REDIRECT** on success, or the value of the two lower bits
- * 		of the **flags* argument on error.
+ * 		of the *flags* argument on error.
  *
  * int bpf_sk_redirect_map(struct sk_buff *skb, struct bpf_map *map, u32 key, u64 flags)
  * 	Description

From 7ae84fedcb15815a7488558b0b319cffdd12a565 Mon Sep 17 00:00:00 2001
From: Jacob Keller <jacob.e.keller@intel.com>
Date: Tue, 28 Apr 2020 13:43:24 -0700
Subject: [PATCH 19/25] devlink: add support for DEVLINK_CMD_REGION_NEW

Add support to request that a new snapshot be taken immediately for
a devlink region. To avoid confusion, the desired snapshot id must be
provided.

Note that if a region does not support snapshots on demand, the kernel
will reject the request with -EOPNOTSUP.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 devlink/devlink.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index f67fe6dd..16602abf 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -6464,10 +6464,27 @@ static int cmd_region_read(struct dl *dl)
 	return err;
 }
 
+static int cmd_region_snapshot_new(struct dl *dl)
+{
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = mnlg_msg_prepare(dl->nlg, DEVLINK_CMD_REGION_NEW,
+			       NLM_F_REQUEST | NLM_F_ACK);
+
+	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE_REGION |
+				DL_OPT_REGION_SNAPSHOT_ID, 0);
+	if (err)
+		return err;
+
+	return _mnlg_socket_sndrcv(dl->nlg, nlh, NULL, NULL);
+}
+
 static void cmd_region_help(void)
 {
 	pr_err("Usage: devlink region show [ DEV/REGION ]\n");
 	pr_err("       devlink region del DEV/REGION snapshot SNAPSHOT_ID\n");
+	pr_err("       devlink region new DEV/REGION snapshot SNAPSHOT_ID\n");
 	pr_err("       devlink region dump DEV/REGION [ snapshot SNAPSHOT_ID ]\n");
 	pr_err("       devlink region read DEV/REGION [ snapshot SNAPSHOT_ID ] address ADDRESS length LENGTH\n");
 }
@@ -6491,6 +6508,9 @@ static int cmd_region(struct dl *dl)
 	} else if (dl_argv_match(dl, "read")) {
 		dl_arg_inc(dl);
 		return cmd_region_read(dl);
+	} else if (dl_argv_match(dl, "new")) {
+		dl_arg_inc(dl);
+		return cmd_region_snapshot_new(dl);
 	}
 	pr_err("Command \"%s\" not found\n", dl_argv(dl));
 	return -ENOENT;

From 594b2d7799041f3962b5a9cf620ca8c90d7860d6 Mon Sep 17 00:00:00 2001
From: Benjamin Poirier <bpoirier@cumulusnetworks.com>
Date: Fri, 1 May 2020 17:47:15 +0900
Subject: [PATCH 20/25] bridge: Use consistent column names in vlan output

Fix singular vs plural. Add a hyphen to clarify that each of those are
single fields.

Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 bridge/vlan.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/bridge/vlan.c b/bridge/vlan.c
index 205851e4..ac0796f6 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -538,9 +538,9 @@ static int vlan_show(int argc, char **argv, int subject)
 		}
 
 		if (!is_json_context()) {
-			printf("port\tvlan ids");
+			printf("port\tvlan-id");
 			if (subject == VLAN_SHOW_TUNNELINFO)
-				printf("\ttunnel id");
+				printf("\ttunnel-id");
 			printf("\n");
 		}
 
@@ -559,7 +559,7 @@ static int vlan_show(int argc, char **argv, int subject)
 		}
 
 		if (!is_json_context())
-			printf("%-16s vlan id\n", "port");
+			printf("%-16s vlan-id\n", "port");
 
 		if (rtnl_dump_filter(&rth, print_vlan_stats, stdout) < 0) {
 			fprintf(stderr, "Dump terminated\n");

From 91b1b49ed3934eb423308130bb3365a0afebf5af Mon Sep 17 00:00:00 2001
From: Benjamin Poirier <bpoirier@cumulusnetworks.com>
Date: Fri, 1 May 2020 17:47:16 +0900
Subject: [PATCH 21/25] bridge: Fix typo

Fixes: 7abf5de677e3 ("bridge: vlan: add support to display per-vlan statistics")
Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 bridge/vlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bridge/vlan.c b/bridge/vlan.c
index ac0796f6..37ff2973 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -546,7 +546,7 @@ static int vlan_show(int argc, char **argv, int subject)
 
 		ret = rtnl_dump_filter(&rth, print_vlan, &subject);
 		if (ret < 0) {
-			fprintf(stderr, "Dump ternminated\n");
+			fprintf(stderr, "Dump terminated\n");
 			exit(1);
 		}
 	} else {

From b262a9becbcb9d0816b7953fd223dd9a7add8e12 Mon Sep 17 00:00:00 2001
From: Benjamin Poirier <bpoirier@cumulusnetworks.com>
Date: Fri, 1 May 2020 17:47:17 +0900
Subject: [PATCH 22/25] bridge: Fix output with empty vlan lists

Consider this configuration:

ip link add br0 type bridge
ip link add vx0 type vxlan dstport 4789 external
ip link set dev vx0 master br0
bridge vlan del vid 1 dev vx0
ip link add vx1 type vxlan dstport 4790 external
ip link set dev vx1 master br0

	root@vsid:/src/iproute2# ./bridge/bridge vlan
	port    vlan-id
	br0      1 PVID Egress Untagged

	vx0     None
	vx1      1 PVID Egress Untagged

	root@vsid:/src/iproute2#

Note the useless and inconsistent empty lines.

	root@vsid:/src/iproute2# ./bridge/bridge vlan tunnelshow
	port    vlan-id tunnel-id
	br0
	vx0     None
	vx1

What's the difference between "None" and ""?

	root@vsid:/src/iproute2# ./bridge/bridge -j -p vlan tunnelshow
	[ {
		"ifname": "br0",
		"tunnels": [ ]
	    },{
		"ifname": "vx1",
		"tunnels": [ ]
	    } ]

Why does vx0 appear in normal output and not json output?
Why output an empty list for br0 and vx1?

Fix these inconsistencies and avoid outputting entries with no values. This
makes the behavior consistent with other iproute2 commands, for example
`ip -6 addr`: if an interface doesn't have any ipv6 addresses, it is not
part of the listing.

Fixes: 8652eeb3ab12 ("bridge: vlan: support for per vlan tunnel info")
Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 bridge/vlan.c                            | 36 +++++++++++++-----------
 testsuite/tests/bridge/vlan/show.t       | 30 ++++++++++++++++++++
 testsuite/tests/bridge/vlan/tunnelshow.t |  2 +-
 3 files changed, 50 insertions(+), 18 deletions(-)
 create mode 100755 testsuite/tests/bridge/vlan/show.t

diff --git a/bridge/vlan.c b/bridge/vlan.c
index 37ff2973..b126fae2 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -290,8 +290,7 @@ static void print_vlan_tunnel_info(struct rtattr *tb, int ifindex)
 	int rem = RTA_PAYLOAD(list);
 	__u16 last_vid_start = 0;
 	__u32 last_tunid_start = 0;
-
-	open_vlan_port(ifindex, "%s", VLAN_SHOW_TUNNELINFO);
+	bool opened = false;
 
 	for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
 		struct rtattr *ttb[IFLA_BRIDGE_VLAN_TUNNEL_MAX+1];
@@ -331,13 +330,20 @@ static void print_vlan_tunnel_info(struct rtattr *tb, int ifindex)
 		else if (vcheck_ret == 0)
 			continue;
 
+		if (!opened) {
+			open_vlan_port(ifindex, "%s", VLAN_SHOW_TUNNELINFO);
+			opened = true;
+		}
+
 		open_json_object(NULL);
 		print_range("vlan", last_vid_start, tunnel_vid);
 		print_range("tunid", last_tunid_start, tunnel_id);
 		close_json_object();
 		print_string(PRINT_FP, NULL, "%s", _SL_);
 	}
-	close_vlan_port();
+
+	if (opened)
+		close_vlan_port();
 }
 
 static int print_vlan(struct nlmsghdr *n, void *arg)
@@ -366,16 +372,8 @@ static int print_vlan(struct nlmsghdr *n, void *arg)
 		return 0;
 
 	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifm), len);
-
-	/* if AF_SPEC isn't there, vlan table is not preset for this port */
-	if (!tb[IFLA_AF_SPEC]) {
-		if (!filter_vlan && !is_json_context()) {
-			color_fprintf(stdout, COLOR_IFNAME, "%s",
-				      ll_index_to_name(ifm->ifi_index));
-			fprintf(stdout, "\tNone\n");
-		}
+	if (!tb[IFLA_AF_SPEC])
 		return 0;
-	}
 
 	switch (*subject) {
 	case VLAN_SHOW_VLAN:
@@ -385,9 +383,7 @@ static int print_vlan(struct nlmsghdr *n, void *arg)
 		print_vlan_tunnel_info(tb[IFLA_AF_SPEC], ifm->ifi_index);
 		break;
 	}
-	print_string(PRINT_FP, NULL, "%s", _SL_);
 
-	fflush(stdout);
 	return 0;
 }
 
@@ -588,8 +584,7 @@ void print_vlan_info(struct rtattr *tb, int ifindex)
 	struct rtattr *i, *list = tb;
 	int rem = RTA_PAYLOAD(list);
 	__u16 last_vid_start = 0;
-
-	open_vlan_port(ifindex, "%s", VLAN_SHOW_VLAN);
+	bool opened = false;
 
 	for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
 		struct bridge_vlan_info *vinfo;
@@ -608,6 +603,11 @@ void print_vlan_info(struct rtattr *tb, int ifindex)
 		else if (vcheck_ret == 0)
 			continue;
 
+		if (!opened) {
+			open_vlan_port(ifindex, "%s", VLAN_SHOW_VLAN);
+			opened = true;
+		}
+
 		open_json_object(NULL);
 		print_range("vlan", last_vid_start, vinfo->vid);
 
@@ -615,7 +615,9 @@ void print_vlan_info(struct rtattr *tb, int ifindex)
 		close_json_object();
 		print_string(PRINT_FP, NULL, "%s", _SL_);
 	}
-	close_vlan_port();
+
+	if (opened)
+		close_vlan_port();
 }
 
 int do_vlan(int argc, char **argv)
diff --git a/testsuite/tests/bridge/vlan/show.t b/testsuite/tests/bridge/vlan/show.t
new file mode 100755
index 00000000..3def2022
--- /dev/null
+++ b/testsuite/tests/bridge/vlan/show.t
@@ -0,0 +1,30 @@
+#!/bin/sh
+
+. lib/generic.sh
+
+ts_log "[Testing vlan show]"
+
+BR_DEV="$(rand_dev)"
+VX0_DEV="$(rand_dev)"
+VX1_DEV="$(rand_dev)"
+
+ts_ip "$0" "Add $BR_DEV bridge interface" link add $BR_DEV type bridge
+
+ts_ip "$0" "Add $VX0_DEV vxlan interface" \
+	link add $VX0_DEV type vxlan dstport 4789 external
+ts_ip "$0" "Enslave $VX0_DEV under $BR_DEV" \
+	link set dev $VX0_DEV master $BR_DEV
+ts_bridge "$0" "Delete default vlan from $VX0_DEV" \
+	vlan del dev $VX0_DEV vid 1
+ts_ip "$0" "Add $VX1_DEV vxlan interface" \
+	link add $VX1_DEV type vxlan dstport 4790 external
+ts_ip "$0" "Enslave $VX1_DEV under $BR_DEV" \
+	link set dev $VX1_DEV master $BR_DEV
+
+# Test that bridge ports without vlans do not appear in the output
+ts_bridge "$0" "Show vlan" vlan
+test_on_not "$VX0_DEV"
+
+# Test that bridge ports without tunnels do not appear in the output
+ts_bridge "$0" "Show vlan tunnel info" vlan tunnelshow
+test_lines_count 1 # header only
diff --git a/testsuite/tests/bridge/vlan/tunnelshow.t b/testsuite/tests/bridge/vlan/tunnelshow.t
index fd41bfcb..3e9c12a2 100755
--- a/testsuite/tests/bridge/vlan/tunnelshow.t
+++ b/testsuite/tests/bridge/vlan/tunnelshow.t
@@ -28,6 +28,6 @@ ts_bridge "$0" "Add tunnel with vni > 16k" \
 
 ts_bridge "$0" "Show tunnel info" vlan tunnelshow dev $VX_DEV
 test_on "1030\s+65556"
-test_lines_count 5
+test_lines_count 4
 
 ts_bridge "$0" "Dump tunnel info" -j vlan tunnelshow dev $VX_DEV

From 5a07a5df5a4793c2ee2bf93ced638268053e5a19 Mon Sep 17 00:00:00 2001
From: Benjamin Poirier <bpoirier@cumulusnetworks.com>
Date: Fri, 1 May 2020 17:47:18 +0900
Subject: [PATCH 23/25] json_print: Return number of characters printed

When outputting in normal mode, forward the return value from
color_fprintf().

Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 include/json_print.h | 24 ++++++-----
 lib/json_print.c     | 95 +++++++++++++++++++++++++++-----------------
 2 files changed, 73 insertions(+), 46 deletions(-)

diff --git a/include/json_print.h b/include/json_print.h
index 34444793..50e71de4 100644
--- a/include/json_print.h
+++ b/include/json_print.h
@@ -44,20 +44,24 @@ void close_json_array(enum output_type type, const char *delim);
 void print_nl(void);
 
 #define _PRINT_FUNC(type_name, type)					\
-	void print_color_##type_name(enum output_type t,		\
-				     enum color_attr color,		\
-				     const char *key,			\
-				     const char *fmt,			\
-				     type value);			\
+	int print_color_##type_name(enum output_type t,			\
+				    enum color_attr color,		\
+				    const char *key,			\
+				    const char *fmt,			\
+				    type value);			\
 									\
-	static inline void print_##type_name(enum output_type t,	\
-					     const char *key,		\
-					     const char *fmt,		\
-					     type value)		\
+	static inline int print_##type_name(enum output_type t,		\
+					    const char *key,		\
+					    const char *fmt,		\
+					    type value)			\
 	{								\
-		print_color_##type_name(t, COLOR_NONE, key, fmt, value);	\
+		return print_color_##type_name(t, COLOR_NONE, key, fmt,	\
+					       value);			\
 	}
 
+/* These functions return 0 if printing to a JSON context, number of
+ * characters printed otherwise (as calculated by printf(3)).
+ */
 _PRINT_FUNC(int, int)
 _PRINT_FUNC(s64, int64_t)
 _PRINT_FUNC(bool, bool)
diff --git a/lib/json_print.c b/lib/json_print.c
index 8e7f32dc..fe0705bf 100644
--- a/lib/json_print.c
+++ b/lib/json_print.c
@@ -123,20 +123,22 @@ void close_json_array(enum output_type type, const char *str)
  */
 #define _PRINT_FUNC(type_name, type)					\
 	__attribute__((format(printf, 4, 0)))				\
-	void print_color_##type_name(enum output_type t,		\
-				     enum color_attr color,		\
-				     const char *key,			\
-				     const char *fmt,			\
-				     type value)			\
+	int print_color_##type_name(enum output_type t,			\
+				    enum color_attr color,		\
+				    const char *key,			\
+				    const char *fmt,			\
+				    type value)				\
 	{								\
+		int ret = 0;						\
 		if (_IS_JSON_CONTEXT(t)) {				\
 			if (!key)					\
 				jsonw_##type_name(_jw, value);		\
 			else						\
 				jsonw_##type_name##_field(_jw, key, value); \
 		} else if (_IS_FP_CONTEXT(t)) {				\
-			color_fprintf(stdout, color, fmt, value);          \
+			ret = color_fprintf(stdout, color, fmt, value); \
 		}							\
+		return ret;						\
 	}
 _PRINT_FUNC(int, int);
 _PRINT_FUNC(s64, int64_t);
@@ -162,12 +164,14 @@ _PRINT_NAME_VALUE_FUNC(uint, unsigned int, u);
 _PRINT_NAME_VALUE_FUNC(string, const char*, s);
 #undef _PRINT_NAME_VALUE_FUNC
 
-void print_color_string(enum output_type type,
-			enum color_attr color,
-			const char *key,
-			const char *fmt,
-			const char *value)
+int print_color_string(enum output_type type,
+		       enum color_attr color,
+		       const char *key,
+		       const char *fmt,
+		       const char *value)
 {
+	int ret = 0;
+
 	if (_IS_JSON_CONTEXT(type)) {
 		if (key && !value)
 			jsonw_name(_jw, key);
@@ -176,8 +180,10 @@ void print_color_string(enum output_type type,
 		else
 			jsonw_string_field(_jw, key, value);
 	} else if (_IS_FP_CONTEXT(type)) {
-		color_fprintf(stdout, color, fmt, value);
+		ret = color_fprintf(stdout, color, fmt, value);
 	}
+
+	return ret;
 }
 
 /*
@@ -185,47 +191,58 @@ void print_color_string(enum output_type type,
  * a value to it, you will need to use "is_json_context()" to have different
  * branch for json and regular output. grep -r "print_bool" for example
  */
-void print_color_bool(enum output_type type,
-		      enum color_attr color,
-		      const char *key,
-		      const char *fmt,
-		      bool value)
+int print_color_bool(enum output_type type,
+		     enum color_attr color,
+		     const char *key,
+		     const char *fmt,
+		     bool value)
 {
+	int ret = 0;
+
 	if (_IS_JSON_CONTEXT(type)) {
 		if (key)
 			jsonw_bool_field(_jw, key, value);
 		else
 			jsonw_bool(_jw, value);
 	} else if (_IS_FP_CONTEXT(type)) {
-		color_fprintf(stdout, color, fmt, value ? "true" : "false");
+		ret = color_fprintf(stdout, color, fmt,
+				    value ? "true" : "false");
 	}
+
+	return ret;
 }
 
 /*
  * In JSON context uses hardcode %#x format: 42 -> 0x2a
  */
-void print_color_0xhex(enum output_type type,
-		       enum color_attr color,
-		       const char *key,
-		       const char *fmt,
-		       unsigned long long hex)
+int print_color_0xhex(enum output_type type,
+		      enum color_attr color,
+		      const char *key,
+		      const char *fmt,
+		      unsigned long long hex)
 {
+	int ret = 0;
+
 	if (_IS_JSON_CONTEXT(type)) {
 		SPRINT_BUF(b1);
 
 		snprintf(b1, sizeof(b1), "%#llx", hex);
 		print_string(PRINT_JSON, key, NULL, b1);
 	} else if (_IS_FP_CONTEXT(type)) {
-		color_fprintf(stdout, color, fmt, hex);
+		ret = color_fprintf(stdout, color, fmt, hex);
 	}
+
+	return ret;
 }
 
-void print_color_hex(enum output_type type,
-		     enum color_attr color,
-		     const char *key,
-		     const char *fmt,
-		     unsigned int hex)
+int print_color_hex(enum output_type type,
+		    enum color_attr color,
+		    const char *key,
+		    const char *fmt,
+		    unsigned int hex)
 {
+	int ret = 0;
+
 	if (_IS_JSON_CONTEXT(type)) {
 		SPRINT_BUF(b1);
 
@@ -235,28 +252,34 @@ void print_color_hex(enum output_type type,
 		else
 			jsonw_string(_jw, b1);
 	} else if (_IS_FP_CONTEXT(type)) {
-		color_fprintf(stdout, color, fmt, hex);
+		ret = color_fprintf(stdout, color, fmt, hex);
 	}
+
+	return ret;
 }
 
 /*
  * In JSON context we don't use the argument "value" we simply call jsonw_null
  * whereas FP context can use "value" to output anything
  */
-void print_color_null(enum output_type type,
-		      enum color_attr color,
-		      const char *key,
-		      const char *fmt,
-		      const char *value)
+int print_color_null(enum output_type type,
+		     enum color_attr color,
+		     const char *key,
+		     const char *fmt,
+		     const char *value)
 {
+	int ret = 0;
+
 	if (_IS_JSON_CONTEXT(type)) {
 		if (key)
 			jsonw_null_field(_jw, key);
 		else
 			jsonw_null(_jw);
 	} else if (_IS_FP_CONTEXT(type)) {
-		color_fprintf(stdout, color, fmt, value);
+		ret = color_fprintf(stdout, color, fmt, value);
 	}
+
+	return ret;
 }
 
 /* Print line separator (if not in JSON mode) */

From e0c457b1a5a26035ccf7c0e6f271d987dbeb8bda Mon Sep 17 00:00:00 2001
From: Benjamin Poirier <bpoirier@cumulusnetworks.com>
Date: Fri, 1 May 2020 17:47:19 +0900
Subject: [PATCH 24/25] bridge: Align output columns

Use fixed column widths to improve readability.

Before:
root@vsid:/src/iproute2# ./bridge/bridge vlan tunnelshow
port    vlan-id tunnel-id
vx0      2       2
         1010-1020       1010-1020
         1030    65556
vx-longname      2       2

After:
root@vsid:/src/iproute2# ./bridge/bridge vlan tunnelshow
port              vlan-id    tunnel-id
vx0               2          2
                  1010-1020  1010-1020
                  1030       65556
vx-longname       2          2

Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 bridge/vlan.c | 73 ++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 52 insertions(+), 21 deletions(-)

diff --git a/bridge/vlan.c b/bridge/vlan.c
index b126fae2..8753d4c3 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -22,6 +22,11 @@ enum vlan_show_subject {
 	VLAN_SHOW_TUNNELINFO,
 };
 
+#define VLAN_ID_LEN 9
+
+#define __stringify_1(x...) #x
+#define __stringify(x...) __stringify_1(x)
+
 static void usage(void)
 {
 	fprintf(stderr,
@@ -256,11 +261,11 @@ static int filter_vlan_check(__u16 vid, __u16 flags)
 	return 1;
 }
 
-static void open_vlan_port(int ifi_index, const char *fmt,
-			   enum vlan_show_subject subject)
+static void open_vlan_port(int ifi_index, enum vlan_show_subject subject)
 {
 	open_json_object(NULL);
-	print_color_string(PRINT_ANY, COLOR_IFNAME, "ifname", fmt,
+	print_color_string(PRINT_ANY, COLOR_IFNAME, "ifname",
+			   "%-" __stringify(IFNAMSIZ) "s  ",
 			   ll_index_to_name(ifi_index));
 	open_json_array(PRINT_JSON,
 			subject == VLAN_SHOW_VLAN ? "vlans": "tunnels");
@@ -272,16 +277,18 @@ static void close_vlan_port(void)
 	close_json_object();
 }
 
-static void print_range(const char *name, __u32 start, __u32 id)
+static unsigned int print_range(const char *name, __u32 start, __u32 id)
 {
 	char end[64];
+	int width;
 
 	snprintf(end, sizeof(end), "%sEnd", name);
 
-	print_uint(PRINT_ANY, name, "\t %u", start);
+	width = print_uint(PRINT_ANY, name, "%u", start);
 	if (start != id)
-		print_uint(PRINT_ANY, end, "-%u", id);
+		width += print_uint(PRINT_ANY, end, "-%u", id);
 
+	return width;
 }
 
 static void print_vlan_tunnel_info(struct rtattr *tb, int ifindex)
@@ -297,6 +304,7 @@ static void print_vlan_tunnel_info(struct rtattr *tb, int ifindex)
 		__u32 tunnel_id = 0;
 		__u16 tunnel_vid = 0;
 		__u16 tunnel_flags = 0;
+		unsigned int width;
 		int vcheck_ret;
 
 		if (i->rta_type != IFLA_BRIDGE_VLAN_TUNNEL_INFO)
@@ -331,12 +339,25 @@ static void print_vlan_tunnel_info(struct rtattr *tb, int ifindex)
 			continue;
 
 		if (!opened) {
-			open_vlan_port(ifindex, "%s", VLAN_SHOW_TUNNELINFO);
+			open_vlan_port(ifindex, VLAN_SHOW_TUNNELINFO);
 			opened = true;
+		} else {
+			print_string(PRINT_FP, NULL,
+				     "%-" __stringify(IFNAMSIZ) "s  ", "");
 		}
 
 		open_json_object(NULL);
-		print_range("vlan", last_vid_start, tunnel_vid);
+		width = print_range("vlan", last_vid_start, tunnel_vid);
+		if (width <= VLAN_ID_LEN) {
+			char buf[VLAN_ID_LEN + 1];
+
+			snprintf(buf, sizeof(buf), "%-*s",
+				 VLAN_ID_LEN - width, "");
+			print_string(PRINT_FP, NULL, "%s  ", buf);
+		} else {
+			fprintf(stderr, "BUG: vlan range too wide, %u\n",
+				width);
+		}
 		print_range("tunid", last_tunid_start, tunnel_id);
 		close_json_object();
 		print_string(PRINT_FP, NULL, "%s", _SL_);
@@ -404,20 +425,23 @@ static void print_vlan_flags(__u16 flags)
 static void print_one_vlan_stats(const struct bridge_vlan_xstats *vstats)
 {
 	open_json_object(NULL);
-	print_hu(PRINT_ANY, "vid", " %hu", vstats->vid);
 
+	print_hu(PRINT_ANY, "vid", "%hu", vstats->vid);
 	print_vlan_flags(vstats->flags);
+	print_nl();
 
-	print_lluint(PRINT_ANY, "rx_bytes",
-		     "\n                   RX: %llu bytes",
+	print_string(PRINT_FP, NULL, "%-" __stringify(IFNAMSIZ) "s    ", "");
+	print_lluint(PRINT_ANY, "rx_bytes", "RX: %llu bytes",
 		     vstats->rx_bytes);
 	print_lluint(PRINT_ANY, "rx_packets", " %llu packets\n",
-		vstats->rx_packets);
-	print_lluint(PRINT_ANY, "tx_bytes",
-		     "                   TX: %llu bytes",
+		     vstats->rx_packets);
+
+	print_string(PRINT_FP, NULL, "%-" __stringify(IFNAMSIZ) "s    ", "");
+	print_lluint(PRINT_ANY, "tx_bytes", "TX: %llu bytes",
 		     vstats->tx_bytes);
 	print_lluint(PRINT_ANY, "tx_packets", " %llu packets\n",
-		vstats->tx_packets);
+		     vstats->tx_packets);
+
 	close_json_object();
 }
 
@@ -452,10 +476,11 @@ static void print_vlan_stats_attr(struct rtattr *attr, int ifindex)
 
 		/* found vlan stats, first time print the interface name */
 		if (!found_vlan) {
-			open_vlan_port(ifindex, "%-16s", VLAN_SHOW_VLAN);
+			open_vlan_port(ifindex, VLAN_SHOW_VLAN);
 			found_vlan = true;
 		} else {
-			print_string(PRINT_FP, NULL, "%-16s", "");
+			print_string(PRINT_FP, NULL,
+				     "%-" __stringify(IFNAMSIZ) "s  ", "");
 		}
 		print_one_vlan_stats(vstats);
 	}
@@ -534,9 +559,11 @@ static int vlan_show(int argc, char **argv, int subject)
 		}
 
 		if (!is_json_context()) {
-			printf("port\tvlan-id");
+			printf("%-" __stringify(IFNAMSIZ) "s  %-"
+			       __stringify(VLAN_ID_LEN) "s", "port",
+			       "vlan-id");
 			if (subject == VLAN_SHOW_TUNNELINFO)
-				printf("\ttunnel-id");
+				printf("  tunnel-id");
 			printf("\n");
 		}
 
@@ -555,7 +582,8 @@ static int vlan_show(int argc, char **argv, int subject)
 		}
 
 		if (!is_json_context())
-			printf("%-16s vlan-id\n", "port");
+			printf("%-" __stringify(IFNAMSIZ) "s  vlan-id\n",
+			       "port");
 
 		if (rtnl_dump_filter(&rth, print_vlan_stats, stdout) < 0) {
 			fprintf(stderr, "Dump terminated\n");
@@ -604,8 +632,11 @@ void print_vlan_info(struct rtattr *tb, int ifindex)
 			continue;
 
 		if (!opened) {
-			open_vlan_port(ifindex, "%s", VLAN_SHOW_VLAN);
+			open_vlan_port(ifindex, VLAN_SHOW_VLAN);
 			opened = true;
+		} else {
+			print_string(PRINT_FP, NULL, "%-"
+				     __stringify(IFNAMSIZ) "s  ", "");
 		}
 
 		open_json_object(NULL);

From 0501fe734fd75ed93b3417266127bfc37b07459b Mon Sep 17 00:00:00 2001
From: Benjamin Poirier <bpoirier@cumulusnetworks.com>
Date: Fri, 1 May 2020 17:47:20 +0900
Subject: [PATCH 25/25] Replace open-coded instances of print_nl()

Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 bridge/vlan.c     |  4 ++--
 tc/m_action.c     | 14 +++++++-------
 tc/m_connmark.c   |  4 ++--
 tc/m_ctinfo.c     |  4 ++--
 tc/m_ife.c        |  4 ++--
 tc/m_mpls.c       |  2 +-
 tc/m_nat.c        |  4 ++--
 tc/m_sample.c     |  4 ++--
 tc/m_skbedit.c    |  4 ++--
 tc/m_tunnel_key.c | 16 ++++++++--------
 tc/q_taprio.c     |  8 ++++----
 tc/tc_util.c      |  4 ++--
 12 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/bridge/vlan.c b/bridge/vlan.c
index 8753d4c3..0d142bc9 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -360,7 +360,7 @@ static void print_vlan_tunnel_info(struct rtattr *tb, int ifindex)
 		}
 		print_range("tunid", last_tunid_start, tunnel_id);
 		close_json_object();
-		print_string(PRINT_FP, NULL, "%s", _SL_);
+		print_nl();
 	}
 
 	if (opened)
@@ -644,7 +644,7 @@ void print_vlan_info(struct rtattr *tb, int ifindex)
 
 		print_vlan_flags(vinfo->flags);
 		close_json_object();
-		print_string(PRINT_FP, NULL, "%s", _SL_);
+		print_nl();
 	}
 
 	if (opened)
diff --git a/tc/m_action.c b/tc/m_action.c
index b41782de..66e67245 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -177,7 +177,7 @@ static void print_hw_stats(const struct rtattr *arg, bool print_used)
 			print_string(PRINT_ANY, NULL, " %s", item->str);
 	}
 	close_json_array(PRINT_JSON, NULL);
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 }
 
 static int parse_hw_stats(const char *str, struct nlmsghdr *n)
@@ -376,11 +376,11 @@ static int tc_print_one_action(FILE *f, struct rtattr *arg)
 
 	if (show_stats && tb[TCA_ACT_STATS]) {
 		print_string(PRINT_FP, NULL, "\tAction statistics:", NULL);
-		print_string(PRINT_FP, NULL, "%s", _SL_);
+		print_nl();
 		open_json_object("stats");
 		print_tcstats2_attr(f, tb[TCA_ACT_STATS], "\t", NULL);
 		close_json_object();
-		print_string(PRINT_FP, NULL, "%s", _SL_);
+		print_nl();
 	}
 	if (tb[TCA_ACT_COOKIE]) {
 		int strsz = RTA_PAYLOAD(tb[TCA_ACT_COOKIE]);
@@ -389,7 +389,7 @@ static int tc_print_one_action(FILE *f, struct rtattr *arg)
 		print_string(PRINT_ANY, "cookie", "\tcookie %s",
 			     hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
 					   strsz, b1, sizeof(b1)));
-		print_string(PRINT_FP, NULL, "%s", _SL_);
+		print_nl();
 	}
 	if (tb[TCA_ACT_FLAGS]) {
 		struct nla_bitfield32 *flags = RTA_DATA(tb[TCA_ACT_FLAGS]);
@@ -398,7 +398,7 @@ static int tc_print_one_action(FILE *f, struct rtattr *arg)
 			print_bool(PRINT_ANY, "no_percpu", "\tno_percpu",
 				   flags->value &
 				   TCA_ACT_FLAGS_NO_PERCPU_STATS);
-		print_string(PRINT_FP, NULL, "%s", _SL_);
+		print_nl();
 	}
 	if (tb[TCA_ACT_HW_STATS])
 		print_hw_stats(tb[TCA_ACT_HW_STATS], false);
@@ -458,7 +458,7 @@ tc_print_action(FILE *f, const struct rtattr *arg, unsigned short tot_acts)
 	for (i = 0; i <= tot_acts; i++) {
 		if (tb[i]) {
 			open_json_object(NULL);
-			print_string(PRINT_FP, NULL, "%s", _SL_);
+			print_nl();
 			print_uint(PRINT_ANY, "order",
 				   "\taction order %u: ", i);
 			if (tc_print_one_action(f, tb[i]) < 0) {
@@ -497,7 +497,7 @@ int print_action(struct nlmsghdr *n, void *arg)
 	open_json_object(NULL);
 	print_uint(PRINT_ANY, "total acts", "total acts %u",
 		   tot_acts ? *tot_acts : 0);
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	close_json_object();
 	if (tb[TCA_ACT_TAB] == NULL) {
 		if (n->nlmsg_type != RTM_GETACTION)
diff --git a/tc/m_connmark.c b/tc/m_connmark.c
index eac23489..4b2dc4e2 100644
--- a/tc/m_connmark.c
+++ b/tc/m_connmark.c
@@ -125,7 +125,7 @@ static int print_connmark(struct action_util *au, FILE *f, struct rtattr *arg)
 	print_uint(PRINT_ANY, "zone", "zone %u", ci->zone);
 	print_action_control(f, " ", ci->action, "");
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	print_uint(PRINT_ANY, "index", "\t index %u", ci->index);
 	print_int(PRINT_ANY, "ref", " ref %d", ci->refcnt);
 	print_int(PRINT_ANY, "bind", " bind %d", ci->bindcnt);
@@ -137,7 +137,7 @@ static int print_connmark(struct action_util *au, FILE *f, struct rtattr *arg)
 			print_tm(f, tm);
 		}
 	}
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 
 	return 0;
 }
diff --git a/tc/m_ctinfo.c b/tc/m_ctinfo.c
index 5e451f87..e5c1b436 100644
--- a/tc/m_ctinfo.c
+++ b/tc/m_ctinfo.c
@@ -238,7 +238,7 @@ static int print_ctinfo(struct action_util *au, FILE *f, struct rtattr *arg)
 	print_hu(PRINT_ANY, "zone", "zone %u", zone);
 	print_action_control(f, " ", ci->action, "");
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	print_uint(PRINT_ANY, "index", "\t index %u", ci->index);
 	print_int(PRINT_ANY, "ref", " ref %d", ci->refcnt);
 	print_int(PRINT_ANY, "bind", " bind %d", ci->bindcnt);
@@ -256,7 +256,7 @@ static int print_ctinfo(struct action_util *au, FILE *f, struct rtattr *arg)
 	if (show_stats)
 		print_ctinfo_stats(f, tb);
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 
 	return 0;
 }
diff --git a/tc/m_ife.c b/tc/m_ife.c
index 7c612c02..6a85e087 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -311,7 +311,7 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg)
 					 sizeof(b2)));
 	}
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	print_uint(PRINT_ANY, "index", "\t index %u", p->index);
 	print_int(PRINT_ANY, "ref", " ref %d", p->refcnt);
 	print_int(PRINT_ANY, "bind", " bind %d", p->bindcnt);
@@ -324,7 +324,7 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg)
 		}
 	}
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 
 	return 0;
 }
diff --git a/tc/m_mpls.c b/tc/m_mpls.c
index 50eba01c..3d5d9b25 100644
--- a/tc/m_mpls.c
+++ b/tc/m_mpls.c
@@ -265,7 +265,7 @@ static int print_mpls(struct action_util *au, FILE *f, struct rtattr *arg)
 		}
 	}
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 
 	return 0;
 }
diff --git a/tc/m_nat.c b/tc/m_nat.c
index c4b02a83..56e8f47c 100644
--- a/tc/m_nat.c
+++ b/tc/m_nat.c
@@ -172,7 +172,7 @@ print_nat(struct action_util *au, FILE * f, struct rtattr *arg)
 		     format_host_r(AF_INET, 4, &sel->new_addr, buf1, sizeof(buf1)));
 
 	print_action_control(f, " ", sel->action, "");
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	print_uint(PRINT_ANY, "index", "\t index %u", sel->index);
 	print_int(PRINT_ANY, "ref", " ref %d", sel->refcnt);
 	print_int(PRINT_ANY, "bind", " bind %d", sel->bindcnt);
@@ -185,7 +185,7 @@ print_nat(struct action_util *au, FILE * f, struct rtattr *arg)
 		}
 	}
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 
 	return 0;
 }
diff --git a/tc/m_sample.c b/tc/m_sample.c
index c068e632..4a30513a 100644
--- a/tc/m_sample.c
+++ b/tc/m_sample.c
@@ -167,7 +167,7 @@ static int print_sample(struct action_util *au, FILE *f, struct rtattr *arg)
 
 	print_action_control(f, " ", p->action, "");
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	print_uint(PRINT_ANY, "index", "\t index %u", p->index);
 	print_int(PRINT_ANY, "ref", " ref %d", p->refcnt);
 	print_int(PRINT_ANY, "bind", " bind %d", p->bindcnt);
@@ -179,7 +179,7 @@ static int print_sample(struct action_util *au, FILE *f, struct rtattr *arg)
 			print_tm(f, tm);
 		}
 	}
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	return 0;
 }
 
diff --git a/tc/m_skbedit.c b/tc/m_skbedit.c
index 761cad58..9afe2f0c 100644
--- a/tc/m_skbedit.c
+++ b/tc/m_skbedit.c
@@ -254,7 +254,7 @@ static int print_skbedit(struct action_util *au, FILE *f, struct rtattr *arg)
 
 	print_action_control(f, " ", p->action, "");
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	print_uint(PRINT_ANY, "index", "\t index %u", p->index);
 	print_int(PRINT_ANY, "ref", " ref %d", p->refcnt);
 	print_int(PRINT_ANY, "bind", " bind %d", p->bindcnt);
@@ -267,7 +267,7 @@ static int print_skbedit(struct action_util *au, FILE *f, struct rtattr *arg)
 		}
 	}
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 
 	return 0;
 }
diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
index 8fde6891..1f6921f3 100644
--- a/tc/m_tunnel_key.c
+++ b/tc/m_tunnel_key.c
@@ -367,7 +367,7 @@ static void tunnel_key_print_ip_addr(FILE *f, const char *name,
 	else
 		return;
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	if (matches(name, "src_ip") == 0)
 		print_string(PRINT_ANY, "src_ip", "\tsrc_ip %s",
 			     rt_addr_n2a_rta(family, attr));
@@ -381,7 +381,7 @@ static void tunnel_key_print_key_id(FILE *f, const char *name,
 {
 	if (!attr)
 		return;
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	print_uint(PRINT_ANY, "key_id", "\tkey_id %u", rta_getattr_be32(attr));
 }
 
@@ -390,7 +390,7 @@ static void tunnel_key_print_dst_port(FILE *f, char *name,
 {
 	if (!attr)
 		return;
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	print_uint(PRINT_ANY, "dst_port", "\tdst_port %u",
 		   rta_getattr_be16(attr));
 }
@@ -401,7 +401,7 @@ static void tunnel_key_print_flag(FILE *f, const char *name_on,
 {
 	if (!attr)
 		return;
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	print_string(PRINT_ANY, "flag", "\t%s",
 		     rta_getattr_u8(attr) ? name_on : name_off);
 }
@@ -473,11 +473,11 @@ static void tunnel_key_print_tos_ttl(FILE *f, char *name,
 		return;
 
 	if (matches(name, "tos") == 0 && rta_getattr_u8(attr) != 0) {
-		print_string(PRINT_FP, NULL, "%s", _SL_);
+		print_nl();
 		print_uint(PRINT_ANY, "tos", "\ttos 0x%x",
 			   rta_getattr_u8(attr));
 	} else if (matches(name, "ttl") == 0 && rta_getattr_u8(attr) != 0) {
-		print_string(PRINT_FP, NULL, "%s", _SL_);
+		print_nl();
 		print_uint(PRINT_ANY, "ttl", "\tttl %u",
 			   rta_getattr_u8(attr));
 	}
@@ -531,7 +531,7 @@ static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr *arg)
 	}
 	print_action_control(f, " ", parm->action, "");
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	print_uint(PRINT_ANY, "index", "\t index %u", parm->index);
 	print_int(PRINT_ANY, "ref", " ref %d", parm->refcnt);
 	print_int(PRINT_ANY, "bind", " bind %d", parm->bindcnt);
@@ -544,7 +544,7 @@ static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr *arg)
 		}
 	}
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 
 	return 0;
 }
diff --git a/tc/q_taprio.c b/tc/q_taprio.c
index b9954436..e43db9d0 100644
--- a/tc/q_taprio.c
+++ b/tc/q_taprio.c
@@ -368,7 +368,7 @@ static int print_sched_list(FILE *f, struct rtattr *list)
 
 	open_json_array(PRINT_JSON, "schedule");
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 
 	for (item = RTA_DATA(list); RTA_OK(item, rem); item = RTA_NEXT(item, rem)) {
 		struct rtattr *tb[TCA_TAPRIO_SCHED_ENTRY_MAX + 1];
@@ -396,7 +396,7 @@ static int print_sched_list(FILE *f, struct rtattr *list)
 		print_uint(PRINT_ANY, "interval", " interval %u", interval);
 		close_json_object();
 
-		print_string(PRINT_FP, NULL, "%s", _SL_);
+		print_nl();
 	}
 
 	close_json_array(PRINT_ANY, "");
@@ -454,7 +454,7 @@ static int taprio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 		print_uint(PRINT_ANY, NULL, " %u", qopt->prio_tc_map[i]);
 	close_json_array(PRINT_ANY, "");
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 
 	open_json_array(PRINT_ANY, "queues");
 	for (i = 0; i < qopt->num_tc; i++) {
@@ -465,7 +465,7 @@ static int taprio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 	}
 	close_json_array(PRINT_ANY, "");
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 
 	if (tb[TCA_TAPRIO_ATTR_SCHED_CLOCKID])
 		clockid = rta_getattr_s32(tb[TCA_TAPRIO_ATTR_SCHED_CLOCKID]);
diff --git a/tc/tc_util.c b/tc/tc_util.c
index 68938fb0..12f865cc 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -788,7 +788,7 @@ static void print_tcstats_basic_hw(struct rtattr **tbs, char *prefix)
 			   sizeof(bs)));
 
 		if (bs.bytes >= bs_hw.bytes && bs.packets >= bs_hw.packets) {
-			print_string(PRINT_FP, NULL, "%s", _SL_);
+			print_nl();
 			print_string(PRINT_FP, NULL, "%s", prefix);
 			print_lluint(PRINT_ANY, "sw_bytes",
 				     "Sent software %llu bytes",
@@ -798,7 +798,7 @@ static void print_tcstats_basic_hw(struct rtattr **tbs, char *prefix)
 		}
 	}
 
-	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_nl();
 	print_string(PRINT_FP, NULL, "%s", prefix);
 	print_lluint(PRINT_ANY, "hw_bytes", "Sent hardware %llu bytes",
 		     bs_hw.bytes);