2005-04-02 Andrew J. Schorr <ajschorr@alumni.princeton.edu>

	Fix problems when netlink interfaces are renamed (same ifindex used
	for a new interface).  Start cleaning up some problems with the way
	interface names are handled.
	* interface.c: (if_new_intern_ifindex) Remove obsolete function.
	  (if_delete_update) After distributing the interface deletion message,
	  set ifp->ifindex to IFINDEX_INTERNAL.
	  (if_dump_vty) Detect pseudo interface by checking if ifp->ifindex is
	  IFINDEX_INTERNAL.
	  (zebra_interface) Check return code from interface_cmd.func.
	  Do not set internal ifindex values to if_new_intern_ifindex(),
	  since we now use IFINDEX_INTERNAL for all pseudo interfaces.
	* kernel_socket.c: (ifm_read) Fix code and comments to reflect that
	  all internal interfaces now have ifp->ifindex set to IFINDEX_INTERNAL.
        * rt_netlink.c: (set_ifindex) New function used to update ifp->ifindex.
	  Detects interface rename events by checking if that ifindex is already
	  being used.  If it is, delete the old interface before assigning
	  the ifindex to the new interface.
	  (netlink_interface, netlink_link_change) Call set_ifindex to update
	  the ifindex.
	* if.h: Remove define for IFINDEX_INTERNBASE and add define
	  IFINDEX_INTERNAL 0, since all internal (i.e. non-kernel) pseudo-
	  interfaces should have ifindex set to 0.
	  (if_new) Remove function.
	  (if_delete_retain) New function to delete an interface without
	  removing from iflist and freeing the structure.
	  (ifname2ifindex) New function.
	* if.c: (if_new) Remove function (absorb into if_create).
	  (if_create) Replace function if_new with call to calloc.
	  Set ifp->ifindex to IFINDEX_INTERNAL.  Fix off-by-one error
	  in assert to check length of interface name.  Add error message
	  if interface with this name already exists.
	  (if_delete_retain) New function to delete an interface without
	  removing from iflist and freeing the structure.
	  (if_delete) Implement with help of if_delete_retain.
	  (ifindex2ifname) Reimplement using if_lookup_by_index.
	  (ifname2ifindex) New function to complement ifindex2ifname.
	  (interface) The interface command should check the name length
	  and fail with a warning message if it is too long.
	  (no_interface) Fix spelling in warning message.
	  (if_nametoindex) Reimplement using if_lookup_by_name.
	  (if_indextoname, ifaddr_ipv4_lookup) Reimplement using
	  if_lookup_by_index.
	* bgp_zebra.c: (bgp_interface_delete) After deleting, set ifp->ifindex
	  to IFINDEX_INTERNAL.
	* isis_zebra.c: (isis_zebra_if_del) Call if_delete_retain instead
	  of if_delete, since it is generally not safe to remove interface
	  structures.  After deleting, set ifp->ifindex to IFINDEX_INTERNAL.
	  (zebra_interface_if_lookup) Tighten up code.
	* ospf6_zebra.c: (ospf6_zebra_if_del) Previously, this whole function
	  was commented out.  But this is not safe: we should at least update
	  the ifindex when the interface is deleted.  So the new version
	  updates the interface status and sets ifp->ifindex to
	  IFINDEX_INTERNAL.
	  (ospf6_zebra_route_update) Use if_indextoname properly.
	* ospf_vty.c: (show_ip_ospf_interface_sub) Show ifindex and interface
	  flags to help with debugging.
	* ospf_zebra.c: (ospf_interface_delete) After deleting, set ifp->ifindex
	  to IFINDEX_INTERNAL.
	  (zebra_interface_if_lookup) Make function static.  Tighten up code.
	* rip_interface.c: (rip_interface_delete) After deleting, set
	  ifp->ifindex to IFINDEX_INTERNAL.
	* ripng_interface.c: (ripng_interface_delete) After deleting, set
	  ifp->ifindex to IFINDEX_INTERNAL.

1 files changed, 15 insertions, 30 deletions

diff --git a/zebra/interface.c b/zebra/interface.c
index 55717545..0f294a56 100644
--- a/zebra/interface.c
+++ b/zebra/interface.c
@@ -40,27 +40,7 @@
 #include "zebra/redistribute.h"
 #include "zebra/debug.h"
 #include "zebra/irdp.h"
-
-/* Allocate a new internal interface index 
- * This works done from the top so that %d macros
- * print a - sign! 
- */
-static unsigned int
-if_new_intern_ifindex (void)
-{
-  /* Start here so that first one assigned is 0xFFFFFFFF */
-  static unsigned int ifindex = IFINDEX_INTERNBASE + 1;
-
-  for (;;) 
-    {
-      ifindex--;
-      if ( ifindex <= IFINDEX_INTERNBASE )
-       ifindex = 0xFFFFFFFF;
 
-      if (if_lookup_by_index(ifindex) == NULL)
-       return ifindex;
-    }
-}
 
 /* Called when new interface is added. */
 int
@@ -418,6 +398,13 @@ if_delete_update (struct interface *ifp)
 	}
     }
   zebra_interface_delete_update (ifp);
+
+  /* Update ifindex after distributing the delete message.  This is in
+     case any client needs to have the old value of ifindex available
+     while processing the deletion.  Each client daemon is responsible
+     for setting ifindex to IFINDEX_INTERNAL after processing the
+     interface deletion message. */
+  ifp->ifindex = IFINDEX_INTERNAL;
 }
 #endif /* (defined(RTM_IFANNOUNCE) || defined(HAVE_NETLINK) */
 
@@ -680,9 +667,9 @@ if_dump_vty (struct vty *vty, struct interface *ifp)
   if (ifp->desc)
     vty_out (vty, "  Description: %s%s", ifp->desc,
 	     VTY_NEWLINE);
-  if (ifp->ifindex <= 0)
+  if (ifp->ifindex == IFINDEX_INTERNAL)
     {
-      vty_out(vty, "  index %d pseudo interface%s", ifp->ifindex, VTY_NEWLINE);
+      vty_out(vty, "  pseudo interface%s", VTY_NEWLINE);
       return;
     }
   else if (! CHECK_FLAG (ifp->status, ZEBRA_INTERFACE_ACTIVE))
@@ -861,17 +848,15 @@ DEFUN_NOSH (zebra_interface,
   struct interface * ifp;
   
   /* Call lib interface() */
-  ret = interface_cmd.func (self, vty, argc, argv);
+  if ((ret = interface_cmd.func (self, vty, argc, argv)) != CMD_SUCCESS)
+    return ret;
 
   ifp = vty->index;  
 
-  /* Set ifindex 
-     this only happens if interface is NOT in kernel */
-  if (ifp->ifindex == 0)
-    {
-      ifp->ifindex = if_new_intern_ifindex ();
-      UNSET_FLAG (ifp->status, ZEBRA_INTERFACE_ACTIVE);
-    }
+  if (ifp->ifindex == IFINDEX_INTERNAL)
+    /* Is this really necessary?  Shouldn't status be initialized to 0
+       in that case? */
+    UNSET_FLAG (ifp->status, ZEBRA_INTERFACE_ACTIVE);
 
   return ret;
 }