*: 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>

diff --git a/lib/if.c b/lib/if.c
index 590893b68c372d1cf8202fb955d956e06f98293b..c704ea9bc9b3e8f3f0619e871eb9008930c3edd7 100644 (file)
--- a/lib/if.c
+++ b/lib/if.c
@@ -141,25 +141,16 @@ 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,
-                                          vrf_id_t vrf_id)
+static struct interface *if_new(vrf_id_t vrf_id)
 {
-       struct vrf *vrf = vrf_get(vrf_id, NULL);
        struct interface *ifp;
 
        ifp = XCALLOC(MTYPE_IF, sizeof(struct interface));
-       ifp->vrf_id = vrf_id;
 
-       if (name) {
-               strlcpy(ifp->name, name, sizeof(ifp->name));
-               IFNAME_RB_INSERT(vrf, ifp);
-       } else
-               ifp->name[0] = '\0';
+       ifp->ifindex = IFINDEX_INTERNAL;
+       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->vrf_id = vrf_id;
 
        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)
 {

diff --git a/lib/if.h b/lib/if.h
index 51db7bb8ed1435ab6e9169ebbbd2631250b31717..1ee6561e8d481b5900e1fcf0c1575bc80d2b9724 100644 (file)
--- a/lib/if.h
+++ b/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

diff --git a/ospfd/ospf_interface.c b/ospfd/ospf_interface.c
index 3407d1bad145f97744e91f8d759ee65147ee8e8f..1f5e0da9440f84494913e9799b8a17506cb5887b 100644 (file)
--- a/ospfd/ospf_interface.c
+++ b/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);

diff --git a/pimd/pim_iface.c b/pimd/pim_iface.c
index bc8dedc4f6f01bf5cf8722672ea5535133f6aac8..3ee9caebcf6c17d3fe1890d95038b690fe8db817 100644 (file)
--- a/pimd/pim_iface.c
+++ b/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,

diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c
index 35cb3a6f5f00f9f0083feb7b6dda0be6aab26cc7..4e9b093610f7e02a7a3784753d690c2ad5805e37 100644 (file)
--- a/zebra/if_netlink.c
+++ b/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;

diff --git a/zebra/kernel_socket.c b/zebra/kernel_socket.c
index 60fbbcc0598f4eb8e0fefd1be2cee804f0286a1b..f5aca2341deeb2262189c8a558c8da7a118b9113 100644 (file)
--- a/zebra/kernel_socket.c
+++ b/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);