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