Merge pull request #66 from ifupdown-ng/feature/refcounting

refcounting

cmd/ifquery.c

@@ -63,7 +63,7 @@ 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->is_up)
+	if (iface->refcount > 0)
 		return;
 
 	if (parent != NULL)
@@ -87,7 +87,7 @@ print_interface_dot(struct lif_dict *collection, struct lif_interface *iface, st
 		struct lif_interface *child_if = lif_interface_collection_find(collection, tokenp);
 
 		print_interface_dot(collection, child_if, iface);
-		child_if->is_up = true;
+		child_if->refcount++;
 	}
 }
 
@@ -215,7 +215,8 @@ list_state(struct lif_dict *state, struct match_options *opts)
 		    fnmatch(opts->include_pattern, entry->key, 0))
 			continue;
 
-		printf("%s=%s\n", entry->key, (const char *) entry->data);
+		struct lif_state_record *rec = entry->data;
+		printf("%s=%s %zu\n", entry->key, rec->mapped_if, rec->refcount);
 	}
 }
 

cmd/ifupdown.c

@@ -93,6 +93,29 @@ acquire_state_lock(const char *state_path, const char *lifname)
 	return fd;
 }
 
+bool
+skip_interface(struct lif_interface *iface, const char *ifname)
+{
+	if (exec_opts.force)
+		return false;
+
+	if (up && iface->refcount > 0)
+	{
+		fprintf(stderr, "%s: skipping %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);
+		return true;
+	}
+
+	return false;
+}
+
 bool
 change_interface(struct lif_interface *iface, struct lif_dict *collection, struct lif_dict *state, const char *ifname)
 {
@@ -104,6 +127,14 @@ change_interface(struct lif_interface *iface, struct lif_dict *collection, struc
 		return false;
 	}
 
+	if (skip_interface(iface, ifname))
+	{
+		if (lockfd != -1)
+			close(lockfd);
+
+		return true;
+	}
+
 	if (exec_opts.verbose)
 	{
 		fprintf(stderr, "%s: changing state of interface %s to '%s'\n",
@@ -155,6 +186,27 @@ change_auto_interfaces(struct lif_dict *collection, struct lif_dict *state, stru
 	return true;
 }
 
+int
+update_state_file_and_exit(int rc, struct lif_dict *state)
+{
+	if (exec_opts.mock)
+	{
+		exit(rc);
+		return rc;
+	}
+
+	if (!lif_state_write_path(state, exec_opts.state_file))
+	{
+		fprintf(stderr, "%s: could not update %s\n", argv0, exec_opts.state_file);
+
+		exit(EXIT_FAILURE);
+		return EXIT_FAILURE;
+	}
+
+	exit(rc);
+	return rc;
+}
+
 int
 ifupdown_main(int argc, char *argv[])
 {
@@ -186,9 +238,9 @@ ifupdown_main(int argc, char *argv[])
 	if (match_opts.is_auto)
 	{
 		if (!change_auto_interfaces(&collection, &state, &match_opts))
-			return EXIT_FAILURE;
+			return update_state_file_and_exit(EXIT_FAILURE, &state);
 
-		return EXIT_SUCCESS;
+		return update_state_file_and_exit(EXIT_SUCCESS, &state);
 	}
 	else if (optind >= argc)
 		generic_usage(self_applet, EXIT_FAILURE);
@@ -217,23 +269,17 @@ ifupdown_main(int argc, char *argv[])
 			if (entry == NULL)
 			{
 				fprintf(stderr, "%s: unknown interface %s\n", argv0, argv[idx]);
-				return EXIT_FAILURE;
+				return update_state_file_and_exit(EXIT_FAILURE, &state);
 			}
 
 			iface = entry->data;
 		}
 
 		if (!change_interface(iface, &collection, &state, ifname))
-			return EXIT_FAILURE;
+			return update_state_file_and_exit(EXIT_FAILURE, &state);
 	}
 
-	if (!exec_opts.mock && !lif_state_write_path(&state, exec_opts.state_file))
-	{
-		fprintf(stderr, "%s: could not update %s\n", argv0, exec_opts.state_file);
-		return EXIT_FAILURE;
-	}
-
-	return EXIT_SUCCESS;
+	return update_state_file_and_exit(EXIT_SUCCESS, &state);
 }
 
 struct if_applet ifup_applet = {

cmd/multicall-exec-options.c

@@ -68,13 +68,14 @@ set_no_lock(const char *opt_arg)
 }
 
 static void
-no_op(const char *opt_arg)
+set_force(const char *opt_arg)
 {
 	(void) opt_arg;
+	exec_opts.force = true;
 }
 
 static struct if_option exec_options[] = {
-	{'f', "force", NULL, "force (de)configuration", false, no_op},
+	{'f', "force", NULL, "force (de)configuration", false, set_force},
 	{'i', "interfaces", "interfaces FILE", "use FILE for interface definitions", true, set_interfaces_file},
 	{'l', "no-lock", NULL, "do not use a lockfile to serialize state changes", false, set_no_lock},
 	{'n', "no-act", NULL, "do not actually run any commands", false, set_no_act},

libifupdown/execute.h

@@ -23,6 +23,7 @@ struct lif_execute_opts {
 	bool verbose;
 	bool mock;
 	bool no_lock;
+	bool force;
 	const char *executor_path;
 	const char *interfaces_file;
 	const char *state_file;

libifupdown/interface.h

@@ -53,7 +53,7 @@ struct lif_interface {
 
 	struct lif_dict vars;
 
-	bool is_up;
+	size_t refcount;	/* >= 0 if up, else 0 */
 };
 
 #define LIF_INTERFACE_COLLECTION_FOREACH(iter, collection) \

libifupdown/lifecycle.c

@@ -261,6 +261,36 @@ 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);
+
+#ifdef DEBUG_REFCOUNTING
+	fprintf(stderr, "handle_refcounting(): orig_refcount=%zu, refcount=%zu, direction=%s\n",
+		orig_refcount, iface->refcount, up ? "UP" : "DOWN");
+#endif
+
+	/* if going up and orig_refcount > 0 -- we're already configured. */
+	if (up && orig_refcount > 0)
+		return true;
+
+	/* if going down and iface->refcount > 1 -- we still have other dependents. */
+	if (!up && iface->refcount > 1)
+		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 +308,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->is_up)
+		/* 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 +363,7 @@ lif_lifecycle_run(const struct lif_execute_opts *opts, struct lif_interface *ifa
 
 		lif_state_upsert(state, lifname, iface);
 
-		iface->is_up = true;
+		lif_state_ref_if(state, lifname, iface);
 	}
 	else
 	{
@@ -345,9 +380,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->is_up = false;
+		lif_state_unref_if(state, lifname, iface);
 	}
 
 	return true;

libifupdown/state.c

@@ -13,27 +13,41 @@
  * from the use of this software.
  */
 
+#include <limits.h>
 #include <string.h>
 #include "libifupdown/state.h"
 #include "libifupdown/fgetline.h"
+#include "libifupdown/tokenize.h"
 
 bool
 lif_state_read(struct lif_dict *state, FILE *fd)
 {
 	char linebuf[4096];
+
 	while (lif_fgetline(linebuf, sizeof linebuf, fd))
 	{
-		char *ifname = linebuf;
+		char *bufp = linebuf;
+		char *ifname = lif_next_token(&bufp);
+		char *refcount = lif_next_token(&bufp);
+		size_t rc = 1;
 		char *equals_p = strchr(linebuf, '=');
 
+		if (*refcount)
+		{
+			rc = strtoul(refcount, NULL, 10);
+
+			if (rc == 0 || rc == ULONG_MAX)
+				rc = 1;
+		}
+
 		if (equals_p == NULL)
 		{
-			lif_state_upsert(state, ifname, &(struct lif_interface){ .ifname = ifname });
+			lif_state_upsert(state, ifname, &(struct lif_interface){ .ifname = ifname, .refcount = rc });
 			continue;
 		}
 
 		*equals_p++ = '\0';
-		lif_state_upsert(state, ifname, &(struct lif_interface){ .ifname = equals_p });
+		lif_state_upsert(state, ifname, &(struct lif_interface){ .ifname = equals_p, .refcount = rc });
 	}
 
 	return true;
@@ -55,10 +69,38 @@ lif_state_read_path(struct lif_dict *state, const char *path)
 	return ret;
 }
 
+void
+lif_state_ref_if(struct lif_dict *state, const char *ifname, struct lif_interface *iface)
+{
+	iface->refcount++;
+	lif_state_upsert(state, ifname, iface);
+}
+
+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)
+		lif_state_upsert(state, ifname, iface);
+	else
+		lif_state_delete(state, ifname);
+}
+
 void
 lif_state_upsert(struct lif_dict *state, const char *ifname, struct lif_interface *iface)
 {
-	lif_dict_add(state, ifname, strdup(iface->ifname));
+	lif_state_delete(state, ifname);
+
+	struct lif_state_record *rec = calloc(1, sizeof(*rec));
+
+	rec->mapped_if = strdup(iface->ifname);
+	rec->refcount = iface->refcount;
+
+	lif_dict_add(state, ifname, rec);
 }
 
 void
@@ -69,7 +111,10 @@ lif_state_delete(struct lif_dict *state, const char *ifname)
 	if (entry == NULL)
 		return;
 
-	free(entry->data);
+	struct lif_state_record *rec = entry->data;
+	free(rec->mapped_if);
+	free(rec);
+
 	lif_dict_delete_entry(state, entry);
 }
 
@@ -81,8 +126,9 @@ lif_state_write(const struct lif_dict *state, FILE *f)
 	LIF_DICT_FOREACH(iter, state)
 	{
 		struct lif_dict_entry *entry = iter->data;
+		struct lif_state_record *rec = entry->data;
 
-		fprintf(f, "%s=%s\n", entry->key, (const char *) entry->data);
+		fprintf(f, "%s=%s %zu\n", entry->key, rec->mapped_if, rec->refcount);
 	}
 }
 
@@ -108,7 +154,8 @@ lif_state_lookup(struct lif_dict *state, struct lif_dict *if_collection, const c
 	if (entry == NULL)
 		return NULL;
 
-	struct lif_dict_entry *if_entry = lif_dict_find(if_collection, (const char *) entry->data);
+	struct lif_state_record *rec = entry->data;
+	struct lif_dict_entry *if_entry = lif_dict_find(if_collection, rec->mapped_if);
 
 	if (if_entry == NULL)
 		return NULL;
@@ -124,9 +171,10 @@ lif_state_sync(struct lif_dict *state, struct lif_dict *if_collection)
 	LIF_DICT_FOREACH(iter, state)
 	{
 		struct lif_dict_entry *entry = iter->data;
-		struct lif_interface *iface = lif_interface_collection_find(if_collection, entry->key);
+		struct lif_state_record *rec = entry->data;
+		struct lif_interface *iface = lif_interface_collection_find(if_collection, rec->mapped_if);
 
-		iface->is_up = true;
+		iface->refcount = rec->refcount;
 	}
 
 	return true;

libifupdown/state.h

@@ -19,9 +19,16 @@
 #include <stdio.h>
 #include "libifupdown/interface.h"
 
+struct lif_state_record {
+	char *mapped_if;
+	size_t refcount;
+};
+
 extern bool lif_state_read(struct lif_dict *state, FILE *f);
 extern bool lif_state_read_path(struct lif_dict *state, const char *path);
 extern void lif_state_upsert(struct lif_dict *state, const char *ifname, struct lif_interface *iface);
+extern void lif_state_ref_if(struct lif_dict *state, const char *ifname, struct lif_interface *iface);
+extern void lif_state_unref_if(struct lif_dict *state, const char *ifname, struct lif_interface *iface);
 extern void lif_state_delete(struct lif_dict *state, const char *ifname);
 extern void lif_state_write(const struct lif_dict *state, FILE *f);
 extern bool lif_state_write_path(const struct lif_dict *state, const char *path);

tests/fixtures/deferred-teardown-1.ifstate

@@ -0,0 +1,6 @@
+lo=lo 1
+br0=br0 1
+bond0=bond0 2
+eth0=eth0 3
+eth1=eth1 2
+tun0=tun0 1

tests/fixtures/deferred-teardown-1.interfaces

@@ -0,0 +1,9 @@
+auto br0
+iface br0
+	requires bond0
+
+iface bond0
+	requires eth0 eth1
+
+iface tun0
+	requires eth0

tests/fixtures/deferred-teardown-2.ifstate

@@ -0,0 +1,5 @@
+eth0=eth0 5
+tun0=tun0 1
+tun1=tun1 1
+tun2=tun2 1
+tun3=tun3 1

tests/fixtures/deferred-teardown-2.interfaces

@@ -0,0 +1,11 @@
+iface tun0
+	requires eth0
+
+iface tun1
+	requires eth0
+
+iface tun2
+	requires eth0
+
+iface tun3
+	requires eth0

tests/ifdown_test

@@ -19,6 +19,9 @@ tests_init \
 	learned_dependency_2 \
 	learned_executor \
 	implicit_vlan \
+	deferred_teardown_1 \
+	deferred_teardown_2 \
+	deferred_teardown_3 \
 	regress_opt_f
 
 noargs_body() {
@@ -27,56 +30,56 @@ noargs_body() {
 
 lo_always_auto_body() {
 	atf_check -s exit:0 -e ignore -o match:'executors/link' \
-		ifdown -S/dev/null -E $EXECUTORS -i/dev/null -n -a
+		ifdown -f -S/dev/null -E $EXECUTORS -i/dev/null -n -a
 }
 
 dual_stack_body() {
 	atf_check -s exit:0 -e ignore \
 		-o match:'executors/link' \
 		-o match:'executors/static' \
-		ifdown -S/dev/null -E $EXECUTORS -i $FIXTURES/static-eth0.interfaces -n -a
+		ifdown -f -S/dev/null -E $EXECUTORS -i $FIXTURES/static-eth0.interfaces -n -a
 }
 
 static_ipv4_body() {
 	atf_check -s exit:0 -e ignore \
 		-o match:'executors/link' \
 		-o match:'executors/static' \
-		ifdown -S/dev/null -E $EXECUTORS -i $FIXTURES/static-eth0-v4.interfaces -n -a
+		ifdown -f -S/dev/null -E $EXECUTORS -i $FIXTURES/static-eth0-v4.interfaces -n -a
 }
 
 static_ipv4_netmask_body() {
 	atf_check -s exit:0 -e ignore \
 		-o match:'executors/link' \
 		-o match:'executors/static' \
-		ifdown -S/dev/null -E $EXECUTORS -i $FIXTURES/static-eth0-v4-netmask.interfaces -n -a
+		ifdown -f -S/dev/null -E $EXECUTORS -i $FIXTURES/static-eth0-v4-netmask.interfaces -n -a
 }
 
 static_ipv6_body() {
 	atf_check -s exit:0 -e ignore \
 		-o match:'executors/link' \
 		-o match:'executors/static' \
-		ifdown -S/dev/null -E $EXECUTORS -i $FIXTURES/static-eth0-v6.interfaces -n -a
+		ifdown -f -S/dev/null -E $EXECUTORS -i $FIXTURES/static-eth0-v6.interfaces -n -a
 }
 
 static_ipv6_netmask_body() {
 	atf_check -s exit:0 -e ignore \
 		-o match:'executors/link' \
 		-o match:'executors/static' \
-		ifdown -S/dev/null -E $EXECUTORS -i $FIXTURES/static-eth0-v6-netmask.interfaces -n -a
+		ifdown -f -S/dev/null -E $EXECUTORS -i $FIXTURES/static-eth0-v6-netmask.interfaces -n -a
 }
 
 inet_dhcp_body() {
 	atf_check -s exit:0 -e ignore \
 		-o match:'executors/link' \
 		-o match:'executors/dhcp' \
-		ifdown -S/dev/null -E $EXECUTORS -i $FIXTURES/dhcp-eth0.interfaces -n -a
+		ifdown -f -S/dev/null -E $EXECUTORS -i $FIXTURES/dhcp-eth0.interfaces -n -a
 }
 
 use_dhcp_body() {
 	atf_check -s exit:0 -e ignore \
 		-o match:'executors/link' \
 		-o match:'executors/dhcp' \
-		ifdown -S/dev/null -E $EXECUTORS -i $FIXTURES/use-dhcp-eth0.interfaces -n -a
+		ifdown -f -S/dev/null -E $EXECUTORS -i $FIXTURES/use-dhcp-eth0.interfaces -n -a
 }
 
 alias_eth0_home_body() {
@@ -142,6 +145,33 @@ implicit_vlan_body() {
 		ifdown -n -S $FIXTURES/vlan.ifstate -E $EXECUTORS -i $FIXTURES/vlan.interfaces eth0.8
 }
 
+deferred_teardown_1_body() {
+	atf_check -s exit:0 -o ignore \
+		-e match:"skipping dependent interface eth0 \\(of bond0\\) -- transient dependencies still exist" \
+		-e match:"changing state of dependent interface eth1 \\(of bond0\\) to down" \
+		ifdown -n -S $FIXTURES/deferred-teardown-1.ifstate -E $EXECUTORS \
+			-i $FIXTURES/deferred-teardown-1.interfaces br0
+}
+
+deferred_teardown_2_body() {
+	atf_check -s exit:0 -o ignore \
+		-e match:"skipping dependent interface eth0 \\(of tun0\\) -- transient dependencies still exist" \
+		-e match:"skipping dependent interface eth0 \\(of tun1\\) -- transient dependencies still exist" \
+		-e match:"skipping dependent interface eth0 \\(of tun2\\) -- transient dependencies still exist" \
+		ifdown -n -S $FIXTURES/deferred-teardown-2.ifstate -E $EXECUTORS \
+			-i $FIXTURES/deferred-teardown-2.interfaces tun0 tun1 tun2
+}
+
+deferred_teardown_3_body() {
+	atf_check -s exit:0 -o ignore \
+		-e match:"skipping dependent interface eth0 \\(of tun0\\) -- transient dependencies still exist" \
+		-e match:"skipping dependent interface eth0 \\(of tun1\\) -- transient dependencies still exist" \
+		-e match:"skipping dependent interface eth0 \\(of tun2\\) -- transient dependencies still exist" \
+		-e match:"changing state of dependent interface eth0 \\(of tun3\\) to down" \
+		ifdown -n -S $FIXTURES/deferred-teardown-2.ifstate -E $EXECUTORS \
+			-i $FIXTURES/deferred-teardown-2.interfaces tun0 tun1 tun2 tun3
+}
+
 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