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 +
