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.

bgpd/ChangeLog

@@ -1,3 +1,8 @@
+2005-04-02 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
+
+	* bgp_zebra.c: (bgp_interface_delete) After deleting, set ifp->ifindex
+	  to IFINDEX_INTERNAL.
+
 2005-03-21 Hasso Tepper <hasso at quagga.net>
 
 	* bgp_route.c: Don't crash while clearing route tables if there is

bgpd/bgp_zebra.c

@@ -81,6 +81,7 @@ bgp_interface_delete (int command, struct zclient *zclient,
 
   s = zclient->ibuf;
   ifp = zebra_interface_state_read (s);
+  ifp->ifindex = IFINDEX_INTERNAL;
 
   return 0;
 }

isisd/ChangeLog

@@ -1,3 +1,10 @@
+2005-04-02 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
+
+	* 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.
+
 2005-03-07 Michael Sandee <voidptr@voidptr.sboost.org>
 
 	* isis_spf.c: host.name might be NULL.

isisd/isis_zebra.c

@@ -95,30 +95,28 @@ isis_zebra_if_del (int command, struct zclient *zclient, zebra_size_t length)
   zlog_debug ("Zebra I/F delete: %s index %d flags %ld metric %d mtu %d",
 	      ifp->name, ifp->ifindex, ifp->flags, ifp->metric, ifp->mtu);
 
-  if_delete (ifp);
+
+  /* Cannot call if_delete because we should retain the pseudo interface
+     in case there is configuration info attached to it. */
+  if_delete_retain(ifp);
 
   isis_csm_state_change (IF_DOWN_FROM_Z, circuit_scan_by_ifp (ifp), ifp);
 
+  ifp->ifindex = IFINDEX_INTERNAL;
+
   return 0;
 }
 
 static struct interface *
 zebra_interface_if_lookup (struct stream *s)
 {
-  struct interface *ifp;
   u_char ifname_tmp[INTERFACE_NAMSIZ];
 
   /* Read interface name. */
   stream_get (ifname_tmp, s, INTERFACE_NAMSIZ);
 
-  /* Lookup this by interface index. */
-  ifp = if_lookup_by_name ((char *) ifname_tmp);
-
-  /* If such interface does not exist, indicate an error */
-  if (!ifp)
-    return NULL;
-
-  return ifp;
+  /* And look it up. */
+  return if_lookup_by_name ((char *) ifname_tmp);
 }
 
 static int

lib/ChangeLog

@@ -1,3 +1,29 @@
+2005-04-02 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
+
+	* 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.
+
 2005-04-02 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
 
 	* zebra.h: Should include str.h to pick up missing functions.

lib/if.c

@@ -112,29 +112,23 @@ if_cmp_func (struct interface *ifp1, struct interface *ifp2)
 }
 
 /* Create new interface structure. */
-struct interface *
-if_new ()
-{
-  struct interface *ifp;
-
-  ifp = XMALLOC (MTYPE_IF, sizeof (struct interface));
-  memset (ifp, 0, sizeof (struct interface));
-  return ifp;
-}
-
 struct interface *
 if_create (const char *name, int namelen)
 {
   struct interface *ifp;
 
-  ifp = if_new ();
+  ifp = XCALLOC (MTYPE_IF, sizeof (struct interface));
+  ifp->ifindex = IFINDEX_INTERNAL;
   
   assert (name);
-  assert (namelen <= (INTERFACE_NAMSIZ + 1));
+  assert (namelen <= INTERFACE_NAMSIZ);	/* Need space for '\0' at end. */
   strncpy (ifp->name, name, namelen);
-  ifp->name[INTERFACE_NAMSIZ] = '\0';
+  ifp->name[namelen] = '\0';
   if (if_lookup_by_name(ifp->name) == NULL)
     listnode_add_sort (iflist, ifp);
+  else
+    zlog_err("if_create(%s): corruption detected -- interface with this "
+	     "name exists already!", ifp->name);
   ifp->connected = list_new ();
   ifp->connected->del = (void (*) (void *)) connected_free;
 
@@ -144,17 +138,24 @@ if_create (const char *name, int namelen)
   return ifp;
 }
 
-/* Delete and free interface structure. */
+/* Delete interface structure. */
 void
-if_delete (struct interface *ifp)
+if_delete_retain (struct interface *ifp)
 {
-  listnode_delete (iflist, ifp);
-
   if (if_master.if_delete_hook)
     (*if_master.if_delete_hook) (ifp);
 
   /* Free connected address list */
   list_delete (ifp->connected);
+}
+
+/* Delete and free interface structure. */
+void
+if_delete (struct interface *ifp)
+{
+  listnode_delete (iflist, ifp);
+
+  if_delete_retain(ifp);
 
   XFREE (MTYPE_IF, ifp);
 }
@@ -194,16 +195,18 @@ if_lookup_by_index (unsigned int index)
 char *
 ifindex2ifname (unsigned int index)
 {
-  struct listnode *node;
   struct interface *ifp;
 
-  for (node = listhead (iflist); node; nextnode (node))
-    {
-      ifp = getdata (node);
-      if (ifp->ifindex == index)
-	return ifp->name;
-    }
-  return (char *) "unknown";
+  return ((ifp = if_lookup_by_index(index)) != NULL) ?
+  	 ifp->name : (char *)"unknown";
+}
+
+unsigned int
+ifname2ifindex (const char *name)
+{
+  struct interface *ifp;
+
+  return ((ifp = if_lookup_by_name(name)) != NULL) ? ifp->ifindex : 0;
 }
 
 /* Interface existance check by interface name. */
@@ -491,11 +494,20 @@ DEFUN (interface,
        "Interface's name\n")
 {
   struct interface *ifp;
+  size_t sl;
+
+  if ((sl = strlen(argv[0])) > INTERFACE_NAMSIZ)
+    {
+      vty_out (vty, "%% Interface name %s is invalid: length exceeds "
+		    "%d characters%s",
+	       argv[0], INTERFACE_NAMSIZ, VTY_NEWLINE);
+      return CMD_WARNING;
+    }
 
   ifp = if_lookup_by_name (argv[0]);
 
   if (ifp == NULL)
-    ifp = if_create (argv[0], INTERFACE_NAMSIZ);
+    ifp = if_create (argv[0], sl);
   vty->index = ifp;
   vty->node = INTERFACE_NODE;
 
@@ -515,17 +527,17 @@ DEFUN_NOSH (no_interface,
   ifp = if_lookup_by_name (argv[0]);
 
   if (ifp == NULL)
-  {
-    vty_out (vty, "%% Inteface %s does not exist%s", argv[0], VTY_NEWLINE);
-    return CMD_WARNING;
-  }
+    {
+      vty_out (vty, "%% Interface %s does not exist%s", argv[0], VTY_NEWLINE);
+      return CMD_WARNING;
+    }
 
   if (CHECK_FLAG (ifp->status, ZEBRA_INTERFACE_ACTIVE)) 
-  {
-    vty_out (vty, "%% Only inactive interfaces can be deleted%s",
-            VTY_NEWLINE);
-    return CMD_WARNING;
-  }
+    {
+      vty_out (vty, "%% Only inactive interfaces can be deleted%s",
+	      VTY_NEWLINE);
+      return CMD_WARNING;
+    }
 
   if_delete(ifp);
 
@@ -726,16 +738,9 @@ connected_add_by_prefix (struct interface *ifp, struct prefix *p,
 unsigned int
 if_nametoindex (const char *name)
 {
-  struct listnode *node;
   struct interface *ifp;
 
-  for (node = listhead (iflist); node; nextnode (node))
-    {
-      ifp = getdata (node);
-      if (strcmp (ifp->name, name) == 0)
-	return ifp->ifindex;
-    }
-  return 0;
+  return ((ifp = if_lookup_by_name(name)) != NULL) ? ifp->ifindex : 0;
 }
 #endif
 
@@ -743,19 +748,12 @@ if_nametoindex (const char *name)
 char *
 if_indextoname (unsigned int ifindex, char *name)
 {
-  struct listnode *node;
   struct interface *ifp;
 
-  for (node = listhead (iflist); node; nextnode (node))
-    {
-      ifp = getdata (node);
-      if (ifp->ifindex == ifindex)
-	{
-	  memcpy (name, ifp->name, IFNAMSIZ);
-	  return ifp->name;
-	}
-    }
-  return NULL;
+  if (!(ifp = if_lookup_by_index(ifindex)))
+    return NULL;
+  strncpy (name, ifp->name, IFNAMSIZ);
+  return ifp->name;
 }
 #endif
 
@@ -832,16 +830,7 @@ ifaddr_ipv4_lookup (struct in_addr *addr, unsigned int ifindex)
       return ifp;
     }
   else
-    {
-      for (node = listhead (iflist); node; nextnode (node))
-	{
-	  ifp = getdata (node);
-
-	  if (ifp->ifindex == ifindex)
-	    return ifp;
-	}
-    }
-  return NULL;
+    return if_lookup_by_index(ifindex);
 }
 
 /* Initialize interface list. */

lib/if.h

@@ -36,10 +36,6 @@ Boston, MA 02111-1307, USA.  */
 #define INTERFACE_NAMSIZ      20
 #define INTERFACE_HWADDR_MAX  20
 
-/* Internal If indexes start at 0xFFFFFFFF and go down to 1 greater
-   than this */
-#define IFINDEX_INTERNBASE 0x80000000
-
 #ifdef HAVE_PROC_NET_DEV
 struct if_stats
 {
@@ -75,11 +71,19 @@ struct if_stats
 /* Interface structure */
 struct interface 
 {
-  /* Interface name. */
+  /* Interface name.  This should probably never be changed after the
+     interface is created, because the configuration info for this interface
+     is associated with this structure.  For that reason, the interface
+     should also never be deleted (to avoid losing configuration info).
+     To delete, just set ifindex to IFINDEX_INTERNAL to indicate that the
+     interface does not exist in the kernel.
+   */
   char name[INTERFACE_NAMSIZ + 1];
 
-  /* Interface index. */
+  /* Interface index (should be IFINDEX_INTERNAL for non-kernel or
+     deleted interfaces). */
   unsigned int ifindex;
+#define IFINDEX_INTERNAL	0
 
   /* Zebra internal interface status */
   u_char status;
@@ -208,14 +212,22 @@ struct connected
 
 /* Prototypes. */
 int if_cmp_func (struct interface *, struct interface *);
-struct interface *if_new (void);
 struct interface *if_create (const char *name, int namelen);
 struct interface *if_lookup_by_index (unsigned int);
 struct interface *if_lookup_by_name (const char *);
 struct interface *if_lookup_exact_address (struct in_addr);
 struct interface *if_lookup_address (struct in_addr);
 struct interface *if_get_by_name (const char *);
-void if_delete (struct interface *);
+
+/* 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 
+   structure because there may be configuration information attached. */
+extern void if_delete_retain (struct interface *);
+
+/* Delete and free the interface structure: calls if_delete_retain and then
+   deletes it from the interface list and frees the structure. */
+extern void if_delete (struct interface *);
+
 int if_is_up (struct interface *);
 int if_is_running (struct interface *);
 int if_is_operative (struct interface *);
@@ -226,9 +238,18 @@ int if_is_multicast (struct interface *);
 void if_add_hook (int, int (*)(struct interface *));
 void if_init ();
 void if_dump_all ();
-char *ifindex2ifname (unsigned int);
 extern const char *if_flag_dump(unsigned long);
 
+/* Please use ifindex2ifname instead of if_indextoname where possible;
+   ifindex2ifname uses internal interface info, whereas if_indextoname must
+   make a system call. */
+extern char *ifindex2ifname (unsigned int);
+
+/* Please use ifname2ifindex instead of if_nametoindex where possible;
+   ifname2ifindex uses internal interface info, whereas if_nametoindex must
+   make a system call. */
+extern unsigned int ifname2ifindex(const char *ifname);
+
 /* Connected address functions. */
 struct connected *connected_new ();
 void connected_free (struct connected *);

ospf6d/ChangeLog

@@ -1,3 +1,12 @@
+2005-04-02 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
+
+	* 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.
+
 2005-04-02 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
 
 	* ospf6_route.c: (ospf6_route_show, ospf6_route_show_detail) Protect

ospf6d/ospf6_zebra.c

@@ -104,16 +104,24 @@ ospf6_zebra_if_add (int command, struct zclient *zclient, zebra_size_t length)
 int
 ospf6_zebra_if_del (int command, struct zclient *zclient, zebra_size_t length)
 {
-#if 0
   struct interface *ifp;
 
-  ifp = zebra_interface_delete_read (zclient->ibuf);
+  if (!(ifp = zebra_interface_state_read(zclient->ibuf)))
+    return 0;
+
+  if (if_is_up (ifp))
+    zlog_warn ("Zebra: got delete of %s, but interface is still up", ifp->name);
+
   if (IS_OSPF6_DEBUG_ZEBRA (RECV))
     zlog_debug ("Zebra Interface delete: %s index %d mtu %d",
 		ifp->name, ifp->ifindex, ifp->mtu6);
 
+#if 0
+  /* Why is this commented out? */
   ospf6_interface_if_del (ifp);
 #endif /*0*/
+
+  ifp->ifindex = IFINDEX_INTERNAL;
   return 0;
 }
 
@@ -348,7 +356,7 @@ static void
 ospf6_zebra_route_update (int type, struct ospf6_route *request)
 {
   struct zapi_ipv6 api;
-  char buf[64], ifname[IFNAMSIZ];
+  char buf[64];
   int nhcount;
   struct in6_addr **nexthops;
   unsigned int *ifindexes;
@@ -432,13 +440,15 @@ ospf6_zebra_route_update (int type, struct ospf6_route *request)
   for (i = 0; i < nhcount; i++)
     {
       if (IS_OSPF6_DEBUG_ZEBRA (SEND))
-        {
-          inet_ntop (AF_INET6, &request->nexthop[i].address,
-                     buf, sizeof (buf));
-          if_indextoname (request->nexthop[i].ifindex, ifname);
-          zlog_debug ("  nexthop: %s%%%s(%d)", buf, ifname,
+	{
+	  char ifname[IFNAMSIZ];
+	  inet_ntop (AF_INET6, &request->nexthop[i].address,
+		     buf, sizeof (buf));
+	  if (!if_indextoname(request->nexthop[i].ifindex, ifname))
+	    strlcpy(ifname, "unknown", sizeof(ifname));
+	  zlog_debug ("  nexthop: %s%%%.*s(%d)", buf, IFNAMSIZ, ifname,
 		      request->nexthop[i].ifindex);
-        }
+	}
       nexthops[i] = &request->nexthop[i].address;
       ifindexes[i] = request->nexthop[i].ifindex;
     }

ospfd/ChangeLog

@@ -1,3 +1,11 @@
+2005-04-02 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
+
+	* 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.
+
 2005-03-31 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
 
 	* ospf_dump.c: (show_debugging_ospf) Show if ospf event debugging

ospfd/ospf_vty.c

@@ -2582,8 +2582,9 @@ show_ip_ospf_interface_sub (struct vty *vty, struct ospf *ospf,
   /* Is interface up? */
   vty_out (vty, "%s is %s%s", ifp->name,
 	   ((is_up = if_is_operative(ifp)) ? "up" : "down"), VTY_NEWLINE);
-  vty_out (vty, "  MTU %u bytes, BW %u Kbit%s",
-  	   ifp->mtu, ifp->bandwidth, VTY_NEWLINE);
+  vty_out (vty, "  ifindex %u, MTU %u bytes, BW %u Kbit %s%s",
+  	   ifp->ifindex, ifp->mtu, ifp->bandwidth, if_flag_dump(ifp->flags),
+	   VTY_NEWLINE);
 
   /* Is interface OSPF enabled? */
   if (ospf_oi_count(ifp) == 0)

ospfd/ospf_zebra.c

@@ -141,26 +141,20 @@ ospf_interface_delete (int command, struct zclient *zclient,
     if (rn->info)
       ospf_if_free ((struct ospf_interface *) rn->info);
 
+  ifp->ifindex = IFINDEX_INTERNAL;
   return 0;
 }
 
-struct interface *
+static struct interface *
 zebra_interface_if_lookup (struct stream *s)
 {
-  struct interface *ifp;
   u_char ifname_tmp[INTERFACE_NAMSIZ];
 
   /* Read interface name. */
   stream_get (ifname_tmp, s, INTERFACE_NAMSIZ);
 
-  /* Lookup this by interface index. */
-  ifp = if_lookup_by_name ((char *) ifname_tmp);
-
-  /* If such interface does not exist, indicate an error */
-  if (!ifp)
-    return NULL;
-
-  return ifp;
+  /* And look it up. */
+  return if_lookup_by_name ((char *) ifname_tmp);
 }
 
 int

ripd/ChangeLog

@@ -1,3 +1,8 @@
+2005-04-02 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
+
+	* rip_interface.c: (rip_interface_delete) After deleting, set
+	  ifp->ifindex to IFINDEX_INTERNAL.
+
 2005-02-04 Paul Jakma <paul@dishone.st>
 
 	* ripd.c: Untangle the construction of RIP auth data.

ripd/rip_interface.c

@@ -598,6 +598,7 @@ rip_interface_delete (int command, struct zclient *zclient,
   
   /* To support pseudo interface do not free interface structure.  */
   /* if_delete(ifp); */
+  ifp->ifindex = IFINDEX_INTERNAL;
 
   return 0;
 }

ripngd/ChangeLog

@@ -1,3 +1,8 @@
+2005-04-02 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
+
+	* ripng_interface.c: (ripng_interface_delete) After deleting, set
+	  ifp->ifindex to IFINDEX_INTERNAL.
+
 2005-01-30 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
 
 	* ripng_interface.c: (ripng_multicast_join) Save errno before calling

ripngd/ripng_interface.c

@@ -331,6 +331,7 @@ ripng_interface_delete (int command, struct zclient *zclient,
 
   /* To support pseudo interface do not free interface structure.  */
   /* if_delete(ifp); */
+  ifp->ifindex = IFINDEX_INTERNAL;
 
   return 0;
 }

zebra/ChangeLog

@@ -1,3 +1,22 @@
+2005-04-02 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
+
+	* 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.
+
 2005-03-31 Hasso Tepper <hasso at quagga.net>
 
 	* rt_netlink.c (netlink_talk_filter): Show always warning message,

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;
 }

zebra/kernel_socket.c

@@ -301,8 +301,8 @@ ifm_read (struct if_msghdr *ifm)
    * If lookup by index was unsuccessful and we have a name, try
    * looking up by name.  Interfaces specified in the configuration
    * file for which the ifindex has not been determined will have
-   * ifindex == -1, and such interfaces are found by this search, and
-   * then their ifindex values can be filled in.
+   * ifindex == IFINDEX_INTERNAL, and such interfaces are found by this search,
+   * and then their ifindex values can be filled in.
    */
   if (ifp == NULL && sdl != NULL)
     {
@@ -322,14 +322,10 @@ ifm_read (struct if_msghdr *ifm)
     }
 
   /*
-   * If ifp does not exist or has an invalid index (-1), create or
+   * If ifp does not exist or has an invalid index (IFINDEX_INTERNAL), create or
    * fill in an interface.
    */
-  /*
-   * XXX warning: comparison between signed and unsigned
-   * ifindex should probably be signed
-   */
-  if ((ifp == NULL) || (ifp->ifindex == -1))
+  if ((ifp == NULL) || (ifp->ifindex == IFINDEX_INTERNAL))
     {
       /*
        * To create or fill in an interface, a sockaddr_dl (via
@@ -348,7 +344,7 @@ ifm_read (struct if_msghdr *ifm)
 
       /* 
        * Fill in newly created interface structure, or larval
-       * structure with ifindex -1.
+       * structure with ifindex IFINDEX_INTERNAL.
        */
       ifp->ifindex = ifm->ifm_index;
       ifp->flags = ifm->ifm_flags;

zebra/rt_netlink.c

@@ -86,6 +86,33 @@ extern struct zebra_privs_t zserv_privs;
 
 extern u_int32_t nl_rcvbufsize;
 
+/* Note: on netlink systems, there should be a 1-to-1 mapping between interface
+   names and ifindex values. */
+static void
+set_ifindex(struct interface *ifp, unsigned int ifi_index)
+{
+  struct interface *oifp;
+
+  if (((oifp = if_lookup_by_index(ifi_index)) != NULL) && (oifp != ifp))
+    {
+      if (ifi_index == IFINDEX_INTERNAL)
+        zlog_err("Netlink is setting interface %s ifindex to reserved "
+		 "internal value %u", ifp->name, ifi_index);
+      else
+        {
+	  if (IS_ZEBRA_DEBUG_KERNEL)
+	    zlog_debug("interface index %d was renamed from %s to %s",
+	    	       ifi_index, oifp->name, ifp->name);
+	  if (if_is_up(oifp))
+	    zlog_err("interface rename detected on up interface: index %d "
+		     "was renamed from %s to %s, results are uncertain!", 
+	    	     ifi_index, oifp->name, ifp->name);
+	  if_delete_update(oifp);
+        }
+    }
+  ifp->ifindex = ifi_index;
+}
+
 /* Make socket for Linux netlink interface. */
 static int
 netlink_socket (struct nlsock *nl, unsigned long groups)
@@ -504,8 +531,7 @@ netlink_interface (struct sockaddr_nl *snl, struct nlmsghdr *h)
 
   /* Add interface. */
   ifp = if_get_by_name (name);
-
-  ifp->ifindex = ifi->ifi_index;
+  set_ifindex(ifp, ifi->ifi_index);
   ifp->flags = ifi->ifi_flags & 0x0000fffff;
   ifp->mtu6 = ifp->mtu = *(int *) RTA_DATA (tb[IFLA_MTU]);
   ifp->metric = 1;
@@ -981,7 +1007,7 @@ netlink_link_change (struct sockaddr_nl *snl, struct nlmsghdr *h)
           if (ifp == NULL)
             ifp = if_get_by_name (name);
 
-          ifp->ifindex = ifi->ifi_index;
+          set_ifindex(ifp, ifi->ifi_index);
           ifp->flags = ifi->ifi_flags & 0x0000fffff;
           ifp->mtu6 = ifp->mtu = *(int *) RTA_DATA (tb[IFLA_MTU]);
           ifp->metric = 1;
@@ -992,7 +1018,7 @@ netlink_link_change (struct sockaddr_nl *snl, struct nlmsghdr *h)
       else
         {
           /* Interface status change. */
-          ifp->ifindex = ifi->ifi_index;
+          set_ifindex(ifp, ifi->ifi_index);
           ifp->mtu6 = ifp->mtu = *(int *) RTA_DATA (tb[IFLA_MTU]);
           ifp->metric = 1;