ok with whitespace fixes below
Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.05.10 15:34:22 +0200:
> This change is from a much larger patch I'm working on. This cleans up
> up_generate_attr() from a hardcoded implementation to a loop-switch
> construct. This way attributes are always dumped in ascending order as
> suggested by the RFC and adding special attributes is simpler than in the
> current way. There is an exception whit the MP attributes because those
> are added at a later point.
>
> Works for me(tm)
> --
> :wq Claudio
>
> Index: rde_update.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
> retrieving revision 1.108
> diff -u -p -r1.108 rde_update.c
> --- rde_update.c 21 Jan 2019 02:07:56 -0000 1.108
> +++ rde_update.c 10 May 2019 11:51:28 -0000
> @@ -295,96 +295,155 @@ up_generate_attr(u_char *buf, int len, s
> struct filterstate *state, u_int8_t aid)
> {
> struct rde_aspath *asp = &state->aspath;
> - struct attr *oa, *newaggr = NULL;
> + struct attr *oa = NULL, *newaggr = NULL;
> u_char *pdata;
> u_int32_t tmp32;
> in_addr_t nexthop;
> int flags, r, neednewpath = 0;
> u_int16_t wlen = 0, plen;
> - u_int8_t l;
> - u_int16_t nlen = 0;
> - u_char *ndata;
> -
> - /* origin */
> - if ((r = attr_write(buf + wlen, len, ATTR_WELL_KNOWN,
> - ATTR_ORIGIN, &asp->origin, 1)) == -1)
> - return (-1);
> - wlen += r; len -= r;
> -
> - /* aspath */
> - if (!peer->conf.ebgp ||
> - peer->conf.flags & PEERFLAG_TRANS_AS)
> - pdata = aspath_prepend(asp->aspath, peer->conf.local_as, 0,
> - &plen);
> - else
> - pdata = aspath_prepend(asp->aspath, peer->conf.local_as, 1,
> - &plen);
> -
> - if (!rde_as4byte(peer))
> - pdata = aspath_deflate(pdata, &plen, &neednewpath);
> -
> - if ((r = attr_write(buf + wlen, len, ATTR_WELL_KNOWN,
> - ATTR_ASPATH, pdata, plen)) == -1)
> - return (-1);
> - wlen += r; len -= r;
> - free(pdata);
> -
> - switch (aid) {
> - case AID_INET:
> - nexthop = up_get_nexthop(peer, state);
> - if ((r = attr_write(buf + wlen, len, ATTR_WELL_KNOWN,
> - ATTR_NEXTHOP, &nexthop, 4)) == -1)
> - return (-1);
> - wlen += r; len -= r;
> - break;
> - default:
> - break;
> - }
> -
> - /*
> - * The old MED from other peers MUST not be announced to others
> - * unless the MED is originating from us or the peer is an IBGP one.
> - * Only exception are routers with "transparent-as yes" set.
> - */
> - if (asp->flags & F_ATTR_MED && (!peer->conf.ebgp ||
> - asp->flags & F_ATTR_MED_ANNOUNCE ||
> - peer->conf.flags & PEERFLAG_TRANS_AS)) {
> - tmp32 = htonl(asp->med);
> - if ((r = attr_write(buf + wlen, len, ATTR_OPTIONAL,
> - ATTR_MED, &tmp32, 4)) == -1)
> - return (-1);
> - wlen += r; len -= r;
> - }
> -
> - if (!peer->conf.ebgp) {
> - /* local preference, only valid for ibgp */
> - tmp32 = htonl(asp->lpref);
> - if ((r = attr_write(buf + wlen, len, ATTR_WELL_KNOWN,
> - ATTR_LOCALPREF, &tmp32, 4)) == -1)
> - return (-1);
> - wlen += r; len -= r;
> - }
> -
> - /*
> - * dump all other path attributes. Following rules apply:
> - * 1. well-known attrs: ATTR_ATOMIC_AGGREGATE and ATTR_AGGREGATOR
> - * pass unmodified (enforce flags to correct values)
> - * Actually ATTR_AGGREGATOR may be deflated for OLD 2-byte peers.
> - * 2. non-transitive attrs: don't re-announce to ebgp peers
> - * 3. transitive known attrs: announce unmodified
> - * 4. transitive unknown attrs: set partial bit and re-announce
> - */
> - for (l = 0; l < asp->others_len; l++) {
> - if ((oa = asp->others[l]) == NULL)
> + u_int8_t oalen = 0, type;
> +
> + if (asp->others_len > 0)
> + oa = asp->others[oalen++];
> +
> + /* dump attributes in ascending order */
> + for (type = ATTR_ORIGIN; type < 255; type++) {
> + r = 0;
> +
> + while (oa && oa->type < type) {
> + if (oalen < asp->others_len)
> + oa = asp->others[oalen++];
trailing tab
> + else
> + oa = NULL;
> + }
> +
> + switch (type) {
> + /*
> + * Attributes stored in rde_aspath
> + */
> + case ATTR_ORIGIN:
> + if ((r = attr_write(buf + wlen, len, ATTR_WELL_KNOWN,
> + ATTR_ORIGIN, &asp->origin, 1)) == -1)
> + return (-1);
> break;
> - switch (oa->type) {
> + case ATTR_ASPATH:
> + if (!peer->conf.ebgp ||
> + peer->conf.flags & PEERFLAG_TRANS_AS)
> + pdata = aspath_prepend(asp->aspath,
> + peer->conf.local_as, 0, &plen);
> + else
> + pdata = aspath_prepend(asp->aspath,
> + peer->conf.local_as, 1, &plen);
> +
> + if (!rde_as4byte(peer))
> + pdata = aspath_deflate(pdata, &plen,
> + &neednewpath);
> +
> + if ((r = attr_write(buf + wlen, len, ATTR_WELL_KNOWN,
> + ATTR_ASPATH, pdata, plen)) == -1)
> + return (-1);
> + free(pdata);
> + break;
> + case ATTR_NEXTHOP:
> + switch (aid) {
> + case AID_INET:
> + nexthop = up_get_nexthop(peer, state);
> + if ((r = attr_write(buf + wlen, len,
> + ATTR_WELL_KNOWN, ATTR_NEXTHOP, &nexthop,
> + 4)) == -1)
> + return (-1);
> + break;
> + default:
> + break;
> + }
> + break;
> + case ATTR_MED:
> + /*
> + * The old MED from other peers MUST not be announced
> + * to others unless the MED is originating from us or
> + * the peer is an IBGP one. Only exception are routers
> + * with "transparent-as yes" set.
> + */
> + if (asp->flags & F_ATTR_MED && (!peer->conf.ebgp ||
> + asp->flags & F_ATTR_MED_ANNOUNCE ||
> + peer->conf.flags & PEERFLAG_TRANS_AS)) {
> + tmp32 = htonl(asp->med);
> + if ((r = attr_write(buf + wlen, len,
> + ATTR_OPTIONAL, ATTR_MED, &tmp32, 4)) == -1)
> + return (-1);
> + }
> + break;
> + case ATTR_LOCALPREF:
> + if (!peer->conf.ebgp) {
> + /* local preference, only valid for ibgp */
> + tmp32 = htonl(asp->lpref);
> + if ((r = attr_write(buf + wlen, len,
> + ATTR_WELL_KNOWN, ATTR_LOCALPREF, &tmp32,
> + 4)) == -1)
> + return (-1);
> + }
> + break;
> + /*
> + * multiprotocol attributes are handled elsewhere
> + */
> + case ATTR_MP_REACH_NLRI:
> + case ATTR_MP_UNREACH_NLRI:
> + break;
> + /*
> + * NEW to OLD conversion when sending stuff to a 2byte AS peer
> + */
> + case ATTR_AS4_PATH:
> + if (neednewpath) {
> + if (!peer->conf.ebgp ||
> + peer->conf.flags & PEERFLAG_TRANS_AS)
> + pdata = aspath_prepend(asp->aspath,
> + peer->conf.local_as, 0, &plen);
> + else
> + pdata = aspath_prepend(asp->aspath,
> + peer->conf.local_as, 1, &plen);
> + flags = ATTR_OPTIONAL|ATTR_TRANSITIVE;
> + if (!(asp->flags & F_PREFIX_ANNOUNCED))
> + flags |= ATTR_PARTIAL;
> + if (plen == 0)
> + r = 0;
> + else if ((r = attr_write(buf + wlen, len, flags,
> + ATTR_AS4_PATH, pdata, plen)) == -1)
> + return (-1);
> + free(pdata);
> + }
> + break;
> + case ATTR_AS4_AGGREGATOR:
> + if (newaggr) {
> + flags = ATTR_OPTIONAL|ATTR_TRANSITIVE;
> + if (!(asp->flags & F_PREFIX_ANNOUNCED))
> + flags |= ATTR_PARTIAL;
> + if ((r = attr_write(buf + wlen, len, flags,
> + ATTR_AS4_AGGREGATOR, newaggr->data,
> + newaggr->len)) == -1)
> + return (-1);
> + }
> + break;
> + /*
> + * dump all other path attributes. Following rules apply:
> + * 1. well-known attrs: ATTR_ATOMIC_AGGREGATE and
> + * ATTR_AGGREGATOR pass unmodified (enforce flags
> + * to correct values). Actually ATTR_AGGREGATOR may be
> + * deflated for OLD 2-byte peers.
> + * 2. non-transitive attrs: don't re-announce to ebgp peers
> + * 3. transitive known attrs: announce unmodified
> + * 4. transitive unknown attrs: set partial bit and re-announce
> + */
> case ATTR_ATOMIC_AGGREGATE:
> + if (oa == NULL || oa->type != type)
> + break;
> if ((r = attr_write(buf + wlen, len,
> ATTR_WELL_KNOWN, ATTR_ATOMIC_AGGREGATE,
> NULL, 0)) == -1)
> return (-1);
> break;
> case ATTR_AGGREGATOR:
> + if (oa == NULL || oa->type != type)
> + break;
> if (!rde_as4byte(peer)) {
> /* need to deflate the aggregator */
> u_int8_t t[6];
> @@ -417,6 +476,8 @@ up_generate_attr(u_char *buf, int len, s
> case ATTR_ORIGINATOR_ID:
> case ATTR_CLUSTER_LIST:
> case ATTR_LARGE_COMMUNITIES:
> + if (oa == NULL || oa->type != type)
> + break;
> if ((!(oa->flags & ATTR_TRANSITIVE)) &&
> peer->conf.ebgp) {
> r = 0;
> @@ -427,8 +488,13 @@ up_generate_attr(u_char *buf, int len, s
> return (-1);
> break;
> case ATTR_EXT_COMMUNITIES:
> + if (oa == NULL || oa->type != type)
> + break;
> /* handle (non-)transitive extended communities */
> if (peer->conf.ebgp) {
> + u_char *ndata;
> + u_int16_t nlen;
> +
> ndata = community_ext_delete_non_trans(oa->data,
> oa->len, &nlen);
>
> @@ -440,10 +506,8 @@ up_generate_attr(u_char *buf, int len, s
> return (-1);
> }
> free(ndata);
> - } else {
> - /* everything got removed */
> - r = 0;
> - }
> + }
trailing space
> + /* else everything got removed */
> } else {
> if ((r = attr_write(buf + wlen, len, oa->flags,
> oa->type, oa->data, oa->len)) == -1)
> @@ -451,6 +515,9 @@ up_generate_attr(u_char *buf, int len, s
> }
> break;
> default:
> + if (oa == NULL || oa->type != type)
> + break;
> +
> /* unknown attribute */
> if (!(oa->flags & ATTR_TRANSITIVE)) {
> /*
> @@ -459,7 +526,6 @@ up_generate_attr(u_char *buf, int len, s
> * attributes must be quietly ignored and
> * not passed along to other BGP peers.
> */
> - r = 0;
> break;
> }
> if ((r = attr_write(buf + wlen, len,
> @@ -468,36 +534,6 @@ up_generate_attr(u_char *buf, int len, s
> return (-1);
> break;
> }
> - wlen += r; len -= r;
> - }
> -
> - /* NEW to OLD conversion when going sending stuff to a 2byte AS peer */
> - if (neednewpath) {
> - if (!peer->conf.ebgp ||
> - peer->conf.flags & PEERFLAG_TRANS_AS)
> - pdata = aspath_prepend(asp->aspath, peer->conf.local_as,
> - 0, &plen);
> - else
> - pdata = aspath_prepend(asp->aspath, peer->conf.local_as,
> - 1, &plen);
> - flags = ATTR_OPTIONAL|ATTR_TRANSITIVE;
> - if (!(asp->flags & F_PREFIX_ANNOUNCED))
> - flags |= ATTR_PARTIAL;
> - if (plen == 0)
> - r = 0;
> - else if ((r = attr_write(buf + wlen, len, flags,
> - ATTR_AS4_PATH, pdata, plen)) == -1)
> - return (-1);
> - wlen += r; len -= r;
> - free(pdata);
> - }
> - if (newaggr) {
> - flags = ATTR_OPTIONAL|ATTR_TRANSITIVE;
> - if (!(asp->flags & F_PREFIX_ANNOUNCED))
> - flags |= ATTR_PARTIAL;
> - if ((r = attr_write(buf + wlen, len, flags,
> - ATTR_AS4_AGGREGATOR, newaggr->data, newaggr->len)) == -1)
> - return (-1);
> wlen += r; len -= r;
> }
>
>
