Job Snijders(j...@instituut.net) on 2017.01.08 20:24:19 +0100: > Dear OpenBSD developers, > > This patch adds support for the "BGP Administrative Shutdown > Communication" to bgpd(8) and bgpctl(8).
Hi Job and Peter, thanks, this is nice! some comments below. > The draft-ietf-idr-shutdown > (https://tools.ietf.org/html/draft-ietf-idr-shutdown) > document specifies a mechanism to transmit a short freeform message > across the wire as part of an Administrative Shutdown Cease NOTIFICATION > message. This message serves to inform the neighbor why the BGP session > was shut down. This is useful to communicate for instance a ticket > reference, contact person or other information to the neighbor. > > Example usage: > > [job@kiera ~]$ bgpctl show > Neighbor AS MsgRcvd MsgSent OutQ Up/Down State/PrfRcvd > 165.254.255.24 15562 70684 60 0 00:21:16 33653 > > [job@kiera ~]$ bgpctl neighbor 165.254.255.24 down "goodbye, we are > upgrading to openbsd 6.1" > request processed > > [job@kiera ~]$ bgpctl show > Neighbor AS MsgRcvd MsgSent OutQ Up/Down State/PrfRcvd > 165.254.255.24 15562 70708 62 0 00:00:08 Idle > > [job@kiera ~]$ bgpctl show neighbor 165.254.255.24 | grep reason > BGP state = Idle, marked down with shutdown reason "goodbye, we are > upgrading to openbsd 6.1", down for 00:00:17 > > On the neighbor's side you'll see the following in syslog: > > Jan 8 19:28:54 shutdown bgpd[50719]: neighbor 165.254.255.26: received > notification: Cease, administratively down > Jan 8 19:28:54 shutdown bgpd[50719]: neighbor 165.254.255.26: received > shutdown reason: "goodbye, we are upgrading to openbsd 6.1" > > and this is also visible through bgpctl(8): > > [job@shutdown~]$ bgpctl show neighbor 165.254.255.26 | grep reason > Last received shutdown reason: "goodbye, we are upgrading to openbsd > 6.1" > > This work has been tested against pmacct and exabgp which also > support draft-ietf-idr-shutdown. > > The BGP Administrative Shutdown Communication feature for OpenBGPD was > developed by Peter van Dijk <peter.van.d...@powerdns.com> and Job Snijders > <j...@ntt.net>. > > Kind regards, > > Job > > > Index: usr.sbin/bgpctl/bgpctl.8 > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.8,v > retrieving revision 1.69 > diff -u -p -r1.69 bgpctl.8 > --- usr.sbin/bgpctl/bgpctl.8 25 May 2016 14:15:59 -0000 1.69 > +++ usr.sbin/bgpctl/bgpctl.8 8 Jan 2017 17:45:24 -0000 > @@ -104,8 +104,14 @@ Destroy a previously cloned peer. > The peer must be down before calling this function. > .Ar peer > may be the neighbor's address or description. > -.It Cm neighbor Ar peer Cm down > -Take the BGP session to the specified neighbor down. > +.It Cm neighbor Ar peer Cm down Op Ar reason > +Take the BGP session to the specified neighbor down. If a > +.Ar reason > +is provided, the > +.Ar reason > +is send as Administrative Shutdown Communication to the neighbor. The > +.Ar reason > +cannot exceed 128 octets. > .Ar peer > may be the neighbor's address or description. > .It Cm neighbor Ar peer Cm refresh > Index: usr.sbin/bgpctl/bgpctl.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v > retrieving revision 1.188 > diff -u -p -r1.188 bgpctl.c > --- usr.sbin/bgpctl/bgpctl.c 3 Jun 2016 17:36:37 -0000 1.188 > +++ usr.sbin/bgpctl/bgpctl.c 8 Jan 2017 17:45:24 -0000 > @@ -159,6 +159,7 @@ main(int argc, char *argv[]) > > memcpy(&neighbor.addr, &res->peeraddr, sizeof(neighbor.addr)); > strlcpy(neighbor.descr, res->peerdesc, sizeof(neighbor.descr)); > + strlcpy(neighbor.shutcomm, res->shutcomm, sizeof(neighbor.shutcomm)); > > if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) > err(1, "control_init: socket"); > @@ -702,6 +703,13 @@ show_neighbor_msg(struct imsg *imsg, enu > inet_ntoa(ina)); > printf("%s\n", print_auth_method(p->auth.method)); > printf(" BGP state = %s", statenames[p->state]); > + if (p->conf.down) { > + printf(", marked down"); > + if (*(p->conf.shutcomm)) { > + printf(" with shutdown reason \"%s\"", > + log_shutcomm(p->conf.shutcomm)); > + } > + } > if (p->stats.last_updown != 0) > printf(", %s for %s", > p->state == STATE_ESTABLISHED ? "up" : "down", > @@ -736,6 +744,10 @@ show_neighbor_msg(struct imsg *imsg, enu > break; > print_neighbor_msgstats(p); > printf("\n"); > + if (*(p->stats.last_shutcomm)) { > + printf(" Last received shutdown reason: \"%s\"\n", > + log_shutcomm(p->stats.last_shutcomm)); > + } > if (p->state == STATE_IDLE) { > static const char *errstr; > > Index: usr.sbin/bgpctl/parser.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v > retrieving revision 1.73 > diff -u -p -r1.73 parser.c > --- usr.sbin/bgpctl/parser.c 11 Oct 2015 19:53:57 -0000 1.73 > +++ usr.sbin/bgpctl/parser.c 8 Jan 2017 17:45:24 -0000 > @@ -43,6 +43,7 @@ enum token_type { > PREFIX, > PEERDESC, > RIBNAME, > + SHUTDOWN_COMMUNICATION, > COMMUNITY, > LOCALPREF, > MED, > @@ -239,9 +240,15 @@ static const struct token t_neighbor[] = > { ENDTOKEN, "", NONE, NULL} > }; > > +static const struct token t_neighbor_modifiers_shutcomm[] = { > + { NOTOKEN, "", NONE, NULL}, > + { SHUTDOWN_COMMUNICATION, "", NONE, NULL}, > + { ENDTOKEN, "", NONE, NULL} > +}; > + > static const struct token t_neighbor_modifiers[] = { > { KEYWORD, "up", NEIGHBOR_UP, NULL}, > - { KEYWORD, "down", NEIGHBOR_DOWN, NULL}, > + { KEYWORD, "down", NEIGHBOR_DOWN, > t_neighbor_modifiers_shutcomm}, > { KEYWORD, "clear", NEIGHBOR_CLEAR, NULL}, > { KEYWORD, "refresh", NEIGHBOR_RREFRESH, NULL}, > { KEYWORD, "destroy", NEIGHBOR_DESTROY, NULL}, > @@ -549,6 +556,16 @@ match_token(int *argc, char **argv[], co > t = &table[i]; > } > break; > + case SHUTDOWN_COMMUNICATION: > + if (!match && word != NULL && wordlen > 0) { > + if (strlcpy(res.shutcomm, word, > + sizeof(res.shutcomm)) >= > + sizeof(res.shutcomm)) > + errx(1, "shutdown reason too long"); > + match++; > + t = &table[i]; > + } > + break; > case COMMUNITY: > if (word != NULL && wordlen > 0 && > parse_community(word, &res)) { > @@ -664,6 +681,9 @@ show_valid_args(const struct token table > break; > case RIBNAME: > fprintf(stderr, " <rib name>\n"); > + break; > + case SHUTDOWN_COMMUNICATION: > + fprintf(stderr, " <shutdown reason>\n"); > break; > case COMMUNITY: > fprintf(stderr, " <community>\n"); > Index: usr.sbin/bgpctl/parser.h > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpctl/parser.h,v > retrieving revision 1.27 > diff -u -p -r1.27 parser.h > --- usr.sbin/bgpctl/parser.h 17 Apr 2015 07:51:09 -0000 1.27 > +++ usr.sbin/bgpctl/parser.h 8 Jan 2017 17:45:24 -0000 > @@ -65,6 +65,7 @@ struct parse_result { > struct filter_community community; > char peerdesc[PEER_DESCR_LEN]; > char rib[PEER_DESCR_LEN]; > + char shutcomm[SHUT_COMM_LEN]; > char *irr_outdir; > int flags; > u_int rtableid; > Index: usr.sbin/bgpd/bgpd.8 > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.8,v > retrieving revision 1.48 > diff -u -p -r1.48 bgpd.8 > --- usr.sbin/bgpd/bgpd.8 14 Aug 2013 06:32:36 -0000 1.48 > +++ usr.sbin/bgpd/bgpd.8 8 Jan 2017 17:45:24 -0000 > @@ -238,6 +238,26 @@ control socket > .Re > .Pp > .Rs > +.%A J. Snijders > +.%A J. Heitz > +.%A K. Patel > +.%A I. Bagdonas > +.%A A. Simpson > +.%D January 2017 > +.%R draft-ietf-idr-large-community > +.%T Large BGP Communities Attribute this duplicates this entry also newer stuff sould go to the bottom of the list i believe. > +.Re > +.Pp > +.Rs > +.%A J. Snijders > +.%A J. Heitz > +.%A J. Scudder > +.%D January 2017 > +.%R draft-ietf-idr-shutdown > +.%T BGP Administrative Shutdown Communication > +.Re > +.Pp > +.Rs > .%A A. Heffernan > .%D August 1998 > .%R RFC 2385 > Index: usr.sbin/bgpd/bgpd.conf.5 > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v > retrieving revision 1.144 > diff -u -p -r1.144 bgpd.conf.5 > --- usr.sbin/bgpd/bgpd.conf.5 28 Jun 2016 16:59:14 -0000 1.144 > +++ usr.sbin/bgpd/bgpd.conf.5 8 Jan 2017 17:45:24 -0000 > @@ -722,9 +722,17 @@ The description is used when logging nei > reports, for specifying neighbors, etc., but has no further meaning to > .Xr bgpd 8 . > .Pp > -.It Ic down > +.It Ic down Op Ar reason > Do not start the session when bgpd comes up but stay in > .Em IDLE . > +If the session is cleared at runtime, after a > +.Ic down > +.Ar reason > +was configured at runtime, the > +.Ar reason > +is sent as Administrative Shutdown Communication. The > +.Ar reason > +cannot exceed 128 octets. > .Pp > .It Xo > .Ic dump > Index: usr.sbin/bgpd/bgpd.h > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v > retrieving revision 1.294 > diff -u -p -r1.294 bgpd.h > --- usr.sbin/bgpd/bgpd.h 6 Jun 2016 15:59:10 -0000 1.294 > +++ usr.sbin/bgpd/bgpd.h 8 Jan 2017 17:45:24 -0000 > @@ -38,6 +38,7 @@ > #define CONFFILE "/etc/bgpd.conf" > #define BGPD_USER "_bgpd" > #define PEER_DESCR_LEN 32 > +#define SHUT_COMM_LEN 129 > #define PFTABLE_LEN 32 > #define TCP_MD5_KEY_LEN 80 > #define IPSEC_ENC_KEY_LEN 32 > @@ -296,6 +297,7 @@ struct peer_config { > struct capabilities capabilities; > char group[PEER_DESCR_LEN]; > char descr[PEER_DESCR_LEN]; > + char shutcomm[SHUT_COMM_LEN]; > char rib[PEER_DESCR_LEN]; > char if_depend[IFNAMSIZ]; > char demote_group[IFNAMSIZ]; > @@ -579,6 +581,7 @@ struct ctl_show_nexthop { > struct ctl_neighbor { > struct bgpd_addr addr; > char descr[PEER_DESCR_LEN]; > + char shutcomm[SHUT_COMM_LEN]; > int show_timers; > }; > > @@ -1060,6 +1063,7 @@ const char *log_sockaddr(struct sockaddr > const char *log_as(u_int32_t); > const char *log_rd(u_int64_t); > const char *log_ext_subtype(u_int8_t); > +const char *log_shutcomm(const char *); > int aspath_snprint(char *, size_t, void *, u_int16_t); > int aspath_asprint(char **, void *, u_int16_t); > size_t aspath_strlen(void *, u_int16_t); > Index: usr.sbin/bgpd/control.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/control.c,v > retrieving revision 1.82 > diff -u -p -r1.82 control.c > --- usr.sbin/bgpd/control.c 5 Dec 2015 18:28:04 -0000 1.82 > +++ usr.sbin/bgpd/control.c 8 Jan 2017 17:45:24 -0000 > @@ -337,9 +337,15 @@ control_dispatch_msg(struct pollfd *pfd, > switch (imsg.hdr.type) { > case IMSG_CTL_NEIGHBOR_UP: > bgp_fsm(p, EVNT_START); > + p->conf.down = 0; > + p->conf.shutcomm[0] = '\0'; > control_result(c, CTL_RES_OK); > break; > case IMSG_CTL_NEIGHBOR_DOWN: > + p->conf.down = 1; > + strlcpy(p->conf.shutcomm, > + neighbor->shutcomm, > + sizeof(neighbor->shutcomm)); > session_stop(p, ERR_CEASE_ADMIN_DOWN); > control_result(c, CTL_RES_OK); > break; > Index: usr.sbin/bgpd/parse.y > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v > retrieving revision 1.288 > diff -u -p -r1.288 parse.y > --- usr.sbin/bgpd/parse.y 21 Jun 2016 21:35:24 -0000 1.288 > +++ usr.sbin/bgpd/parse.y 8 Jan 2017 17:45:24 -0000 > @@ -1029,8 +1029,19 @@ peeropts : REMOTEAS as4number { > | PASSIVE { > curpeer->conf.passive = 1; > } > - | DOWN { > + | DOWN { > curpeer->conf.down = 1; > + } > + | DOWN STRING { > + curpeer->conf.down = 1; > + if (strlcpy(curpeer->conf.shutcomm, $2, > + sizeof(curpeer->conf.shutcomm)) >= > + sizeof(curpeer->conf.shutcomm)) { > + yyerror("shutdown reason too long"); > + free($2); > + YYERROR; > + } > + free($2); > } > | RIB STRING { > if (!find_rib($2)) { > Index: usr.sbin/bgpd/session.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/session.c,v > retrieving revision 1.351 > diff -u -p -r1.351 session.c > --- usr.sbin/bgpd/session.c 25 Jul 2016 14:29:28 -0000 1.351 > +++ usr.sbin/bgpd/session.c 8 Jan 2017 17:45:24 -0000 > @@ -552,6 +552,9 @@ session_main(int debug, int verbose) > > while ((p = peers) != NULL) { > peers = p->next; > + strlcpy(p->conf.shutcomm, > + "bgpd shutting down", > + sizeof(p->conf.shutcomm)); > session_stop(p, ERR_CEASE_ADMIN_DOWN); > pfkey_remove(p); > free(p); > @@ -2206,6 +2209,7 @@ parse_notification(struct peer *peer) > u_int8_t subcode; > u_int8_t capa_code; > u_int8_t capa_len; > + u_int8_t shutcomm_len; > u_int8_t i; > > /* just log */ > @@ -2300,6 +2304,31 @@ parse_notification(struct peer *peer) > return (1); > } > > + if (errcode == ERR_CEASE && subcode == ERR_CEASE_ADMIN_DOWN) { > + if (datalen >= sizeof(shutcomm_len)) { > + memcpy(&shutcomm_len, p, sizeof(shutcomm_len)); > + p += sizeof(shutcomm_len); > + datalen -= sizeof(shutcomm_len); > + if(datalen < shutcomm_len) { > + log_peer_warnx(&peer->conf, > + "received truncated shutdown reason"); > + return (0); > + } > + if (shutcomm_len > (SHUT_COMM_LEN-1)) { > + log_peer_warnx(&peer->conf, > + "received overly long shutdown reason"); > + return (0); > + } > + memcpy(peer->stats.last_shutcomm, p, shutcomm_len); > + peer->stats.last_shutcomm[shutcomm_len] = '\0'; > + log_peer_warnx(&peer->conf, > + "received shutdown reason: \"%s\"", > + log_shutcomm(peer->stats.last_shutcomm)); > + p+=shutcomm_len; please add spaces around += > + datalen-=shutcomm_len; same > + } > + } > + > return (0); > } > > @@ -3180,11 +3209,29 @@ session_demote(struct peer *p, int level > void > session_stop(struct peer *peer, u_int8_t subcode) > { > + char data[SHUT_COMM_LEN]; > + uint8_t datalen; > + uint8_t shutcomm_len; > + char *communication; > + > + datalen=0; same > + > + communication = peer->conf.shutcomm; > + > + if (subcode == ERR_CEASE_ADMIN_DOWN && communication && > + *communication) { > + shutcomm_len=strlen(communication); same > + if(shutcomm_len < SHUT_COMM_LEN) { > + data[0] = shutcomm_len; > + datalen = shutcomm_len + sizeof(data[0]); > + memcpy(data + 1, communication, shutcomm_len); > + } > + } > switch (peer->state) { > case STATE_OPENSENT: > case STATE_OPENCONFIRM: > case STATE_ESTABLISHED: > - session_notification(peer, ERR_CEASE, subcode, NULL, 0); > + session_notification(peer, ERR_CEASE, subcode, data, datalen); > break; > default: > /* session not open, no need to send notification */ > Index: usr.sbin/bgpd/session.h > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/session.h,v > retrieving revision 1.121 > diff -u -p -r1.121 session.h > --- usr.sbin/bgpd/session.h 25 Oct 2015 18:49:01 -0000 1.121 > +++ usr.sbin/bgpd/session.h 8 Jan 2017 17:45:24 -0000 > @@ -168,6 +168,7 @@ struct peer_stats { > u_int32_t prefix_cnt; > u_int8_t last_sent_errcode; > u_int8_t last_sent_suberr; > + char last_shutcomm[SHUT_COMM_LEN]; > }; > > enum Timer { > Index: usr.sbin/bgpd/util.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/util.c,v > retrieving revision 1.21 > diff -u -p -r1.21 util.c > --- usr.sbin/bgpd/util.c 3 Jun 2016 17:36:37 -0000 1.21 > +++ usr.sbin/bgpd/util.c 8 Jan 2017 17:45:24 -0000 > @@ -24,6 +24,7 @@ > #include <stdlib.h> > #include <stdio.h> > #include <string.h> > +#include <vis.h> > > #include "bgpd.h" > #include "rde.h" > @@ -156,6 +157,24 @@ log_ext_subtype(u_int8_t subtype) > snprintf(etype, sizeof(etype), "[%u]", subtype); > return (etype); > } > +} > + > +const char * > +log_shutcomm(const char *communication) { > + static char buf[(SHUT_COMM_LEN - 1) * 4 + 1]; > + const char *p; > + char *q; > + > + p = communication; > + for (q = buf; *p && q < &buf[sizeof(buf) - 1]; p++) { > + if (*p == '\n') > + *q++ = ' '; > + else > + q = vis(q, *p, 0, 0); > + } > + *q = '\0'; > + > + return buf; while i think this is correct, would it not be easier to encode \n as control char with VIS_NL? then you could just use strnvis() > } > > const char * >
