Re: bgpctl show neighbor errors better
On Tue, Jan 21, 2020 at 09:31:36AM +0100, Claudio Jeker wrote:
> So the 'Last error:' message for sessions that failed do often not include
> the suberror and also it is only shown for NOTIFICATIONS we sent but not
> for received notifications. The following diff fixes this.
>
> So instead of:
> Last error: Cease
> or
> Last error: unknown error code
>
> I now get:
> Last error sent: Cease, recieved max-prefix exceeded
> or
> Last error received: Cease, recieved max-prefix exceeded
>
> --
> :wq Claudio
>
> Index: bgpctl/bgpctl.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.256
> diff -u -p -r1.256 bgpctl.c
> --- bgpctl/bgpctl.c 9 Jan 2020 11:57:04 - 1.256
> +++ bgpctl/bgpctl.c 21 Jan 2020 08:18:59 -
> @@ -1300,36 +1300,68 @@ send_filterset(struct imsgbuf *i, struct
> const char *
> get_errstr(u_int8_t errcode, u_int8_t subcode)
> {
> - static const char *errstr = NULL;
> + static char errbuf[256];
> + const char *errstr = NULL;
> + const char *suberr = NULL;
> + int uk = 0;
>
> - if (errcode && errcode < sizeof(errnames)/sizeof(char *))
> + if (errcode == 0) /* no error */
> + return NULL;
> +
> + if (errcode < sizeof(errnames)/sizeof(char *))
> errstr = errnames[errcode];
>
> switch (errcode) {
> case ERR_HEADER:
> - if (subcode &&
> - subcode < sizeof(suberr_header_names)/sizeof(char *))
> - errstr = suberr_header_names[subcode];
> + if (subcode < sizeof(suberr_header_names)/sizeof(char *))
> + suberr = suberr_header_names[subcode];
> + else
> + uk = 1;
Would it be possible to have that chunk before the switch to avoid duplication ?
> break;
> case ERR_OPEN:
> - if (subcode &&
> - subcode < sizeof(suberr_open_names)/sizeof(char *))
> - errstr = suberr_open_names[subcode];
> + if (subcode < sizeof(suberr_open_names)/sizeof(char *))
> + suberr = suberr_open_names[subcode];
> + else
> + uk = 1;
> break;
> case ERR_UPDATE:
> - if (subcode &&
> - subcode < sizeof(suberr_update_names)/sizeof(char *))
> - errstr = suberr_update_names[subcode];
> + if (subcode < sizeof(suberr_update_names)/sizeof(char *))
> + suberr = suberr_update_names[subcode];
> + else
> + uk = 1;
> break;
> case ERR_HOLDTIMEREXPIRED:
> + if (subcode != 0)
> + uk = 1;
> + break;
> case ERR_FSM:
> + if (subcode < sizeof(suberr_fsm_names)/sizeof(char *))
> + suberr = suberr_fsm_names[subcode];
> + else
> + uk = 1;
> + break;
> case ERR_CEASE:
> + if (subcode < sizeof(suberr_cease_names)/sizeof(char *))
> + suberr = suberr_cease_names[subcode];
> + else
> + uk = 1;
> break;
> default:
> - return ("unknown error code");
> + snprintf(errbuf, sizeof(errbuf),
> + "unknown error code %u subcode %u", errcode, subcode);
> + return (errbuf);
> }
>
> - return (errstr);
> + if (uk)
> + snprintf(errbuf, sizeof(errbuf),
> + "%s, unknown subcode %u", errstr, subcode);
> + else if (suberr == NULL)
> + return (errstr);
> + else
> + snprintf(errbuf, sizeof(errbuf),
> + "%s, %s", errstr, suberr);
> +
> + return (errbuf);
> }
>
> void
> Index: bgpctl/output.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/output.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 output.c
> --- bgpctl/output.c 9 Jan 2020 11:57:04 - 1.3
> +++ bgpctl/output.c 21 Jan 2020 08:18:59 -
> @@ -221,12 +221,16 @@ show_neighbor_full(struct peer *p, struc
> log_shutcomm(p->stats.last_shutcomm));
> }
> if (p->state == STATE_IDLE) {
> - static const char *errstr;
> + const char *errstr;
>
> errstr = get_errstr(p->stats.last_sent_errcode,
> p->stats.last_sent_suberr);
> if (errstr)
> - printf(" Last error: %s\n\n", errstr);
> + printf(" Last error sent: %s\n\n", errstr);
> + errstr = get_errstr(p->stats.last_rcvd_errcode,
> + p->stats.last_rcvd_suberr);
> + if (errstr)
> + printf(" Last error received: %s\n\n", errstr);
>
bgpctl show neighbor errors better
So the 'Last error:' message for sessions that failed do often not include
the suberror and also it is only shown for NOTIFICATIONS we sent but not
for received notifications. The following diff fixes this.
So instead of:
Last error: Cease
or
Last error: unknown error code
I now get:
Last error sent: Cease, recieved max-prefix exceeded
or
Last error received: Cease, recieved max-prefix exceeded
--
:wq Claudio
Index: bgpctl/bgpctl.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
retrieving revision 1.256
diff -u -p -r1.256 bgpctl.c
--- bgpctl/bgpctl.c 9 Jan 2020 11:57:04 - 1.256
+++ bgpctl/bgpctl.c 21 Jan 2020 08:18:59 -
@@ -1300,36 +1300,68 @@ send_filterset(struct imsgbuf *i, struct
const char *
get_errstr(u_int8_t errcode, u_int8_t subcode)
{
- static const char *errstr = NULL;
+ static char errbuf[256];
+ const char *errstr = NULL;
+ const char *suberr = NULL;
+ int uk = 0;
- if (errcode && errcode < sizeof(errnames)/sizeof(char *))
+ if (errcode == 0) /* no error */
+ return NULL;
+
+ if (errcode < sizeof(errnames)/sizeof(char *))
errstr = errnames[errcode];
switch (errcode) {
case ERR_HEADER:
- if (subcode &&
- subcode < sizeof(suberr_header_names)/sizeof(char *))
- errstr = suberr_header_names[subcode];
+ if (subcode < sizeof(suberr_header_names)/sizeof(char *))
+ suberr = suberr_header_names[subcode];
+ else
+ uk = 1;
break;
case ERR_OPEN:
- if (subcode &&
- subcode < sizeof(suberr_open_names)/sizeof(char *))
- errstr = suberr_open_names[subcode];
+ if (subcode < sizeof(suberr_open_names)/sizeof(char *))
+ suberr = suberr_open_names[subcode];
+ else
+ uk = 1;
break;
case ERR_UPDATE:
- if (subcode &&
- subcode < sizeof(suberr_update_names)/sizeof(char *))
- errstr = suberr_update_names[subcode];
+ if (subcode < sizeof(suberr_update_names)/sizeof(char *))
+ suberr = suberr_update_names[subcode];
+ else
+ uk = 1;
break;
case ERR_HOLDTIMEREXPIRED:
+ if (subcode != 0)
+ uk = 1;
+ break;
case ERR_FSM:
+ if (subcode < sizeof(suberr_fsm_names)/sizeof(char *))
+ suberr = suberr_fsm_names[subcode];
+ else
+ uk = 1;
+ break;
case ERR_CEASE:
+ if (subcode < sizeof(suberr_cease_names)/sizeof(char *))
+ suberr = suberr_cease_names[subcode];
+ else
+ uk = 1;
break;
default:
- return ("unknown error code");
+ snprintf(errbuf, sizeof(errbuf),
+ "unknown error code %u subcode %u", errcode, subcode);
+ return (errbuf);
}
- return (errstr);
+ if (uk)
+ snprintf(errbuf, sizeof(errbuf),
+ "%s, unknown subcode %u", errstr, subcode);
+ else if (suberr == NULL)
+ return (errstr);
+ else
+ snprintf(errbuf, sizeof(errbuf),
+ "%s, %s", errstr, suberr);
+
+ return (errbuf);
}
void
Index: bgpctl/output.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/output.c,v
retrieving revision 1.3
diff -u -p -r1.3 output.c
--- bgpctl/output.c 9 Jan 2020 11:57:04 - 1.3
+++ bgpctl/output.c 21 Jan 2020 08:18:59 -
@@ -221,12 +221,16 @@ show_neighbor_full(struct peer *p, struc
log_shutcomm(p->stats.last_shutcomm));
}
if (p->state == STATE_IDLE) {
- static const char *errstr;
+ const char *errstr;
errstr = get_errstr(p->stats.last_sent_errcode,
p->stats.last_sent_suberr);
if (errstr)
- printf(" Last error: %s\n\n", errstr);
+ printf(" Last error sent: %s\n\n", errstr);
+ errstr = get_errstr(p->stats.last_rcvd_errcode,
+ p->stats.last_rcvd_suberr);
+ if (errstr)
+ printf(" Last error received: %s\n\n", errstr);
} else {
printf(" Local host: %20s, Local port: %5u\n",
log_addr(&p->local), p->local_port);
Index: bgpd/bgpd.h
===
RC
