From 175f002b3e50c8b01ba416c00eea9d3bca60f7f8 Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Fri, 25 Sep 2020 08:23:51 -0600 Subject: [PATCH 1/6] lifecycle: add lif_interface.is_pending to break dependency cycles --- libifupdown/interface.h | 1 + libifupdown/lifecycle.c | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/libifupdown/interface.h b/libifupdown/interface.h index eeaa898..4a33ac7 100644 --- a/libifupdown/interface.h +++ b/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 */ diff --git a/libifupdown/lifecycle.c b/libifupdown/lifecycle.c index 3da5dae..9a07f32 100644 --- a/libifupdown/lifecycle.c +++ b/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; From b2e657cdf014f5834a0fcd0b41dbc9f015265081 Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Thu, 8 Oct 2020 02:00:18 -0600 Subject: [PATCH 2/6] tests: add dependency loop fixture --- tests/fixtures/dependency-loop.interfaces | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 tests/fixtures/dependency-loop.interfaces diff --git a/tests/fixtures/dependency-loop.interfaces b/tests/fixtures/dependency-loop.interfaces new file mode 100644 index 0000000..e164928 --- /dev/null +++ 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 From 122a54377ddf675cd19168e2c9ca06c156369bcd Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Thu, 8 Oct 2020 02:01:46 -0600 Subject: [PATCH 3/6] lifecycle: break dependency cycles when calculating the full dependency graph --- libifupdown/lifecycle.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libifupdown/lifecycle.c b/libifupdown/lifecycle.c index 9a07f32..4e59526 100644 --- a/libifupdown/lifecycle.c +++ b/libifupdown/lifecycle.c @@ -476,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] = {}; @@ -499,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; } From 0cade539f85aaf81e2c224f28ca8c9d607d584fc Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Thu, 8 Oct 2020 02:05:43 -0600 Subject: [PATCH 4/6] tests: ifup: add dependency loop breaking test --- tests/ifup_test | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/ifup_test b/tests/ifup_test index a5ef49b..5c619a4 100755 --- a/tests/ifup_test +++ b/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 +} From 959617df88b63b8b9be26974ee9f436f6136cccd Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Thu, 8 Oct 2020 02:09:30 -0600 Subject: [PATCH 5/6] tests: add dependency loop breaking test for ifdown --- tests/fixtures/dependency-loop.ifstate | 3 +++ tests/ifdown_test | 9 ++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/dependency-loop.ifstate diff --git a/tests/fixtures/dependency-loop.ifstate b/tests/fixtures/dependency-loop.ifstate new file mode 100644 index 0000000..4b7a3ef --- /dev/null +++ b/tests/fixtures/dependency-loop.ifstate @@ -0,0 +1,3 @@ +lo=lo +a=a +b=b diff --git a/tests/ifdown_test b/tests/ifdown_test index 5aafc86..fb6cf8e 100755 --- a/tests/ifdown_test +++ b/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 +} From 4c354ebf35dfb3b085df240fac6b67801af648ec Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Thu, 8 Oct 2020 02:17:13 -0600 Subject: [PATCH 6/6] ifquery: implement more robust loop breaking strategy --- cmd/ifquery.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/cmd/ifquery.c b/cmd/ifquery.c index 9eb1f58..f62a58b 100644 --- a/cmd/ifquery.c +++ b/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; } }