This diff does nothing else than prep for the possibility to use
struct rde_aspath objects that sit on the stack instead of being
malloc()-ed. Currently a lot of copies are created when filter modify
aspath entries when in practice it is not needed.
The main changes are:
- path_copy() gets a second argument (dst, src)
- a new function path_prep() is introduced to initialize a aspath object
Additionally some const where added to the *_copy functions to make it
more obvious which is the source and target. Also the pftable_ref and
rtlabel_ref functions return now the id so you can do:
rttable_id = rttable_ref(another_id)
Which is nicer.
This does not yet reduce the copy overhead. Working on that next.
--
:wq Claudio
Index: bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.319
diff -u -p -r1.319 bgpd.h
--- bgpd.h 25 Jun 2018 14:28:33 -0000 1.319
+++ bgpd.h 29 Jun 2018 10:01:13 -0000
@@ -1110,11 +1110,11 @@ void rib_ref(u_int16_t);
u_int16_t rtlabel_name2id(const char *);
const char *rtlabel_id2name(u_int16_t);
void rtlabel_unref(u_int16_t);
-void rtlabel_ref(u_int16_t);
+u_int16_t rtlabel_ref(u_int16_t);
u_int16_t pftable_name2id(const char *);
const char *pftable_id2name(u_int16_t);
void pftable_unref(u_int16_t);
-void pftable_ref(u_int16_t);
+u_int16_t pftable_ref(u_int16_t);
/* parse.y */
int cmdline_symset(char *);
Index: name2id.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/name2id.c,v
retrieving revision 1.9
diff -u -p -r1.9 name2id.c
--- name2id.c 4 Jun 2009 04:46:42 -0000 1.9
+++ name2id.c 29 Jun 2018 10:03:48 -0000
@@ -41,7 +41,7 @@ TAILQ_HEAD(n2id_labels, n2id_label);
u_int16_t _name2id(struct n2id_labels *, const char *);
const char *_id2name(struct n2id_labels *, u_int16_t);
void _unref(struct n2id_labels *, u_int16_t);
-void _ref(struct n2id_labels *, u_int16_t);
+u_int16_t _ref(struct n2id_labels *, u_int16_t);
struct n2id_labels rt_labels = TAILQ_HEAD_INITIALIZER(rt_labels);
struct n2id_labels pftable_labels = TAILQ_HEAD_INITIALIZER(pftable_labels);
@@ -64,10 +64,10 @@ rtlabel_unref(u_int16_t id)
_unref(&rt_labels, id);
}
-void
+u_int16_t
rtlabel_ref(u_int16_t id)
{
- _ref(&rt_labels, id);
+ return (_ref(&rt_labels, id));
}
u_int16_t
@@ -88,10 +88,10 @@ pftable_unref(u_int16_t id)
_unref(&pftable_labels, id);
}
-void
+u_int16_t
pftable_ref(u_int16_t id)
{
- _ref(&pftable_labels, id);
+ return (_ref(&pftable_labels, id));
}
u_int16_t
@@ -180,17 +180,20 @@ _unref(struct n2id_labels *head, u_int16
}
}
-void
+u_int16_t
_ref(struct n2id_labels *head, u_int16_t id)
{
struct n2id_label *label;
if (id == 0)
- return;
+ return (0);
TAILQ_FOREACH(label, head, entry)
if (label->id == id) {
++label->ref;
- break;
+ return (id);
}
+
+ /* id not found, treat like no id */
+ return (0);
}
Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
retrieving revision 1.174
diff -u -p -r1.174 rde.h
--- rde.h 28 Jun 2018 09:54:48 -0000 1.174
+++ rde.h 29 Jun 2018 10:11:27 -0000
@@ -351,7 +351,7 @@ void attr_shutdown(void);
int attr_optadd(struct rde_aspath *, u_int8_t, u_int8_t,
void *, u_int16_t);
struct attr *attr_optget(const struct rde_aspath *, u_int8_t);
-void attr_copy(struct rde_aspath *, struct rde_aspath *);
+void attr_copy(struct rde_aspath *, const struct rde_aspath *);
int attr_compare(struct rde_aspath *, struct rde_aspath *);
void attr_freeall(struct rde_aspath *);
void attr_free(struct rde_aspath *, struct attr *);
@@ -473,7 +473,8 @@ void path_remove(struct rde_aspath *);
u_int32_t path_remove_stale(struct rde_aspath *, u_int8_t);
void path_destroy(struct rde_aspath *);
int path_empty(struct rde_aspath *);
-struct rde_aspath *path_copy(struct rde_aspath *);
+struct rde_aspath *path_copy(struct rde_aspath *, const struct rde_aspath *);
+struct rde_aspath *path_prep(struct rde_aspath *);
struct rde_aspath *path_get(void);
void path_put(struct rde_aspath *);
Index: rde_attr.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_attr.c,v
retrieving revision 1.101
diff -u -p -r1.101 rde_attr.c
--- rde_attr.c 2 Apr 2018 19:25:33 -0000 1.101
+++ rde_attr.c 29 Jun 2018 10:09:44 -0000
@@ -209,7 +209,7 @@ attr_optget(const struct rde_aspath *asp
}
void
-attr_copy(struct rde_aspath *t, struct rde_aspath *s)
+attr_copy(struct rde_aspath *t, const struct rde_aspath *s)
{
u_int8_t l;
Index: rde_filter.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_filter.c,v
retrieving revision 1.93
diff -u -p -r1.93 rde_filter.c
--- rde_filter.c 28 Jun 2018 09:54:48 -0000 1.93
+++ rde_filter.c 29 Jun 2018 10:13:31 -0000
@@ -305,8 +305,7 @@ rde_apply_set(struct filter_set_head *sh
/* FALLTHROUGH */
case ACTION_PFTABLE_ID:
pftable_unref(asp->pftableid);
- asp->pftableid = set->action.id;
- pftable_ref(asp->pftableid);
+ asp->pftableid = pftable_ref(set->action.id);
break;
case ACTION_RTLABEL:
/* convert the route label to an id for faster access */
@@ -315,8 +314,7 @@ rde_apply_set(struct filter_set_head *sh
/* FALLTHROUGH */
case ACTION_RTLABEL_ID:
rtlabel_unref(asp->rtlabelid);
- asp->rtlabelid = set->action.id;
- rtlabel_ref(asp->rtlabelid);
+ asp->rtlabelid = rtlabel_ref(set->action.id);
break;
case ACTION_SET_ORIGIN:
asp->origin = set->action.origin;
@@ -1023,7 +1021,7 @@ rde_filter(struct filter_head *rules, st
if (asp != NULL && new != NULL) {
/* asp may get modified so create a copy */
if (*new == NULL) {
- *new = path_copy(asp);
+ *new = path_copy(path_get(), asp);
/* ... and use the copy from now on */
asp = *new;
}
Index: rde_rib.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
retrieving revision 1.164
diff -u -p -r1.164 rde_rib.c
--- rde_rib.c 28 Jun 2018 08:55:56 -0000 1.164
+++ rde_rib.c 29 Jun 2018 10:23:28 -0000
@@ -420,7 +420,7 @@ path_update(struct rib *rib, struct rde_
*/
if ((asp = path_lookup(nasp, peer)) == NULL) {
/* Path not available, create and link a new one. */
- asp = path_copy(nasp);
+ asp = path_copy(path_get(), nasp);
path_link(asp, peer);
}
@@ -638,46 +638,36 @@ path_link(struct rde_aspath *asp, struct
}
/*
- * copy asp to a new UNLINKED one mainly for filtering
+ * Copy asp to a new UNLINKED aspath
+ * On dst either path_get() or path_prep() had to be called before.
*/
struct rde_aspath *
-path_copy(struct rde_aspath *asp)
+path_copy(struct rde_aspath *dst, const struct rde_aspath *src)
{
- struct rde_aspath *nasp;
-
- nasp = path_get();
- nasp->aspath = asp->aspath;
- if (nasp->aspath != NULL) {
- nasp->aspath->refcnt++;
+ dst->aspath = src->aspath;
+ if (dst->aspath != NULL) {
+ dst->aspath->refcnt++;
rdemem.aspath_refs++;
}
- nasp->nexthop = nexthop_ref(asp->nexthop);
- nasp->med = asp->med;
- nasp->lpref = asp->lpref;
- nasp->weight = asp->weight;
- nasp->origin = asp->origin;
- nasp->rtlabelid = asp->rtlabelid;
- rtlabel_ref(nasp->rtlabelid);
- nasp->pftableid = asp->pftableid;
- pftable_ref(nasp->pftableid);
+ dst->nexthop = nexthop_ref(src->nexthop);
+ dst->med = src->med;
+ dst->lpref = src->lpref;
+ dst->weight = src->weight;
+ dst->origin = src->origin;
+ dst->rtlabelid = rtlabel_ref(src->rtlabelid);
+ dst->pftableid = pftable_ref(src->pftableid);
- nasp->flags = asp->flags & ~(F_ATTR_LINKED | F_ATTR_UPDATE);
- attr_copy(nasp, asp);
+ dst->flags = src->flags & ~(F_ATTR_LINKED | F_ATTR_UPDATE);
+ attr_copy(dst, src);
- return (nasp);
+ return (dst);
}
-/* alloc and initialize new entry. May not fail. */
+/* initialize or pepare an aspath for use */
struct rde_aspath *
-path_get(void)
+path_prep(struct rde_aspath *asp)
{
- struct rde_aspath *asp;
-
- asp = calloc(1, sizeof(*asp));
- if (asp == NULL)
- fatal("path_alloc");
- rdemem.path_cnt++;
-
+ memset(asp, 0, sizeof(*asp));
TAILQ_INIT(&asp->prefixes);
TAILQ_INIT(&asp->updates);
asp->origin = ORIGIN_INCOMPLETE;
@@ -687,6 +677,20 @@ path_get(void)
/* rtlabel = 0 */
return (asp);
+}
+
+/* alloc and initialize new entry. May not fail. */
+struct rde_aspath *
+path_get(void)
+{
+ struct rde_aspath *asp;
+
+ asp = malloc(sizeof(*asp));
+ if (asp == NULL)
+ fatal("path_get");
+ rdemem.path_cnt++;
+
+ return (path_prep(asp));
}
/* free an unlinked element */
