All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2 0/8] bridge vlan tunnelshow fixes
@ 2019-12-16  6:43 Benjamin Poirier
  2019-12-16  6:43 ` [PATCH iproute2 1/8] json_print: Remove declaration without implementation Benjamin Poirier
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Benjamin Poirier @ 2019-12-16  6:43 UTC (permalink / raw)
  To: netdev; +Cc: Roopa Prabhu

Fix various problems in and around normal and json output of `bridge vlan
tunnelshow`.

Can be tested using:
ip link add bridge type bridge

ip link add vxlan0 type vxlan dstport 4789 external
ip link set dev vxlan0 master bridge
ip link set dev vxlan0 type bridge_slave vlan_tunnel on

bridge vlan add dev vxlan0 vid 1000
bridge vlan add dev vxlan0 vid 1000 tunnel_info id 1000
bridge vlan add dev vxlan0 vid 1010-1020
bridge vlan add dev vxlan0 vid 1010-1020 tunnel_info id 1010-1020
bridge vlan add dev vxlan0 vid 1030
bridge vlan add dev vxlan0 vid 1030 tunnel_info id 65556

Benjamin Poirier (8):
  json_print: Remove declaration without implementation
  testsuite: Fix line count test
  bridge: Fix typo in error messages
  bridge: Fix src_vni argument in man page
  bridge: Fix BRIDGE_VLAN_TUNNEL attribute sizes
  bridge: Fix vni printing
  bridge: Deduplicate vlan show functions
  bridge: Fix tunnelshow json output

 bridge/vlan.c                            | 138 ++++++++---------------
 include/json_print.h                     |   2 -
 man/man8/bridge.8                        |   4 +-
 testsuite/Makefile                       |   3 +-
 testsuite/lib/generic.sh                 |   8 +-
 testsuite/tests/bridge/vlan/tunnelshow.t |  33 ++++++
 6 files changed, 88 insertions(+), 100 deletions(-)
 create mode 100755 testsuite/tests/bridge/vlan/tunnelshow.t

-- 
2.24.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH iproute2 1/8] json_print: Remove declaration without implementation
  2019-12-16  6:43 [PATCH iproute2 0/8] bridge vlan tunnelshow fixes Benjamin Poirier
@ 2019-12-16  6:43 ` Benjamin Poirier
  2019-12-16  6:43 ` [PATCH iproute2 2/8] testsuite: Fix line count test Benjamin Poirier
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Benjamin Poirier @ 2019-12-16  6:43 UTC (permalink / raw)
  To: netdev; +Cc: Roopa Prabhu

Fixes: 6377572f0aa8 ("ip: ip_print: add new API to print JSON or regular format output")
Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
---
 include/json_print.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/json_print.h b/include/json_print.h
index fe92d14c..6695654f 100644
--- a/include/json_print.h
+++ b/include/json_print.h
@@ -34,8 +34,6 @@ void delete_json_obj(void);
 
 bool is_json_context(void);
 
-void fflush_fp(void);
-
 void open_json_object(const char *str);
 void close_json_object(void);
 void open_json_array(enum output_type type, const char *delim);
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH iproute2 2/8] testsuite: Fix line count test
  2019-12-16  6:43 [PATCH iproute2 0/8] bridge vlan tunnelshow fixes Benjamin Poirier
  2019-12-16  6:43 ` [PATCH iproute2 1/8] json_print: Remove declaration without implementation Benjamin Poirier
@ 2019-12-16  6:43 ` Benjamin Poirier
  2019-12-16  6:43 ` [PATCH iproute2 3/8] bridge: Fix typo in error messages Benjamin Poirier
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Benjamin Poirier @ 2019-12-16  6:43 UTC (permalink / raw)
  To: netdev; +Cc: Roopa Prabhu

a substring match is not enough, ex: 10 != 1

Fixes: 30383b074de1 ("tests: Add output testing")
Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
---
 testsuite/lib/generic.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testsuite/lib/generic.sh b/testsuite/lib/generic.sh
index f92260fc..e909008a 100644
--- a/testsuite/lib/generic.sh
+++ b/testsuite/lib/generic.sh
@@ -121,7 +121,7 @@ test_on_not()
 test_lines_count()
 {
 	echo -n "test on lines count ($1): "
-	if cat "$STD_OUT" | wc -l | grep -q "$1"
+	if [ $(cat "$STD_OUT" | wc -l) -eq "$1" ]
 	then
 		pr_success
 	else
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH iproute2 3/8] bridge: Fix typo in error messages
  2019-12-16  6:43 [PATCH iproute2 0/8] bridge vlan tunnelshow fixes Benjamin Poirier
  2019-12-16  6:43 ` [PATCH iproute2 1/8] json_print: Remove declaration without implementation Benjamin Poirier
  2019-12-16  6:43 ` [PATCH iproute2 2/8] testsuite: Fix line count test Benjamin Poirier
@ 2019-12-16  6:43 ` Benjamin Poirier
  2019-12-16  6:43 ` [PATCH iproute2 4/8] bridge: Fix src_vni argument in man page Benjamin Poirier
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Benjamin Poirier @ 2019-12-16  6:43 UTC (permalink / raw)
  To: netdev; +Cc: Roopa Prabhu

Fixes: 9eff0e5cc447 ("bridge: Add vlan configuration support")
Fixes: 7abf5de677e3 ("bridge: vlan: add support to display per-vlan statistics")
Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
---
 bridge/vlan.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/bridge/vlan.c b/bridge/vlan.c
index 6d33b0a9..6dc694b6 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -576,7 +576,7 @@ static int vlan_show(int argc, char **argv)
 					     (compress_vlans ?
 					      RTEXT_FILTER_BRVLAN_COMPRESSED :
 					      RTEXT_FILTER_BRVLAN)) < 0) {
-			perror("Cannont send dump request");
+			perror("Cannot send dump request");
 			exit(1);
 		}
 
@@ -601,7 +601,7 @@ static int vlan_show(int argc, char **argv)
 
 		filt_mask = IFLA_STATS_FILTER_BIT(IFLA_STATS_LINK_XSTATS);
 		if (rtnl_statsdump_req_filter(&rth, AF_UNSPEC, filt_mask) < 0) {
-			perror("Cannont send dump request");
+			perror("Cannot send dump request");
 			exit(1);
 		}
 
@@ -615,7 +615,7 @@ static int vlan_show(int argc, char **argv)
 
 		filt_mask = IFLA_STATS_FILTER_BIT(IFLA_STATS_LINK_XSTATS_SLAVE);
 		if (rtnl_statsdump_req_filter(&rth, AF_UNSPEC, filt_mask) < 0) {
-			perror("Cannont send slave dump request");
+			perror("Cannot send slave dump request");
 			exit(1);
 		}
 
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH iproute2 4/8] bridge: Fix src_vni argument in man page
  2019-12-16  6:43 [PATCH iproute2 0/8] bridge vlan tunnelshow fixes Benjamin Poirier
                   ` (2 preceding siblings ...)
  2019-12-16  6:43 ` [PATCH iproute2 3/8] bridge: Fix typo in error messages Benjamin Poirier
@ 2019-12-16  6:43 ` Benjamin Poirier
  2019-12-16  6:43 ` [PATCH iproute2 5/8] bridge: Fix BRIDGE_VLAN_TUNNEL attribute sizes Benjamin Poirier
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Benjamin Poirier @ 2019-12-16  6:43 UTC (permalink / raw)
  To: netdev; +Cc: Roopa Prabhu

"SRC VNI" is only one argument and should appear as such. Moreover, this
argument to the src_vni option is documented under three forms: "SRC_VNI",
"SRC VNI" and "VNI" in different places. Consistenly use the simplest form,
"VNI".

Fixes: c5b176e5ba1f ("bridge: fdb: add support for src_vni option")
Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
---
 man/man8/bridge.8 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index 10f6cf0e..1804f0b4 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -71,7 +71,7 @@ bridge \- show / manipulate bridge addresses and devices
 .B dst
 .IR IPADDR " ] [ "
 .B src_vni
-.IR SRC_VNI " ] ["
+.IR VNI " ] ["
 .B vni
 .IR VNI " ] ["
 .B port
@@ -498,7 +498,7 @@ the IP address of the destination
 VXLAN tunnel endpoint where the Ethernet MAC ADDRESS resides.
 
 .TP
-.BI src_vni " SRC VNI"
+.BI src_vni " VNI"
 the src VNI Network Identifier (or VXLAN Segment ID)
 this entry belongs to. Used only when the vxlan device is in
 external or collect metadata mode. If omitted the value specified at
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH iproute2 5/8] bridge: Fix BRIDGE_VLAN_TUNNEL attribute sizes
  2019-12-16  6:43 [PATCH iproute2 0/8] bridge vlan tunnelshow fixes Benjamin Poirier
                   ` (3 preceding siblings ...)
  2019-12-16  6:43 ` [PATCH iproute2 4/8] bridge: Fix src_vni argument in man page Benjamin Poirier
@ 2019-12-16  6:43 ` Benjamin Poirier
  2019-12-16  6:43 ` [PATCH iproute2 6/8] bridge: Fix vni printing Benjamin Poirier
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Benjamin Poirier @ 2019-12-16  6:43 UTC (permalink / raw)
  To: netdev; +Cc: Roopa Prabhu

As per the kernel's vlan_tunnel_policy, IFLA_BRIDGE_VLAN_TUNNEL_VID and
IFLA_BRIDGE_VLAN_TUNNEL_FLAGS have type NLA_U16.

Fixes: 8652eeb3ab12 ("bridge: vlan: support for per vlan tunnel info")
Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
---
 bridge/vlan.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/bridge/vlan.c b/bridge/vlan.c
index 6dc694b6..c0294aa6 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -71,8 +71,8 @@ static int add_tunnel_info(struct nlmsghdr *n, int reqsize,
 
 	tinfo = addattr_nest(n, reqsize, IFLA_BRIDGE_VLAN_TUNNEL_INFO);
 	addattr32(n, reqsize, IFLA_BRIDGE_VLAN_TUNNEL_ID, tun_id);
-	addattr32(n, reqsize, IFLA_BRIDGE_VLAN_TUNNEL_VID, vid);
-	addattr32(n, reqsize, IFLA_BRIDGE_VLAN_TUNNEL_FLAGS, flags);
+	addattr16(n, reqsize, IFLA_BRIDGE_VLAN_TUNNEL_VID, vid);
+	addattr16(n, reqsize, IFLA_BRIDGE_VLAN_TUNNEL_FLAGS, flags);
 
 	addattr_nest_end(n, tinfo);
 
@@ -304,7 +304,7 @@ static void print_vlan_tunnel_info(FILE *fp, struct rtattr *tb, int ifindex)
 
 		if (ttb[IFLA_BRIDGE_VLAN_TUNNEL_VID])
 			tunnel_vid =
-				rta_getattr_u32(ttb[IFLA_BRIDGE_VLAN_TUNNEL_VID]);
+				rta_getattr_u16(ttb[IFLA_BRIDGE_VLAN_TUNNEL_VID]);
 		else
 			continue;
 
@@ -314,7 +314,7 @@ static void print_vlan_tunnel_info(FILE *fp, struct rtattr *tb, int ifindex)
 
 		if (ttb[IFLA_BRIDGE_VLAN_TUNNEL_FLAGS])
 			tunnel_flags =
-				rta_getattr_u32(ttb[IFLA_BRIDGE_VLAN_TUNNEL_FLAGS]);
+				rta_getattr_u16(ttb[IFLA_BRIDGE_VLAN_TUNNEL_FLAGS]);
 
 		if (!(tunnel_flags & BRIDGE_VLAN_INFO_RANGE_END)) {
 			last_vid_start = tunnel_vid;
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH iproute2 6/8] bridge: Fix vni printing
  2019-12-16  6:43 [PATCH iproute2 0/8] bridge vlan tunnelshow fixes Benjamin Poirier
                   ` (4 preceding siblings ...)
  2019-12-16  6:43 ` [PATCH iproute2 5/8] bridge: Fix BRIDGE_VLAN_TUNNEL attribute sizes Benjamin Poirier
@ 2019-12-16  6:43 ` Benjamin Poirier
  2019-12-16  6:43 ` [PATCH iproute2 7/8] bridge: Deduplicate vlan show functions Benjamin Poirier
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Benjamin Poirier @ 2019-12-16  6:43 UTC (permalink / raw)
  To: netdev; +Cc: Roopa Prabhu

Since commit c7c1a1ef51ae ("bridge: colorize output and use JSON print
library"), print_range() is used for vid (16bits) and vni. However, the
latter are 32bits so they get truncated. They got truncated even before
that commit though.

Fixes: 8652eeb3ab12 ("bridge: vlan: support for per vlan tunnel info")
Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
---
 bridge/vlan.c                            |  6 +++---
 testsuite/Makefile                       |  3 ++-
 testsuite/lib/generic.sh                 |  6 +++++-
 testsuite/tests/bridge/vlan/tunnelshow.t | 24 ++++++++++++++++++++++++
 4 files changed, 34 insertions(+), 5 deletions(-)
 create mode 100755 testsuite/tests/bridge/vlan/tunnelshow.t

diff --git a/bridge/vlan.c b/bridge/vlan.c
index c0294aa6..428eeee3 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -266,15 +266,15 @@ static void close_vlan_port(void)
 	close_json_object();
 }
 
-static void print_range(const char *name, __u16 start, __u16 id)
+static void print_range(const char *name, __u32 start, __u32 id)
 {
 	char end[64];
 
 	snprintf(end, sizeof(end), "%sEnd", name);
 
-	print_hu(PRINT_ANY, name, "\t %hu", start);
+	print_uint(PRINT_ANY, name, "\t %u", start);
 	if (start != id)
-		print_hu(PRINT_ANY, end, "-%hu", id);
+		print_uint(PRINT_ANY, end, "-%u", id);
 
 }
 
diff --git a/testsuite/Makefile b/testsuite/Makefile
index 4451f316..fb50f618 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -82,7 +82,8 @@ endif
 		TMP_OUT=`mktemp /tmp/tc_testsuite.XXXXXX`; \
 		. $(KENVFN); \
 		STD_ERR="$$TMP_ERR" STD_OUT="$$TMP_OUT" \
-		TC="$$i/tc/tc" IP="$$i/ip/ip" SS=$$i/misc/ss DEV="$(DEV)" IPVER="$@" SNAME="$$i" \
+		TC="$$i/tc/tc" IP="$$i/ip/ip" SS=$$i/misc/ss BRIDGE="$$i/bridge/bridge" \
+		DEV="$(DEV)" IPVER="$@" SNAME="$$i" \
 		ERRF="$(RESULTS_DIR)/$@.$$o.err" $(PREFIX) tests/$@ > $(RESULTS_DIR)/$@.$$o.out; \
 		if [ "$$?" = "127" ]; then \
 			printf "\033[1;35mSKIPPED\033[0m\n"; \
diff --git a/testsuite/lib/generic.sh b/testsuite/lib/generic.sh
index e909008a..8b339ec1 100644
--- a/testsuite/lib/generic.sh
+++ b/testsuite/lib/generic.sh
@@ -1,4 +1,3 @@
-
 export DEST="127.0.0.1"
 
 ts_log()
@@ -66,6 +65,11 @@ ts_ss()
 	__ts_cmd "$SS" "$@"
 }
 
+ts_bridge()
+{
+	__ts_cmd "$BRIDGE" "$@"
+}
+
 ts_qdisc_available()
 {
 	HELPOUT=`$TC qdisc add $1 help 2>&1`
diff --git a/testsuite/tests/bridge/vlan/tunnelshow.t b/testsuite/tests/bridge/vlan/tunnelshow.t
new file mode 100755
index 00000000..1583abb9
--- /dev/null
+++ b/testsuite/tests/bridge/vlan/tunnelshow.t
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+. lib/generic.sh
+
+ts_log "[Testing tunnelshow]"
+
+BR_DEV="$(rand_dev)"
+VX_DEV="$(rand_dev)"
+
+ts_ip "$0" "Add $BR_DEV bridge interface" link add $BR_DEV type bridge
+
+ts_ip "$0" "Add $VX_DEV vxlan interface" \
+	link add $VX_DEV type vxlan dstport 4789 external
+ts_ip "$0" "Enslave $VX_DEV under $BR_DEV" \
+	link set dev $VX_DEV master $BR_DEV
+ts_ip "$0" "Set vlan_tunnel on $VX_DEV" \
+	link set dev $VX_DEV type bridge_slave vlan_tunnel on
+
+ts_bridge "$0" "Add single vlan" vlan add dev $VX_DEV vid 1030
+ts_bridge "$0" "Add tunnel with vni > 16k" \
+	vlan add dev $VX_DEV vid 1030 tunnel_info id 65556
+
+ts_bridge "$0" "Show tunnel info" vlan tunnelshow dev $VX_DEV
+test_on "1030\s+65556"
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH iproute2 7/8] bridge: Deduplicate vlan show functions
  2019-12-16  6:43 [PATCH iproute2 0/8] bridge vlan tunnelshow fixes Benjamin Poirier
                   ` (5 preceding siblings ...)
  2019-12-16  6:43 ` [PATCH iproute2 6/8] bridge: Fix vni printing Benjamin Poirier
@ 2019-12-16  6:43 ` Benjamin Poirier
  2019-12-16  6:43 ` [PATCH iproute2 8/8] bridge: Fix tunnelshow json output Benjamin Poirier
  2019-12-17  4:51 ` [PATCH iproute2 0/8] bridge vlan tunnelshow fixes Stephen Hemminger
  8 siblings, 0 replies; 10+ messages in thread
From: Benjamin Poirier @ 2019-12-16  6:43 UTC (permalink / raw)
  To: netdev; +Cc: Roopa Prabhu

print_vlan() and print_vlan_tunnel() are almost identical copies, save for
a missing newline in the latter which leads to broken output of "vlan
tunnelshow" in normal mode.

Fixes: c7c1a1ef51ae ("bridge: colorize output and use JSON print library")
Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
---
 bridge/vlan.c                            | 91 +++++++-----------------
 testsuite/tests/bridge/vlan/tunnelshow.t |  7 ++
 2 files changed, 34 insertions(+), 64 deletions(-)

diff --git a/bridge/vlan.c b/bridge/vlan.c
index 428eeee3..19283bca 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -16,7 +16,11 @@
 #include "utils.h"
 
 static unsigned int filter_index, filter_vlan;
-static int show_vlan_tunnel_info = 0;
+
+enum vlan_show_subject {
+	VLAN_SHOW_VLAN,
+	VLAN_SHOW_TUNNELINFO,
+};
 
 static void usage(void)
 {
@@ -278,7 +282,7 @@ static void print_range(const char *name, __u32 start, __u32 id)
 
 }
 
-static void print_vlan_tunnel_info(FILE *fp, struct rtattr *tb, int ifindex)
+static void print_vlan_tunnel_info(struct rtattr *tb, int ifindex)
 {
 	struct rtattr *i, *list = tb;
 	int rem = RTA_PAYLOAD(list);
@@ -347,52 +351,9 @@ static void print_vlan_tunnel_info(FILE *fp, struct rtattr *tb, int ifindex)
 		close_vlan_port();
 }
 
-static int print_vlan_tunnel(struct nlmsghdr *n, void *arg)
-{
-	struct ifinfomsg *ifm = NLMSG_DATA(n);
-	struct rtattr *tb[IFLA_MAX+1];
-	int len = n->nlmsg_len;
-	FILE *fp = arg;
-
-	if (n->nlmsg_type != RTM_NEWLINK) {
-		fprintf(stderr, "Not RTM_NEWLINK: %08x %08x %08x\n",
-			n->nlmsg_len, n->nlmsg_type, n->nlmsg_flags);
-		return 0;
-	}
-
-	len -= NLMSG_LENGTH(sizeof(*ifm));
-	if (len < 0) {
-		fprintf(stderr, "BUG: wrong nlmsg len %d\n", len);
-		return -1;
-	}
-
-	if (ifm->ifi_family != AF_BRIDGE)
-		return 0;
-
-	if (filter_index && filter_index != ifm->ifi_index)
-		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(fp, COLOR_IFNAME, "%s",
-				      ll_index_to_name(ifm->ifi_index));
-			fprintf(fp, "\tNone\n");
-		}
-		return 0;
-	}
-
-	print_vlan_tunnel_info(fp, tb[IFLA_AF_SPEC], ifm->ifi_index);
-
-	fflush(fp);
-	return 0;
-}
-
 static int print_vlan(struct nlmsghdr *n, void *arg)
 {
-	FILE *fp = arg;
+	enum vlan_show_subject *subject = arg;
 	struct ifinfomsg *ifm = NLMSG_DATA(n);
 	int len = n->nlmsg_len;
 	struct rtattr *tb[IFLA_MAX+1];
@@ -420,17 +381,24 @@ static int print_vlan(struct nlmsghdr *n, void *arg)
 	/* 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(fp, COLOR_IFNAME, "%s",
+			color_fprintf(stdout, COLOR_IFNAME, "%s",
 				      ll_index_to_name(ifm->ifi_index));
-			fprintf(fp, "\tNone\n");
+			fprintf(stdout, "\tNone\n");
 		}
 		return 0;
 	}
 
-	print_vlan_info(tb[IFLA_AF_SPEC], ifm->ifi_index);
+	switch (*subject) {
+	case VLAN_SHOW_VLAN:
+		print_vlan_info(tb[IFLA_AF_SPEC], ifm->ifi_index);
+		break;
+	case VLAN_SHOW_TUNNELINFO:
+		print_vlan_tunnel_info(tb[IFLA_AF_SPEC], ifm->ifi_index);
+		break;
+	}
 	print_string(PRINT_FP, NULL, "%s", _SL_);
 
-	fflush(fp);
+	fflush(stdout);
 	return 0;
 }
 
@@ -543,7 +511,7 @@ static int print_vlan_stats(struct nlmsghdr *n, void *arg)
 	return 0;
 }
 
-static int vlan_show(int argc, char **argv)
+static int vlan_show(int argc, char **argv, int subject)
 {
 	char *filter_dev = NULL;
 	int ret = 0;
@@ -581,17 +549,13 @@ static int vlan_show(int argc, char **argv)
 		}
 
 		if (!is_json_context()) {
-			if (show_vlan_tunnel_info)
-				printf("port\tvlan ids\ttunnel id\n");
-			else
-				printf("port\tvlan ids\n");
+			printf("port\tvlan ids");
+			if (subject == VLAN_SHOW_TUNNELINFO)
+				printf("\ttunnel id");
+			printf("\n");
 		}
 
-		if (show_vlan_tunnel_info)
-			ret = rtnl_dump_filter(&rth, print_vlan_tunnel,
-					       stdout);
-		else
-			ret = rtnl_dump_filter(&rth, print_vlan, stdout);
+		ret = rtnl_dump_filter(&rth, print_vlan, &subject);
 		if (ret < 0) {
 			fprintf(stderr, "Dump ternminated\n");
 			exit(1);
@@ -677,15 +641,14 @@ int do_vlan(int argc, char **argv)
 		if (matches(*argv, "show") == 0 ||
 		    matches(*argv, "lst") == 0 ||
 		    matches(*argv, "list") == 0)
-			return vlan_show(argc-1, argv+1);
+			return vlan_show(argc-1, argv+1, VLAN_SHOW_VLAN);
 		if (matches(*argv, "tunnelshow") == 0) {
-			show_vlan_tunnel_info = 1;
-			return vlan_show(argc-1, argv+1);
+			return vlan_show(argc-1, argv+1, VLAN_SHOW_TUNNELINFO);
 		}
 		if (matches(*argv, "help") == 0)
 			usage();
 	} else {
-		return vlan_show(0, NULL);
+		return vlan_show(0, NULL, VLAN_SHOW_VLAN);
 	}
 
 	fprintf(stderr, "Command \"%s\" is unknown, try \"bridge vlan help\".\n", *argv);
diff --git a/testsuite/tests/bridge/vlan/tunnelshow.t b/testsuite/tests/bridge/vlan/tunnelshow.t
index 1583abb9..b2141e7c 100755
--- a/testsuite/tests/bridge/vlan/tunnelshow.t
+++ b/testsuite/tests/bridge/vlan/tunnelshow.t
@@ -16,9 +16,16 @@ ts_ip "$0" "Enslave $VX_DEV under $BR_DEV" \
 ts_ip "$0" "Set vlan_tunnel on $VX_DEV" \
 	link set dev $VX_DEV type bridge_slave vlan_tunnel on
 
+ts_bridge "$0" "Add single vlan" vlan add dev $VX_DEV vid 1000
+ts_bridge "$0" "Add single tunnel" \
+	vlan add dev $VX_DEV vid 1000 tunnel_info id 1000
+ts_bridge "$0" "Add vlan range" vlan add dev $VX_DEV vid 1010-1020
+ts_bridge "$0" "Add tunnel range" \
+	vlan add dev $VX_DEV vid 1010-1020 tunnel_info id 1010-1020
 ts_bridge "$0" "Add single vlan" vlan add dev $VX_DEV vid 1030
 ts_bridge "$0" "Add tunnel with vni > 16k" \
 	vlan add dev $VX_DEV vid 1030 tunnel_info id 65556
 
 ts_bridge "$0" "Show tunnel info" vlan tunnelshow dev $VX_DEV
 test_on "1030\s+65556"
+test_lines_count 5
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH iproute2 8/8] bridge: Fix tunnelshow json output
  2019-12-16  6:43 [PATCH iproute2 0/8] bridge vlan tunnelshow fixes Benjamin Poirier
                   ` (6 preceding siblings ...)
  2019-12-16  6:43 ` [PATCH iproute2 7/8] bridge: Deduplicate vlan show functions Benjamin Poirier
@ 2019-12-16  6:43 ` Benjamin Poirier
  2019-12-17  4:51 ` [PATCH iproute2 0/8] bridge vlan tunnelshow fixes Stephen Hemminger
  8 siblings, 0 replies; 10+ messages in thread
From: Benjamin Poirier @ 2019-12-16  6:43 UTC (permalink / raw)
  To: netdev; +Cc: Roopa Prabhu

repeats for "vlan tunnelshow" what commit 0f36267485e3 ("bridge: fix vlan
show formatting") did for "vlan show". This fixes problems in json output.

Note that the resulting json output format of "vlan tunnelshow" is not the
same as the original, introduced in commit 8652eeb3ab12 ("bridge: vlan:
support for per vlan tunnel info"). Changes similar to the ones done for
"vlan show" in commit 0f36267485e3 ("bridge: fix vlan show formatting") are
carried over to "vlan tunnelshow".

Fixes: c7c1a1ef51ae ("bridge: colorize output and use JSON print library")
Fixes: 0f36267485e3 ("bridge: fix vlan show formatting")
Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com>
---
 bridge/vlan.c                            | 27 +++++++-----------------
 testsuite/tests/bridge/vlan/tunnelshow.t |  2 ++
 2 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/bridge/vlan.c b/bridge/vlan.c
index 19283bca..205851e4 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -256,12 +256,14 @@ static int filter_vlan_check(__u16 vid, __u16 flags)
 	return 1;
 }
 
-static void open_vlan_port(int ifi_index, const char *fmt)
+static void open_vlan_port(int ifi_index, const char *fmt,
+			   enum vlan_show_subject subject)
 {
 	open_json_object(NULL);
 	print_color_string(PRINT_ANY, COLOR_IFNAME, "ifname", fmt,
 			   ll_index_to_name(ifi_index));
-	open_json_array(PRINT_JSON, "vlans");
+	open_json_array(PRINT_JSON,
+			subject == VLAN_SHOW_VLAN ? "vlans": "tunnels");
 }
 
 static void close_vlan_port(void)
@@ -289,10 +291,8 @@ static void print_vlan_tunnel_info(struct rtattr *tb, int ifindex)
 	__u16 last_vid_start = 0;
 	__u32 last_tunid_start = 0;
 
-	if (!filter_vlan)
-		open_vlan_port(ifindex, "%s");
+	open_vlan_port(ifindex, "%s", VLAN_SHOW_TUNNELINFO);
 
-	open_json_array(PRINT_JSON, "tunnel");
 	for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
 		struct rtattr *ttb[IFLA_BRIDGE_VLAN_TUNNEL_MAX+1];
 		__u32 tunnel_id = 0;
@@ -331,24 +331,13 @@ static void print_vlan_tunnel_info(struct rtattr *tb, int ifindex)
 		else if (vcheck_ret == 0)
 			continue;
 
-		if (tunnel_flags & BRIDGE_VLAN_INFO_RANGE_BEGIN)
-			continue;
-
-		if (filter_vlan)
-			open_vlan_port(ifindex, "%s");
-
 		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_);
-		if (filter_vlan)
-			close_vlan_port();
 	}
-
-	if (!filter_vlan)
-		close_vlan_port();
+	close_vlan_port();
 }
 
 static int print_vlan(struct nlmsghdr *n, void *arg)
@@ -467,7 +456,7 @@ 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");
+			open_vlan_port(ifindex, "%-16s", VLAN_SHOW_VLAN);
 			found_vlan = true;
 		} else {
 			print_string(PRINT_FP, NULL, "%-16s", "");
@@ -600,7 +589,7 @@ void print_vlan_info(struct rtattr *tb, int ifindex)
 	int rem = RTA_PAYLOAD(list);
 	__u16 last_vid_start = 0;
 
-	open_vlan_port(ifindex, "%s");
+	open_vlan_port(ifindex, "%s", VLAN_SHOW_VLAN);
 
 	for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
 		struct bridge_vlan_info *vinfo;
diff --git a/testsuite/tests/bridge/vlan/tunnelshow.t b/testsuite/tests/bridge/vlan/tunnelshow.t
index b2141e7c..fd41bfcb 100755
--- a/testsuite/tests/bridge/vlan/tunnelshow.t
+++ b/testsuite/tests/bridge/vlan/tunnelshow.t
@@ -29,3 +29,5 @@ 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
+
+ts_bridge "$0" "Dump tunnel info" -j vlan tunnelshow dev $VX_DEV
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH iproute2 0/8] bridge vlan tunnelshow fixes
  2019-12-16  6:43 [PATCH iproute2 0/8] bridge vlan tunnelshow fixes Benjamin Poirier
                   ` (7 preceding siblings ...)
  2019-12-16  6:43 ` [PATCH iproute2 8/8] bridge: Fix tunnelshow json output Benjamin Poirier
@ 2019-12-17  4:51 ` Stephen Hemminger
  8 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2019-12-17  4:51 UTC (permalink / raw)
  To: Benjamin Poirier; +Cc: netdev, Roopa Prabhu

On Mon, 16 Dec 2019 15:43:36 +0900
Benjamin Poirier <bpoirier@cumulusnetworks.com> wrote:

> Fix various problems in and around normal and json output of `bridge vlan
> tunnelshow`.
> 
> Can be tested using:
> ip link add bridge type bridge
> 
> ip link add vxlan0 type vxlan dstport 4789 external
> ip link set dev vxlan0 master bridge
> ip link set dev vxlan0 type bridge_slave vlan_tunnel on
> 
> bridge vlan add dev vxlan0 vid 1000
> bridge vlan add dev vxlan0 vid 1000 tunnel_info id 1000
> bridge vlan add dev vxlan0 vid 1010-1020
> bridge vlan add dev vxlan0 vid 1010-1020 tunnel_info id 1010-1020
> bridge vlan add dev vxlan0 vid 1030
> bridge vlan add dev vxlan0 vid 1030 tunnel_info id 65556
> 
> Benjamin Poirier (8):
>   json_print: Remove declaration without implementation
>   testsuite: Fix line count test
>   bridge: Fix typo in error messages
>   bridge: Fix src_vni argument in man page
>   bridge: Fix BRIDGE_VLAN_TUNNEL attribute sizes
>   bridge: Fix vni printing
>   bridge: Deduplicate vlan show functions
>   bridge: Fix tunnelshow json output
> 
>  bridge/vlan.c                            | 138 ++++++++---------------
>  include/json_print.h                     |   2 -
>  man/man8/bridge.8                        |   4 +-
>  testsuite/Makefile                       |   3 +-
>  testsuite/lib/generic.sh                 |   8 +-
>  testsuite/tests/bridge/vlan/tunnelshow.t |  33 ++++++
>  6 files changed, 88 insertions(+), 100 deletions(-)
>  create mode 100755 testsuite/tests/bridge/vlan/tunnelshow.t
> 

Thanks for cleaning this up. Applied

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-12-17  4:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-16  6:43 [PATCH iproute2 0/8] bridge vlan tunnelshow fixes Benjamin Poirier
2019-12-16  6:43 ` [PATCH iproute2 1/8] json_print: Remove declaration without implementation Benjamin Poirier
2019-12-16  6:43 ` [PATCH iproute2 2/8] testsuite: Fix line count test Benjamin Poirier
2019-12-16  6:43 ` [PATCH iproute2 3/8] bridge: Fix typo in error messages Benjamin Poirier
2019-12-16  6:43 ` [PATCH iproute2 4/8] bridge: Fix src_vni argument in man page Benjamin Poirier
2019-12-16  6:43 ` [PATCH iproute2 5/8] bridge: Fix BRIDGE_VLAN_TUNNEL attribute sizes Benjamin Poirier
2019-12-16  6:43 ` [PATCH iproute2 6/8] bridge: Fix vni printing Benjamin Poirier
2019-12-16  6:43 ` [PATCH iproute2 7/8] bridge: Deduplicate vlan show functions Benjamin Poirier
2019-12-16  6:43 ` [PATCH iproute2 8/8] bridge: Fix tunnelshow json output Benjamin Poirier
2019-12-17  4:51 ` [PATCH iproute2 0/8] bridge vlan tunnelshow fixes Stephen Hemminger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.