On Mon, Jul 09, 2018 at 05:47:11PM +0200, Claudio Jeker wrote:
> Similar to the rde_filter code there is no need to path_get() the aspath
> used in rde_update_dispatch(). Also makes the code a bit easier since the
> cleanup can be done all the time.
>
looks good, compiles fine, runs well, OK denis@
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.386
> diff -u -p -r1.386 rde.c
> --- rde.c 9 Jul 2018 14:44:02 -0000 1.386
> +++ rde.c 9 Jul 2018 14:50:21 -0000
> @@ -938,10 +938,10 @@ rde_dispatch_imsg_parent(struct imsgbuf
> int
> rde_update_dispatch(struct imsg *imsg)
> {
> + struct rde_aspath asp;
> struct bgpd_addr prefix;
> struct mpattr mpa;
> struct rde_peer *peer;
> - struct rde_aspath *asp = NULL;
> u_char *p, *mpp = NULL;
> int error = -1, pos = 0;
> u_int16_t afi, len, mplen;
> @@ -986,11 +986,11 @@ rde_update_dispatch(struct imsg *imsg)
> imsg->hdr.len - IMSG_HEADER_SIZE - 4 - withdrawn_len - attrpath_len;
> bzero(&mpa, sizeof(mpa));
>
> + path_prep(&asp);
> if (attrpath_len != 0) { /* 0 = no NLRI information in this message */
> /* parse path attributes */
> - asp = path_get();
> while (len > 0) {
> - if ((pos = rde_attr_parse(p, len, peer, asp,
> + if ((pos = rde_attr_parse(p, len, peer, &asp,
> &mpa)) < 0)
> goto done;
> p += pos;
> @@ -998,19 +998,19 @@ rde_update_dispatch(struct imsg *imsg)
> }
>
> /* check for missing but necessary attributes */
> - if ((subtype = rde_attr_missing(asp, peer->conf.ebgp,
> + if ((subtype = rde_attr_missing(&asp, peer->conf.ebgp,
> nlri_len))) {
> rde_update_err(peer, ERR_UPDATE, ERR_UPD_MISSNG_WK_ATTR,
> &subtype, sizeof(u_int8_t));
> goto done;
> }
>
> - rde_as4byte_fixup(peer, asp);
> + rde_as4byte_fixup(peer, &asp);
>
> /* enforce remote AS if requested */
> - if (asp->flags & F_ATTR_ASPATH &&
> + if (asp.flags & F_ATTR_ASPATH &&
> peer->conf.enforce_as == ENFORCE_AS_ON) {
> - fas = aspath_neighbor(asp->aspath);
> + fas = aspath_neighbor(asp.aspath);
> if (peer->conf.remote_as != fas) {
> log_peer_warnx(&peer->conf, "bad path, "
> "starting with %s, "
> @@ -1021,7 +1021,7 @@ rde_update_dispatch(struct imsg *imsg)
> }
> }
>
> - rde_reflector(peer, asp);
> + rde_reflector(peer, &asp);
> }
>
> p = imsg->data;
> @@ -1103,7 +1103,7 @@ rde_update_dispatch(struct imsg *imsg)
> goto done;
> }
>
> - if ((asp->flags & ~F_ATTR_MP_UNREACH) == 0 && mplen == 0) {
> + if ((asp.flags & ~F_ATTR_MP_UNREACH) == 0 && mplen == 0) {
> /* EoR marker */
> peer_recv_eor(peer, aid);
> }
> @@ -1166,7 +1166,7 @@ rde_update_dispatch(struct imsg *imsg)
> break;
> }
>
> - if ((asp->flags & ~F_ATTR_MP_UNREACH) == 0) {
> + if ((asp.flags & ~F_ATTR_MP_UNREACH) == 0) {
> error = 0;
> goto done;
> }
> @@ -1178,8 +1178,8 @@ rde_update_dispatch(struct imsg *imsg)
> /* aspath needs to be loop free nota bene this is not a hard error */
> if (peer->conf.ebgp &&
> peer->conf.enforce_local_as == ENFORCE_AS_ON &&
> - !aspath_loopfree(asp->aspath, peer->conf.local_as))
> - asp->flags |= F_ATTR_LOOP;
> + !aspath_loopfree(asp.aspath, peer->conf.local_as))
> + asp.flags |= F_ATTR_LOOP;
>
> /* parse nlri prefix */
> while (nlri_len > 0) {
> @@ -1208,7 +1208,7 @@ rde_update_dispatch(struct imsg *imsg)
> goto done;
> }
>
> - if (rde_update_update(peer, asp, &prefix, prefixlen) == -1)
> + if (rde_update_update(peer, &asp, &prefix, prefixlen) == -1)
> goto done;
>
> }
> @@ -1244,11 +1244,11 @@ rde_update_dispatch(struct imsg *imsg)
> * this works because asp is not linked.
> * But first unlock the previously locked nexthop.
> */
> - if (asp->nexthop) {
> - (void)nexthop_put(asp->nexthop);
> - asp->nexthop = NULL;
> + if (asp.nexthop) {
> + (void)nexthop_put(asp.nexthop);
> + asp.nexthop = NULL;
> }
> - if ((pos = rde_get_mp_nexthop(mpp, mplen, aid, asp)) == -1) {
> + if ((pos = rde_get_mp_nexthop(mpp, mplen, aid, &asp)) == -1) {
> log_peer_warnx(&peer->conf, "bad nlri prefix");
> rde_update_err(peer, ERR_UPDATE, ERR_UPD_OPTATTR,
> mpa.reach, mpa.reach_len);
> @@ -1279,7 +1279,7 @@ rde_update_dispatch(struct imsg *imsg)
> mpp += pos;
> mplen -= pos;
>
> - if (rde_update_update(peer, asp, &prefix,
> + if (rde_update_update(peer, &asp, &prefix,
> prefixlen) == -1)
> goto done;
> }
> @@ -1305,7 +1305,7 @@ rde_update_dispatch(struct imsg *imsg)
> mpp += pos;
> mplen -= pos;
>
> - if (rde_update_update(peer, asp, &prefix,
> + if (rde_update_update(peer, &asp, &prefix,
> prefixlen) == -1)
> goto done;
> }
> @@ -1317,10 +1317,7 @@ rde_update_dispatch(struct imsg *imsg)
> }
>
> done:
> - if (attrpath_len != 0) {
> - /* free allocated attribute memory that is no longer used */
> - path_put(asp);
> - }
> + path_clean(&asp);
>
> return (error);
> }
>
