Download raw body.
bgpd: remove announce capabilities option
Running BGP without capabilities is so 90is that I think it is time to not
allow it anymore.
When we get an optional parameter in OPEN error then stop the session.
There is no way a session will establish like this. Normally bgpd just
retries over and over again so maybe we should do the same here as well
and just handle this case like any other notification.
Adjust session_stop to handle the shutdown reason as argument to simplify
some other code paths. Remove the no punish case in parse_open since this
actually causes more issues than it solves.
Idle sessions need to clear a possible Timer_IdleHold when an EVNT_STOP
happens. This should prevent an idle session from reactivating after
'bgpctl nei XYZ down'.
Last but not least, parse_notification() is now a void function and does
the state change to IDLE in the function itself.
--
:wq Claudio
Index: bgpctl/output_json.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpctl/output_json.c,v
diff -u -p -r1.42 output_json.c
--- bgpctl/output_json.c 31 Jan 2024 11:23:20 -0000 1.42
+++ bgpctl/output_json.c 9 Apr 2024 09:46:55 -0000
@@ -250,7 +250,6 @@ json_neighbor_full(struct peer *p)
json_do_string("role", log_policy(p->conf.role));
/* capabilities */
- json_do_bool("announce_capabilities", p->conf.announce_capa);
json_neighbor_capabilities(&p->conf.capabilities);
json_do_end();
Index: bgpd/bgpd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v
diff -u -p -r1.239 bgpd.conf.5
--- bgpd/bgpd.conf.5 9 Apr 2024 09:03:18 -0000 1.239
+++ bgpd/bgpd.conf.5 9 Apr 2024 12:44:25 -0000
@@ -1064,17 +1064,6 @@ The default is
.Ic yes .
.Pp
.It Xo
-.Ic announce capabilities
-.Pq Ic yes Ns | Ns Ic no
-.Xc
-If set to
-.Ic no ,
-capability negotiation is disabled during the establishment of the session.
-This can be helpful to connect to old or broken BGP implementations.
-The default is
-.Ic yes .
-.Pp
-.It Xo
.Ic announce enhanced refresh
.Pq Ic yes Ns | Ns Ic no Ns | Ns Ic enforce
.Xc
Index: bgpd/bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
diff -u -p -r1.491 bgpd.h
--- bgpd/bgpd.h 9 Apr 2024 12:09:19 -0000 1.491
+++ bgpd/bgpd.h 10 Apr 2024 08:20:06 -0000
@@ -479,7 +479,6 @@ struct peer_config {
uint8_t distance; /* 1 = direct, >1 = multihop */
uint8_t passive;
uint8_t down;
- uint8_t announce_capa;
uint8_t reflector_client;
uint8_t ttlsec; /* TTL security hack */
uint8_t flags;
Index: bgpd/control.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/control.c,v
diff -u -p -r1.116 control.c
--- bgpd/control.c 11 Jan 2024 15:46:25 -0000 1.116
+++ bgpd/control.c 9 Apr 2024 10:32:01 -0000
@@ -393,31 +393,28 @@ control_dispatch_msg(struct pollfd *pfd,
case IMSG_CTL_NEIGHBOR_DOWN:
neighbor.reason[
sizeof(neighbor.reason) - 1] = '\0';
- strlcpy(p->conf.reason,
- neighbor.reason,
- sizeof(p->conf.reason));
p->conf.down = 1;
- session_stop(p, ERR_CEASE_ADMIN_DOWN);
+ session_stop(p, ERR_CEASE_ADMIN_DOWN,
+ neighbor.reason);
control_result(c, CTL_RES_OK);
break;
case IMSG_CTL_NEIGHBOR_CLEAR:
neighbor.reason[
sizeof(neighbor.reason) - 1] = '\0';
- strlcpy(p->conf.reason,
- neighbor.reason,
- sizeof(p->conf.reason));
p->IdleHoldTime =
INTERVAL_IDLE_HOLD_INITIAL;
p->errcnt = 0;
if (!p->conf.down) {
session_stop(p,
- ERR_CEASE_ADMIN_RESET);
+ ERR_CEASE_ADMIN_RESET,
+ neighbor.reason);
timer_set(&p->timers,
Timer_IdleHold,
SESSION_CLEAR_DELAY);
} else {
session_stop(p,
- ERR_CEASE_ADMIN_DOWN);
+ ERR_CEASE_ADMIN_DOWN,
+ neighbor.reason);
}
control_result(c, CTL_RES_OK);
break;
Index: bgpd/parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
diff -u -p -r1.460 parse.y
--- bgpd/parse.y 9 Apr 2024 12:40:01 -0000 1.460
+++ bgpd/parse.y 10 Apr 2024 08:20:06 -0000
@@ -246,7 +246,7 @@ typedef struct {
%token EBGP IBGP
%token FLOWSPEC PROTO FLAGS FRAGMENT TOS LENGTH ICMPTYPE CODE
%token LOCALAS REMOTEAS DESCR LOCALADDR MULTIHOP PASSIVE MAXPREFIX RESTART
-%token ANNOUNCE CAPABILITIES REFRESH AS4BYTE CONNECTRETRY ENHANCED ADDPATH
+%token ANNOUNCE REFRESH AS4BYTE CONNECTRETRY ENHANCED ADDPATH
%token SEND RECV PLUS POLICY ROLE
%token DEMOTE ENFORCE NEIGHBORAS ASOVERRIDE REFLECTOR DEPEND DOWN
%token DUMP IN OUT SOCKET RESTRICTED
@@ -1912,9 +1912,6 @@ peeropts : REMOTEAS as4number {
curpeer->conf.capabilities.mp[aid] = 1;
}
}
- | ANNOUNCE CAPABILITIES yesno {
- curpeer->conf.announce_capa = $3;
- }
| ANNOUNCE REFRESH yesnoenforce {
curpeer->conf.capabilities.refresh = $3;
}
@@ -3522,7 +3519,6 @@ lookup(char *s)
{ "aspa-set", ASPASET},
{ "avs", AVS},
{ "blackhole", BLACKHOLE},
- { "capabilities", CAPABILITIES},
{ "community", COMMUNITY},
{ "compare", COMPARE},
{ "connect-retry", CONNECTRETRY},
@@ -4635,7 +4631,6 @@ alloc_peer(void)
p->reconf_action = RECONF_REINIT;
p->conf.distance = 1;
p->conf.export_type = EXPORT_UNSET;
- p->conf.announce_capa = 1;
p->conf.capabilities.refresh = 1;
p->conf.capabilities.grestart.restart = 1;
p->conf.capabilities.as4byte = 1;
Index: bgpd/printconf.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v
diff -u -p -r1.171 printconf.c
--- bgpd/printconf.c 9 Apr 2024 09:03:18 -0000 1.171
+++ bgpd/printconf.c 9 Apr 2024 10:01:51 -0000
@@ -918,9 +918,6 @@ print_announce(struct peer_config *p, co
uint8_t aid;
int match = 0;
- if (p->announce_capa == 0)
- printf("%s\tannounce capabilities no\n", c);
-
for (aid = AID_MIN; aid < AID_MAX; aid++)
if (p->capabilities.mp[aid] == 2) {
printf("%s\tannounce %s enforce\n", c, aid2str(aid));
Index: bgpd/session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
diff -u -p -r1.469 session.c
--- bgpd/session.c 10 Apr 2024 09:05:32 -0000 1.469
+++ bgpd/session.c 11 Apr 2024 08:25:44 -0000
@@ -66,7 +66,6 @@ int session_setup_socket(struct peer *);
void session_accept(int);
int session_connect(struct peer *);
void session_tcp_established(struct peer *);
-void session_capa_ann_none(struct peer *);
int session_capa_add(struct ibuf *, uint8_t, uint8_t);
int session_capa_add_mp(struct ibuf *, uint8_t);
int session_capa_add_afi(struct ibuf *, uint8_t, uint8_t);
@@ -87,7 +86,7 @@ int parse_header(struct peer *, u_char *
int parse_open(struct peer *);
int parse_update(struct peer *);
int parse_rrefresh(struct peer *);
-int parse_notification(struct peer *);
+void parse_notification(struct peer *);
int parse_capabilities(struct peer *, u_char *, uint16_t, uint32_t *);
int capa_neg_calc(struct peer *);
void session_dispatch_imsg(struct imsgbuf *, int, u_int *);
@@ -266,11 +265,12 @@ session_main(int debug, int verbose)
if (p->demoted)
session_demote(p, -1);
p->conf.demote_group[0] = 0;
- session_stop(p, ERR_CEASE_PEER_UNCONF);
+ session_stop(p, ERR_CEASE_PEER_UNCONF,
+ NULL);
timer_remove_all(&p->timers);
tcp_md5_del_listener(conf, p);
- log_peer_warnx(&p->conf, "removed");
RB_REMOVE(peer_head, &conf->peers, p);
+ log_peer_warnx(&p->conf, "removed");
free(p);
peer_cnt--;
continue;
@@ -513,12 +513,10 @@ session_main(int debug, int verbose)
}
RB_FOREACH_SAFE(p, peer_head, &conf->peers, next) {
- RB_REMOVE(peer_head, &conf->peers, p);
- strlcpy(p->conf.reason,
- "bgpd shutting down",
- sizeof(p->conf.reason));
- session_stop(p, ERR_CEASE_ADMIN_DOWN);
+ session_stop(p, ERR_CEASE_ADMIN_DOWN, "bgpd shutting down");
timer_remove_all(&p->timers);
+ tcp_md5_del_listener(conf, p);
+ RB_REMOVE(peer_head, &conf->peers, p);
free(p);
}
@@ -624,6 +622,9 @@ bgp_fsm(struct peer *peer, enum session_
}
peer->passive = 0;
break;
+ case EVNT_STOP:
+ timer_stop(&peer->timers, Timer_IdleHold);
+ break;
default:
/* ignore */
break;
@@ -723,13 +724,7 @@ bgp_fsm(struct peer *peer, enum session_
change_state(peer, STATE_OPENCONFIRM, event);
break;
case EVNT_RCVD_NOTIFICATION:
- if (parse_notification(peer)) {
- change_state(peer, STATE_IDLE, event);
- /* don't punish, capa negotiation */
- timer_set(&peer->timers, Timer_IdleHold, 0);
- peer->IdleHoldTime /= 2;
- } else
- change_state(peer, STATE_IDLE, event);
+ parse_notification(peer);
break;
default:
session_notification(peer,
@@ -769,7 +764,6 @@ bgp_fsm(struct peer *peer, enum session_
break;
case EVNT_RCVD_NOTIFICATION:
parse_notification(peer);
- change_state(peer, STATE_IDLE, event);
break;
default:
session_notification(peer,
@@ -815,7 +809,6 @@ bgp_fsm(struct peer *peer, enum session_
break;
case EVNT_RCVD_NOTIFICATION:
parse_notification(peer);
- change_state(peer, STATE_IDLE, event);
break;
default:
session_notification(peer,
@@ -937,8 +930,6 @@ change_state(struct peer *peer, enum ses
/* initialize capability negotiation structures */
memcpy(&peer->capa.ann, &peer->conf.capabilities,
sizeof(peer->capa.ann));
- if (!peer->conf.announce_capa)
- session_capa_ann_none(peer);
}
break;
case STATE_CONNECT:
@@ -1336,12 +1327,6 @@ session_tcp_established(struct peer *pee
&peer->if_scope);
}
-void
-session_capa_ann_none(struct peer *peer)
-{
- memset(&peer->capa.ann, 0, sizeof(peer->capa.ann));
-}
-
int
session_capa_add(struct ibuf *opb, uint8_t capa_code, uint8_t capa_len)
{
@@ -2326,9 +2311,6 @@ bad_len:
session_notification(peer, ERR_OPEN, ERR_OPEN_OPT,
NULL);
change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
- /* no punish */
- timer_set(&peer->timers, Timer_IdleHold, 0);
- peer->IdleHoldTime /= 2;
return (-1);
}
}
@@ -2493,7 +2475,7 @@ parse_rrefresh(struct peer *peer)
return (0);
}
-int
+void
parse_notification(struct peer *peer)
{
struct ibuf ibuf;
@@ -2518,7 +2500,7 @@ parse_notification(struct peer *peer)
if (ibuf_get_n8(&ibuf, &errcode) == -1 ||
ibuf_get_n8(&ibuf, &subcode) == -1) {
log_peer_warnx(&peer->conf, "received bad notification");
- return (-1);
+ goto done;
}
peer->errcnt++;
@@ -2541,12 +2523,15 @@ parse_notification(struct peer *peer)
}
}
+ /* other side does not support capabilities, give up */
if (errcode == ERR_OPEN && subcode == ERR_OPEN_OPT) {
- session_capa_ann_none(peer);
- return (1);
+ change_state(peer, STATE_IDLE, EVNT_RCVD_NOTIFICATION);
+ session_stop(peer, ERR_CEASE_CONN_REJECT, NULL);
+ return;
}
- return (0);
+done:
+ change_state(peer, STATE_IDLE, EVNT_RCVD_NOTIFICATION);
}
int
@@ -3165,7 +3150,8 @@ session_dispatch_imsg(struct imsgbuf *im
} else if (!depend_ok && p->depend_ok) {
p->depend_ok = depend_ok;
session_stop(p,
- ERR_CEASE_OTHER_CHANGE);
+ ERR_CEASE_OTHER_CHANGE,
+ NULL);
}
}
break;
@@ -3631,21 +3617,21 @@ session_demote(struct peer *p, int level
}
void
-session_stop(struct peer *peer, uint8_t subcode)
+session_stop(struct peer *peer, uint8_t subcode, const char *reason)
{
struct ibuf *ibuf;
- char *communication;
- communication = peer->conf.reason;
+ if (reason != NULL)
+ strlcpy(peer->conf.reason, reason, sizeof(peer->conf.reason));
ibuf = ibuf_dynamic(0, REASON_LEN);
if ((subcode == ERR_CEASE_ADMIN_DOWN ||
subcode == ERR_CEASE_ADMIN_RESET) &&
- communication != NULL && *communication != '\0' &&
+ reason != NULL && *reason != '\0' &&
ibuf != NULL) {
- if (ibuf_add_n8(ibuf, strlen(communication)) == -1 ||
- ibuf_add(ibuf, communication, strlen(communication))) {
+ if (ibuf_add_n8(ibuf, strlen(reason)) == -1 ||
+ ibuf_add(ibuf, reason, strlen(reason))) {
log_peer_warnx(&peer->conf,
"trying to send overly long shutdown reason");
ibuf_free(ibuf);
@@ -3660,6 +3646,13 @@ session_stop(struct peer *peer, uint8_t
break;
default:
/* session not open, no need to send notification */
+ if (subcode >= sizeof(suberr_cease_names) / sizeof(char *) ||
+ suberr_cease_names[subcode] == NULL)
+ log_peer_warnx(&peer->conf, "session stop: %s, "
+ "unknown subcode %u", errnames[ERR_CEASE], subcode);
+ else
+ log_peer_warnx(&peer->conf, "session stop: %s, %s",
+ errnames[ERR_CEASE], suberr_cease_names[subcode]);
break;
}
ibuf_free(ibuf);
Index: bgpd/session.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.h,v
diff -u -p -r1.168 session.h
--- bgpd/session.h 22 Mar 2024 07:19:28 -0000 1.168
+++ bgpd/session.h 9 Apr 2024 09:48:06 -0000
@@ -331,7 +331,7 @@ int peer_matched(struct peer *, struct
int imsg_ctl_parent(struct imsg *);
int imsg_ctl_rde(struct imsg *);
int imsg_ctl_rde_msg(int, uint32_t, pid_t);
-void session_stop(struct peer *, uint8_t);
+void session_stop(struct peer *, uint8_t, const char *);
/* timer.c */
struct timer *timer_get(struct timer_head *, enum Timer);
bgpd: remove announce capabilities option