From 6603ff1000281c525b9391f5dd0d3c6e5c5173c6 Mon Sep 17 00:00:00 2001
From: Ariadne Conill <ariadne@dereferenced.org>
Date: Tue, 8 Sep 2020 15:28:36 -0600
Subject: [PATCH] lifecycle: use refcounting to defer interface teardown

---
 libifupdown/lifecycle.c | 44 +++++++++++++++++++++++++++++++++--------
 libifupdown/state.c     |  3 +++
 2 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/libifupdown/lifecycle.c b/libifupdown/lifecycle.c
index 2b77ed0..81f8587 100644
--- a/libifupdown/lifecycle.c
+++ b/libifupdown/lifecycle.c
@@ -261,6 +261,31 @@ handle_error:
 	return false;
 }
 
+/* this function returns true if we can skip processing the interface for now,
+ * otherwise false.
+ */
+static bool
+handle_refcounting(struct lif_dict *state, struct lif_interface *iface, bool up)
+{
+	size_t orig_refcount = iface->refcount;
+
+	if (up)
+		lif_state_ref_if(state, iface->ifname, iface);
+	else
+		lif_state_unref_if(state, iface->ifname, iface);
+
+	/* if going up and orig_refcount > 0 -- we're already configured. */
+	if (up && orig_refcount > 0)
+		return true;
+
+	/* if going down and iface->refcount > 0 -- we still have other dependents. */
+	if (!up && iface->refcount > 0)
+		return true;
+
+	/* we can change this interface -- no blocking dependents. */
+	return false;
+}
+
 static bool
 handle_dependents(const struct lif_execute_opts *opts, struct lif_interface *parent, struct lif_dict *collection, struct lif_dict *state, bool up)
 {
@@ -278,12 +303,17 @@ handle_dependents(const struct lif_execute_opts *opts, struct lif_interface *par
 	{
 		struct lif_interface *iface = lif_interface_collection_find(collection, tokenp);
 
-		/* already up or down, skip */
-		if (up && iface->refcount > 0)
+		/* if handle_refcounting returns true, it means we've already
+		 * configured the interface, or it is too soon to deconfigure
+		 * the interface.
+		 */
+		if (handle_refcounting(state, iface, up))
 		{
 			if (opts->verbose)
-				fprintf(stderr, "ifupdown: skipping dependent interface %s (of %s)\n",
-					iface->ifname, parent->ifname);
+				fprintf(stderr, "ifupdown: skipping dependent interface %s (of %s) -- %s\n",
+					iface->ifname, parent->ifname,
+					up ? "already configured" : "transient dependencies still exist");
+
 			continue;
 		}
 
@@ -328,7 +358,7 @@ lif_lifecycle_run(const struct lif_execute_opts *opts, struct lif_interface *ifa
 
 		lif_state_upsert(state, lifname, iface);
 
-		iface->refcount++;
+		lif_state_ref_if(state, lifname, iface);
 	}
 	else
 	{
@@ -345,9 +375,7 @@ lif_lifecycle_run(const struct lif_execute_opts *opts, struct lif_interface *ifa
 		if (!handle_dependents(opts, iface, collection, state, up))
 			return false;
 
-		lif_state_delete(state, lifname);
-
-		iface->refcount--;
+		lif_state_unref_if(state, lifname, iface);
 	}
 
 	return true;
diff --git a/libifupdown/state.c b/libifupdown/state.c
index f3a4260..c2a1a99 100644
--- a/libifupdown/state.c
+++ b/libifupdown/state.c
@@ -73,6 +73,9 @@ lif_state_ref_if(struct lif_dict *state, const char *ifname, struct lif_interfac
 void
 lif_state_unref_if(struct lif_dict *state, const char *ifname, struct lif_interface *iface)
 {
+	if (iface->refcount == 0)
+		return;
+
 	iface->refcount--;
 
 	if (iface->refcount)