Re: bgpctl sh nei [group XX] terse: add peer address

2019-09-25 Thread Claudio Jeker
On Wed, Sep 25, 2019 at 12:19:23PM +0100, Stuart Henderson wrote:
> On 2019/09/24 22:06, Sebastian Benoit wrote:
> > Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.09.24 17:01:21 +0200:
> > > On Tue, Sep 24, 2019 at 03:51:43PM +0100, Stuart Henderson wrote:
> > > > On 2019/09/24 11:10, Claudio Jeker wrote:
> > > > > On Tue, Sep 24, 2019 at 10:06:51AM +0100, Stuart Henderson wrote:
> > > > > > On 2019/09/23 22:48, Stuart Henderson wrote:
> > > > > > > "bgpctl XX nei" functions can now take "group XX" - when used as
> > > > > > > "show nei group XX terse" you can't tell which entry relates to 
> > > > > > > each
> > > > > > > neighbour.
> > > > > > > 
> > > > > > > OK to add the address to the end of the line where it's reasonably
> > > > > > > out of the way of existing parsers?
> > > > > > 
> > > > > > missing free, pointed out by benno. (not that bgpctl will stick 
> > > > > > around
> > > > > > for long anyway :)
> > > > > 
> > > > > This is fine with me. I wonder if other data e.g. the peer 
> > > > > description or
> > > > > peer AS number should be added as well.
> > > > 
> > > > That might be useful, though as the peer description could contain
> > > > spaces we'd want to be reasonably sure we have already included any
> > > > other useful data first so that it can go right at the end of the line
> > > > (so that parsing the output isn't too awkward).
> > > > 
> > > 
> > > I was thinking the same maybe even put the name in "" to make it look more
> > > like a string.
> > 
> > Yes, that makes it matchable again at least.
> > 
> > > At least adding the AS number should be done.
> > 
> > +1
> > 

OK claudio@, my diff looks the same.

> Index: bgpctl.8
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.8,v
> retrieving revision 1.90
> diff -u -p -r1.90 bgpctl.8
> --- bgpctl.8  24 Sep 2019 14:46:09 -  1.90
> +++ bgpctl.8  25 Sep 2019 11:18:28 -
> @@ -290,7 +290,8 @@ The printed numbers are the sent and rec
>  notifications, sent and received updates, sent and received keepalives, and
>  sent and received route refresh messages plus the current and maximum
>  prefix count, the number of sent and received updates, sent and
> -received withdraws, and finally the neighbor's address.
> +received withdraws, the neighbor's address (or subnet, for a template),
> +AS number, and finally description.
>  .It Cm timers
>  Show the BGP timers.
>  .El
> Index: bgpctl.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.244
> diff -u -p -r1.244 bgpctl.c
> --- bgpctl.c  24 Sep 2019 14:46:09 -  1.244
> +++ bgpctl.c  25 Sep 2019 11:18:28 -
> @@ -619,8 +619,8 @@ show_neighbor_terse(struct imsg *imsg)
>   NULL)
>   err(1, "strdup");
>  
> - printf("%llu %llu %llu %llu %llu %llu %llu "
> - "%llu %llu %llu %u %u %llu %llu %llu %llu %s\n",
> + printf("%llu %llu %llu %llu %llu %llu %llu %llu %llu "
> + "%llu %u %u %llu %llu %llu %llu %s %s \"%s\"\n",
>   p->stats.msg_sent_open, p->stats.msg_rcvd_open,
>   p->stats.msg_sent_notification,
>   p->stats.msg_rcvd_notification,
> @@ -630,7 +630,8 @@ show_neighbor_terse(struct imsg *imsg)
>   p->stats.prefix_cnt, p->conf.max_prefix,
>   p->stats.prefix_sent_update, p->stats.prefix_rcvd_update,
>   p->stats.prefix_sent_withdraw,
> - p->stats.prefix_rcvd_withdraw, s);
> + p->stats.prefix_rcvd_withdraw, s,
> + log_as(p->conf.remote_as), p->conf.descr);
>   free(s);
>   break;
>   case IMSG_CTL_END:
> 

-- 
:wq Claudio



Re: bgpctl sh nei [group XX] terse: add peer address

2019-09-25 Thread Stuart Henderson
On 2019/09/24 22:06, Sebastian Benoit wrote:
> Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.09.24 17:01:21 +0200:
> > On Tue, Sep 24, 2019 at 03:51:43PM +0100, Stuart Henderson wrote:
> > > On 2019/09/24 11:10, Claudio Jeker wrote:
> > > > On Tue, Sep 24, 2019 at 10:06:51AM +0100, Stuart Henderson wrote:
> > > > > On 2019/09/23 22:48, Stuart Henderson wrote:
> > > > > > "bgpctl XX nei" functions can now take "group XX" - when used as
> > > > > > "show nei group XX terse" you can't tell which entry relates to each
> > > > > > neighbour.
> > > > > > 
> > > > > > OK to add the address to the end of the line where it's reasonably
> > > > > > out of the way of existing parsers?
> > > > > 
> > > > > missing free, pointed out by benno. (not that bgpctl will stick around
> > > > > for long anyway :)
> > > > 
> > > > This is fine with me. I wonder if other data e.g. the peer description 
> > > > or
> > > > peer AS number should be added as well.
> > > 
> > > That might be useful, though as the peer description could contain
> > > spaces we'd want to be reasonably sure we have already included any
> > > other useful data first so that it can go right at the end of the line
> > > (so that parsing the output isn't too awkward).
> > > 
> > 
> > I was thinking the same maybe even put the name in "" to make it look more
> > like a string.
> 
> Yes, that makes it matchable again at least.
> 
> > At least adding the AS number should be done.
> 
> +1
> 
> > 
> > -- 
> > :wq Claudio
> > 
> 

Index: bgpctl.8
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.8,v
retrieving revision 1.90
diff -u -p -r1.90 bgpctl.8
--- bgpctl.824 Sep 2019 14:46:09 -  1.90
+++ bgpctl.825 Sep 2019 11:18:28 -
@@ -290,7 +290,8 @@ The printed numbers are the sent and rec
 notifications, sent and received updates, sent and received keepalives, and
 sent and received route refresh messages plus the current and maximum
 prefix count, the number of sent and received updates, sent and
-received withdraws, and finally the neighbor's address.
+received withdraws, the neighbor's address (or subnet, for a template),
+AS number, and finally description.
 .It Cm timers
 Show the BGP timers.
 .El
Index: bgpctl.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
retrieving revision 1.244
diff -u -p -r1.244 bgpctl.c
--- bgpctl.c24 Sep 2019 14:46:09 -  1.244
+++ bgpctl.c25 Sep 2019 11:18:28 -
@@ -619,8 +619,8 @@ show_neighbor_terse(struct imsg *imsg)
NULL)
err(1, "strdup");
 
-   printf("%llu %llu %llu %llu %llu %llu %llu "
-   "%llu %llu %llu %u %u %llu %llu %llu %llu %s\n",
+   printf("%llu %llu %llu %llu %llu %llu %llu %llu %llu "
+   "%llu %u %u %llu %llu %llu %llu %s %s \"%s\"\n",
p->stats.msg_sent_open, p->stats.msg_rcvd_open,
p->stats.msg_sent_notification,
p->stats.msg_rcvd_notification,
@@ -630,7 +630,8 @@ show_neighbor_terse(struct imsg *imsg)
p->stats.prefix_cnt, p->conf.max_prefix,
p->stats.prefix_sent_update, p->stats.prefix_rcvd_update,
p->stats.prefix_sent_withdraw,
-   p->stats.prefix_rcvd_withdraw, s);
+   p->stats.prefix_rcvd_withdraw, s,
+   log_as(p->conf.remote_as), p->conf.descr);
free(s);
break;
case IMSG_CTL_END:



Re: bgpctl sh nei [group XX] terse: add peer address

2019-09-24 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.09.24 17:01:21 +0200:
> On Tue, Sep 24, 2019 at 03:51:43PM +0100, Stuart Henderson wrote:
> > On 2019/09/24 11:10, Claudio Jeker wrote:
> > > On Tue, Sep 24, 2019 at 10:06:51AM +0100, Stuart Henderson wrote:
> > > > On 2019/09/23 22:48, Stuart Henderson wrote:
> > > > > "bgpctl XX nei" functions can now take "group XX" - when used as
> > > > > "show nei group XX terse" you can't tell which entry relates to each
> > > > > neighbour.
> > > > > 
> > > > > OK to add the address to the end of the line where it's reasonably
> > > > > out of the way of existing parsers?
> > > > 
> > > > missing free, pointed out by benno. (not that bgpctl will stick around
> > > > for long anyway :)
> > > 
> > > This is fine with me. I wonder if other data e.g. the peer description or
> > > peer AS number should be added as well.
> > 
> > That might be useful, though as the peer description could contain
> > spaces we'd want to be reasonably sure we have already included any
> > other useful data first so that it can go right at the end of the line
> > (so that parsing the output isn't too awkward).
> > 
> 
> I was thinking the same maybe even put the name in "" to make it look more
> like a string.

Yes, that makes it matchable again at least.

> At least adding the AS number should be done.

+1

> 
> -- 
> :wq Claudio
> 



Re: bgpctl sh nei [group XX] terse: add peer address

2019-09-24 Thread Claudio Jeker
On Tue, Sep 24, 2019 at 03:51:43PM +0100, Stuart Henderson wrote:
> On 2019/09/24 11:10, Claudio Jeker wrote:
> > On Tue, Sep 24, 2019 at 10:06:51AM +0100, Stuart Henderson wrote:
> > > On 2019/09/23 22:48, Stuart Henderson wrote:
> > > > "bgpctl XX nei" functions can now take "group XX" - when used as
> > > > "show nei group XX terse" you can't tell which entry relates to each
> > > > neighbour.
> > > > 
> > > > OK to add the address to the end of the line where it's reasonably
> > > > out of the way of existing parsers?
> > > 
> > > missing free, pointed out by benno. (not that bgpctl will stick around
> > > for long anyway :)
> > 
> > This is fine with me. I wonder if other data e.g. the peer description or
> > peer AS number should be added as well.
> 
> That might be useful, though as the peer description could contain
> spaces we'd want to be reasonably sure we have already included any
> other useful data first so that it can go right at the end of the line
> (so that parsing the output isn't too awkward).
> 

I was thinking the same maybe even put the name in "" to make it look more
like a string. At least adding the AS number should be done.

-- 
:wq Claudio



Re: bgpctl sh nei [group XX] terse: add peer address

2019-09-24 Thread Stuart Henderson
On 2019/09/24 11:10, Claudio Jeker wrote:
> On Tue, Sep 24, 2019 at 10:06:51AM +0100, Stuart Henderson wrote:
> > On 2019/09/23 22:48, Stuart Henderson wrote:
> > > "bgpctl XX nei" functions can now take "group XX" - when used as
> > > "show nei group XX terse" you can't tell which entry relates to each
> > > neighbour.
> > > 
> > > OK to add the address to the end of the line where it's reasonably
> > > out of the way of existing parsers?
> > 
> > missing free, pointed out by benno. (not that bgpctl will stick around
> > for long anyway :)
> 
> This is fine with me. I wonder if other data e.g. the peer description or
> peer AS number should be added as well.

That might be useful, though as the peer description could contain
spaces we'd want to be reasonably sure we have already included any
other useful data first so that it can go right at the end of the line
(so that parsing the output isn't too awkward).



Re: bgpctl sh nei [group XX] terse: add peer address

2019-09-24 Thread Claudio Jeker
On Tue, Sep 24, 2019 at 10:06:51AM +0100, Stuart Henderson wrote:
> On 2019/09/23 22:48, Stuart Henderson wrote:
> > "bgpctl XX nei" functions can now take "group XX" - when used as
> > "show nei group XX terse" you can't tell which entry relates to each
> > neighbour.
> > 
> > OK to add the address to the end of the line where it's reasonably
> > out of the way of existing parsers?
> 
> missing free, pointed out by benno. (not that bgpctl will stick around
> for long anyway :)

This is fine with me. I wonder if other data e.g. the peer description or
peer AS number should be added as well.
 
> Index: bgpctl.8
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.8,v
> retrieving revision 1.89
> diff -u -p -r1.89 bgpctl.8
> --- bgpctl.8  28 Jun 2019 12:12:06 -  1.89
> +++ bgpctl.8  24 Sep 2019 09:06:13 -
> @@ -289,7 +289,8 @@ Show statistics in an easily parseable t
>  The printed numbers are the sent and received open, sent and received
>  notifications, sent and received updates, sent and received keepalives, and
>  sent and received route refresh messages plus the current and maximum
> -prefix count, the number of sent and received updates, and withdraws.
> +prefix count, the number of sent and received updates, sent and
> +received withdraws, and finally the neighbor's address.
>  .It Cm timers
>  Show the BGP timers.
>  .El
> Index: bgpctl.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.243
> diff -u -p -r1.243 bgpctl.c
> --- bgpctl.c  5 Aug 2019 12:51:32 -   1.243
> +++ bgpctl.c  24 Sep 2019 09:06:13 -
> @@ -601,12 +601,26 @@ int
>  show_neighbor_terse(struct imsg *imsg)
>  {
>   struct peer *p;
> + char*s;
>  
>   switch (imsg->hdr.type) {
>   case IMSG_CTL_SHOW_NEIGHBOR:
>   p = imsg->data;
> + if ((p->conf.remote_addr.aid == AID_INET &&
> + p->conf.remote_masklen != 32) ||
> + (p->conf.remote_addr.aid == AID_INET6 &&
> + p->conf.remote_masklen != 128)) {
> + if (asprintf(, "%s/%u",
> + log_addr(>conf.remote_addr),
> + p->conf.remote_masklen) == -1)
> + err(1, NULL);
> + } else
> + if ((s = strdup(log_addr(>conf.remote_addr))) ==
> + NULL)
> + err(1, "strdup");
> +
>   printf("%llu %llu %llu %llu %llu %llu %llu "
> - "%llu %llu %llu %u %u %llu %llu %llu %llu\n",
> + "%llu %llu %llu %u %u %llu %llu %llu %llu %s\n",
>   p->stats.msg_sent_open, p->stats.msg_rcvd_open,
>   p->stats.msg_sent_notification,
>   p->stats.msg_rcvd_notification,
> @@ -616,7 +630,8 @@ show_neighbor_terse(struct imsg *imsg)
>   p->stats.prefix_cnt, p->conf.max_prefix,
>   p->stats.prefix_sent_update, p->stats.prefix_rcvd_update,
>   p->stats.prefix_sent_withdraw,
> - p->stats.prefix_rcvd_withdraw);
> + p->stats.prefix_rcvd_withdraw, s);
> + free(s);
>   break;
>   case IMSG_CTL_END:
>   return (1);
> 

-- 
:wq Claudio



Re: bgpctl sh nei [group XX] terse: add peer address

2019-09-24 Thread Stuart Henderson
On 2019/09/23 22:48, Stuart Henderson wrote:
> "bgpctl XX nei" functions can now take "group XX" - when used as
> "show nei group XX terse" you can't tell which entry relates to each
> neighbour.
> 
> OK to add the address to the end of the line where it's reasonably
> out of the way of existing parsers?

missing free, pointed out by benno. (not that bgpctl will stick around
for long anyway :)

Index: bgpctl.8
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.8,v
retrieving revision 1.89
diff -u -p -r1.89 bgpctl.8
--- bgpctl.828 Jun 2019 12:12:06 -  1.89
+++ bgpctl.824 Sep 2019 09:06:13 -
@@ -289,7 +289,8 @@ Show statistics in an easily parseable t
 The printed numbers are the sent and received open, sent and received
 notifications, sent and received updates, sent and received keepalives, and
 sent and received route refresh messages plus the current and maximum
-prefix count, the number of sent and received updates, and withdraws.
+prefix count, the number of sent and received updates, sent and
+received withdraws, and finally the neighbor's address.
 .It Cm timers
 Show the BGP timers.
 .El
Index: bgpctl.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
retrieving revision 1.243
diff -u -p -r1.243 bgpctl.c
--- bgpctl.c5 Aug 2019 12:51:32 -   1.243
+++ bgpctl.c24 Sep 2019 09:06:13 -
@@ -601,12 +601,26 @@ int
 show_neighbor_terse(struct imsg *imsg)
 {
struct peer *p;
+   char*s;
 
switch (imsg->hdr.type) {
case IMSG_CTL_SHOW_NEIGHBOR:
p = imsg->data;
+   if ((p->conf.remote_addr.aid == AID_INET &&
+   p->conf.remote_masklen != 32) ||
+   (p->conf.remote_addr.aid == AID_INET6 &&
+   p->conf.remote_masklen != 128)) {
+   if (asprintf(, "%s/%u",
+   log_addr(>conf.remote_addr),
+   p->conf.remote_masklen) == -1)
+   err(1, NULL);
+   } else
+   if ((s = strdup(log_addr(>conf.remote_addr))) ==
+   NULL)
+   err(1, "strdup");
+
printf("%llu %llu %llu %llu %llu %llu %llu "
-   "%llu %llu %llu %u %u %llu %llu %llu %llu\n",
+   "%llu %llu %llu %u %u %llu %llu %llu %llu %s\n",
p->stats.msg_sent_open, p->stats.msg_rcvd_open,
p->stats.msg_sent_notification,
p->stats.msg_rcvd_notification,
@@ -616,7 +630,8 @@ show_neighbor_terse(struct imsg *imsg)
p->stats.prefix_cnt, p->conf.max_prefix,
p->stats.prefix_sent_update, p->stats.prefix_rcvd_update,
p->stats.prefix_sent_withdraw,
-   p->stats.prefix_rcvd_withdraw);
+   p->stats.prefix_rcvd_withdraw, s);
+   free(s);
break;
case IMSG_CTL_END:
return (1);



bgpctl sh nei [group XX] terse: add peer address

2019-09-23 Thread Stuart Henderson
"bgpctl XX nei" functions can now take "group XX" - when used as
"show nei group XX terse" you can't tell which entry relates to each
neighbour.

OK to add the address to the end of the line where it's reasonably
out of the way of existing parsers?

Index: bgpctl.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
retrieving revision 1.243
diff -u -p -r1.243 bgpctl.c
--- bgpctl.c5 Aug 2019 12:51:32 -   1.243
+++ bgpctl.c23 Sep 2019 21:42:52 -
@@ -601,12 +601,26 @@ int
 show_neighbor_terse(struct imsg *imsg)
 {
struct peer *p;
+   char*s;
 
switch (imsg->hdr.type) {
case IMSG_CTL_SHOW_NEIGHBOR:
p = imsg->data;
+   if ((p->conf.remote_addr.aid == AID_INET &&
+   p->conf.remote_masklen != 32) ||
+   (p->conf.remote_addr.aid == AID_INET6 &&
+   p->conf.remote_masklen != 128)) {
+   if (asprintf(, "%s/%u",
+   log_addr(>conf.remote_addr),
+   p->conf.remote_masklen) == -1)
+   err(1, NULL);
+   } else
+   if ((s = strdup(log_addr(>conf.remote_addr))) ==
+   NULL)
+   err(1, "strdup");
+
printf("%llu %llu %llu %llu %llu %llu %llu "
-   "%llu %llu %llu %u %u %llu %llu %llu %llu\n",
+   "%llu %llu %llu %u %u %llu %llu %llu %llu %s\n",
p->stats.msg_sent_open, p->stats.msg_rcvd_open,
p->stats.msg_sent_notification,
p->stats.msg_rcvd_notification,
@@ -616,7 +630,7 @@ show_neighbor_terse(struct imsg *imsg)
p->stats.prefix_cnt, p->conf.max_prefix,
p->stats.prefix_sent_update, p->stats.prefix_rcvd_update,
p->stats.prefix_sent_withdraw,
-   p->stats.prefix_rcvd_withdraw);
+   p->stats.prefix_rcvd_withdraw, s);
break;
case IMSG_CTL_END:
return (1);
Index: bgpctl.8
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.8,v
retrieving revision 1.89
diff -u -p -r1.89 bgpctl.8
--- bgpctl.828 Jun 2019 12:12:06 -  1.89
+++ bgpctl.823 Sep 2019 21:47:58 -
@@ -289,7 +289,8 @@ Show statistics in an easily parseable t
 The printed numbers are the sent and received open, sent and received
 notifications, sent and received updates, sent and received keepalives, and
 sent and received route refresh messages plus the current and maximum
-prefix count, the number of sent and received updates, and withdraws.
+prefix count, the number of sent and received updates, sent and
+received withdraws, and finally the neighbor's address.
 .It Cm timers
 Show the BGP timers.
 .El