0
0
mirror of https://gitlab.nic.cz/labs/bird.git synced 2025-03-14 10:27:03 +00:00

Nest: Removed rte_get_temp(), rte_update() now does its own copy.

This changes behaviour of rte_update() and rte_update2().

Formerly, the caller of rte_update2() took care of caching rta and
allocating rte; then rte_update2() freed the structures if it didn't use them.

Now rte_update2() does the permanent/global copies of rte+rta itself.
Allocation of rte+rta passed to rte_update2() is considered temporary
and the caller is also responsible to free these structures properly.
This is true for any kind of allocation: rte_update2() will always
allocate its own copy of the data passed (either really or just by
raising usecount in rta cache).

It is now possible (and in many protocols now used) to allocate rte+rta
on stack and pass them to rte_update(). It will deal with them correctly.
This commit is contained in:
Jan Maria Matejka 2018-02-09 15:49:34 +01:00
parent 9a5557ea8f
commit 54ac17dc13
14 changed files with 152 additions and 141 deletions

View File

@ -306,6 +306,7 @@ void rte_free(rte *);
rte *rte_do_cow(rte *);
static inline rte * rte_cow(rte *r) { return (r->flags & REF_COW) ? rte_do_cow(r) : r; }
rte *rte_cow_rta(rte *r, linpool *lp);
rte *rte_clone(rte *);
void rt_dump(rtable *);
void rt_dump_all(void);
int rt_feed_channel(struct channel *c);
@ -617,6 +618,7 @@ static inline size_t rta_size(const rta *a) { return sizeof(rta) + sizeof(u32)*a
rta *rta_lookup(rta *); /* Get rta equivalent to this one, uc++ */
static inline int rta_is_cached(rta *r) { return r->aflags & RTAF_CACHED; }
static inline rta *rta_clone(rta *r) { r->uc++; return r; }
int rta_same(rta *x, rta *y);
void rta__free(rta *r);
static inline void rta_free(rta *r) { if (r && !--r->uc) rta__free(r); }
rta *rta_do_cow(rta *o, linpool *lp);

View File

@ -1062,7 +1062,7 @@ rta_hash(rta *a)
return mem_hash_value(&h) ^ nexthop_hash(&(a->nh)) ^ ea_hash(a->eattrs);
}
static inline int
int
rta_same(rta *x, rta *y)
{
return (x->src == y->src &&

View File

@ -71,9 +71,6 @@ dev_ifa_notify(struct proto *P, uint flags, struct ifa *ad)
}
else if (flags & IF_CHANGE_UP)
{
rta *a;
rte *e;
DBG("dev_if_notify: %s:%I going up\n", ad->iface->name, ad->ip);
if (cf->check_link && !(ad->iface->flags & IF_LINK_UP))
@ -90,10 +87,12 @@ dev_ifa_notify(struct proto *P, uint flags, struct ifa *ad)
.nh.iface = ad->iface,
};
a = rta_lookup(&a0);
e = rte_get_temp(a);
e->pflags = 0;
rte_update2(c, net, e, src);
rte e = {
.pflags = 0,
.attrs = &a0,
};
rte_update2(c, net, &e, src);
}
}

View File

@ -261,26 +261,6 @@ rte_find(net *net, struct rte_src *src)
return e;
}
/**
* rte_get_temp - get a temporary &rte
* @a: attributes to assign to the new route (a &rta; in case it's
* un-cached, rte_update() will create a cached copy automatically)
*
* Create a temporary &rte and bind it with the attributes @a.
* Also set route preference to the default preference set for
* the protocol.
*/
rte *
rte_get_temp(rta *a)
{
rte *e = sl_alloc(rte_slab);
e->attrs = a;
e->flags = 0;
e->pref = 0;
return e;
}
rte *
rte_do_cow(rte *r)
{
@ -292,6 +272,25 @@ rte_do_cow(rte *r)
return e;
}
/**
* rte_clone - do a complete copy of given rte
* @e: route to clone
*/
rte *
rte_clone(rte *r)
{
rte *e = sl_alloc(rte_slab);
memcpy(e, r, sizeof(rte));
if (rta_is_cached(r->attrs))
e->attrs = rta_clone(r->attrs);
else
e->attrs = rta_lookup(r->attrs);
return e;
}
/**
* rte_cow_rta - get a private writable copy of &rte with writable &rta
* @r: a route entry to be copied
@ -1274,11 +1273,8 @@ rte_unhide_dummy_routes(net *net, rte **dummy)
*
* This function is called by the routing protocols whenever they discover
* a new route or wish to update/remove an existing route. The right announcement
* sequence is to build route attributes first (either un-cached with @aflags set
* to zero or a cached one using rta_lookup(); in this case please note that
* you need to increase the use count of the attributes yourself by calling
* rta_clone()), call rte_get_temp() to obtain a temporary &rte, fill in all
* the appropriate data and finally submit the new &rte by calling rte_update().
* sequence is to build route attributes first, allocate a temporary &rte, fill in all
* the appropriate data and submit the new &rte by calling rte_update().
*
* @src specifies the protocol that originally created the route and the meaning
* of protocol-dependent data of @new. If @new is not %NULL, @src have to be the
@ -1367,9 +1363,8 @@ rte_update2(struct channel *c, const net_addr *n, rte *new, struct rte_src *src)
src->proto->store_tmp_attrs(new);
}
}
if (!rta_is_cached(new->attrs)) /* Need to copy attributes */
new->attrs = rta_lookup(new->attrs);
new->flags |= REF_COW;
new = rte_clone(new);
}
else
{
@ -1391,7 +1386,6 @@ rte_update2(struct channel *c, const net_addr *n, rte *new, struct rte_src *src)
return;
drop:
rte_free(new);
new = NULL;
goto recalc;
}

View File

@ -640,15 +640,18 @@ babel_announce_rte(struct babel_proto *p, struct babel_entry *e)
.nh.iface = r->neigh->ifa->iface,
};
rta *a = rta_lookup(&a0);
rte *rte = rte_get_temp(a);
rte->u.babel.seqno = r->seqno;
rte->u.babel.metric = r->metric;
rte->u.babel.router_id = r->router_id;
rte->pflags = 0;
rte rte = {
.attrs = &a0,
.u.babel = {
.seqno = r->seqno,
.metric = r->metric,
.router_id = r->router_id,
},
.pflags = 0,
};
e->unreachable = 0;
rte_update2(c, e->n.addr, rte, p->p.main_source);
rte_update2(c, e->n.addr, &rte, p->p.main_source);
}
else if (e->valid && (e->router_id != p->router_id))
{
@ -660,14 +663,15 @@ babel_announce_rte(struct babel_proto *p, struct babel_entry *e)
.dest = RTD_UNREACHABLE,
};
rta *a = rta_lookup(&a0);
rte *rte = rte_get_temp(a);
memset(&rte->u.babel, 0, sizeof(rte->u.babel));
rte->pflags = 0;
rte->pref = 1;
rte rte = {
.attrs = &a0,
.u.babel = {},
.pflags = 0,
.pref = 1,
};
e->unreachable = 1;
rte_update2(c, e->n.addr, rte, p->p.main_source);
rte_update2(c, e->n.addr, &rte, p->p.main_source);
}
else
{

View File

@ -1190,17 +1190,18 @@ bgp_rte_update(struct bgp_parse_state *s, net_addr *n, u32 path_id, rta *a0)
/* Workaround for rta_lookup() breaking eattrs */
ea_list *ea = a0->eattrs;
s->cached_rta = rta_lookup(a0);
s->cached_rta = rta_lookup(a0); /* This rta use is finally freed in bgp_decode_nlri() */
a0->eattrs = ea;
}
rta *a = rta_clone(s->cached_rta);
rte *e = rte_get_temp(a);
rte e = {
.attrs = s->cached_rta,
.pflags = 0,
.u.bgp.suppressed = 0,
.u.bgp.stale = -1,
};
e->pflags = 0;
e->u.bgp.suppressed = 0;
e->u.bgp.stale = -1;
rte_update2(&s->channel->c, n, e, s->last_src);
rte_update2(&s->channel->c, n, &e, s->last_src);
}
static void
@ -2285,7 +2286,7 @@ bgp_decode_nlri(struct bgp_parse_state *s, u32 afi, byte *nlri, uint len, ea_lis
c->desc->decode_nlri(s, nlri, len, a);
rta_free(s->cached_rta);
rta_free(s->cached_rta); /* Freeing the rta used in bgp_rte_update */
s->cached_rta = NULL;
}

View File

@ -2013,19 +2013,25 @@ again1:
if (reload || ort_changed(nf, &a0))
{
rta *a = rta_lookup(&a0);
rte *e = rte_get_temp(a);
rta_free(nf->old_rta);
nf->old_rta = rta_clone(a);
e->u.ospf.metric1 = nf->old_metric1 = nf->n.metric1;
e->u.ospf.metric2 = nf->old_metric2 = nf->n.metric2;
e->u.ospf.tag = nf->old_tag = nf->n.tag;
e->u.ospf.router_id = nf->old_rid = nf->n.rid;
e->pflags = 0;
rte e = {
.attrs = a,
.u.ospf = {
.metric1 = nf->old_metric1 = nf->n.metric1,
.metric2 = nf->old_metric2 = nf->n.metric2,
.tag = nf->old_tag = nf->n.tag,
.router_id = nf->old_rid = nf->n.rid,
},
.pflags = 0,
};
DBG("Mod rte type %d - %N via %I on iface %s, met %d\n",
a0.source, nf->fn.addr, a0.gw, a0.iface ? a0.iface->name : "(none)", nf->n.metric1);
rte_update(&p->p, nf->fn.addr, e);
rte_update(&p->p, nf->fn.addr, &e);
rta_free(a);
}
}
else if (nf->old_rta)

View File

@ -50,6 +50,7 @@ pipe_rt_notify(struct proto *P, struct channel *src_ch, net *n, rte *new, rte *o
struct channel *dst = (src_ch == p->pri) ? p->sec : p->pri;
struct rte_src *src;
rte ee = {};
rte *e;
rta *a;
@ -70,8 +71,10 @@ pipe_rt_notify(struct proto *P, struct channel *src_ch, net *n, rte *new, rte *o
a->aflags = 0;
a->hostentry = NULL;
e = rte_get_temp(a);
e = ⅇ
e->pflags = 0;
e->attrs = a;
/* Copy protocol specific embedded attributes. */
memcpy(&(e->u), &(new->u), sizeof(e->u));

View File

@ -187,16 +187,17 @@ rip_announce_rte(struct rip_proto *p, struct rip_entry *en)
a0.nh.iface = rt->from->nbr->iface;
}
rta *a = rta_lookup(&a0);
rte *e = rte_get_temp(a);
rte e = {
.attrs = &a0,
.u.rip = {
.from = a0.nh.iface,
.metric = rt_metric,
.tag = rt_tag,
},
.pflags = 0,
};
e->u.rip.from = a0.nh.iface;
e->u.rip.metric = rt_metric;
e->u.rip.tag = rt_tag;
e->pflags = 0;
rte_update(&p->p, en->n.addr, e);
rte_update(&p->p, en->n.addr, &e);
}
else
{

View File

@ -126,13 +126,13 @@ rpki_table_add_roa(struct rpki_cache *cache, struct channel *channel, const net_
.scope = SCOPE_UNIVERSE,
.dest = RTD_NONE,
};
rte e = {
.attrs = &a0,
.pflags = 0,
};
rta *a = rta_lookup(&a0);
rte *e = rte_get_temp(a);
e->pflags = 0;
rte_update2(channel, &pfxr->n, e, a0.src);
rte_update2(channel, &pfxr->n, &e, a0.src);
}
void

View File

@ -98,14 +98,17 @@ static_announce_rte(struct static_proto *p, struct static_route *r)
if (r->state == SRS_CLEAN)
return;
/* We skip rta_lookup() here */
rte *e = rte_get_temp(a);
e->pflags = 0;
rte e = {
.attrs = a,
.pflags = 0
};
rte *ep = &e;
if (r->cmds)
f_eval_rte(r->cmds, &e, static_lp);
f_eval_rte(r->cmds, &ep, static_lp);
rte_update(&p->p, r->net, e);
rte_update(&p->p, r->net, ep);
r->state = SRS_CLEAN;
if (r->cmds)

View File

@ -371,7 +371,6 @@ krt_read_route(struct ks_msg *msg, struct krt_proto *p, int scan)
/* p is NULL iff KRT_SHARED_SOCKET and !scan */
int ipv6;
rte *e;
net *net;
sockaddr dst, gate, mask;
ip_addr idst, igate, imask;
@ -497,6 +496,15 @@ krt_read_route(struct ks_msg *msg, struct krt_proto *p, int scan)
.scope = SCOPE_UNIVERSE,
};
rte e = {
.attrs = &a,
.net = net,
.u.krt = {
.src = src,
.proto = src2,
},
};
/* reject/blackhole routes have also set RTF_GATEWAY,
we wil check them first. */
@ -547,18 +555,10 @@ krt_read_route(struct ks_msg *msg, struct krt_proto *p, int scan)
}
done:
e = rte_get_temp(&a);
e->net = net;
e->u.krt.src = src;
e->u.krt.proto = src2;
e->u.krt.seen = 0;
e->u.krt.best = 0;
e->u.krt.metric = 0;
if (scan)
krt_got_route(p, e);
krt_got_route(p, &e);
else
krt_got_route_async(p, e, new);
krt_got_route_async(p, &e, new);
}
static void

View File

@ -1413,18 +1413,22 @@ nl_mergable_route(struct nl_parse_state *s, net *net, struct krt_proto *p, uint
static void
nl_announce_route(struct nl_parse_state *s)
{
rte *e = rte_get_temp(s->attrs);
e->net = s->net;
e->u.krt.src = s->krt_src;
e->u.krt.proto = s->krt_proto;
e->u.krt.seen = 0;
e->u.krt.best = 0;
e->u.krt.metric = s->krt_metric;
rte e = {
.attrs = s->attrs,
.net = s->net,
.u.krt = {
.src = s->krt_src,
.proto = s->krt_proto,
.seen = 0,
.best = 0,
.metric = s->krt_metric,
},
};
if (s->scan)
krt_got_route(s->proto, e);
krt_got_route(s->proto, &e);
else
krt_got_route_async(s->proto, e, s->new);
krt_got_route_async(s->proto, &e, s->new);
s->net = NULL;
s->attrs = NULL;

View File

@ -286,7 +286,7 @@ krt_same_key(rte *a, rte *b)
static inline int
krt_uptodate(rte *a, rte *b)
{
if (a->attrs != b->attrs)
if (!rta_same(a->attrs, b->attrs))
return 0;
if (a->u.krt.proto != b->u.krt.proto)
@ -298,12 +298,7 @@ krt_uptodate(rte *a, rte *b)
static void
krt_learn_announce_update(struct krt_proto *p, rte *e)
{
net *n = e->net;
rta *aa = rta_clone(e->attrs);
rte *ee = rte_get_temp(aa);
ee->pflags = 0;
ee->u.krt = e->u.krt;
rte_update(&p->p, n->n.addr, ee);
rte_update(&p->p, e->net->n.addr, e);
}
static void
@ -312,7 +307,8 @@ krt_learn_announce_delete(struct krt_proto *p, net *n)
rte_update(&p->p, n->n.addr, NULL);
}
/* Called when alien route is discovered during scan */
/* Called when alien route is discovered during scan.
* The route is a temporary rte. */
static void
krt_learn_scan(struct krt_proto *p, rte *e)
{
@ -320,17 +316,15 @@ krt_learn_scan(struct krt_proto *p, rte *e)
net *n = net_get(&p->krt_table, n0->n.addr);
rte *m, **mm;
e->attrs = rta_lookup(e->attrs);
for(mm=&n->routes; m = *mm; mm=&m->next)
if (krt_same_key(m, e))
break;
if (m)
if (m) /* Route found */
{
if (krt_uptodate(m, e))
{
krt_trace_in_rl(&rl_alien, p, e, "[alien] seen");
rte_free(e);
m->u.krt.seen = 1;
}
else
@ -343,11 +337,13 @@ krt_learn_scan(struct krt_proto *p, rte *e)
}
else
krt_trace_in(p, e, "[alien] created");
if (!m)
if (!m) /* Route created or updated -> inserting the new route. */
{
e->next = n->routes;
n->routes = e;
e->u.krt.seen = 1;
rte *ee = rte_clone(e);
ee->next = n->routes;
n->routes = ee;
ee->u.krt.seen = 1;
}
}
@ -426,6 +422,11 @@ again:
p->reload = 0;
}
/*
* This is called when the low-level code learns a route asynchronously.
* The given route is considered temporary.
*/
static void
krt_learn_async(struct krt_proto *p, rte *e, int new)
{
@ -433,8 +434,6 @@ krt_learn_async(struct krt_proto *p, rte *e, int new)
net *n = net_get(&p->krt_table, n0->n.addr);
rte *g, **gg, *best, **bestp, *old_best;
e->attrs = rta_lookup(e->attrs);
old_best = n->routes;
for(gg=&n->routes; g = *gg; gg = &g->next)
if (krt_same_key(g, e))
@ -446,7 +445,6 @@ krt_learn_async(struct krt_proto *p, rte *e, int new)
if (krt_uptodate(g, e))
{
krt_trace_in(p, e, "[alien async] same");
rte_free(e);
return;
}
krt_trace_in(p, e, "[alien async] updated");
@ -456,20 +454,19 @@ krt_learn_async(struct krt_proto *p, rte *e, int new)
else
krt_trace_in(p, e, "[alien async] created");
e = rte_clone(e);
e->next = n->routes;
n->routes = e;
}
else if (!g)
{
krt_trace_in(p, e, "[alien async] delete failed");
rte_free(e);
return;
}
else
{
krt_trace_in(p, e, "[alien async] removed");
*gg = g->next;
rte_free(e);
rte_free(g);
}
best = n->routes;
@ -639,10 +636,7 @@ krt_got_route(struct krt_proto *p, rte *e)
if (KRT_CF->learn)
krt_learn_scan(p, e);
else
{
krt_trace_in_rl(&rl_alien, p, e, "[alien] ignored");
rte_free(e);
}
krt_trace_in_rl(&rl_alien, p, e, "[alien] ignored");
return;
}
#endif
@ -652,7 +646,6 @@ krt_got_route(struct krt_proto *p, rte *e)
{
/* Route to this destination was already seen. Strange, but it happens... */
krt_trace_in(p, e, "already seen");
rte_free(e);
return;
}
@ -691,15 +684,12 @@ krt_got_route(struct krt_proto *p, rte *e)
net->n.flags = (net->n.flags & ~KRF_VERDICT_MASK) | verdict;
if (verdict == KRF_UPDATE || verdict == KRF_DELETE)
{
/* Get a cached copy of attributes and temporarily link the route */
rta *a = e->attrs;
a->source = RTS_DUMMY;
e->attrs = rta_lookup(a);
e->next = net->routes;
net->routes = e;
/* Get a cached copy of route and temporarily link it */
e->attrs->source = RTS_DUMMY;
rte *ee = rte_clone(e);
ee->next = net->routes;
net->routes = ee;
}
else
rte_free(e);
}
static void
@ -776,6 +766,11 @@ krt_prune(struct krt_proto *p)
p->initialized = 1;
}
/*
* This is called when the low-level code gets a route asynchronously.
* The given route is considered temporary.
*/
void
krt_got_route_async(struct krt_proto *p, rte *e, int new)
{
@ -805,7 +800,6 @@ krt_got_route_async(struct krt_proto *p, rte *e, int new)
}
#endif
}
rte_free(e);
}
/*