From 67dce280d78f07d5a66c0ea5336cea09d5f34cc2 Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Wed, 9 Sep 2020 17:13:11 -0600 Subject: [PATCH 1/8] interface: add lif_interface.rdepends_count --- libifupdown/interface.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libifupdown/interface.h b/libifupdown/interface.h index a18c4b7..d627610 100644 --- a/libifupdown/interface.h +++ b/libifupdown/interface.h @@ -53,7 +53,8 @@ struct lif_interface { struct lif_dict vars; - size_t refcount; /* >= 0 if up, else 0 */ + size_t refcount; /* > 0 if up, else 0 */ + size_t rdepends_count; /* > 0 if any reverse dependency */ }; #define LIF_INTERFACE_COLLECTION_FOREACH(iter, collection) \ From a5eebda391e00b3b2f9db080aade68dadd0e96a4 Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Wed, 9 Sep 2020 17:14:58 -0600 Subject: [PATCH 2/8] ifquery: include reverse dependency count in dot output --- cmd/ifquery.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/ifquery.c b/cmd/ifquery.c index 25a09d3..8968d86 100644 --- a/cmd/ifquery.c +++ b/cmd/ifquery.c @@ -67,9 +67,9 @@ print_interface_dot(struct lif_dict *collection, struct lif_interface *iface, st return; if (parent != NULL) - printf("\"%s\" -> ", parent->ifname); + printf("\"%s (%zu)\" -> ", parent->ifname, parent->rdepends_count); - printf("\"%s\"", iface->ifname); + printf("\"%s (%zu)\"", iface->ifname, iface->rdepends_count); printf("\n"); From 2569503afa6fa8edb45433be62bff38406d9c0d8 Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Wed, 9 Sep 2020 17:39:26 -0600 Subject: [PATCH 3/8] lifecycle: add lif_lifecycle_count_rdepends(), which calculates reverse dependency depth --- cmd/ifquery.c | 6 +++++ cmd/ifupdown.c | 6 +++++ libifupdown/lifecycle.c | 55 +++++++++++++++++++++++++++++++++++++++++ libifupdown/lifecycle.h | 1 + 4 files changed, 68 insertions(+) diff --git a/cmd/ifquery.c b/cmd/ifquery.c index 8968d86..855cbb0 100644 --- a/cmd/ifquery.c +++ b/cmd/ifquery.c @@ -290,6 +290,12 @@ ifquery_main(int argc, char *argv[]) return EXIT_FAILURE; } + if (!lif_lifecycle_count_rdepends(&exec_opts, &collection)) + { + fprintf(stderr, "%s: could not validate dependency tree\n", argv0); + return EXIT_FAILURE; + } + /* --list --state is not allowed */ if (listing && listing_stat) generic_usage(self_applet, EXIT_FAILURE); diff --git a/cmd/ifupdown.c b/cmd/ifupdown.c index bc8b657..18bf8eb 100644 --- a/cmd/ifupdown.c +++ b/cmd/ifupdown.c @@ -229,6 +229,12 @@ ifupdown_main(int argc, char *argv[]) return EXIT_FAILURE; } + if (!lif_lifecycle_count_rdepends(&exec_opts, &collection)) + { + fprintf(stderr, "%s: could not validate dependency tree\n", argv0); + return EXIT_FAILURE; + } + if (!lif_state_sync(&state, &collection)) { fprintf(stderr, "%s: could not sync state\n", argv0); diff --git a/libifupdown/lifecycle.c b/libifupdown/lifecycle.c index 5d50248..75f5743 100644 --- a/libifupdown/lifecycle.c +++ b/libifupdown/lifecycle.c @@ -389,3 +389,58 @@ lif_lifecycle_run(const struct lif_execute_opts *opts, struct lif_interface *ifa return true; } + +static bool +count_interface_rdepends(const struct lif_execute_opts *opts, struct lif_dict *collection, struct lif_interface *parent, size_t depth) +{ + /* 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->rdepends_count = depth; + + struct lif_dict_entry *requires = lif_dict_find(&parent->vars, "requires"); + + /* no dependents, nothing to worry about */ + if (requires == NULL) + return true; + + /* walk any dependents */ + char require_ifs[4096] = {}; + strlcpy(require_ifs, requires->data, sizeof require_ifs); + char *bufp = require_ifs; + + for (char *tokenp = lif_next_token(&bufp); *tokenp; tokenp = lif_next_token(&bufp)) + { + struct lif_interface *iface = lif_interface_collection_find(collection, tokenp); + + if (!count_interface_rdepends(opts, collection, iface, depth + 1)) + return false; + } + + return true; +} + +bool +lif_lifecycle_count_rdepends(const struct lif_execute_opts *opts, struct lif_dict *collection) +{ + struct lif_node *iter; + + LIF_DICT_FOREACH(iter, collection) + { + struct lif_dict_entry *entry = iter->data; + struct lif_interface *iface = entry->data; + + /* start depth at interface's rdepends_count, which will be 0 for the root, + * but will be more if additional rdepends are found... + */ + if (!count_interface_rdepends(opts, collection, iface, iface->rdepends_count)) + { + fprintf(stderr, "ifupdown: dependency graph is broken for interface %s\n", iface->ifname); + return false; + } + } + + return true; +} diff --git a/libifupdown/lifecycle.h b/libifupdown/lifecycle.h index aebbf6e..1dcf2e9 100644 --- a/libifupdown/lifecycle.h +++ b/libifupdown/lifecycle.h @@ -22,6 +22,7 @@ extern bool lif_lifecycle_query_dependents(const struct lif_execute_opts *opts, struct lif_interface *iface, const char *lifname); extern bool lif_lifecycle_run_phase(const struct lif_execute_opts *opts, struct lif_interface *iface, const char *phase, const char *lifname, bool up); extern 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); +extern bool lif_lifecycle_count_rdepends(const struct lif_execute_opts *opts, struct lif_dict *collection); #endif From 8c8727e30f8386b41bb5d380d819509d128c28cd Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Wed, 9 Sep 2020 17:40:57 -0600 Subject: [PATCH 4/8] lifecycle: drop redundant lif_lifecycle_query_dependents(). since we now precompile the full dependency tree, including depths, we no longer need to re-query each leaf for dependency information. --- libifupdown/lifecycle.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/libifupdown/lifecycle.c b/libifupdown/lifecycle.c index 75f5743..7f0b032 100644 --- a/libifupdown/lifecycle.c +++ b/libifupdown/lifecycle.c @@ -339,9 +339,6 @@ lif_lifecycle_run(const struct lif_execute_opts *opts, struct lif_interface *ifa if (lifname == NULL) lifname = iface->ifname; - if (!lif_lifecycle_query_dependents(opts, iface, lifname)) - return false; - if (up) { /* when going up, dependents go up first. */ From 258e2e8a526c7d1ef3d42ba4c4a426e6d5433665 Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Wed, 9 Sep 2020 18:11:10 -0600 Subject: [PATCH 5/8] lifecycle: return the maximum tree depth for the dependency tree when counting depth --- cmd/ifquery.c | 2 +- cmd/ifupdown.c | 2 +- libifupdown/lifecycle.c | 18 +++++++++++++++--- libifupdown/lifecycle.h | 2 +- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/cmd/ifquery.c b/cmd/ifquery.c index 855cbb0..fb91883 100644 --- a/cmd/ifquery.c +++ b/cmd/ifquery.c @@ -290,7 +290,7 @@ ifquery_main(int argc, char *argv[]) return EXIT_FAILURE; } - if (!lif_lifecycle_count_rdepends(&exec_opts, &collection)) + if (lif_lifecycle_count_rdepends(&exec_opts, &collection) == -1) { fprintf(stderr, "%s: could not validate dependency tree\n", argv0); return EXIT_FAILURE; diff --git a/cmd/ifupdown.c b/cmd/ifupdown.c index 18bf8eb..0b09d67 100644 --- a/cmd/ifupdown.c +++ b/cmd/ifupdown.c @@ -229,7 +229,7 @@ ifupdown_main(int argc, char *argv[]) return EXIT_FAILURE; } - if (!lif_lifecycle_count_rdepends(&exec_opts, &collection)) + if (lif_lifecycle_count_rdepends(&exec_opts, &collection) == -1) { fprintf(stderr, "%s: could not validate dependency tree\n", argv0); return EXIT_FAILURE; diff --git a/libifupdown/lifecycle.c b/libifupdown/lifecycle.c index 7f0b032..87e0d0a 100644 --- a/libifupdown/lifecycle.c +++ b/libifupdown/lifecycle.c @@ -419,7 +419,7 @@ count_interface_rdepends(const struct lif_execute_opts *opts, struct lif_dict *c return true; } -bool +ssize_t lif_lifecycle_count_rdepends(const struct lif_execute_opts *opts, struct lif_dict *collection) { struct lif_node *iter; @@ -435,9 +435,21 @@ lif_lifecycle_count_rdepends(const struct lif_execute_opts *opts, struct lif_dic if (!count_interface_rdepends(opts, collection, iface, iface->rdepends_count)) { fprintf(stderr, "ifupdown: dependency graph is broken for interface %s\n", iface->ifname); - return false; + return -1; } } - return true; + /* figure out the max depth */ + size_t maxdepth = 0; + + LIF_DICT_FOREACH(iter, collection) + { + struct lif_dict_entry *entry = iter->data; + struct lif_interface *iface = entry->data; + + if (iface->rdepends_count > maxdepth) + maxdepth = iface->rdepends_count; + } + + return maxdepth; } diff --git a/libifupdown/lifecycle.h b/libifupdown/lifecycle.h index 1dcf2e9..fecf9fe 100644 --- a/libifupdown/lifecycle.h +++ b/libifupdown/lifecycle.h @@ -22,7 +22,7 @@ extern bool lif_lifecycle_query_dependents(const struct lif_execute_opts *opts, struct lif_interface *iface, const char *lifname); extern bool lif_lifecycle_run_phase(const struct lif_execute_opts *opts, struct lif_interface *iface, const char *phase, const char *lifname, bool up); extern 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); -extern bool lif_lifecycle_count_rdepends(const struct lif_execute_opts *opts, struct lif_dict *collection); +extern ssize_t lif_lifecycle_count_rdepends(const struct lif_execute_opts *opts, struct lif_dict *collection); #endif From a6e022ad99a9a1e5d4c31bc859803b37e98cb277 Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Wed, 9 Sep 2020 19:21:41 -0600 Subject: [PATCH 6/8] lifecycle: re-sort the interface collection by hand --- libifupdown/lifecycle.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/libifupdown/lifecycle.c b/libifupdown/lifecycle.c index 87e0d0a..9004e3f 100644 --- a/libifupdown/lifecycle.c +++ b/libifupdown/lifecycle.c @@ -451,5 +451,37 @@ lif_lifecycle_count_rdepends(const struct lif_execute_opts *opts, struct lif_dic maxdepth = iface->rdepends_count; } + /* move the collection to a temporary list so we can reorder it */ + struct lif_list temp_list = {}; + struct lif_node *iter_next; + + LIF_LIST_FOREACH_SAFE(iter, iter_next, collection->list.head) + { + void *data = iter->data; + + lif_node_delete(iter, &collection->list); + memset(iter, 0, sizeof *iter); + + lif_node_insert(iter, data, &temp_list); + } + + /* walk backwards from maxdepth to 0, readding nodes */ + for (ssize_t curdepth = maxdepth; curdepth > -1; curdepth--) + { + LIF_LIST_FOREACH_SAFE(iter, iter_next, temp_list.head) + { + struct lif_dict_entry *entry = iter->data; + struct lif_interface *iface = entry->data; + + if ((ssize_t) iface->rdepends_count != curdepth) + continue; + + lif_node_delete(iter, &temp_list); + memset(iter, 0, sizeof *iter); + + lif_node_insert(iter, entry, &collection->list); + } + } + return maxdepth; } From ceb82f4fd209a14d6a442a485e6bd390db8f3312 Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Wed, 9 Sep 2020 19:38:29 -0600 Subject: [PATCH 7/8] ifupdown: only display skip messages in verbose mode --- cmd/ifupdown.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cmd/ifupdown.c b/cmd/ifupdown.c index 0b09d67..ad20df4 100644 --- a/cmd/ifupdown.c +++ b/cmd/ifupdown.c @@ -101,15 +101,17 @@ skip_interface(struct lif_interface *iface, const char *ifname) if (up && iface->refcount > 0) { - fprintf(stderr, "%s: skipping %s (already configured), use --force to force configuration\n", - argv0, ifname); + if (exec_opts.verbose) + fprintf(stderr, "%s: skipping auto interface %s (already configured), use --force to force configuration\n", + argv0, ifname); return true; } if (!up && iface->refcount == 0) { - fprintf(stderr, "%s: skipping %s (already deconfigured), use --force to force deconfiguration\n", - argv0, ifname); + if (exec_opts.verbose) + fprintf(stderr, "%s: skipping auto interface %s (already deconfigured), use --force to force deconfiguration\n", + argv0, ifname); return true; } From e754e836afc03be71bc9826562c04e9d87fecd86 Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Wed, 9 Sep 2020 20:46:50 -0600 Subject: [PATCH 8/8] tests: add tests for auto interface skipping --- tests/fixtures/teardown-dep-ordering.ifstate | 3 +++ tests/fixtures/teardown-dep-ordering.interfaces | 13 +++++++++++++ tests/ifdown_test | 9 +++++++++ tests/ifup_test | 10 +++++++++- 4 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/teardown-dep-ordering.ifstate create mode 100644 tests/fixtures/teardown-dep-ordering.interfaces diff --git a/tests/fixtures/teardown-dep-ordering.ifstate b/tests/fixtures/teardown-dep-ordering.ifstate new file mode 100644 index 0000000..aef421a --- /dev/null +++ b/tests/fixtures/teardown-dep-ordering.ifstate @@ -0,0 +1,3 @@ +dummy=dummy 2 +bat=bat 2 +br=br 1 diff --git a/tests/fixtures/teardown-dep-ordering.interfaces b/tests/fixtures/teardown-dep-ordering.interfaces new file mode 100644 index 0000000..551ad3d --- /dev/null +++ b/tests/fixtures/teardown-dep-ordering.interfaces @@ -0,0 +1,13 @@ +auto dummy +iface dummy + link-type dummy + +auto bat +iface bat + link-type dummy + requires dummy + +auto br +iface br + link-type dummy + requires bat diff --git a/tests/ifdown_test b/tests/ifdown_test index 2ca83c3..5aafc86 100755 --- a/tests/ifdown_test +++ b/tests/ifdown_test @@ -22,6 +22,7 @@ tests_init \ deferred_teardown_1 \ deferred_teardown_2 \ deferred_teardown_3 \ + teardown_dep_ordering \ regress_opt_f noargs_body() { @@ -172,6 +173,14 @@ deferred_teardown_3_body() { -i $FIXTURES/deferred-teardown-2.interfaces tun0 tun1 tun2 tun3 } +teardown_dep_ordering_body() { + atf_check -s exit:0 -o ignore \ + -e match:"skipping auto interface bat" \ + -e match:"skipping auto interface dummy" \ + ifdown -n -i $FIXTURES/teardown-dep-ordering.interfaces \ + -S $FIXTURES/teardown-dep-ordering.ifstate -E $EXECUTORS -a +} + 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 diff --git a/tests/ifup_test b/tests/ifup_test index 594ab11..a5ef49b 100755 --- a/tests/ifup_test +++ b/tests/ifup_test @@ -18,7 +18,8 @@ tests_init \ learned_dependency \ learned_dependency_2 \ learned_executor \ - implicit_vlan + implicit_vlan \ + teardown_dep_ordering noargs_body() { atf_check -s exit:1 -e ignore ifup -S/dev/null @@ -131,3 +132,10 @@ implicit_vlan_body() { -e match:"attempting to run link executor" \ ifup -n -S/dev/null -E $EXECUTORS -i $FIXTURES/vlan.interfaces eth0.8 } + +teardown_dep_ordering_body() { + atf_check -s exit:0 -o ignore \ + -e match:"skipping auto interface bat" \ + -e match:"skipping auto interface dummy" \ + ifup -n -i $FIXTURES/teardown-dep-ordering.interfaces -E $EXECUTORS -a +}