lib: handle failure to change ifindex

This fixes a theoretical bug that could occur when trying to change an
ifindex on an interface to that of an existing interface. We would
remove the interface from the ifindex tree, and change the ifindex, but
when we tried to reinsert the interface, the insert would fail. It was
impossible to know if this failed due to the insertion / deletion macros
capturing the result value of the underlying BSD tree macros. So we
would effectively delete the interface.

Instead of failing on insert, we just check if the prospective ifindex
already exists and return -1 if it does.

Macros have been changed to statement expressions so the result can be
checked, and bubbled up.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

1 files changed, 21 insertions, 5 deletions

diff --git a/lib/if.c b/lib/if.c
index d4d9c4a5a4..cc964106d0 100644
--- a/lib/if.c
+++ b/lib/if.c
@@ -582,23 +582,39 @@ struct interface *if_get_by_ifindex(ifindex_t ifindex, vrf_id_t vrf_id)
 	return NULL;
 }
 
-void if_set_index(struct interface *ifp, ifindex_t ifindex)
+int if_set_index(struct interface *ifp, ifindex_t ifindex)
 {
 	struct vrf *vrf;
 
+	if (ifp->ifindex == ifindex)
+		return 0;
+
 	vrf = vrf_get(ifp->vrf_id, NULL);
 	assert(vrf);
 
-	if (ifp->ifindex == ifindex)
-		return;
+	/*
+	 * If there is already an interface with this ifindex, we will collide
+	 * on insertion, so don't even try.
+	 */
+	if (if_lookup_by_ifindex(ifindex, ifp->vrf_id))
+		return -1;
 
 	if (ifp->ifindex != IFINDEX_INTERNAL)
 		IFINDEX_RB_REMOVE(vrf, ifp);
 
 	ifp->ifindex = ifindex;
 
-	if (ifp->ifindex != IFINDEX_INTERNAL)
-		IFINDEX_RB_INSERT(vrf, ifp)
+	if (ifp->ifindex != IFINDEX_INTERNAL) {
+		/*
+		 * This should never happen, since we checked if there was
+		 * already an interface with the desired ifindex at the top of
+		 * the function. Nevertheless.
+		 */
+		if (IFINDEX_RB_INSERT(vrf, ifp))
+			return -1;
+	}
+
+	return 0;
 }
 
 void if_set_name(struct interface *ifp, const char *name)