bgpd already supports Open Policy (RFC 9234) and the role of the router is set as part of the `announce policy` config. Now ASPA also needs the role so it makes sense to promote setting the role in the config.
I also switched the role to an enum mainly because I want 0 to be the default "NONE" role. Roles only matter on ebgp sessions and ibgp sessions should just use ROLE_NONE. announce policy now only has three options: yes, no and enforce If announce policy is set to yes but no role is defined the policy capability is silently switched to no. I'm not super happy with that behaviour but I think it is temporary and at one point 'role' will become mandatory for ebgp neighbors. Similar to turning off announce policy, ibgp sessions are forced to ROLE_NONE. Roles make no sense for ibgp and so forcing the role to none makes sure they remain consistent. The automatic changes are done so that groups can inherit default values that are fixed up if required. I did the minimal adjustment in bgpd.conf.5 manpage. I think more is needed there. This breaks the policy intergration test but I have a fix for it ready. -- :wq Claudio Index: bgpd.conf.5 =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v retrieving revision 1.227 diff -u -p -r1.227 bgpd.conf.5 --- bgpd.conf.5 22 Dec 2022 19:53:24 -0000 1.227 +++ bgpd.conf.5 3 Jan 2023 17:42:15 -0000 @@ -933,19 +933,11 @@ The default is .Pp .It Xo .Ic announce policy -.Pq Ic no Ns | Ns Ar role -.Op Ic enforce +.Pq Ic yes Ns | Ns Ic no Ns | Ns Ic enforce .Xc If set to -.Ic no , -do not add the open policy role capability. -The role can be one of -.Ar provider , -.Ar customer , -.Ar rs , -.Ar rs-client , -or -.Ar peer . +.Ic yes , +add the open policy role capability. If the role of the neighbor does not correspond to the expected role then the session will be closed. If @@ -1310,6 +1302,22 @@ setting. .Pp .It Ic rib Ar name Bind the neighbor to the specified RIB. +.Pp +.It Ic role Ar role +Set the local role for this eBGP session. +The role can be one of +.Ar none , +.Ar provider , +.Ar customer , +.Ar rs , +.Ar rs-client , +or +.Ar peer . +If the role is set to +.Ar none +the +.Ic announce Ic policy +will also be disabled. .Pp .It Ic route-reflector Op Ar address Act as an RFC 4456 Index: bgpd.h =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v retrieving revision 1.455 diff -u -p -r1.455 bgpd.h --- bgpd.h 18 Nov 2022 10:17:23 -0000 1.455 +++ bgpd.h 3 Jan 2023 16:45:29 -0000 @@ -333,6 +333,15 @@ enum enforce_as { ENFORCE_AS_ON }; +enum role { + ROLE_NONE, + ROLE_CUSTOMER, + ROLE_PROVIDER, + ROLE_RS, + ROLE_RS_CLIENT, + ROLE_PEER, +}; + enum auth_method { AUTH_NONE, AUTH_MD5SIG, @@ -380,12 +389,12 @@ struct capabilities { int8_t flags[AID_MAX]; /* graceful restart per AID flags */ int8_t restart; /* graceful restart, RFC 4724 */ } grestart; + enum role role; /* Open Policy, RFC 9234 */ int8_t mp[AID_MAX]; /* multiprotocol extensions, RFC 4760 */ + int8_t add_path[AID_MAX]; /* ADD_PATH, RFC 7911 */ int8_t refresh; /* route refresh, RFC 2918 */ int8_t as4byte; /* 4-byte ASnum, RFC 4893 */ int8_t enhanced_rr; /* enhanced route refresh, RFC 7313 */ - int8_t add_path[AID_MAX]; /* ADD_PATH, RFC 7911 */ - uint8_t role; /* Open Policy, RFC 9234 */ int8_t role_ena; /* 1 for enable, 2 for enforce */ }; @@ -432,6 +441,7 @@ struct peer_config { enum export_type export_type; enum enforce_as enforce_as; enum enforce_as enforce_local_as; + enum role role; uint16_t max_prefix_restart; uint16_t max_out_prefix_restart; uint16_t holdtime; @@ -1417,7 +1427,7 @@ const char *log_rd(uint64_t); const char *log_ext_subtype(int, uint8_t); const char *log_reason(const char *); const char *log_rtr_error(enum rtr_error); -const char *log_policy(uint8_t); +const char *log_policy(enum role); int aspath_snprint(char *, size_t, void *, uint16_t); int aspath_asprint(char **, void *, uint16_t); size_t aspath_strlen(void *, uint16_t); Index: parse.y =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v retrieving revision 1.437 diff -u -p -r1.437 parse.y --- parse.y 18 Nov 2022 10:17:23 -0000 1.437 +++ parse.y 3 Jan 2023 17:16:38 -0000 @@ -219,7 +219,7 @@ typedef struct { %token EBGP IBGP %token LOCALAS REMOTEAS DESCR LOCALADDR MULTIHOP PASSIVE MAXPREFIX RESTART %token ANNOUNCE CAPABILITIES REFRESH AS4BYTE CONNECTRETRY ENHANCED ADDPATH -%token SEND RECV PLUS POLICY +%token SEND RECV PLUS POLICY ROLE %token DEMOTE ENFORCE NEIGHBORAS ASOVERRIDE REFLECTOR DEPEND DOWN %token DUMP IN OUT SOCKET RESTRICTED %token LOG TRANSPARENT @@ -1640,32 +1640,30 @@ peeropts : REMOTEAS as4number { curpeer->conf.eval.extrapaths = $5; curpeer->conf.eval.maxpaths = $6; } - | ANNOUNCE POLICY STRING enforce { - curpeer->conf.capabilities.role_ena = $4; - if (strcmp($3, "no") == 0) { - curpeer->conf.capabilities.role_ena = 0; - } else if (strcmp($3, "provider") == 0) { - curpeer->conf.capabilities.role = - CAPA_ROLE_PROVIDER; - } else if (strcmp($3, "rs") == 0) { - curpeer->conf.capabilities.role = - CAPA_ROLE_RS; - } else if (strcmp($3, "rs-client") == 0) { - curpeer->conf.capabilities.role = - CAPA_ROLE_RS_CLIENT; - } else if (strcmp($3, "customer") == 0) { - curpeer->conf.capabilities.role = - CAPA_ROLE_CUSTOMER; - } else if (strcmp($3, "peer") == 0) { - curpeer->conf.capabilities.role = - CAPA_ROLE_PEER; + | ANNOUNCE POLICY enforce { + curpeer->conf.capabilities.role_ena = $3; + } + | ROLE STRING { + if (strcmp($2, "provider") == 0) { + curpeer->conf.role = ROLE_PROVIDER; + } else if (strcmp($2, "rs") == 0) { + curpeer->conf.role = ROLE_RS; + } else if (strcmp($2, "rs-client") == 0) { + curpeer->conf.role = ROLE_RS_CLIENT; + } else if (strcmp($2, "customer") == 0) { + curpeer->conf.role = ROLE_CUSTOMER; + } else if (strcmp($2, "peer") == 0) { + curpeer->conf.role = ROLE_PEER; } else { - yyerror("syntax error, one of no, provider, " + yyerror("syntax error, one of none, provider, " "rs, rs-client, customer, peer expected"); - free($3); + free($2); YYERROR; } - free($3); + free($2); + } + | ROLE NONE { + curpeer->conf.role = ROLE_NONE; } | EXPORT NONE { curpeer->conf.export_type = EXPORT_NONE; @@ -2742,7 +2740,7 @@ delete : /* empty */ { $$ = 0; } | DELETE { $$ = 1; } ; -enforce : /* empty */ { $$ = 1; } +enforce : yesno { $$ = $1; } | ENFORCE { $$ = 2; } ; @@ -3248,6 +3246,7 @@ lookup(char *s) { "restricted", RESTRICTED}, { "rib", RIB}, { "roa-set", ROASET }, + { "role", ROLE}, { "route-reflector", REFLECTOR}, { "router-id", ROUTERID}, { "rtable", RTABLE}, @@ -4710,6 +4709,14 @@ neighbor_consistent(struct peer *p) "reflector clusters"); return (-1); } + + /* BGP role and RFC 9234 role are only valid for EBGP neighbors */ + if (p->conf.ebgp) { + if (p->conf.role == ROLE_NONE) + p->conf.capabilities.role_ena = 0; + p->conf.capabilities.role = p->conf.role; + } else + p->conf.role = ROLE_NONE; /* check for duplicate peer definitions */ RB_FOREACH(xp, peer_head, new_peers) Index: printconf.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v retrieving revision 1.160 diff -u -p -r1.160 printconf.c --- printconf.c 18 Nov 2022 10:17:23 -0000 1.160 +++ printconf.c 3 Jan 2023 17:21:25 -0000 @@ -671,6 +671,8 @@ print_peer(struct peer_config *p, struct log_addr(&p->local_addr_v6)); if (p->remote_port != BGP_PORT) printf("%s\tport %hu\n", c, p->remote_port); + if (p->role != ROLE_NONE) + printf("%s\trole %s\n", c, log_policy(p->role)); if (p->max_prefix) { printf("%s\tmax-prefix %u", c, p->max_prefix); if (p->max_prefix_restart) @@ -847,11 +849,9 @@ print_announce(struct peer_config *p, co printf("\n"); } if (p->capabilities.role_ena) { - printf("%s\tannounce policy %s%s\n", c, - log_policy(p->capabilities.role), - p->capabilities.role_ena == 2 ? " enforce" : ""); + printf("%s\tannounce policy %s\n", c, + p->capabilities.role_ena == 2 ? "enforce" : "yes"); } - } void Index: session.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/session.c,v retrieving revision 1.438 diff -u -p -r1.438 session.c --- session.c 28 Dec 2022 21:30:16 -0000 1.438 +++ session.c 4 Jan 2023 11:11:45 -0000 @@ -1412,6 +1412,47 @@ session_sendmsg(struct bgp_msg *msg, str return (0); } +/* + * Translate between internal roles and the value expected by RFC 9234. + */ +static uint8_t +role2capa(enum role role) +{ + switch (role) { + case ROLE_CUSTOMER: + return CAPA_ROLE_CUSTOMER; + case ROLE_PROVIDER: + return CAPA_ROLE_PROVIDER; + case ROLE_RS: + return CAPA_ROLE_RS; + case ROLE_RS_CLIENT: + return CAPA_ROLE_RS_CLIENT; + case ROLE_PEER: + return CAPA_ROLE_PEER; + default: + fatalx("Unsupported role for role capability"); + } +} + +static enum role +capa2role(uint8_t val) +{ + switch (val) { + case CAPA_ROLE_PROVIDER: + return ROLE_PROVIDER; + case CAPA_ROLE_RS: + return ROLE_RS; + case CAPA_ROLE_RS_CLIENT: + return ROLE_RS_CLIENT; + case CAPA_ROLE_CUSTOMER: + return ROLE_CUSTOMER; + case CAPA_ROLE_PEER: + return ROLE_PEER; + default: + return ROLE_NONE; + } +} + void session_open(struct peer *p) { @@ -1442,9 +1483,12 @@ session_open(struct peer *p) errs += session_capa_add(opb, CAPA_REFRESH, 0); /* BGP open policy, RFC 9234, only for ebgp sessions */ - if (p->capa.ann.role_ena && p->conf.ebgp) { + if (p->conf.ebgp && p->capa.ann.role_ena && + p->capa.ann.role != CAPA_NONE) { + uint8_t val; + val = role2capa(p->capa.ann.role); errs += session_capa_add(opb, CAPA_ROLE, 1); - errs += ibuf_add(opb, &p->capa.ann.role, 1); + errs += ibuf_add(opb, &val, 1); } /* graceful restart and End-of-RIB marker, RFC 4724 */ @@ -2628,7 +2672,7 @@ parse_capabilities(struct peer *peer, u_ log_peer_warnx(&peer->conf, "Received role capability on iBGP session"); peer->capa.peer.role_ena = 1; - peer->capa.peer.role = *capa_val; + peer->capa.peer.role = capa2role(*capa_val); break; case CAPA_RESTART: if (capa_len == 2) { @@ -2847,24 +2891,24 @@ capa_neg_calc(struct peer *p, uint8_t *s if (p->capa.ann.role_ena != 0 && p->capa.peer.role_ena != 0 && p->conf.ebgp) { switch (p->capa.ann.role) { - case CAPA_ROLE_PROVIDER: - if (p->capa.peer.role != CAPA_ROLE_CUSTOMER) + case ROLE_PROVIDER: + if (p->capa.peer.role != ROLE_CUSTOMER) goto fail; break; - case CAPA_ROLE_RS: - if (p->capa.peer.role != CAPA_ROLE_RS_CLIENT) + case ROLE_RS: + if (p->capa.peer.role != ROLE_RS_CLIENT) goto fail; break; - case CAPA_ROLE_RS_CLIENT: - if (p->capa.peer.role != CAPA_ROLE_RS) + case ROLE_RS_CLIENT: + if (p->capa.peer.role != ROLE_RS) goto fail; break; - case CAPA_ROLE_CUSTOMER: - if (p->capa.peer.role != CAPA_ROLE_PROVIDER) + case ROLE_CUSTOMER: + if (p->capa.peer.role != ROLE_PROVIDER) goto fail; break; - case CAPA_ROLE_PEER: - if (p->capa.peer.role != CAPA_ROLE_PEER) + case ROLE_PEER: + if (p->capa.peer.role != ROLE_PEER) goto fail; break; default: Index: util.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/util.c,v retrieving revision 1.73 diff -u -p -r1.73 util.c --- util.c 9 Nov 2022 14:23:53 -0000 1.73 +++ util.c 3 Jan 2023 16:42:30 -0000 @@ -198,18 +198,18 @@ log_rtr_error(enum rtr_error err) } const char * -log_policy(uint8_t role) +log_policy(enum role role) { switch (role) { - case CAPA_ROLE_PROVIDER: + case ROLE_PROVIDER: return "provider"; - case CAPA_ROLE_RS: + case ROLE_RS: return "rs"; - case CAPA_ROLE_RS_CLIENT: + case ROLE_RS_CLIENT: return "rs-client"; - case CAPA_ROLE_CUSTOMER: + case ROLE_CUSTOMER: return "customer"; - case CAPA_ROLE_PEER: + case ROLE_PEER: return "peer"; default: return "unknown";