Re: bgpctl: enlarge columns for 4-byte ASN display
On Sun, Jul 27, 2014 at 10:06:12PM +0100, Stuart Henderson wrote: [...] Nice, I like that a lot. What do you think Claudio? [...] Ping. Are there remaining issues with the patch? -- Gregor Best
Re: bgpctl: enlarge columns for 4-byte ASN display
Ping. -- Gregor Best
Re: bgpctl: enlarge columns for 4-byte ASN display
Ping? Are there remaining issues with the patch or should I leave you guys alone until the release stress is over? -- Gregor Best
Re: bgpctl: enlarge columns for 4-byte ASN display
Not a big fan since this makes the bgpctl show output no longer fit 80 chars and so will wrap lines on default terminals. While it is OK to increase the size it should be taken away from other fields in some whay. An option would be to drop the OutQ since that field has only limited value IMO. -- :wq Claudio PS: you only need 11 chars and not 12 for AS-DOT notation On Sat, Jul 26, 2014 at 03:57:16PM +0200, Gregor Best wrote: Hi people, I'm running OpenBGPD with a few peers that have large 4-byte AS numbers. Displaying the table of peers with `bgpctl show` shows the stats just fine, but the length of the AS number shifts the content of the statistics confusingly far to the right, such as in the following example: $ bgpctl show Neighbor ASMsgRcvdMsgSent OutQ Up/Down State/PrfRcvd peer 1 64734.12580316466 0 00:07:04364 peer 2 64734.12578251293 0 00:40:06365 peer 3 64828 17545 13466 0 3d17h35m325 The attached patch makes the 'AS' column 4 characters larger, with the following result: $ bgpctl show Neighbor ASMsgRcvdMsgSent OutQ Up/Down State/PrfRcvd peer 164734.12580317468 0 00:07:51 364 peer 264734.12578252295 0 00:40:53 365 peer 3 64828 17547 13467 0 3d17h36m 325 -- Gregor Best Index: bgpctl.c === RCS file: /mnt/media/cvs/src/usr.sbin/bgpctl/bgpctl.c,v retrieving revision 1.174 diff -u -p -u -r1.174 bgpctl.c --- bgpctl.c 18 Mar 2014 13:47:14 - 1.174 +++ bgpctl.c 26 Jul 2014 13:49:17 - @@ -508,7 +508,7 @@ fmt_peer(const char *descr, const struct void show_summary_head(void) { - printf(%-20s %8s %10s %10s %5s %-8s %s\n, Neighbor, AS, + printf(%-20s %12s %10s %10s %5s %-8s %s\n, Neighbor, AS, MsgRcvd, MsgSent, OutQ, Up/Down, State/PrfRcvd); } @@ -525,7 +525,7 @@ show_summary_msg(struct imsg *imsg, int p-conf.remote_masklen, nodescr); if (strlen(s) = 20) s[20] = 0; - printf(%-20s %8s %10llu %10llu %5u %-8s , + printf(%-20s %12s %10llu %10llu %5u %-8s , s, log_as(p-conf.remote_as), p-stats.msg_rcvd_open + p-stats.msg_rcvd_notification + p-stats.msg_rcvd_update + p-stats.msg_rcvd_keepalive +
Re: bgpctl: enlarge columns for 4-byte ASN display
On Sun, Jul 27, 2014 at 11:15:41AM +0200, Claudio Jeker wrote: Not a big fan since this makes the bgpctl show output no longer fit 80 chars and so will wrap lines on default terminals. [...] Agreed, that's not good. While it is OK to increase the size it should be taken away from other fields in some whay. An option would be to drop the OutQ since that field has only limited value IMO. [...] Right. I dare say on most links the OutQ column is 0 most of the time anyway, or the link has problems that are visible in other ways as well. [...] PS: you only need 11 chars and not 12 for AS-DOT notation [...] Whoops, off by one. The attached patch addresses the issues you pointed out by removing the OutQ field and increasing the AS column width by 3 characters. The output fits into 77 characters now. -- Gregor Best Index: bgpctl.c === RCS file: /mnt/media/cvs/src/usr.sbin/bgpctl/bgpctl.c,v retrieving revision 1.174 diff -u -p -u -r1.174 bgpctl.c --- bgpctl.c18 Mar 2014 13:47:14 - 1.174 +++ bgpctl.c27 Jul 2014 09:53:18 - @@ -508,8 +508,8 @@ fmt_peer(const char *descr, const struct void show_summary_head(void) { - printf(%-20s %8s %10s %10s %5s %-8s %s\n, Neighbor, AS, - MsgRcvd, MsgSent, OutQ, Up/Down, State/PrfRcvd); + printf(%-20s %11s %10s %10s %-8s %s\n, Neighbor, AS, + MsgRcvd, MsgSent, Up/Down, State/PrfRcvd); } int @@ -525,7 +525,7 @@ show_summary_msg(struct imsg *imsg, int p-conf.remote_masklen, nodescr); if (strlen(s) = 20) s[20] = 0; - printf(%-20s %8s %10llu %10llu %5u %-8s , + printf(%-20s %11s %10llu %10llu %-8s , s, log_as(p-conf.remote_as), p-stats.msg_rcvd_open + p-stats.msg_rcvd_notification + p-stats.msg_rcvd_update + p-stats.msg_rcvd_keepalive + @@ -533,7 +533,6 @@ show_summary_msg(struct imsg *imsg, int p-stats.msg_sent_open + p-stats.msg_sent_notification + p-stats.msg_sent_update + p-stats.msg_sent_keepalive + p-stats.msg_sent_rrefresh, - p-wbuf.queued, fmt_timeframe(p-stats.last_updown)); if (p-state == STATE_ESTABLISHED) { printf(%6u, p-stats.prefix_cnt);
Re: bgpctl: enlarge columns for 4-byte ASN display
On 2014/07/27 11:15, Claudio Jeker wrote: Not a big fan since this makes the bgpctl show output no longer fit 80 chars and so will wrap lines on default terminals. While it is OK to increase the size it should be taken away from other fields in some whay. An option would be to drop the OutQ since that field has only limited value IMO. oh, I would miss not having OutQ, it's totally useless when things are working correctly, but I have had a couple of situations (one with openbgp, one with *cough* bay BCN some time ago...) where this has been helpful. It gives quite a clear pointer to problems transmitting to that particular peer without having to look in netstat output and work out which peer is which (some physical layer problems, MTU problems etc). I think the most usable display would be to have neighbor and AS share a column, with priority to AS... i.e. somepeer12345 16419 89603 0 09:42:06100/8000 somepeer-with-long-name 567816049401367282 0 13:39:28100/8000 otherpeer16584484 16420 89606 0 1d00h58m Active otherpeer-with-long- 1658448516049401367282 0 13:40:17100/8000
Re: bgpctl: enlarge columns for 4-byte ASN display
On Sun, Jul 27, 2014 at 03:36:06PM +0100, Stuart Henderson wrote: On 2014/07/27 11:15, Claudio Jeker wrote: Not a big fan since this makes the bgpctl show output no longer fit 80 chars and so will wrap lines on default terminals. While it is OK to increase the size it should be taken away from other fields in some whay. An option would be to drop the OutQ since that field has only limited value IMO. oh, I would miss not having OutQ, it's totally useless when things are working correctly, but I have had a couple of situations (one with openbgp, one with *cough* bay BCN some time ago...) where this has been helpful. It gives quite a clear pointer to problems transmitting to that particular peer without having to look in netstat output and work out which peer is which (some physical layer problems, MTU problems etc). I think the most usable display would be to have neighbor and AS share a column, with priority to AS... i.e. somepeer12345 16419 89603 0 09:42:06100/8000 somepeer-with-long-name 567816049401367282 0 13:39:28100/8000 otherpeer16584484 16420 89606 0 1d00h58m Active otherpeer-with-long- 1658448516049401367282 0 13:40:17100/8000 The attached patch does just that. Maximum line length is now 80 characters (excluding the trailing newline), and the Neighbor and AS columns now take a combined maximum of 29 characters, including the blank separating peer name and AS. The result looks like this: $ bgpctl show Neighbor ASMsgRcvdMsgSent OutQ Up/Down State/PrfRcvd other-peer-with-l 16581.44851 0 0 0 NeverConnect other-peer 1658.44841 0 0 0 NeverActive some-peer-with-long-name 5678 0 0 0 NeverActive some-peer 12345 0 0 0 NeverActive Where before it was $ bgpctl show Neighbor ASMsgRcvdMsgSent OutQ Up/Down State/PrfRcvd other-peer-with-long 16581.44851 0 0 0 NeverConnect other-peer 1658.44841 0 0 0 NeverActive some-peer-with-long- 5678 0 0 0 NeverActive some-peer 12345 0 0 0 NeverConnect -- Gregor Best Index: bgpctl.c === RCS file: /mnt/media/cvs/src/usr.sbin/bgpctl/bgpctl.c,v retrieving revision 1.174 diff -u -p -u -r1.174 bgpctl.c --- bgpctl.c18 Mar 2014 13:47:14 - 1.174 +++ bgpctl.c27 Jul 2014 15:20:59 - @@ -517,16 +517,26 @@ show_summary_msg(struct imsg *imsg, int { struct peer *p; char*s; + const char *a; + char*pa; + size_t alen; switch (imsg-hdr.type) { case IMSG_CTL_SHOW_NEIGHBOR: p = imsg-data; s = fmt_peer(p-conf.descr, p-conf.remote_addr, p-conf.remote_masklen, nodescr); - if (strlen(s) = 20) - s[20] = 0; - printf(%-20s %8s %10llu %10llu %5u %-8s , - s, log_as(p-conf.remote_as), + + a = log_as(p-conf.remote_as); + alen = strlen(a); + + if (asprintf(pa, %-28s, s) == -1) + err(1, NULL); + if (strlen(pa) 28 - alen) + pa[28 - alen] = 0; + + printf(%s %s %10llu %10llu %5u %-8s , + pa, a, p-stats.msg_rcvd_open + p-stats.msg_rcvd_notification + p-stats.msg_rcvd_update + p-stats.msg_rcvd_keepalive + p-stats.msg_rcvd_rrefresh, @@ -544,6 +554,7 @@ show_summary_msg(struct imsg *imsg, int else printf(%s, statenames[p-state]); printf(\n); + free(pa); free(s); break; case IMSG_CTL_END:
Re: bgpctl: enlarge columns for 4-byte ASN display
On Sun, Jul 27, 2014 at 03:36:06PM +0100, Stuart Henderson wrote: On 2014/07/27 11:15, Claudio Jeker wrote: Not a big fan since this makes the bgpctl show output no longer fit 80 chars and so will wrap lines on default terminals. While it is OK to increase the size it should be taken away from other fields in some whay. An option would be to drop the OutQ since that field has only limited value IMO. oh, I would miss not having OutQ, it's totally useless when things are working correctly, but I have had a couple of situations (one with openbgp, one with *cough* bay BCN some time ago...) where this has been helpful. It gives quite a clear pointer to problems transmitting to that particular peer without having to look in netstat output and work out which peer is which (some physical layer problems, MTU problems etc). In that case I would suggest to move that information to the bgpctl show neighbor somepeer detail view. It shows more counters and is probably better for debugging a particular peering that has issues. bgpctl show sum should only give a summary to get an overview. I think the most usable display would be to have neighbor and AS share a column, with priority to AS... i.e. somepeer12345 16419 89603 0 09:42:06100/8000 somepeer-with-long-name 567816049401367282 0 13:39:28100/8000 otherpeer16584484 16420 89606 0 1d00h58m Active otherpeer-with-long- 1658448516049401367282 0 13:40:17100/8000 Yes, that would be better. -- :wq Claudio
Re: bgpctl: enlarge columns for 4-byte ASN display
On 2014/07/27 17:24, Gregor Best wrote: On Sun, Jul 27, 2014 at 03:36:06PM +0100, Stuart Henderson wrote: On 2014/07/27 11:15, Claudio Jeker wrote: Not a big fan since this makes the bgpctl show output no longer fit 80 chars and so will wrap lines on default terminals. While it is OK to increase the size it should be taken away from other fields in some whay. An option would be to drop the OutQ since that field has only limited value IMO. oh, I would miss not having OutQ, it's totally useless when things are working correctly, but I have had a couple of situations (one with openbgp, one with *cough* bay BCN some time ago...) where this has been helpful. It gives quite a clear pointer to problems transmitting to that particular peer without having to look in netstat output and work out which peer is which (some physical layer problems, MTU problems etc). I think the most usable display would be to have neighbor and AS share a column, with priority to AS... i.e. somepeer12345 16419 89603 0 09:42:06 100/8000 somepeer-with-long-name 567816049401367282 0 13:39:28 100/8000 otherpeer16584484 16420 89606 0 1d00h58m Active otherpeer-with-long- 1658448516049401367282 0 13:40:17 100/8000 The attached patch does just that. Maximum line length is now 80 characters (excluding the trailing newline), and the Neighbor and AS columns now take a combined maximum of 29 characters, including the blank separating peer name and AS. The result looks like this: $ bgpctl show Neighbor ASMsgRcvdMsgSent OutQ Up/Down State/PrfRcvd other-peer-with-l 16581.44851 0 0 0 NeverConnect other-peer 1658.44841 0 0 0 NeverActive some-peer-with-long-name 5678 0 0 0 NeverActive some-peer 12345 0 0 0 NeverActive Where before it was $ bgpctl show Neighbor ASMsgRcvdMsgSent OutQ Up/Down State/PrfRcvd other-peer-with-long 16581.44851 0 0 0 NeverConnect other-peer 1658.44841 0 0 0 NeverActive some-peer-with-long- 5678 0 0 0 NeverActive some-peer 12345 0 0 0 NeverConnect Nice, I like that a lot. What do you think Claudio? Index: bgpctl.c === RCS file: /mnt/media/cvs/src/usr.sbin/bgpctl/bgpctl.c,v retrieving revision 1.174 diff -u -p -u -r1.174 bgpctl.c --- bgpctl.c 18 Mar 2014 13:47:14 - 1.174 +++ bgpctl.c 27 Jul 2014 15:20:59 - @@ -517,16 +517,26 @@ show_summary_msg(struct imsg *imsg, int { struct peer *p; char*s; + const char *a; + char*pa; + size_t alen; switch (imsg-hdr.type) { case IMSG_CTL_SHOW_NEIGHBOR: p = imsg-data; s = fmt_peer(p-conf.descr, p-conf.remote_addr, p-conf.remote_masklen, nodescr); - if (strlen(s) = 20) - s[20] = 0; - printf(%-20s %8s %10llu %10llu %5u %-8s , - s, log_as(p-conf.remote_as), + + a = log_as(p-conf.remote_as); + alen = strlen(a); + + if (asprintf(pa, %-28s, s) == -1) + err(1, NULL); + if (strlen(pa) 28 - alen) + pa[28 - alen] = 0; + + printf(%s %s %10llu %10llu %5u %-8s , + pa, a, p-stats.msg_rcvd_open + p-stats.msg_rcvd_notification + p-stats.msg_rcvd_update + p-stats.msg_rcvd_keepalive + p-stats.msg_rcvd_rrefresh, @@ -544,6 +554,7 @@ show_summary_msg(struct imsg *imsg, int else printf(%s, statenames[p-state]); printf(\n); + free(pa); free(s); break; case IMSG_CTL_END:
bgpctl: enlarge columns for 4-byte ASN display
Hi people, I'm running OpenBGPD with a few peers that have large 4-byte AS numbers. Displaying the table of peers with `bgpctl show` shows the stats just fine, but the length of the AS number shifts the content of the statistics confusingly far to the right, such as in the following example: $ bgpctl show Neighbor ASMsgRcvdMsgSent OutQ Up/Down State/PrfRcvd peer 1 64734.12580316466 0 00:07:04364 peer 2 64734.12578251293 0 00:40:06365 peer 3 64828 17545 13466 0 3d17h35m325 The attached patch makes the 'AS' column 4 characters larger, with the following result: $ bgpctl show Neighbor ASMsgRcvdMsgSent OutQ Up/Down State/PrfRcvd peer 164734.12580317468 0 00:07:51364 peer 264734.12578252295 0 00:40:53365 peer 3 64828 17547 13467 0 3d17h36m325 -- Gregor Best Index: bgpctl.c === RCS file: /mnt/media/cvs/src/usr.sbin/bgpctl/bgpctl.c,v retrieving revision 1.174 diff -u -p -u -r1.174 bgpctl.c --- bgpctl.c18 Mar 2014 13:47:14 - 1.174 +++ bgpctl.c26 Jul 2014 13:49:17 - @@ -508,7 +508,7 @@ fmt_peer(const char *descr, const struct void show_summary_head(void) { - printf(%-20s %8s %10s %10s %5s %-8s %s\n, Neighbor, AS, + printf(%-20s %12s %10s %10s %5s %-8s %s\n, Neighbor, AS, MsgRcvd, MsgSent, OutQ, Up/Down, State/PrfRcvd); } @@ -525,7 +525,7 @@ show_summary_msg(struct imsg *imsg, int p-conf.remote_masklen, nodescr); if (strlen(s) = 20) s[20] = 0; - printf(%-20s %8s %10llu %10llu %5u %-8s , + printf(%-20s %12s %10llu %10llu %5u %-8s , s, log_as(p-conf.remote_as), p-stats.msg_rcvd_open + p-stats.msg_rcvd_notification + p-stats.msg_rcvd_update + p-stats.msg_rcvd_keepalive +