dhcpcd-discuss

On 04/03/2019 17:38, Donald Sharp wrote:
> Roy -
>
> Without the rb-tree dhcpcd is taking up this much space after 1
> million routes install/delete:
>
> sharpd@janelle ~/d/src> ps -eo size,pid,user,command | grep dhcpcd
> 16672 27438 root     /usr/sbin/dhcpcd
>
> With the rb-tree dhcpcd is taking up this much space:
>
> sharpd@janelle ~/d/src> ps -eo size,pid,user,command | grep dhcpcd
> 172232 26445 root    dhcpcd/src/dhcpcd
>
> So not great at all and it would be nice to understand what is going on.

Well, there are several factors.
We re-load the kernel routing table a lot, probably more than we need to.
We also maintain a freed route entry list to avoid a lot of mallocs - 
I've attached a patch to disable this which *may* help with memory usage 
so it should shrink between operations.
I don't know enough if that's good or bad for memory fragmentation on 
small systems with a long running program such as dhcpcd.

> On the other hand original non-rbtree code continues to grow after
> every iteration of 1 million routes:
>
> <another 1 million routes added/deleted>
> sharpd@janelle ~/d/src> ps -eo size,pid,user,command | grep dhcpcd
> 27088 27438 root     /usr/sbin/dhcpcd
> <another 1 million routes added/deleted>
> sharpd@janelle ~/d/src> ps -eo size,pid,user,command | grep dhcpcd
> 35024 27438 root     /usr/sbin/dhcpcd
>
> While the new rb-tree code is staying at 172mb for each iteration, so
> there is that at least.  I think it might be worthwhile to figure out
> where all the extra memory is going to.  It seems like a simple data
> structure replacement shouldn't have these memory issues.  I guess
> some more investigation is needed.  I'll see if I can look at it this
> evening some more.

I noted in the first patch that this rb implementation might have fixed 
a minor memory leak. I think you're just seeing this x1 million which 
can explain it.

Lowering memory usage more we can do this - move if_initrt to the top of 
rt_build. This is tricky as from memory some parts of dhcpcd rely on it 
being there outside of route building. We'll need to double check and 
fix these places.
Once that's done we can then dispose of kroutes at the end of rt_build.
If that works, kroutes can be removed from the global context.

But that's for another day.

Roy

diff --git a/src/dhcpcd.h b/src/dhcpcd.h
index a09f40fb..bdcd4377 100644
--- a/src/dhcpcd.h
+++ b/src/dhcpcd.h
@@ -139,7 +139,9 @@ struct dhcpcd_ctx {
 
 	rb_tree_t routes;	/* our routes */
 	rb_tree_t kroutes;	/* all kernel routes */
+#ifdef RT_FREE_ROUTE_TABLE
 	rb_tree_t froutes;	/* free routes for re-use */
+#endif
 
 	int pf_inet_fd;
 	void *priv;
diff --git a/src/route.c b/src/route.c
index d7cfe7ba..cdf4f052 100644
--- a/src/route.c
+++ b/src/route.c
@@ -151,7 +151,9 @@ rt_init(struct dhcpcd_ctx *ctx)
 
 	rb_tree_init(&ctx->routes, &rt_compare_os_ops);
 	rb_tree_init(&ctx->kroutes, &rt_compare_os_ops);
+#ifdef RT_FREE_ROUTE_TABLE
 	rb_tree_init(&ctx->froutes, &rt_compare_free_ops);
+#endif
 }
 
 static void
@@ -207,7 +209,9 @@ rt_headclear0(struct dhcpcd_ctx *ctx, rb_tree_t *rts, int af)
 	if (rts == NULL)
 		return;
 	assert(ctx != NULL);
+#ifdef RT_FREE_ROUTE_TABLE
 	assert(&ctx->froutes != rts);
+#endif
 
 	RB_TREE_FOREACH_SAFE(rt, rts, rtn) {
 		if (af != AF_UNSPEC &&
@@ -247,7 +251,9 @@ rt_dispose(struct dhcpcd_ctx *ctx)
 	assert(ctx != NULL);
 	rt_headfree(&ctx->routes);
 	rt_headfree(&ctx->kroutes);
+#ifdef RT_FREE_ROUTE_TABLE
 	rt_headfree(&ctx->froutes);
+#endif
 }
 
 struct rt *
@@ -256,9 +262,12 @@ rt_new0(struct dhcpcd_ctx *ctx)
 	struct rt *rt;
 
 	assert(ctx != NULL);
+#ifdef RT_FREE_ROUTE_TABLE
 	if ((rt = RB_TREE_MIN(&ctx->froutes)) != NULL)
 		rb_tree_remove_node(&ctx->froutes, rt);
-	else if ((rt = malloc(sizeof(*rt))) == NULL) {
+	else
+#endif
+	if ((rt = malloc(sizeof(*rt))) == NULL) {
 		logerr(__func__);
 		return NULL;
 	}
@@ -293,6 +302,7 @@ rt_new(struct interface *ifp)
 void
 rt_free(struct rt *rt)
 {
+#ifdef RT_FREE_ROUTE_TABLE
 	struct dhcpcd_ctx *ctx;
 
 	assert(rt != NULL);
@@ -301,6 +311,9 @@ rt_free(struct rt *rt)
 
 	ctx = rt->rt_ifp->ctx;
 	rb_tree_insert_node(&ctx->froutes, rt);
+#else
+	free(rt);
+#endif
 }
 
 void