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";

Reply via email to