*: Cleanup interface creation apis

Cleanup the interface creation apis to make it more
clear what they are doing.

Make it explicit that the creation via name/ifindex will
only add it to the appropriate list.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>

lib/if.c

@@ -141,26 +141,17 @@ static int if_cmp_index_func(const struct interface *ifp1,
 }
 
 /* Create new interface structure. */
-static struct interface *if_create_backend(const char *name, ifindex_t ifindex,
+static struct interface *if_new(vrf_id_t vrf_id)
-					   vrf_id_t vrf_id)
 {
-	struct vrf *vrf = vrf_get(vrf_id, NULL);
 	struct interface *ifp;
 
 	ifp = XCALLOC(MTYPE_IF, sizeof(struct interface));
+
+	ifp->ifindex = IFINDEX_INTERNAL;
+	ifp->name[0] = '\0';
+
 	ifp->vrf_id = vrf_id;
 
-	if (name) {
-		strlcpy(ifp->name, name, sizeof(ifp->name));
-		IFNAME_RB_INSERT(vrf, ifp);
-	} else
-		ifp->name[0] = '\0';
-
-	if (ifindex != IFINDEX_INTERNAL)
-		if_set_index(ifp, ifindex);
-	else
-		ifp->ifindex = ifindex; /* doesn't add it to the list */
-
 	ifp->connected = list_new();
 	ifp->connected->del = (void (*)(void *))connected_free;
 
@@ -171,7 +162,6 @@ static struct interface *if_create_backend(const char *name, ifindex_t ifindex,
 	SET_FLAG(ifp->status, ZEBRA_INTERFACE_LINKDETECTION);
 
 	QOBJ_REG(ifp, interface);
-	hook_call(if_add, ifp);
 	return ifp;
 }
 
@@ -203,14 +193,28 @@ void if_down_via_zapi(struct interface *ifp)
 		(*ifp_master.down_hook)(ifp);
 }
 
-struct interface *if_create(const char *name, vrf_id_t vrf_id)
+struct interface *if_create_name(const char *name, vrf_id_t vrf_id)
 {
-	return if_create_backend(name, IFINDEX_INTERNAL, vrf_id);
+	struct interface *ifp;
+
+	ifp = if_new(vrf_id);
+
+	if_set_name(ifp, name);
+
+	hook_call(if_add, ifp);
+	return ifp;
 }
 
 struct interface *if_create_ifindex(ifindex_t ifindex, vrf_id_t vrf_id)
 {
-	return if_create_backend(NULL, ifindex, vrf_id);
+	struct interface *ifp;
+
+	ifp = if_new(vrf_id);
+
+	if_set_index(ifp, ifindex);
+
+	hook_call(if_add, ifp);
+	return ifp;
 }
 
 /* Create new interface structure. */
@@ -516,7 +520,7 @@ struct interface *if_get_by_name(const char *name, vrf_id_t vrf_id)
 		ifp = if_lookup_by_name(name, vrf_id);
 		if (ifp)
 			return ifp;
-		return if_create(name, vrf_id);
+		return if_create_name(name, vrf_id);
 	case VRF_BACKEND_VRF_LITE:
 		ifp = if_lookup_by_name_all_vrf(name);
 		if (ifp) {
@@ -528,7 +532,7 @@ struct interface *if_get_by_name(const char *name, vrf_id_t vrf_id)
 			if_update_to_new_vrf(ifp, vrf_id);
 			return ifp;
 		}
-		return if_create(name, vrf_id);
+		return if_create_name(name, vrf_id);
 	}
 
 	return NULL;
@@ -566,14 +570,14 @@ void if_set_index(struct interface *ifp, ifindex_t ifindex)
 {
 	struct vrf *vrf;
 
-	vrf = vrf_lookup_by_id(ifp->vrf_id);
+	vrf = vrf_get(ifp->vrf_id, NULL);
 	assert(vrf);
 
 	if (ifp->ifindex == ifindex)
 		return;
 
 	if (ifp->ifindex != IFINDEX_INTERNAL)
-		IFINDEX_RB_REMOVE(vrf, ifp)
+		IFINDEX_RB_REMOVE(vrf, ifp);
 
 	ifp->ifindex = ifindex;
 
@@ -581,6 +585,25 @@ void if_set_index(struct interface *ifp, ifindex_t ifindex)
 		IFINDEX_RB_INSERT(vrf, ifp)
 }
 
+void if_set_name(struct interface *ifp, const char *name)
+{
+	struct vrf *vrf;
+
+	vrf = vrf_get(ifp->vrf_id, NULL);
+	assert(vrf);
+
+	if (if_cmp_name_func(ifp->name, name) == 0)
+		return;
+
+	if (ifp->name[0] != '\0')
+		IFNAME_RB_REMOVE(vrf, ifp);
+
+	strlcpy(ifp->name, name, sizeof(ifp->name));
+
+	if (ifp->name[0] != '\0')
+		IFNAME_RB_INSERT(vrf, ifp);
+}
+
 /* Does interface up ? */
 int if_is_up(const struct interface *ifp)
 {

lib/if.h

@@ -479,7 +479,11 @@ extern int if_cmp_name_func(const char *p1, const char *p2);
  * else think before you use VRF_UNKNOWN
  */
 extern void if_update_to_new_vrf(struct interface *, vrf_id_t vrf_id);
-extern struct interface *if_create(const char *name, vrf_id_t vrf_id);
+
+/* Create new interface, adds to name list only */
+extern struct interface *if_create_name(const char *name, vrf_id_t vrf_id);
+
+/* Create new interface, adds to index list only */
 extern struct interface *if_create_ifindex(ifindex_t ifindex, vrf_id_t vrf_id);
 extern struct interface *if_lookup_by_index(ifindex_t, vrf_id_t vrf_id);
 extern struct interface *if_lookup_by_index_all_vrf(ifindex_t);
@@ -492,13 +496,15 @@ extern struct interface *if_lookup_prefix(struct prefix *prefix,
 size_t if_lookup_by_hwaddr(const uint8_t *hw_addr, size_t addrsz,
 			   struct interface ***result, vrf_id_t vrf_id);
 
-/* These 3 functions are to be used when the ifname argument is terminated
-   by a '\0' character: */
 extern struct interface *if_lookup_by_name_all_vrf(const char *ifname);
 extern struct interface *if_lookup_by_name(const char *ifname, vrf_id_t vrf_id);
 extern struct interface *if_get_by_name(const char *ifname, vrf_id_t vrf_id);
 extern struct interface *if_get_by_ifindex(ifindex_t ifindex, vrf_id_t vrf_id);
+
+/* Sets the index and adds to index list */
 extern void if_set_index(struct interface *ifp, ifindex_t ifindex);
+/* Sets the name and adds to name list */
+extern void if_set_name(struct interface *ifp, const char *name);
 
 /* Delete the interface, but do not free the structure, and leave it in the
    interface list.  It is often advisable to leave the pseudo interface

ospfd/ospf_interface.c

@@ -847,9 +847,9 @@ struct ospf_interface *ospf_vl_new(struct ospf *ospf,
 			ospf->vrf_id);
 
 	snprintf(ifname, sizeof(ifname), "VLINK%u", vlink_count);
-	vi = if_create(ifname, ospf->vrf_id);
+	vi = if_create_name(ifname, ospf->vrf_id);
 	/*
-	 * if_create sets ZEBRA_INTERFACE_LINKDETECTION
+	 * if_create_name sets ZEBRA_INTERFACE_LINKDETECTION
 	 * virtual links don't need this.
 	 */
 	UNSET_FLAG(vi->status, ZEBRA_INTERFACE_LINKDETECTION);

pimd/pim_iface.c

@@ -1476,7 +1476,7 @@ void pim_if_create_pimreg(struct pim_instance *pim)
 			snprintf(pimreg_name, sizeof(pimreg_name), "pimreg%u",
 				 pim->vrf->data.l.table_id);
 
-		pim->regiface = if_create(pimreg_name, pim->vrf_id);
+		pim->regiface = if_create_name(pimreg_name, pim->vrf_id);
 		pim->regiface->ifindex = PIM_OIF_PIM_REGISTER_VIF;
 
 		pim_if_new(pim->regiface, false, false, true,

zebra/if_netlink.c

@@ -606,7 +606,6 @@ static int netlink_interface(struct nlmsghdr *h, ns_id_t ns_id, int startup)
 	ifindex_t link_ifindex = IFINDEX_INTERNAL;
 	ifindex_t bond_ifindex = IFINDEX_INTERNAL;
 	struct zebra_if *zif;
-	struct vrf *vrf = NULL;
 
 	zns = zebra_ns_lookup(ns_id);
 	ifi = NLMSG_DATA(h);
@@ -690,7 +689,6 @@ static int netlink_interface(struct nlmsghdr *h, ns_id_t ns_id, int startup)
 	if (tb[IFLA_LINK])
 		link_ifindex = *(ifindex_t *)RTA_DATA(tb[IFLA_LINK]);
 
-	vrf = vrf_get(vrf_id, NULL);
 	/* Add interface.
 	 * We add by index first because in some cases such as the master
 	 * interface, we have the index before we have the name. Fixing
@@ -699,8 +697,9 @@ static int netlink_interface(struct nlmsghdr *h, ns_id_t ns_id, int startup)
 	 */
 	ifp = if_get_by_ifindex(ifi->ifi_index, vrf_id);
 	set_ifindex(ifp, ifi->ifi_index, zns); /* add it to ns struct */
-	strlcpy(ifp->name, name, sizeof(ifp->name));
+
-	IFNAME_RB_INSERT(vrf, ifp);
+	if_set_name(ifp, name);
+
 	ifp->flags = ifi->ifi_flags & 0x0000fffff;
 	ifp->mtu6 = ifp->mtu = *(uint32_t *)RTA_DATA(tb[IFLA_MTU]);
 	ifp->metric = 0;

zebra/kernel_socket.c

@@ -643,7 +643,7 @@ int ifm_read(struct if_msghdr *ifm)
 		if (ifp == NULL) {
 			/* Interface that zebra was not previously aware of, so
 			 * create. */
-			ifp = if_create(ifname, VRF_DEFAULT);
+			ifp = if_create_name(ifname, VRF_DEFAULT);
 			if (IS_ZEBRA_DEBUG_KERNEL)
 				zlog_debug("%s: creating ifp for ifindex %d",
 					   __func__, ifm->ifm_index);