Stuart Henderson([email protected]) on 2012.07.01 13:39:48 +0100:
> On 2012/07/01 10:54, Florian Obser wrote:
> > Hi,
> > after a very brief reading of ospfd sources I think I understand
> > why it touches the route on the external interface. (This might be
> > entirely wrong, but I think it does its spf dance and then puts the
> > best routes into the fib wether they changed or not.)
> >
> > In any case since I understand ospfd a lot less then bgpd so I decided
> > to change bgpd by figuring out if the nexthop actually changed or not.
> >
> > There is already this comment in prefix_updateall:
> > /*
> > * The state of the nexthop did not change. The only
> > * thing that may have changed is the true_nexthop
> > * or other internal infos. This will not change
> > * the routing decision so shortcut here.
> > */
> >
> > The following patch compares true_nexthop, flags, etc with the old
> > values and skips prefix_updateall if noting has changed.
> >
> > This is either obviously correct or horribly wrong - but it fixes the
> > memory consumption in rde :)
>
> I don't know enough about the internal design to know whether it's
> right but it makes enough sense that I'm going to try running one
> machine with it anyway ;)
Hi,
i am running this on 3 machines now and it fixes the memory problem i was
seeing.
/Benno
> It does seem to me that ospfd should probably not update the fib
> in this situation however whatever happens there, this is a fairly
> cheap check which saves bgpd doing extra work at a time when it's
> already heavily loaded, so if it's not *wrong* then it seems useful
> to have. At the time when bgpd is handling bulk route messages it's
> fairly likely to suffer an overflow on the route socket so cutting
> processing time for these is good.
> ? .todo
> ? kroute.c-desync
> ? leakfinder.shar
> Index: rde_rib.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
> retrieving revision 1.133
> diff -u -p -u -1 -3 -r1.133 rde_rib.c
> --- rde_rib.c 1 Jul 2012 11:55:13 -0000 1.133
> +++ rde_rib.c 1 Jul 2012 12:21:12 -0000
> @@ -1046,63 +1046,75 @@ nexthop_shutdown(void)
> (void)nexthop_delete(nh);
> }
> if (!LIST_EMPTY(&nexthoptable.nexthop_hashtbl[i]))
> log_warnx("nexthop_shutdown: non-free table");
> }
>
> free(nexthoptable.nexthop_hashtbl);
> }
>
> void
> nexthop_update(struct kroute_nexthop *msg)
> {
> struct nexthop *nh;
> + struct bgpd_addr oldtrue_nh, oldnh_net;
> struct rde_aspath *asp;
> enum nexthop_state oldstate;
> + u_int8_t oldflags, oldnh_netlen;
>
> nh = nexthop_lookup(&msg->nexthop);
> if (nh == NULL) {
> log_warnx("nexthop_update: non-existent nexthop %s",
> log_addr(&msg->nexthop));
> return;
> }
>
> oldstate = nh->state;
> + oldflags = nh->flags;
> + oldnh_netlen = nh->nexthop_netlen;
> + memcpy(&oldtrue_nh, &nh->true_nexthop, sizeof(oldtrue_nh));
> + memcpy(&oldnh_net, &nh->nexthop_net, sizeof(oldnh_net));
> if (msg->valid)
> nh->state = NEXTHOP_REACH;
> else
> nh->state = NEXTHOP_UNREACH;
>
> if (msg->connected) {
> nh->flags |= NEXTHOP_CONNECTED;
> memcpy(&nh->true_nexthop, &nh->exit_nexthop,
> sizeof(nh->true_nexthop));
> } else
> memcpy(&nh->true_nexthop, &msg->gateway,
> sizeof(nh->true_nexthop));
>
> memcpy(&nh->nexthop_net, &msg->net,
> sizeof(nh->nexthop_net));
> nh->nexthop_netlen = msg->netlen;
>
> if (nexthop_delete(nh))
> /* nexthop no longer used */
> return;
>
> if (rde_noevaluate())
> /*
> * if the decision process is turned off there is no need
> * for the aspath list walk.
> */
> + return;
> +
> + if (oldstate == nh->state && oldflags == nh->flags &&
> + memcmp(&oldtrue_nh, &nh->true_nexthop, sizeof(oldtrue_nh)) == 0 &&
> + memcmp(&oldnh_net, &nh->nexthop_net, sizeof(oldnh_net)) == 0)
> + /* nexthop didn't change */
> return;
>
> LIST_FOREACH(asp, &nh->path_h, nexthop_l) {
> prefix_updateall(asp, nh->state, oldstate);
> }
> }
>
> void
> nexthop_modify(struct rde_aspath *asp, struct bgpd_addr *nexthop,
> enum action_types type, u_int8_t aid)
> {
> struct nexthop *nh;
>
--