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 -0000       1.256
> +++ bgpctl/bgpctl.c   21 Jan 2020 08:18:59 -0000
> @@ -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 -0000       1.3
> +++ bgpctl/output.c   21 Jan 2020 08:18:59 -0000
> @@ -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
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.397
> diff -u -p -r1.397 bgpd.h
> --- bgpd/bgpd.h       9 Jan 2020 11:55:25 -0000       1.397
> +++ bgpd/bgpd.h       21 Jan 2020 08:18:59 -0000
> @@ -383,6 +383,7 @@ struct peer_config {
>       enum enforce_as          enforce_as;
>       enum enforce_as          enforce_local_as;
>       u_int16_t                max_prefix_restart;
> +     u_int16_t                max_out_prefix_restart;
>       u_int16_t                holdtime;
>       u_int16_t                min_holdtime;
>       u_int16_t                local_short_as;
> @@ -574,7 +575,9 @@ enum suberr_cease {
>       ERR_CEASE_CONN_REJECT,
>       ERR_CEASE_OTHER_CHANGE,
>       ERR_CEASE_COLLISION,
> -     ERR_CEASE_RSRC_EXHAUST
> +     ERR_CEASE_RSRC_EXHAUST,
> +     ERR_CEASE_HARD_RESET,
> +     ERR_CEASE_MAX_SENT_PREFIX
>  };
>  
>  struct kroute_node;
> @@ -1432,14 +1435,16 @@ static const char * const suberr_update_
>  
>  static const char * const suberr_cease_names[] = {
>       "none",
> -     "max-prefix exceeded",
> +     "recieved max-prefix exceeded",

"received" here.

>       "administratively down",
>       "peer unconfigured",
>       "administrative reset",
>       "connection rejected",
>       "other config change",
>       "collision",
> -     "resource exhaustion"
> +     "resource exhaustion",
> +     "hard reset",
> +     "sent max-prefix exceeded"
>  };
>  
>  static const char * const ctl_res_strerror[] = {
> Index: bgpd/session.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> retrieving revision 1.396
> diff -u -p -r1.396 session.c
> --- bgpd/session.c    9 Jan 2020 11:51:18 -0000       1.396
> +++ bgpd/session.c    21 Jan 2020 08:19:00 -0000
> @@ -610,6 +610,8 @@ bgp_fsm(struct peer *peer, enum session_
>  
>                       peer->stats.last_sent_errcode = 0;
>                       peer->stats.last_sent_suberr = 0;
> +                     peer->stats.last_rcvd_errcode = 0;
> +                     peer->stats.last_rcvd_suberr = 0;
>  
>                       if (!peer->depend_ok)
>                               timer_stop(peer, Timer_ConnectRetry);
> @@ -2189,6 +2191,8 @@ parse_notification(struct peer *peer)
>  
>       log_notification(peer, errcode, subcode, p, datalen, "received");
>       peer->errcnt++;
> +     peer->stats.last_rcvd_errcode = errcode;
> +     peer->stats.last_rcvd_suberr = subcode;
>  
>       if (errcode == ERR_OPEN && subcode == ERR_OPEN_CAPA) {
>               if (datalen == 0) {     /* zebra likes to send those.. humbug */
> Index: bgpd/session.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session.h,v
> retrieving revision 1.142
> diff -u -p -r1.142 session.h
> --- bgpd/session.h    9 Jan 2020 11:51:18 -0000       1.142
> +++ bgpd/session.h    21 Jan 2020 08:19:00 -0000
> @@ -171,6 +171,8 @@ struct peer_stats {
>       u_int32_t                prefix_cnt;
>       u_int8_t                 last_sent_errcode;
>       u_int8_t                 last_sent_suberr;
> +     u_int8_t                 last_rcvd_errcode;
> +     u_int8_t                 last_rcvd_suberr;
>       char                     last_shutcomm[SHUT_COMM_LEN];
>  };
>  
> 

Reply via email to