Merge pull request #85 from ifupdown-ng/feature/dependency-loop-breaking

lifecycle: add lif_interface.is_pending to break dependency cycles

cmd/ifquery.c

@@ -63,9 +63,6 @@ print_interface_dot(struct lif_dict *collection, struct lif_interface *iface, st
 	if (!lif_lifecycle_query_dependents(&exec_opts, iface, iface->ifname))
 		return;
 
-	if (iface->refcount > 0)
-		return;
-
 	if (parent != NULL)
 		printf("\"%s (%zu)\" -> ", parent->ifname, parent->rdepends_count);
 
@@ -86,8 +83,12 @@ print_interface_dot(struct lif_dict *collection, struct lif_interface *iface, st
 	{
 		struct lif_interface *child_if = lif_interface_collection_find(collection, tokenp);
 
+		if (child_if->is_pending)
+			continue;
+
+		child_if->is_pending = true;
 		print_interface_dot(collection, child_if, iface);
-		child_if->refcount++;
+		child_if->is_pending = false;
 	}
 }
 

libifupdown/interface.h

@@ -52,6 +52,7 @@ struct lif_interface {
 	bool is_bridge;
 	bool is_bond;
 	bool is_template;
+	bool is_pending;
 
 	bool has_config_error;	/* error found in interface configuration */
 

libifupdown/lifecycle.c

@@ -359,6 +359,9 @@ handle_dependents(const struct lif_execute_opts *opts, struct lif_interface *par
 	if (requires == NULL)
 		return true;
 
+	/* set the parent's pending flag to break dependency cycles */
+	parent->is_pending = true;
+
 	char require_ifs[4096] = {};
 	strlcpy(require_ifs, requires->data, sizeof require_ifs);
 	char *bufp = require_ifs;
@@ -399,15 +402,23 @@ handle_dependents(const struct lif_execute_opts *opts, struct lif_interface *par
 				iface->ifname, parent->ifname, up ? "up" : "down");
 
 		if (!lif_lifecycle_run(opts, iface, collection, state, iface->ifname, up))
+		{
+			parent->is_pending = false;
 			return false;
+		}
 	}
 
+	parent->is_pending = false;
 	return true;
 }
 
 bool
 lif_lifecycle_run(const struct lif_execute_opts *opts, struct lif_interface *iface, struct lif_dict *collection, struct lif_dict *state, const char *lifname, bool up)
 {
+	/* if we're already pending, exit */
+	if (iface->is_pending)
+		return true;
+
 	if (iface->is_template)
 		return false;
 
@@ -465,18 +476,26 @@ lif_lifecycle_run(const struct lif_execute_opts *opts, struct lif_interface *ifa
 static bool
 count_interface_rdepends(const struct lif_execute_opts *opts, struct lif_dict *collection, struct lif_interface *parent, size_t depth)
 {
+	/* if we have looped, return true immediately to break the loop. */
+	if (parent->is_pending)
+		return true;
+
 	/* query our dependents if we don't have them already */
 	if (!lif_lifecycle_query_dependents(opts, parent, parent->ifname))
 		return false;
 
 	/* set rdepends_count to depth, dependents will be depth + 1 */
+	parent->is_pending = true;
 	parent->rdepends_count = depth;
 
 	struct lif_dict_entry *requires = lif_dict_find(&parent->vars, "requires");
 
 	/* no dependents, nothing to worry about */
 	if (requires == NULL)
+	{
+		parent->is_pending = false;
 		return true;
+	}
 
 	/* walk any dependents */
 	char require_ifs[4096] = {};
@@ -488,9 +507,13 @@ count_interface_rdepends(const struct lif_execute_opts *opts, struct lif_dict *c
 		struct lif_interface *iface = lif_interface_collection_find(collection, tokenp);
 
 		if (!count_interface_rdepends(opts, collection, iface, depth + 1))
+		{
+			parent->is_pending = false;
 			return false;
+		}
 	}
 
+	parent->is_pending = false;
 	return true;
 }
 

tests/fixtures/dependency-loop.ifstate

@@ -0,0 +1,3 @@
+lo=lo
+a=a
+b=b

tests/fixtures/dependency-loop.interfaces

@@ -0,0 +1,9 @@
+auto a
+iface a
+	use link
+	requires b
+
+auto b
+iface b
+	use link
+	requires a

tests/ifdown_test

@@ -23,7 +23,8 @@ tests_init \
 	deferred_teardown_2 \
 	deferred_teardown_3 \
 	teardown_dep_ordering \
-	regress_opt_f
+	regress_opt_f \
+	dependency_loop_breaking
 
 noargs_body() {
 	atf_check -s exit:1 -e ignore ifdown -S/dev/null
@@ -185,3 +186,9 @@ regress_opt_f_body() {
 	atf_check -s exit:0 -o ignore -e ignore \
 		ifdown -n -S $FIXTURES/vlan.ifstate -E $EXECUTORS -i $FIXTURES/vlan.interfaces -f eth0.8
 }
+
+dependency_loop_breaking_body() {
+	atf_check -s exit:0 -o ignore \
+		-e match:"ifdown: skipping auto interface a \\(already deconfigured\\), use --force to force deconfiguration" \
+		ifdown -n -i $FIXTURES/dependency-loop.interfaces -E $EXECUTORS -a
+}

tests/ifup_test

@@ -19,7 +19,8 @@ tests_init \
 	learned_dependency_2 \
 	learned_executor \
 	implicit_vlan \
-	teardown_dep_ordering
+	teardown_dep_ordering \
+	dependency_loop_breaking
 
 noargs_body() {
 	atf_check -s exit:1 -e ignore ifup -S/dev/null
@@ -139,3 +140,9 @@ teardown_dep_ordering_body() {
 		-e match:"skipping auto interface dummy" \
 		ifup -n -i $FIXTURES/teardown-dep-ordering.interfaces -E $EXECUTORS -a
 }
+
+dependency_loop_breaking_body() {
+	atf_check -s exit:0 -o ignore \
+		-e match:"ifup: skipping auto interface a \\(already configured\\), use --force to force configuration" \
+		ifup -n -i $FIXTURES/dependency-loop.interfaces -E $EXECUTORS -a
+}