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