Always update a route's nexthop as soon as the current nexthop is deleted (and clean up the route update code a bit)

3 files changed, 69 insertions, 59 deletions

diff --git a/src/babel.c b/src/babel.c
index fb2d165..0fab0a7 100644
--- a/src/babel.c
+++ b/src/babel.c
@@ -93,7 +93,7 @@ static void maintain_routes(gmrf_context_t *ctx) {
 		gp_babel_route_t *route = *cur;
 		next = &route->next;
 
-		gp_babel_route_update(ctx, route);
+		gp_babel_route_maintain(ctx, route);
 
 		if (!route->nexthops && gp_babel_since(ctx, route->last_nexthop) > GP_BABEL_PURGE_TIMEOUT) {
 			gmrf_logf(ctx->gmrf, LOG_DEBUG, "node %04x%04x (%u, seqno=%04x): purging.",
diff --git a/src/babel.h b/src/babel.h
index 3564cf8..27748a7 100644
--- a/src/babel.h
+++ b/src/babel.h
@@ -134,7 +134,7 @@ void gp_babel_send_seqno_request(gmrf_context_t *ctx, gp_babel_neigh_t *neigh, g
 gp_babel_route_t* gp_babel_route_new(gmrf_context_t *ctx);
 gp_babel_route_t* gp_babel_route_find(gmrf_context_t *ctx, const gp_babel_node_id_t *node);
 gp_babel_route_t* gp_babel_route_get(gmrf_context_t *ctx, const gp_babel_node_id_t *node);
-void gp_babel_route_update(gmrf_context_t *ctx, gp_babel_route_t *route);
+void gp_babel_route_maintain(gmrf_context_t *ctx, gp_babel_route_t *route);
 void gp_babel_route_free(gmrf_context_t *ctx, gp_babel_route_t *route);
 
 gp_babel_nexthop_t* gp_babel_route_nexthop_new(gp_babel_route_t *route, gp_babel_neigh_t *neigh);
diff --git a/src/route.c b/src/route.c
index e0ff127..fed699c 100644
--- a/src/route.c
+++ b/src/route.c
@@ -94,50 +94,6 @@ static inline bool nexthop_has_expired(gmrf_context_t *ctx, gp_babel_nexthop_t *
 	return (gp_babel_since(ctx, nexthop->last_update) > GP_BABEL_UPDATE_TIMEOUT(nexthop->interval));
 }
 
-void gp_babel_route_nexthop_delete(gmrf_context_t *ctx, gp_babel_route_t *route, gp_babel_nexthop_t *nexthop) {
-	gp_babel_nexthop_t **nexthopp;
-	for (nexthopp = &route->nexthops; *nexthopp; nexthopp = &(*nexthopp)->next) {
-		if (*nexthopp == nexthop)
-			break;
-	}
-	assert(*nexthopp == nexthop);
-
-	*nexthopp = nexthop->next;
-
-	if (route->selected == nexthop)
-		route->selected = NULL;
-
-	gp_babel_neigh_unref(nexthop->neigh);
-	free(nexthop);
-
-	route->last_nexthop = gmrf_now(ctx->gmrf);
-}
-
-static void maintain_nexthop(gmrf_context_t *ctx, gp_babel_route_t *route, gp_babel_nexthop_t *nexthop) {
-	if (!nexthop->neigh) /* local */
-		return;
-
-	if (nexthop_has_expired(ctx, nexthop)) {
-		gp_babel_route_nexthop_delete(ctx, route, nexthop);
-		return;
-	}
-
-	if (gp_babel_since(ctx, nexthop->last_update) > GP_BABEL_UPDATE_REQUEST_TIMEOUT(nexthop->interval)
-	    && route->selected == nexthop && !nexthop->requested_update) {
-		gmrf_logf(ctx->gmrf, LOG_INFO, "route about to expire, requesting update");
-		gp_babel_send_route_request(ctx, NULL, nexthop->neigh, &route->node);
-		nexthop->requested_update = true;
-	}
-}
-
-static void maintain_nexthops(gmrf_context_t *ctx, gp_babel_route_t *route) {
-	gp_babel_nexthop_t *nexthop, *next;
-	for (nexthop = route->nexthops; nexthop; nexthop = next) {
-		next = nexthop->next;
-
-		maintain_nexthop(ctx, route, nexthop);
-	}
-}
 
 static gp_babel_nexthop_t* select_nexthop(gmrf_context_t *ctx, const gp_babel_route_t *route) {
 	uint16_t ret_metric = GP_BABEL_INFINITY;
@@ -162,7 +118,7 @@ static gp_babel_nexthop_t* select_nexthop(gmrf_context_t *ctx, const gp_babel_ro
 	return ret;
 }
 
-gp_babel_metric_seqno_t get_metric(gmrf_context_t *ctx, const gp_babel_route_t *route) {
+static gp_babel_metric_seqno_t get_metric(gmrf_context_t *ctx, const gp_babel_route_t *route) {
 	if (route->selected) {
 		uint32_t metric = route->selected->metric_seqno.metric + gp_babel_neigh_get_cost(ctx, route->selected->neigh);
 
@@ -173,9 +129,7 @@ gp_babel_metric_seqno_t get_metric(gmrf_context_t *ctx, const gp_babel_route_t *
 	return (gp_babel_metric_seqno_t){GP_BABEL_INFINITY, 0};
 }
 
-void gp_babel_route_update(gmrf_context_t *ctx, gp_babel_route_t *route) {
-	maintain_nexthops(ctx, route);
-
+static void update_nexthop(gmrf_context_t *ctx, gp_babel_route_t *route) {
 	route->selected = select_nexthop(ctx, route);
 	route->metric = get_metric(ctx, route);
 
@@ -203,18 +157,74 @@ void gp_babel_route_update(gmrf_context_t *ctx, gp_babel_route_t *route) {
 	}
 }
 
+static void delete_nexthop(gmrf_context_t *ctx, gp_babel_route_t *route, gp_babel_nexthop_t *nexthop) {
+	gp_babel_nexthop_t **nexthopp;
+	for (nexthopp = &route->nexthops; *nexthopp; nexthopp = &(*nexthopp)->next) {
+		if (*nexthopp == nexthop)
+			break;
+	}
+	assert(*nexthopp == nexthop);
+
+	*nexthopp = nexthop->next;
+
+	if (route->selected == nexthop)
+		route->selected = NULL;
+
+	gp_babel_neigh_unref(nexthop->neigh);
+	free(nexthop);
+
+	route->last_nexthop = gmrf_now(ctx->gmrf);
+}
+
+void gp_babel_route_nexthop_delete(gmrf_context_t *ctx, gp_babel_route_t *route, gp_babel_nexthop_t *nexthop) {
+	delete_nexthop(ctx, route, nexthop);
+
+	if (!route->selected)
+		gp_babel_route_maintain(ctx, route);
+}
+
+static void maintain_nexthop(gmrf_context_t *ctx, gp_babel_route_t *route, gp_babel_nexthop_t *nexthop) {
+	if (!nexthop->neigh) /* local */
+		return;
+
+	if (nexthop_has_expired(ctx, nexthop)) {
+		delete_nexthop(ctx, route, nexthop);
+		return;
+	}
+
+	if (gp_babel_since(ctx, nexthop->last_update) > GP_BABEL_UPDATE_REQUEST_TIMEOUT(nexthop->interval)
+	    && route->selected == nexthop && !nexthop->requested_update) {
+		gmrf_logf(ctx->gmrf, LOG_INFO, "route about to expire, requesting update");
+		gp_babel_send_route_request(ctx, NULL, nexthop->neigh, &route->node);
+		nexthop->requested_update = true;
+	}
+}
+
+static void maintain_nexthops(gmrf_context_t *ctx, gp_babel_route_t *route) {
+	gp_babel_nexthop_t *nexthop, *next;
+	for (nexthop = route->nexthops; nexthop; nexthop = next) {
+		next = nexthop->next;
+
+		maintain_nexthop(ctx, route, nexthop);
+	}
+}
+
+void gp_babel_route_maintain(gmrf_context_t *ctx, gp_babel_route_t *route) {
+	maintain_nexthops(ctx, route);
+	update_nexthop(ctx, route);
+}
+
 void gp_babel_route_update_nexthop(gmrf_context_t *ctx, gp_babel_route_t *route, gp_babel_nexthop_t *nexthop, gp_babel_metric_seqno_t ms, uint16_t interval) {
 	if (ms.metric == GP_BABEL_INFINITY) {
 		gmrf_logf(ctx->gmrf, LOG_DEBUG, "retract received, deleting nexthop");
-		gp_babel_route_nexthop_delete(ctx, route, nexthop);
-		gp_babel_route_update(ctx, route);
-		return;
+		delete_nexthop(ctx, route, nexthop);
+	}
+	else {
+		nexthop->metric_seqno = ms;
+		nexthop->interval = interval;
+		nexthop->requested_update = false;
+		nexthop->last_update = gmrf_now(ctx->gmrf);
 	}
 
-	nexthop->metric_seqno = ms;
-	nexthop->interval = interval;
-	nexthop->requested_update = false;
-	nexthop->last_update = gmrf_now(ctx->gmrf);
-
-	gp_babel_route_update(ctx, route);
+	gp_babel_route_maintain(ctx, route);
 }