Hi,

Peter van Dijk pointed me at valgrind. It appears I can improve this
patch a bit more, stay tuned.

Kind regards,

Job

On Thu, Jul 27, 2017 at 05:55:40PM +0200, Job Snijders wrote:
> Hi all,
> 
> Here is a patch to decode received BGP shutdown communication messages
> as specified in RFC 8203. In the following example scenario I'm sending
> a shutdown communication with openbgpd:
> 
>       $ bgpctl neighbor 94.142.241.204 down "TICKET-2331 we are upgrading, 
> back in 30 min"
>       request processed
> 
> And can subsequently observe the message via logging and via 'show protocols 
> all':
> 
>       job@irime:~/source/bird$ sudo ./bird -s test.sock -d -c bird2.conf
>       bird: Started
>       bird: bgp1: Received: Administrative shutdown: "TICKET-2331 we are 
> upgrading, back in 30 min"
> 
>       job@irime:~/source/bird$ sudo ./birdc -s test.sock show protocols all
>         ....
>         BGP state:          Active
>               Neighbor address: 165.254.255.26
>               Neighbor AS:      15562
>               Connect delay:    3/5
>               Last error:       Received: Administrative shutdown
>               Message:          "TICKET-2331 we are upgrading, back in 30 min"
> 
> As for the remaining RFC 8203 work:
> 
> I could use some help with _sending_ bgp shutdown communication
> messages. It is not clear to me where I should hook the message into
> the config/cli infrastructure. Ideally you can do things like:
> 
>     $ birdc disable bgp1 "hi, we will upgrade to bird 1.6.4"
> 
> or via the configuration (also during reconfigure):
> 
>       protocol bgp {
>               disabled "shutdown due to maintenance ticket 2424";
>               description "My BGP uplink";
>               local 94.142.241.204 as 65000;
>               neighbor 165.254.255.26 as 15562;
>       }
> 
> Thoughts?
> 
> Kind regards,
> 
> Job
> 
> ---
>  proto/bgp/bgp.c     | 13 ++++++++++
>  proto/bgp/bgp.h     |  2 ++
>  proto/bgp/packets.c | 73 
> ++++++++++++++++++++++++++++++++++-------------------
>  3 files changed, 62 insertions(+), 26 deletions(-)
> 
> diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
> index f706e76..551b156 100644
> --- a/proto/bgp/bgp.c
> +++ b/proto/bgp/bgp.c
> @@ -1487,6 +1487,16 @@ bgp_last_errmsg(struct bgp_proto *p)
>  }
>  
>  static const char *
> +bgp_last_shutmsg(struct bgp_proto *p)
> +{
> +    unsigned len;
> +    char *buf = calloc (128, sizeof (char));
> +    len = p->last_received_shutmsg[0];
> +    bsnprintf(buf, len, "%s", p->last_received_shutmsg+1);
> +    return(buf);
> +}
> +
> +static const char *
>  bgp_state_dsc(struct bgp_proto *p)
>  {
>    if (p->p.proto_state == PS_DOWN)
> @@ -1580,7 +1590,10 @@ bgp_show_proto_info(struct proto *P)
>      {
>        const char *err1 = bgp_err_classes[p->last_error_class];
>        const char *err2 = bgp_last_errmsg(p);
> +      const char *msg  = bgp_last_shutmsg(p);
>        cli_msg(-1006, "    Last error:       %s%s", err1, err2);
> +      if (p->last_received_shutmsg[0] > 0)
> +        cli_msg(-1006, "    Message:          \"%s\"", msg);
>      }
>  }
>  
> diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
> index e47a0eb..d958716 100644
> --- a/proto/bgp/bgp.h
> +++ b/proto/bgp/bgp.h
> @@ -157,6 +157,8 @@ struct bgp_proto {
>    u8 last_error_class;                       /* Error class of last error */
>    u32 last_error_code;                       /* Error code of last error. 
> BGP protocol errors
>                                          are encoded as (bgp_err_code << 16 | 
> bgp_err_subcode) */
> +  byte last_received_shutmsg[129];           /* RFC 8203 */
> +
>  #ifdef IPV6
>    byte *mp_reach_start, *mp_unreach_start; /* Multiprotocol BGP attribute 
> notes */
>    unsigned mp_reach_len, mp_unreach_len;
> diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
> index ab87bdc..1a021dd 100644
> --- a/proto/bgp/packets.c
> +++ b/proto/bgp/packets.c
> @@ -1460,9 +1460,9 @@ static struct {
>    { 5, 3, "Unexpected message in Established state" },
>    { 6, 0, "Cease" }, /* Subcodes are according to [RFC4486] */
>    { 6, 1, "Maximum number of prefixes reached" },
> -  { 6, 2, "Administrative shutdown" },
> +  { 6, 2, "Administrative shutdown" }, /* RFC 8203 can follow */
>    { 6, 3, "Peer de-configured" },
> -  { 6, 4, "Administrative reset" },
> +  { 6, 4, "Administrative reset" }, /* RFC 8203 can follow */
>    { 6, 5, "Connection rejected" },
>    { 6, 6, "Other configuration change" },
>    { 6, 7, "Connection collision resolution" },
> @@ -1497,35 +1497,56 @@ bgp_error_dsc(unsigned code, unsigned subcode)
>  void
>  bgp_log_error(struct bgp_proto *p, u8 class, char *msg, unsigned code, 
> unsigned subcode, byte *data, unsigned len)
>  {
> -  const byte *name;
> -  byte *t, argbuf[36];
> -  unsigned i;
> +    const byte *name;
> +    byte *t, argbuf[256];
> +    unsigned i;
> +    unsigned shutdown_comm_length;
>  
> -  /* Don't report Cease messages generated by myself */
> -  if (code == 6 && class == BE_BGP_TX)
> -    return;
> +    /* Don't report Cease messages generated by myself */
> +    if (code == 6 && class == BE_BGP_TX)
> +        return;
> +
> +    name = bgp_error_dsc(code, subcode);
> +    t = argbuf;
> +    memset(p->last_received_shutmsg, 0, 129); // clear old RFC 8203 messages
>  
> -  name = bgp_error_dsc(code, subcode);
> -  t = argbuf;
> -  if (len)
> +    if (len)
>      {
> -      *t++ = ':';
> -      *t++ = ' ';
> +        *t++ = ':';
> +        *t++ = ' ';
>  
> -      if ((code == 2) && (subcode == 2) && ((len == 2) || (len == 4)))
> -     {
> -       /* Bad peer AS - we would like to print the AS */
> -       t += bsprintf(t, "%d", (len == 2) ? get_u16(data) : get_u32(data));
> -       goto done;
> -     }
> -      if (len > 16)
> -     len = 16;
> -      for (i=0; i<len; i++)
> -     t += bsprintf(t, "%02x", data[i]);
> +        if ((code == 2) && (subcode == 2) && ((len == 2) || (len == 4)))
> +        {
> +            /* Bad peer AS - we would like to print the AS */
> +            t += bsprintf(t, "%d", (len == 2) ? get_u16(data) : 
> get_u32(data));
> +            goto done;
> +        }
> +        /* RFC 8203 */
> +        if ((code == 6) && ((subcode == 2 || subcode == 4)) && (len <= 129))
> +        {
> +            shutdown_comm_length = data[0];
> +            if ((shutdown_comm_length <= 128) && (len == 
> shutdown_comm_length + 1))
> +            {
> +                t += bsprintf(t, "\"");
> +                for (i=1; i<len; i++)
> +                    t += bsprintf(t, "%c", data[i]);
> +                t += bsprintf(t, "\"");
> +                memcpy(p->last_received_shutmsg, data, shutdown_comm_length 
> + 1);
> +                goto done;
> +            }
> +        }
> +        else
> +        {
> +            if (len > 16)
> +                len = 16;
> +            for (i=0; i<len; i++)
> +                t += bsprintf(t, "%02x", data[i]);
> +        }
>      }
> - done:
> -  *t = 0;
> -  log(L_REMOTE "%s: %s: %s%s", p->p.name, msg, name, argbuf);
> +
> +  done:
> +    *t = 0;
> +    log(L_REMOTE "%s: %s: %s%s", p->p.name, msg, name, argbuf);
>  }
>  
>  static void

Reply via email to