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>

diff --git a/bridge/vlan.c b/bridge/vlan.c
index 37ff2973d7927cd0e84ec00fad01a91299f0f609..b126fae25eb4e666816f2515804a1665c5fb9bdb 100644 (file)
--- 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 (executable)
index 0000000..3def202
--- /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 fd41bfcb32a2bad5be615770ea678e1d9bfb77a0..3e9c12a21a9ccfde1a20ab06bef500eab03b6ad1 100755 (executable)
--- 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