On Wed, Jan 04, 2023 at 12:21:59PM +0100, Claudio Jeker wrote: > 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.
Reads fine. ok with one small comment below. > 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 [...] > @@ -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) { This should be ROLE_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 */