confd: Always generate a valid interface DAG

Fix #938

Before this change, interface dependencies were only setup when
interfaces were created. This led to problems in flows like this:

1.
    configure
        set interface br0
        set interface br0.10
        leave

2.
    configure
        set interface dummy0
        leave

2.
    configure
        no interface br0
        no interface br0.10
        leave

Since neither br0 nor br0.10 was created in (2), no dependencies were
setup. Because of that, when the deletion scripts from generation (2)
are generated when entering (3), dagger is free to execute them in any
order. This means that it may choose to remove br0 before br0.10,
which will mean that br0.10 is already gone when we try to remove it.

Therefore, make sure that we generate the full dependency graph on
every iteration.

Also, get rid of the need to keep on-disk state for VETH pairs, and
use the name as a descriminator for which side to create/delete.

src/confd/src/dagger.c

@@ -155,26 +155,6 @@ int dagger_evolve_or_abandon(struct dagger *d)
 	return err;
 }
 
-void dagger_skip_iface(struct dagger *d, const char *ifname)
-{
-	touchf("%s/%d/skip/%s", d->path, d->next, ifname);
-}
-
-void dagger_skip_current_iface(struct dagger *d, const char *ifname)
-{
-	touchf("%s/%d/skip/%s", d->path, d->current, ifname);
-}
-
-int dagger_should_skip(struct dagger *d, const char *ifname)
-{
-	return fexistf("%s/%d/skip/%s", d->path, d->next, ifname);
-}
-
-int dagger_should_skip_current(struct dagger *d, const char *ifname)
-{
-	return fexistf("%s/%d/skip/%s", d->path, d->current, ifname);
-}
-
 int dagger_is_bootstrap(struct dagger *d)
 {
 	return d->next == 0;

src/confd/src/dagger.h

@@ -27,10 +27,6 @@ int dagger_abandon(struct dagger *d);
 int dagger_evolve(struct dagger *d);
 int dagger_evolve_or_abandon(struct dagger *d);
 
-void dagger_skip_iface(struct dagger *d, const char *ifname);
-void dagger_skip_current_iface(struct dagger *d, const char *ifname);
-int dagger_should_skip(struct dagger *d, const char *ifname);
-int dagger_should_skip_current(struct dagger *d, const char *ifname);
 int dagger_is_bootstrap(struct dagger *d);
 
 int dagger_claim(struct dagger *d, const char *path);

src/confd/src/ietf-interfaces.c

@@ -482,40 +482,36 @@ static bool netdag_must_del(struct lyd_node *dif, struct lyd_node *cif)
 }
 
 static int netdag_gen_iface_del(struct dagger *net, struct lyd_node *dif,
-				       struct lyd_node *cif, bool fixed)
+				struct lyd_node *cif)
 {
 	const char *ifname = lydx_get_cattr(dif, "name");
 	FILE *ip;
 
 	DEBUG_IFACE(dif, "");
 
-	if (dagger_should_skip_current(net, ifname))
-		return 0;
-
-	if (iftype_from_iface(cif) == IFT_VETH) {
-		struct lyd_node *node;
-		const char *peer;
-
-		node = lydx_get_descendant(lyd_child(dif), "veth", NULL);
-		if (!node)
-			return -EINVAL;
-
-		peer = lydx_get_cattr(node, "peer");
-		if (!peer)
-			return -EINVAL;
-
-		dagger_skip_current_iface(net, peer);
-	}
-
 	ip = dagger_fopen_net_exit(net, ifname, NETDAG_EXIT, "exit.ip");
 	if (!ip)
 		return -EIO;
 
-	if (fixed) {
+	switch (iftype_from_iface(dif)) {
+	case IFT_ETH:
+	case IFT_ETHISH:
 		fprintf(ip, "link set dev %s down\n", ifname);
 		fprintf(ip, "addr flush dev %s\n", ifname);
-	} else {
+		break;
+	case IFT_VETH:
+		if (!veth_is_primary(dif))
+			break;
+		/* fallthrough */
+	case IFT_BRIDGE:
+	case IFT_DUMMY:
+	case IFT_GRE:
+	case IFT_GRETAP:
+	case IFT_VLAN:
+	case IFT_VXLAN:
+	case IFT_UNKNOWN:
 		fprintf(ip, "link del dev %s\n", ifname);
+		break;
 	}
 
 	fclose(ip);
@@ -545,7 +541,7 @@ static sr_error_t netdag_gen_iface(sr_session_ctx_t *session, struct dagger *net
 	      (op == LYDX_OP_NONE) ? "mod" : ((op == LYDX_OP_CREATE) ? "add" : "del"));
 
 	if (op == LYDX_OP_DELETE) {
-		err  = netdag_gen_iface_del(net, dif, cif, fixed);
+		err  = netdag_gen_iface_del(net, dif, cif);
 		err += netdag_gen_ipv4_autoconf(net, cif, dif);
 		goto err;
 	}
@@ -559,7 +555,7 @@ static sr_error_t netdag_gen_iface(sr_session_ctx_t *session, struct dagger *net
 	if (op != LYDX_OP_CREATE && netdag_must_del(dif, cif)) {
 		DEBUG_IFACE(dif, "Must delete");
 
-		err = netdag_gen_iface_del(net, dif, cif, fixed);
+		err = netdag_gen_iface_del(net, dif, cif);
 		if (err)
 			goto err;
 
@@ -636,14 +632,48 @@ err:
 	return err ? SR_ERR_INTERNAL : SR_ERR_OK;
 }
 
+static int netdag_init_iface(struct lyd_node *cif)
+{
+	int err;
+
+	err = dagger_add_node(&confd.netdag, lydx_get_cattr(cif, "name"));
+	if (err)
+		return err;
+
+	switch (iftype_from_iface(cif)) {
+	case IFT_BRIDGE:
+		return bridge_add_deps(cif);
+	/* case IFT_LAG: */
+	/* 	return lag_add_deps(cif); */
+	case IFT_VLAN:
+		return vlan_add_deps(cif);
+	case IFT_VETH:
+		return veth_add_deps(cif);
+
+	case IFT_DUMMY:
+	case IFT_ETH:
+	case IFT_ETHISH:
+	case IFT_GRE:
+	case IFT_GRETAP:
+	case IFT_VXLAN:
+	case IFT_UNKNOWN:
+		break;
+	}
+
+	return 0;
+}
+
 static sr_error_t netdag_init(sr_session_ctx_t *session, struct dagger *net,
 			      struct lyd_node *cifs, struct lyd_node *difs)
 {
-	struct lyd_node *iface;
+	struct lyd_node *cif;
+	int err;
 
-	LYX_LIST_FOR_EACH(cifs, iface, "interface")
-		if (dagger_add_node(net, lydx_get_cattr(iface, "name")))
+	LYX_LIST_FOR_EACH(cifs, cif, "interface") {
+		err = netdag_init_iface(cif);
+		if (err)
 			return SR_ERR_INTERNAL;
+	}
 
 	net->session = session;
 	return SR_ERR_OK;

src/confd/src/ietf-interfaces.h

@@ -97,20 +97,24 @@ int netdag_gen_ip_addrs(struct dagger *net, FILE *ip, const char *proto,
 /* infix-if-bridge.c */
 int bridge_mstpd_gen(struct lyd_node *cifs);
 int bridge_gen(struct lyd_node *dif, struct lyd_node *cif, FILE *ip, int add);
+int bridge_add_deps(struct lyd_node *cif);
 /* infix-if-bridge-mcd.c */
 int bridge_mcd_gen(struct lyd_node *cifs);
 /* infix-if-bridge-port.c */
 int bridge_port_gen(struct lyd_node *dif, struct lyd_node *cif, FILE *ip);
 
 /* infix-if-veth.c */
+bool veth_is_primary(struct lyd_node *cif);
 int ifchange_cand_infer_veth(sr_session_ctx_t *session, const char *path);
 int netdag_gen_veth(struct dagger *net, struct lyd_node *dif,
 		    struct lyd_node *cif, FILE *ip);
+int veth_add_deps(struct lyd_node *cif);
 
 /* infix-if-vlan.c */
 int ifchange_cand_infer_vlan(sr_session_ctx_t *session, const char *path);
 int netdag_gen_vlan(struct dagger *net, struct lyd_node *dif,
 		    struct lyd_node *cif, FILE *ip);
+int vlan_add_deps(struct lyd_node *cif);
 
 /* infix-if-gre.c */
 int gre_gen(struct dagger *net, struct lyd_node *dif,

src/confd/src/infix-if-bridge-port.c

@@ -198,7 +198,6 @@ static int gen_link(struct lyd_node *dif, struct lyd_node *cif)
 	const char *brname, *iface;
 	int mrouter;
 	FILE *next;
-	int err;
 
 	if (!lydx_get_child(dif, "bridge-port"))
 		return 0;
@@ -214,10 +213,6 @@ static int gen_link(struct lyd_node *dif, struct lyd_node *cif)
 
 	iface = lydx_get_cattr(cif, "name");
 
-	err = dagger_add_dep(&confd.netdag, brname, iface);
-	if (err)
-		return ERR_IFACE(cif, err, "Unable to add dep \"%s\" to %s", iface, brname);
-
 	next = dagger_fopen_net_init(&confd.netdag, brname,
 				     NETDAG_INIT_LOWERS, "add-ports.ip");
 	if (!next)

src/confd/src/infix-if-bridge.c

@@ -535,4 +535,31 @@ out:
 	return err;
 }
 
+int bridge_add_deps(struct lyd_node *cif)
+{
+	const char *brname = lydx_get_cattr(cif, "name");
+	struct ly_set *brports;
+	const char *portname;
+	int err = 0;
+	uint32_t i;
+
+	brports = lydx_find_xpathf(cif, "../interface[bridge-port/bridge='%s']", brname);
+	if (!brports)
+		return ERR_IFACE(cif, -ENOENT, "Unable to fetch bridge ports");
+
+
+	for (i = 0; i < brports->count; i++) {
+		portname = lydx_get_cattr(brports->dnodes[i], "name");
+
+		err = dagger_add_dep(&confd.netdag, brname, portname);
+		if (err) {
+			ERR_IFACE(cif, err, "Unable to depend on \"%s\"", portname);
+			break;
+		}
+	}
+
+	ly_set_free(brports, NULL);
+	return err;
+}
+
 /* BR */

src/confd/src/infix-if-veth.c

@@ -12,6 +12,34 @@
 
 #include "ietf-interfaces.h"
 
+/*
+ * While the kernel atomically creates/destroys the pair, in `running`
+ * the two sides are distinct interfaces. So we need to figure out
+ * which one is going to create/delete the other - i.e. which side is
+ * the "primary"
+ */
+bool veth_is_primary(struct lyd_node *cif)
+{
+	struct lyd_node *peer, *veth;
+	const char *peername;
+
+	veth = lydx_get_child(cif, "veth");
+	peername = lydx_get_cattr(veth, "peer");
+	peer = lydx_find_by_name(lyd_parent(cif), "interface", peername);
+
+	/* At the moment, CNI code relies on one side of the pair
+	 * remaining in the host namespace, and that that interface
+	 * takes care of creating the pair.
+	 */
+	if (lydx_get_child(cif, "container-network"))
+		return false;
+	if (lydx_get_child(peer, "container-network"))
+		return true;
+
+	return strcmp(lydx_get_cattr(cif, "name"),
+		      lydx_get_cattr(veth, "peer")) < 0;
+}
+
 int ifchange_cand_infer_veth(sr_session_ctx_t *session, const char *path)
 {
 	char *ifname, *type, *peer, *xpath, *val;
@@ -67,36 +95,47 @@ int netdag_gen_veth(struct dagger *net, struct lyd_node *dif,
 		    struct lyd_node *cif, FILE *ip)
 {
 	const char *ifname = lydx_get_cattr(cif, "name");
+	char ifname_args[64] = "", peer_args[64] = "";
+	const char *mac, *peer;
 	struct lyd_node *node;
-	const char *peer;
-	int err;
+
+	if (!veth_is_primary(cif))
+		return 0;
 
 	node = lydx_get_descendant(lyd_child(cif), "veth", NULL);
 	if (!node)
 		return -EINVAL;
 
 	peer = lydx_get_cattr(node, "peer");
-	if (dagger_should_skip(net, ifname)) {
-		err = dagger_add_dep(net, ifname, peer);
-		if (err)
-			return ERR_IFACE(cif, err, "Unable to add dep \"%s\" to %s", peer, ifname);
-	} else {
-		char ifname_args[64] = "", peer_args[64] = "";
-		const char *mac;
 
-		dagger_skip_iface(net, peer);
+	mac = get_phys_addr(dif, NULL);
+	if (mac)
+		snprintf(ifname_args, sizeof(ifname_args), "address %s", mac);
 
-		mac = get_phys_addr(dif, NULL);
-		if (mac)
-			snprintf(ifname_args, sizeof(ifname_args), "address %s", mac);
+	node = lydx_find_by_name(lyd_parent(cif), "interface", peer);
+	if (node && (mac = get_phys_addr(node, NULL)))
+		snprintf(peer_args, sizeof(peer_args), "address %s", mac);
 
-		node = lydx_find_by_name(lyd_parent(cif), "interface", peer);
-		if (node && (mac = get_phys_addr(node, NULL)))
-			snprintf(peer_args, sizeof(peer_args), "address %s", mac);
+	fprintf(ip, "link add dev %s %s type veth peer %s %s\n",
+		ifname, ifname_args, peer, peer_args);
 
-		fprintf(ip, "link add dev %s %s type veth peer %s %s\n",
-			ifname, ifname_args, peer, peer_args);
-	}
+	return 0;
+}
+
+int veth_add_deps(struct lyd_node *cif)
+{
+	struct lyd_node *veth = lydx_get_child(cif, "veth");
+	const char *peer;
+	int err;
+
+	if (veth_is_primary(cif))
+		return 0;
+
+	peer = lydx_get_cattr(veth, "peer");
+
+	err = dagger_add_dep(&confd.netdag, lydx_get_cattr(cif, "name"), peer);
+	if (err)
+		return ERR_IFACE(cif, err, "Unable to depend on \"%s\"", peer);
 
 	return 0;
 }

src/confd/src/infix-if-vlan.c

@@ -160,11 +160,6 @@ int netdag_gen_vlan(struct dagger *net, struct lyd_node *dif,
 	lower_if = lydx_get_cattr(vlan, "lower-layer-if");
 	DEBUG("ifname %s lower if %s\n", ifname, lower_if);
 
-	err = dagger_add_dep(net, ifname, lower_if);
-	if (err)
-		return ERR_IFACE(cif, err, "Unable to add dep \"%s\"", lower_if);
-
-
 	fprintf(ip, "link add dev %s down link %s type vlan", ifname, lower_if);
 
 	if (lydx_get_diff(lydx_get_child(vlan, "tag-type"), &typed)) {
@@ -190,3 +185,18 @@ int netdag_gen_vlan(struct dagger *net, struct lyd_node *dif,
 
 	return 0;
 }
+
+int vlan_add_deps(struct lyd_node *cif)
+{
+	struct lyd_node *vlan = lydx_get_child(cif, "vlan");
+	const char *lower;
+	int err;
+
+	lower = lydx_get_cattr(vlan, "lower-layer-if");
+
+	err = dagger_add_dep(&confd.netdag, lydx_get_cattr(cif, "name"), lower);
+	if (err)
+		return ERR_IFACE(cif, err, "Unable to depend on \"%s\"", lower);
+
+	return 0;
+}