* iproute2 broken vlan stats output
@ 2019-04-16 12:38 Nikolay Aleksandrov
2019-04-16 14:28 ` Roopa Prabhu
0 siblings, 1 reply; 6+ messages in thread
From: Nikolay Aleksandrov @ 2019-04-16 12:38 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: network dev, David Ahern, Roopa Prabhu
Hi Stephen,
It seems standard (non-json) vlan stats output has been broken since commit:
0f36267485e3 ("bridge: fix vlan show formatting")
bridge -s vlan show output before commit 0f36267485e3:
port vlan id
br0 1 PVID untagged
RX: 0 bytes 0 packets
TX: 0 bytes 0 packets
eth4 1 PVID untagged
RX: 0 bytes 0 packets
TX: 0 bytes 0 packets
bridge -s vlan show output after commit 0f36267485e3:
root@debian~/# bridge -s vlan
port vlan id
br0br0 1 PVID Egress Untagged
RX: 0 bytes 0 packets
TX: 0 bytes 0 packetseth4eth4 1 PVID Egress Untagged
RX: 0 bytes 0 packets
TX: 0 bytes 0 packetsroot@debian:~/#
Cheers,
Nik
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: iproute2 broken vlan stats output 2019-04-16 12:38 iproute2 broken vlan stats output Nikolay Aleksandrov @ 2019-04-16 14:28 ` Roopa Prabhu 2019-04-16 14:33 ` Nikolay Aleksandrov 0 siblings, 1 reply; 6+ messages in thread From: Roopa Prabhu @ 2019-04-16 14:28 UTC (permalink / raw) To: Nikolay Aleksandrov; +Cc: Stephen Hemminger, network dev, David Ahern On Tue, Apr 16, 2019 at 5:38 AM Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote: > > Hi Stephen, > It seems standard (non-json) vlan stats output has been broken since commit: > 0f36267485e3 ("bridge: fix vlan show formatting") > > bridge -s vlan show output before commit 0f36267485e3: > port vlan id > br0 1 PVID untagged > RX: 0 bytes 0 packets > TX: 0 bytes 0 packets > eth4 1 PVID untagged > RX: 0 bytes 0 packets > TX: 0 bytes 0 packets > > bridge -s vlan show output after commit 0f36267485e3: > root@debian~/# bridge -s vlan > port vlan id > br0br0 1 PVID Egress Untagged > RX: 0 bytes 0 packets > TX: 0 bytes 0 packetseth4eth4 1 PVID Egress Untagged > RX: 0 bytes 0 packets > TX: 0 bytes 0 packetsroot@debian:~/# > :(. Looks like iproute2 should have selftests too. Its hard to catch these in code review. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: iproute2 broken vlan stats output 2019-04-16 14:28 ` Roopa Prabhu @ 2019-04-16 14:33 ` Nikolay Aleksandrov 2019-04-16 15:14 ` [PATCH iproute2] bridge: vlan: fix standard " Nikolay Aleksandrov 0 siblings, 1 reply; 6+ messages in thread From: Nikolay Aleksandrov @ 2019-04-16 14:33 UTC (permalink / raw) To: Roopa Prabhu; +Cc: Stephen Hemminger, network dev, David Ahern On 16/04/2019 17:28, Roopa Prabhu wrote: > On Tue, Apr 16, 2019 at 5:38 AM Nikolay Aleksandrov > <nikolay@cumulusnetworks.com> wrote: >> >> Hi Stephen, >> It seems standard (non-json) vlan stats output has been broken since commit: >> 0f36267485e3 ("bridge: fix vlan show formatting") >> >> bridge -s vlan show output before commit 0f36267485e3: >> port vlan id >> br0 1 PVID untagged >> RX: 0 bytes 0 packets >> TX: 0 bytes 0 packets >> eth4 1 PVID untagged >> RX: 0 bytes 0 packets >> TX: 0 bytes 0 packets >> >> bridge -s vlan show output after commit 0f36267485e3: >> root@debian~/# bridge -s vlan >> port vlan id >> br0br0 1 PVID Egress Untagged >> RX: 0 bytes 0 packets >> TX: 0 bytes 0 packetseth4eth4 1 PVID Egress Untagged >> RX: 0 bytes 0 packets >> TX: 0 bytes 0 packetsroot@debian:~/# >> > > :(. Looks like iproute2 should have selftests too. Its hard to catch > these in code review. > I made a mistake about the commit, the actual culprits are: 45fca4ed9412 ("bridge: fix vlan show stats formatting") c7c1a1ef51ae ("bridge: colorize output and use JSON print library") They both broke a different thing about standard stats output: - 45fca4ed9412 ("bridge: fix vlan show stats formatting") Added a second print of an interface name (e.g. eth4eth4) - c7c1a1ef51ae ("bridge: colorize output and use JSON print library") Broke normal vlan stats output by not printing a new line after them I'll post a fix in a minute. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH iproute2] bridge: vlan: fix standard stats output 2019-04-16 14:33 ` Nikolay Aleksandrov @ 2019-04-16 15:14 ` Nikolay Aleksandrov 2019-04-16 15:30 ` [PATCH iproute2 v2] " Nikolay Aleksandrov 0 siblings, 1 reply; 6+ messages in thread From: Nikolay Aleksandrov @ 2019-04-16 15:14 UTC (permalink / raw) To: netdev; +Cc: roopa, Stephen Hemminger, Nikolay Aleksandrov Each of the commits below broke the vlan stats output in a different way: - 45fca4ed9412 ("bridge: fix vlan show stats formatting") Added a second print of an interface name (e.g. eth4eth4) - c7c1a1ef51ae ("bridge: colorize output and use JSON print library") Broke normal vlan stats output by not printing a new line after them Also printed interfaces without any vlans when printing stats This fix is not pretty but it brings back the previous behaviour. Before this fix: $ bridge -s vlan show port vlan id br0br0 1 PVID Egress Untagged RX: 0 bytes 0 packets TX: 0 bytes 0 packets 4 RX: 0 bytes 0 packets TX: 0 bytes 0 packetseth4eth4 4 RX: 0 bytes 0 packets TX: 0 bytes 0 packetsroot@debian:~/ After this fix: $ bridge -s vlan show port vlan id br0 1 PVID Egress Untagged RX: 0 bytes 0 packets TX: 0 bytes 0 packets 4 RX: 0 bytes 0 packets TX: 0 bytes 0 packets eth4 4 RX: 0 bytes 0 packets TX: 0 bytes 0 packets Fixes: 45fca4ed9412 ("bridge: fix vlan show stats formatting") Fixes: c7c1a1ef51ae ("bridge: colorize output and use JSON print library") Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> --- bridge/vlan.c | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/bridge/vlan.c b/bridge/vlan.c index d075a42d139b..dbed7fbd8c02 100644 --- a/bridge/vlan.c +++ b/bridge/vlan.c @@ -252,11 +252,11 @@ static int filter_vlan_check(__u16 vid, __u16 flags) return 1; } -static void open_vlan_port(int ifi_index) +static void open_vlan_port(int ifi_index, const char *fmt) { open_json_object(NULL); - print_string(PRINT_ANY, "ifname", "%s", - ll_index_to_name(ifi_index)); + print_color_string(PRINT_ANY, COLOR_IFNAME, "ifname", fmt, + ll_index_to_name(ifi_index)); open_json_array(PRINT_JSON, "vlans"); } @@ -286,7 +286,7 @@ static void print_vlan_tunnel_info(FILE *fp, struct rtattr *tb, int ifindex) __u32 last_tunid_start = 0; if (!filter_vlan) - open_vlan_port(ifindex); + open_vlan_port(ifindex, "%s"); open_json_array(PRINT_JSON, "tunnel"); for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) { @@ -331,7 +331,7 @@ static void print_vlan_tunnel_info(FILE *fp, struct rtattr *tb, int ifindex) continue; if (filter_vlan) - open_vlan_port(ifindex); + open_vlan_port(ifindex, "%s"); open_json_object(NULL); print_range("vlan", last_vid_start, tunnel_vid); @@ -448,9 +448,15 @@ static void print_vlan_flags(__u16 flags) close_json_array(PRINT_JSON, NULL); } -static void print_one_vlan_stats(const struct bridge_vlan_xstats *vstats) +static void print_one_vlan_stats(const struct bridge_vlan_xstats *vstats, + int vlan_idx) { open_json_object(NULL); + /* we need to know if this is the initial vlan to align vlan ids + * properly under it + */ + if (vlan_idx > 0) + print_string(PRINT_FP, NULL, "%-16s", ""); print_hu(PRINT_ANY, "vid", " %hu", vstats->vid); print_vlan_flags(vstats->flags); @@ -463,7 +469,7 @@ static void print_one_vlan_stats(const struct bridge_vlan_xstats *vstats) print_lluint(PRINT_ANY, "tx_bytes", " TX: %llu bytes", vstats->tx_bytes); - print_lluint(PRINT_ANY, "tx_packets", " %llu packets", + print_lluint(PRINT_ANY, "tx_packets", " %llu packets\n", vstats->tx_packets); close_json_object(); } @@ -472,8 +478,7 @@ static void print_vlan_stats_attr(struct rtattr *attr, int ifindex) { struct rtattr *brtb[LINK_XSTATS_TYPE_MAX+1]; struct rtattr *i, *list; - const char *ifname; - int rem; + int rem, vlan_idx; parse_rtattr(brtb, LINK_XSTATS_TYPE_MAX, RTA_DATA(attr), RTA_PAYLOAD(attr)); @@ -483,13 +488,8 @@ static void print_vlan_stats_attr(struct rtattr *attr, int ifindex) list = brtb[LINK_XSTATS_TYPE_BRIDGE]; rem = RTA_PAYLOAD(list); - ifname = ll_index_to_name(ifindex); - open_vlan_port(ifindex); - - print_color_string(PRINT_FP, COLOR_IFNAME, - NULL, "%-16s", ifname); - - for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) { + for (i = RTA_DATA(list), vlan_idx = 0; RTA_OK(i, rem); + i = RTA_NEXT(i, rem)) { const struct bridge_vlan_xstats *vstats = RTA_DATA(i); if (i->rta_type != BRIDGE_XSTATS_VLAN) @@ -503,9 +503,17 @@ static void print_vlan_stats_attr(struct rtattr *attr, int ifindex) !(vstats->flags & BRIDGE_VLAN_INFO_BRENTRY)) continue; - print_one_vlan_stats(vstats); + /* found vlan stats, print the interface name */ + if (vlan_idx == 0) + open_vlan_port(ifindex, "%-16s"); + + print_one_vlan_stats(vstats, vlan_idx); + vlan_idx++; } - close_vlan_port(); + + /* vlan port is opened only if there are any vlan stats */ + if (vlan_idx > 0) + close_vlan_port(); } static int print_vlan_stats(struct nlmsghdr *n, void *arg) @@ -632,7 +640,7 @@ void print_vlan_info(struct rtattr *tb, int ifindex) int rem = RTA_PAYLOAD(list); __u16 last_vid_start = 0; - open_vlan_port(ifindex); + open_vlan_port(ifindex, "%s"); for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) { struct bridge_vlan_info *vinfo; -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH iproute2 v2] bridge: vlan: fix standard stats output 2019-04-16 15:14 ` [PATCH iproute2] bridge: vlan: fix standard " Nikolay Aleksandrov @ 2019-04-16 15:30 ` Nikolay Aleksandrov 2019-04-17 23:34 ` Stephen Hemminger 0 siblings, 1 reply; 6+ messages in thread From: Nikolay Aleksandrov @ 2019-04-16 15:30 UTC (permalink / raw) To: netdev; +Cc: roopa, Stephen Hemminger, Nikolay Aleksandrov Each of the commits below broke the vlan stats output in a different way: - 45fca4ed9412 ("bridge: fix vlan show stats formatting") Added a second print of an interface name (e.g. eth4eth4) - c7c1a1ef51ae ("bridge: colorize output and use JSON print library") Broke normal vlan stats output by not printing a new line after them Also printed interfaces without any vlans when printing stats This fix is not pretty but it brings back the previous behaviour. Before this fix: $ bridge -s vlan show port vlan id br0br0 1 PVID Egress Untagged RX: 0 bytes 0 packets TX: 0 bytes 0 packets 4 RX: 0 bytes 0 packets TX: 0 bytes 0 packetseth4eth4 4 RX: 0 bytes 0 packets TX: 0 bytes 0 packetsroot@debian:~/ After this fix: $ bridge -s vlan show port vlan id br0 1 PVID Egress Untagged RX: 0 bytes 0 packets TX: 0 bytes 0 packets 4 RX: 0 bytes 0 packets TX: 0 bytes 0 packets eth4 4 RX: 0 bytes 0 packets TX: 0 bytes 0 packets Fixes: 45fca4ed9412 ("bridge: fix vlan show stats formatting") Fixes: c7c1a1ef51ae ("bridge: colorize output and use JSON print library") Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> --- v2: simplify print_vlan_stats_attr bridge/vlan.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/bridge/vlan.c b/bridge/vlan.c index d075a42d139b..6d33b0a990a5 100644 --- a/bridge/vlan.c +++ b/bridge/vlan.c @@ -252,11 +252,11 @@ static int filter_vlan_check(__u16 vid, __u16 flags) return 1; } -static void open_vlan_port(int ifi_index) +static void open_vlan_port(int ifi_index, const char *fmt) { open_json_object(NULL); - print_string(PRINT_ANY, "ifname", "%s", - ll_index_to_name(ifi_index)); + print_color_string(PRINT_ANY, COLOR_IFNAME, "ifname", fmt, + ll_index_to_name(ifi_index)); open_json_array(PRINT_JSON, "vlans"); } @@ -286,7 +286,7 @@ static void print_vlan_tunnel_info(FILE *fp, struct rtattr *tb, int ifindex) __u32 last_tunid_start = 0; if (!filter_vlan) - open_vlan_port(ifindex); + open_vlan_port(ifindex, "%s"); open_json_array(PRINT_JSON, "tunnel"); for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) { @@ -331,7 +331,7 @@ static void print_vlan_tunnel_info(FILE *fp, struct rtattr *tb, int ifindex) continue; if (filter_vlan) - open_vlan_port(ifindex); + open_vlan_port(ifindex, "%s"); open_json_object(NULL); print_range("vlan", last_vid_start, tunnel_vid); @@ -463,7 +463,7 @@ static void print_one_vlan_stats(const struct bridge_vlan_xstats *vstats) print_lluint(PRINT_ANY, "tx_bytes", " TX: %llu bytes", vstats->tx_bytes); - print_lluint(PRINT_ANY, "tx_packets", " %llu packets", + print_lluint(PRINT_ANY, "tx_packets", " %llu packets\n", vstats->tx_packets); close_json_object(); } @@ -472,7 +472,7 @@ static void print_vlan_stats_attr(struct rtattr *attr, int ifindex) { struct rtattr *brtb[LINK_XSTATS_TYPE_MAX+1]; struct rtattr *i, *list; - const char *ifname; + bool found_vlan = false; int rem; parse_rtattr(brtb, LINK_XSTATS_TYPE_MAX, RTA_DATA(attr), @@ -483,12 +483,6 @@ static void print_vlan_stats_attr(struct rtattr *attr, int ifindex) list = brtb[LINK_XSTATS_TYPE_BRIDGE]; rem = RTA_PAYLOAD(list); - ifname = ll_index_to_name(ifindex); - open_vlan_port(ifindex); - - print_color_string(PRINT_FP, COLOR_IFNAME, - NULL, "%-16s", ifname); - for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) { const struct bridge_vlan_xstats *vstats = RTA_DATA(i); @@ -503,9 +497,19 @@ static void print_vlan_stats_attr(struct rtattr *attr, int ifindex) !(vstats->flags & BRIDGE_VLAN_INFO_BRENTRY)) continue; + /* found vlan stats, first time print the interface name */ + if (!found_vlan) { + open_vlan_port(ifindex, "%-16s"); + found_vlan = true; + } else { + print_string(PRINT_FP, NULL, "%-16s", ""); + } print_one_vlan_stats(vstats); } - close_vlan_port(); + + /* vlan_port is opened only if there are any vlan stats */ + if (found_vlan) + close_vlan_port(); } static int print_vlan_stats(struct nlmsghdr *n, void *arg) @@ -632,7 +636,7 @@ void print_vlan_info(struct rtattr *tb, int ifindex) int rem = RTA_PAYLOAD(list); __u16 last_vid_start = 0; - open_vlan_port(ifindex); + open_vlan_port(ifindex, "%s"); for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) { struct bridge_vlan_info *vinfo; -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH iproute2 v2] bridge: vlan: fix standard stats output 2019-04-16 15:30 ` [PATCH iproute2 v2] " Nikolay Aleksandrov @ 2019-04-17 23:34 ` Stephen Hemminger 0 siblings, 0 replies; 6+ messages in thread From: Stephen Hemminger @ 2019-04-17 23:34 UTC (permalink / raw) To: Nikolay Aleksandrov; +Cc: netdev, roopa On Tue, 16 Apr 2019 18:30:12 +0300 Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote: > Each of the commits below broke the vlan stats output in a different > way: > - 45fca4ed9412 ("bridge: fix vlan show stats formatting") > Added a second print of an interface name (e.g. eth4eth4) > - c7c1a1ef51ae ("bridge: colorize output and use JSON print library") > Broke normal vlan stats output by not printing a new line after them > Also printed interfaces without any vlans when printing stats > > This fix is not pretty but it brings back the previous behaviour. Ok, applied. The output format here makes it hard to code. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-17 23:34 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-16 12:38 iproute2 broken vlan stats output Nikolay Aleksandrov 2019-04-16 14:28 ` Roopa Prabhu 2019-04-16 14:33 ` Nikolay Aleksandrov 2019-04-16 15:14 ` [PATCH iproute2] bridge: vlan: fix standard " Nikolay Aleksandrov 2019-04-16 15:30 ` [PATCH iproute2 v2] " Nikolay Aleksandrov 2019-04-17 23:34 ` 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.