lib/zebra: put vrf_get() on a diet

Also, for some reason we had two functions to search a VRF by its name:
zebra_vrf_lookup_by_name() and zebra_vrf_list_lookup_by_name().  The first
one would loop through vrf_table and the other one through vrf_list. This
is not necessary anymore, so remove zebra_vrf_lookup_by_name() and rename
zebra_vrf_list_lookup_by_name() to zebra_vrf_lookup_by_name().

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

diff --git a/lib/vrf.c b/lib/vrf.c
index 3def16151ad67ebcae3b71fcadfc6ba979ab939d..aa1bfeeabda3fa2f94f092ddb1e5edc537d12ffa 100644 (file)
--- a/lib/vrf.c
+++ b/lib/vrf.c
@@ -64,7 +64,7 @@ static void vrf_disable (struct vrf *vrf);
 
 /* VRF list existance check by name. */
 struct vrf *
-vrf_list_lookup_by_name (const char *name)
+vrf_lookup_by_name (const char *name)
 {
   struct vrf vrf;
   strlcpy (vrf.name, name, sizeof (vrf.name));
@@ -93,157 +93,59 @@ vrf_name_compare (struct vrf *a, struct vrf *b)
 struct vrf *
 vrf_get (vrf_id_t vrf_id, const char *name)
 {
-  struct vrf *vrf;
+  struct vrf *vrf = NULL;
+  int new = 0;
 
   if (debug_vrf)
     zlog_debug ("VRF_GET: %s(%d)", name, vrf_id);
 
-  /*
-   * Nothing to see, move along here
-   */
+  /* Nothing to see, move along here */
   if (!name && vrf_id == VRF_UNKNOWN)
     return NULL;
 
-  /*
-   * Valid vrf name and unknown vrf_id case
-   *
-   * This is called when we are configured from
-   * the cli but we have no kernel information yet.
-   */
-  if (name && vrf_id == VRF_UNKNOWN)
-    {
-      vrf = vrf_list_lookup_by_name (name);
-      if (vrf)
-       return vrf;
+  /* Try to find VRF both by ID and name */
+  if (vrf_id != VRF_UNKNOWN)
+    vrf = vrf_lookup_by_id (vrf_id);
+  if (! vrf && name)
+    vrf = vrf_lookup_by_name (name);
 
+  if (vrf == NULL)
+    {
       vrf = XCALLOC (MTYPE_VRF, sizeof (struct vrf));
-      if (debug_vrf)
-        zlog_debug ("VRF(%u) %s is created.",
-                   vrf_id, (name) ? name : "(NULL)");
-      strcpy (vrf->name, name);
       vrf->vrf_id = VRF_UNKNOWN;
-      RB_INSERT (vrf_name_head, &vrfs_by_name, vrf);
       if_init (&vrf->iflist);
       QOBJ_REG (vrf, vrf);
-      if (vrf_master.vrf_new_hook)
-       {
-         (*vrf_master.vrf_new_hook) (vrf_id, name, &vrf->info);
+      new = 1;
 
-         if (debug_vrf && vrf->info)
-           zlog_info ("zvrf is created.");
-       }
       if (debug_vrf)
-        zlog_debug("Vrf Created: %p", vrf);
-      return vrf;
+       zlog_debug ("VRF(%u) %s is created.",
+                   vrf_id, (name) ? name : "(NULL)");
     }
-  /*
-   * Valid vrf name and valid vrf_id case
-   *
-   * This can be passed from the kernel
-   */
-  else if (name && vrf_id != VRF_UNKNOWN)
+
+  /* Set identifier */
+  if (vrf_id != VRF_UNKNOWN && vrf->vrf_id == VRF_UNKNOWN)
     {
-      vrf = vrf_list_lookup_by_name (name);
-      if (vrf)
-       {
-          /*
-           * If the passed in vrf_id and name match
-          * return, nothing to do here.
-           */
-         if (vrf->vrf_id == vrf_id)
-           return vrf;
-
-         vrf->vrf_id = vrf_id;
-         RB_INSERT (vrf_id_head, &vrfs_by_id, vrf);
-         if (vrf_master.vrf_new_hook)
-           (*vrf_master.vrf_new_hook) (vrf_id, name, &vrf->info);
-
-          if (debug_vrf)
-            zlog_debug("Vrf found matched stuff up: %p", vrf);
-
-         return vrf;
-       }
-      else
-       {
-         /*
-          * We can have 1 of two situations here
-          * We've already been told about the vrf_id
-          * or we haven't.
-          */
-         vrf = vrf_lookup_by_id (vrf_id);
-         if (vrf)
-           {
-             /*
-              * We know at this point that the vrf->name is not
-              * right because we would have caught it above.
-              * so let's set it.
-              */
-             strcpy (vrf->name, name);
-             RB_INSERT (vrf_name_head, &vrfs_by_name, vrf);
-             if (vrf_master.vrf_new_hook)
-               {
-                 (*vrf_master.vrf_new_hook) (vrf_id, name, &vrf->info);
-
-                 if (debug_vrf && vrf->info)
-                   zlog_info ("zvrf is created.");
-               }
-             if (debug_vrf)
-               zlog_debug("Vrf Created: %p", vrf);
-             return vrf;
-           }
-         else
-           {
-             vrf = XCALLOC (MTYPE_VRF, sizeof (struct vrf));
-             vrf->vrf_id = vrf_id;
-             strcpy (vrf->name, name);
-             RB_INSERT (vrf_name_head, &vrfs_by_name, vrf);
-             RB_INSERT (vrf_id_head, &vrfs_by_id, vrf);
-             if_init (&vrf->iflist);
-             QOBJ_REG (vrf, vrf);
-             if (vrf_master.vrf_new_hook)
-               {
-                 (*vrf_master.vrf_new_hook) (vrf_id, name, &vrf->info);
-
-                 if (debug_vrf && vrf->info)
-                   zlog_info ("zvrf is created.");
-               }
-             if (debug_vrf)
-               zlog_debug("Vrf Created: %p", vrf);
-             return vrf;
-           }
-       }
+      vrf->vrf_id = vrf_id;
+      RB_INSERT (vrf_id_head, &vrfs_by_id, vrf);
     }
-  /*
-   * The final case, we've been passed a valid vrf_id
-   * but no name.  So we create the route node
-   * if it hasn't already been created.
-   */
-  else if (!name)
+
+  /* Set name */
+  if (name && vrf->name[0] != '\0' && strcmp (name, vrf->name))
     {
-      vrf = vrf_lookup_by_id (vrf_id);
-      if (debug_vrf)
-        zlog_debug("Vrf found: %p", vrf);
-
-      if (vrf)
-       return vrf;
-      else
-       {
-         vrf = XCALLOC (MTYPE_VRF, sizeof (struct vrf));
-         vrf->vrf_id = vrf_id;
-          if_init (&vrf->iflist);
-         RB_INSERT (vrf_id_head, &vrfs_by_id, vrf);
-          QOBJ_REG (vrf, vrf);
-         if (debug_vrf)
-            zlog_debug("Vrf Created: %p", vrf);
-         return vrf;
-        }
+      RB_REMOVE (vrf_name_head, &vrfs_by_name, vrf);
+      strlcpy (vrf->name, name, sizeof (vrf->name));
+      RB_INSERT (vrf_name_head, &vrfs_by_name, vrf);
+    }
+  else if (name && vrf->name[0] == '\0')
+    {
+      strlcpy (vrf->name, name, sizeof (vrf->name));
+      RB_INSERT (vrf_name_head, &vrfs_by_name, vrf);
     }
 
-  /*
-   * We shouldn't get here and if we do
-   * something has gone wrong.
-   */
-  return NULL;
+  if (new && vrf_master.vrf_new_hook)
+    (*vrf_master.vrf_new_hook) (vrf_id, name, &vrf->info);
+
+  return vrf;
 }
 
 /* Delete a VRF. This is called in vrf_terminate(). */
@@ -264,7 +166,8 @@ vrf_delete (struct vrf *vrf)
 
   if (vrf->vrf_id != VRF_UNKNOWN)
     RB_REMOVE (vrf_id_head, &vrfs_by_id, vrf);
-  RB_REMOVE (vrf_name_head, &vrfs_by_name, vrf);
+  if (vrf->name[0] != '\0')
+    RB_REMOVE (vrf_name_head, &vrfs_by_name, vrf);
 
   XFREE (MTYPE_VRF, vrf);
 }
@@ -279,18 +182,12 @@ vrf_lookup_by_id (vrf_id_t vrf_id)
 }
 
 /*
- * Check whether the VRF is enabled - that is, whether the VRF
- * is ready to allocate resources. Currently there's only one
- * type of resource: socket.
+ * Check whether the VRF is enabled.
  */
 static int
 vrf_is_enabled (struct vrf *vrf)
 {
   return vrf && CHECK_FLAG (vrf->status, VRF_ACTIVE);
-
-  /*Pending: figure out the real use of this routine.. it used to be..
-  return vrf && vrf->vrf_id == VRF_DEFAULT;
-  */
 }
 
 /*
@@ -303,11 +200,13 @@ vrf_is_enabled (struct vrf *vrf)
 int
 vrf_enable (struct vrf *vrf)
 {
+  if (vrf_is_enabled (vrf))
+    return 1;
+
   if (debug_vrf)
     zlog_debug ("VRF %u is enabled.", vrf->vrf_id);
 
-  if (!CHECK_FLAG (vrf->status, VRF_ACTIVE))
-    SET_FLAG (vrf->status, VRF_ACTIVE);
+  SET_FLAG (vrf->status, VRF_ACTIVE);
 
   if (vrf_master.vrf_enable_hook)
     (*vrf_master.vrf_enable_hook) (vrf->vrf_id, vrf->name, &vrf->info);
@@ -323,20 +222,19 @@ vrf_enable (struct vrf *vrf)
 static void
 vrf_disable (struct vrf *vrf)
 {
-  if (vrf_is_enabled (vrf))
-    {
-      UNSET_FLAG (vrf->status, VRF_ACTIVE);
+  if (! vrf_is_enabled (vrf))
+    return;
 
-      if (debug_vrf)
-       zlog_debug ("VRF %u is to be disabled.", vrf->vrf_id);
+  UNSET_FLAG (vrf->status, VRF_ACTIVE);
 
-      /* Till now, nothing to be done for the default VRF. */
-      //Pending: see why this statement.
+  if (debug_vrf)
+    zlog_debug ("VRF %u is to be disabled.", vrf->vrf_id);
 
-      if (vrf_master.vrf_disable_hook)
-        (*vrf_master.vrf_disable_hook) (vrf->vrf_id, vrf->name, &vrf->info);
-    }
+  /* Till now, nothing to be done for the default VRF. */
+  //Pending: see why this statement.
 
+  if (vrf_master.vrf_disable_hook)
+    (*vrf_master.vrf_disable_hook) (vrf->vrf_id, vrf->name, &vrf->info);
 }
 
 
@@ -366,19 +264,6 @@ vrf_add_hook (int type, int (*func)(vrf_id_t, const char *, void **))
   }
 }
 
-/* Look up a VRF by name. */
-struct vrf *
-vrf_lookup_by_name (const char *name)
-{
-  struct vrf *vrf;
-
-  RB_FOREACH (vrf, vrf_id_head, &vrfs_by_id)
-    if (!strcmp(vrf->name, name))
-      return vrf;
-
-  return NULL;
-}
-
 vrf_id_t
 vrf_name_to_id (const char *name)
 {
@@ -571,6 +456,8 @@ vrf_terminate (void)
 
   while ((vrf = RB_ROOT (&vrfs_by_id)) != NULL)
     vrf_delete (vrf);
+  while ((vrf = RB_ROOT (&vrfs_by_name)) != NULL)
+    vrf_delete (vrf);
 }
 
 /* Create a socket for the VRF. */
@@ -618,7 +505,7 @@ DEFUN_NOSH (no_vrf,
 {
   struct vrf *vrfp;
 
-  vrfp = vrf_list_lookup_by_name (argv[0]);
+  vrfp = vrf_lookup_by_name (argv[0]);
 
   if (vrfp == NULL)
     {

diff --git a/lib/vrf.h b/lib/vrf.h
index 9dfaae21fcd81518623d2a3b9c881317aefe6bf2..feaf7689698743e78c86b1463e42e47a234e7fc5 100644 (file)
--- a/lib/vrf.h
+++ b/lib/vrf.h
@@ -112,7 +112,6 @@ extern void vrf_add_hook (int, int (*)(vrf_id_t, const char *, void **));
 
 extern struct vrf *vrf_lookup_by_id (vrf_id_t);
 extern struct vrf *vrf_lookup_by_name (const char *);
-extern struct vrf *vrf_list_lookup_by_name (const char *);
 extern struct vrf *vrf_get (vrf_id_t, const char *);
 extern void vrf_delete (struct vrf *);
 extern int vrf_enable (struct vrf *);
@@ -121,7 +120,7 @@ extern vrf_id_t vrf_name_to_id (const char *);
 #define VRF_GET_ID(V,NAME)      \
   do {                          \
       struct vrf *vrf; \
-      if (!(vrf = vrf_list_lookup_by_name(NAME))) \
+      if (!(vrf = vrf_lookup_by_name(NAME))) \
         {                                                           \
           vty_out (vty, "%% VRF %s not found%s", NAME, VTY_NEWLINE);\
           return CMD_WARNING;                                       \

diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c
index abf1c781a31f7a9d642faebff59aa192a106e2a9..be7a5ac78de232452fd926f952a285e11af1660d 100644 (file)
--- a/zebra/if_netlink.c
+++ b/zebra/if_netlink.c
@@ -208,13 +208,6 @@ netlink_vrf_change (struct nlmsghdr *h, struct rtattr *tb, const char *name)
 
   if (h->nlmsg_type == RTM_NEWLINK)
     {
-      /* If VRF already exists, we just return; status changes are handled
-       * against the VRF "interface".
-       */
-      vrf = vrf_lookup_by_id ((vrf_id_t)ifi->ifi_index);
-      if (vrf && vrf->info)
-        return;
-
       if (IS_ZEBRA_DEBUG_KERNEL)
         zlog_debug ("RTM_NEWLINK for VRF %s(%u) table %u",
                     name, ifi->ifi_index, nl_table_id);

diff --git a/zebra/zebra_vrf.c b/zebra/zebra_vrf.c
index e28f97d763dfcdd5b03cadde1c6d25511dcbd767..94e5d49c2c0b310d7df7579528a041e7b8f4fe6b 100644 (file)
--- a/zebra/zebra_vrf.c
+++ b/zebra/zebra_vrf.c
@@ -87,7 +87,7 @@ zebra_vrf_new (vrf_id_t vrf_id, const char *name, void **info)
 
   if (! zvrf)
     {
-      zvrf = zebra_vrf_list_lookup_by_name (name);
+      zvrf = zebra_vrf_lookup_by_name (name);
       if (!zvrf)
        {
          zvrf = zebra_vrf_alloc (vrf_id, name);
@@ -345,14 +345,14 @@ zebra_vrf_lookup_by_id (vrf_id_t vrf_id)
 
 /* Lookup VRF by name.  */
 struct zebra_vrf *
-zebra_vrf_list_lookup_by_name (const char *name)
+zebra_vrf_lookup_by_name (const char *name)
 {
   struct vrf *vrf;
 
   if (!name)
     name = VRF_DEFAULT_NAME;
 
-  vrf = vrf_list_lookup_by_name (name);
+  vrf = vrf_lookup_by_name (name);
   if (vrf)
     return ((struct zebra_vrf *) vrf->info);
 

diff --git a/zebra/zebra_vrf.h b/zebra/zebra_vrf.h
index c7814e6c998a0eddb4b353c9151401048d735adb..8247365644d0789256b60ecbd80c26d5bd684dd2 100644 (file)
--- a/zebra/zebra_vrf.h
+++ b/zebra/zebra_vrf.h
@@ -93,7 +93,7 @@ zebra_vrf_table_with_table_id (afi_t afi, safi_t safi,
 extern void zebra_vrf_static_route_interface_fixup (struct interface *ifp);
 extern void zebra_vrf_update_all (struct zserv *client);
 extern struct zebra_vrf *zebra_vrf_lookup_by_id (vrf_id_t vrf_id);
-extern struct zebra_vrf *zebra_vrf_list_lookup_by_name (const char *);
+extern struct zebra_vrf *zebra_vrf_lookup_by_name (const char *);
 extern struct zebra_vrf *zebra_vrf_alloc (vrf_id_t, const char *);
 extern struct route_table *zebra_vrf_table (afi_t, safi_t, vrf_id_t);
 extern struct route_table *zebra_vrf_static_table (afi_t, safi_t, struct zebra_vrf *zvrf);

diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c
index d069c318930bac70f49ef39c6a95c716bce82469..4ae12559ad443d51ac1126b7796126dc0d65c647 100644 (file)
--- a/zebra/zebra_vty.c
+++ b/zebra/zebra_vty.c
@@ -107,7 +107,7 @@ zebra_static_ipv4 (struct vty *vty, safi_t safi, int add_cmd,
     tag = atol(tag_str);
 
   /* VRF id */
-  zvrf = zebra_vrf_list_lookup_by_name (vrf_id_str);
+  zvrf = zebra_vrf_lookup_by_name (vrf_id_str);
 
   if (!zvrf)
     {
@@ -2450,7 +2450,7 @@ do_show_ip_route (struct vty *vty, const char *vrf_name, safi_t safi,
   json_object *json = NULL;
   json_object *json_prefix = NULL;
 
-  if (!(zvrf = zebra_vrf_list_lookup_by_name (vrf_name)))
+  if (!(zvrf = zebra_vrf_lookup_by_name (vrf_name)))
     {
       if (use_json)
         vty_out (vty, "{}%s", VTY_NEWLINE);
@@ -3784,7 +3784,7 @@ static_ipv6_func (struct vty *vty, int add_cmd, const char *dest_str,
   ret = inet_pton (AF_INET6, gate_str, &gate_addr);
 
   /* VRF id */
-  zvrf = zebra_vrf_list_lookup_by_name (vrf_id_str);
+  zvrf = zebra_vrf_lookup_by_name (vrf_id_str);
 
   if (!zvrf)
     {
@@ -4925,7 +4925,7 @@ DEFUN (show_ipv6_route,
 
   if (argc > 0 && argv[0] && strcmp(argv[0], "json") != 0)
     {
-     if (!(zvrf = zebra_vrf_list_lookup_by_name (argv[0])))
+     if (!(zvrf = zebra_vrf_lookup_by_name (argv[0])))
         {
           if (uj)
             vty_out (vty, "{}%s", VTY_NEWLINE);