[quagga-dev 5864] [PATCH] zebra: handle netlink renames better

Stephen Hemminger shemminger at vyatta.com
Tue Sep 2 16:54:17 BST 2008


Zebra needs to rely on ifindex as primary key, and not use name for
lookups because devices can be renamed. This code (which matches current
testing version of Vyatta) does lookup based on index and tracks renames
better. It also adds a log message when new interfaces are discovered
or change state.

--- a/zebra/rt_netlink.c	2008-09-02 07:52:16.000000000 -0700
+++ b/zebra/rt_netlink.c	2008-09-02 08:48:22.000000000 -0700
@@ -85,32 +85,6 @@ 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;
-}
 
 static int
 netlink_recvbuf (struct nlsock *nl, uint32_t newsize)
@@ -435,7 +409,7 @@ netlink_interface (struct sockaddr_nl *s
   struct ifinfomsg *ifi;
   struct rtattr *tb[IFLA_MAX + 1];
   struct interface *ifp;
-  char *name;
+  const char *name;
   int i;
 
   ifi = NLMSG_DATA (h);
@@ -447,7 +421,6 @@ netlink_interface (struct sockaddr_nl *s
   if (len < 0)
     return -1;
 
-  /* Looking up interface name. */
   memset (tb, 0, sizeof tb);
   netlink_parse_rtattr (tb, IFLA_MAX, IFLA_RTA (ifi), len);
   
@@ -462,12 +435,26 @@ netlink_interface (struct sockaddr_nl *s
 #endif /* IFLA_WIRELESS */
 
   if (tb[IFLA_IFNAME] == NULL)
-    return -1;
-  name = (char *) RTA_DATA (tb[IFLA_IFNAME]);
+    {
+      zlog_err("%s: missing interface name in message", __func__);
+      return -1;
+    }
+  name = RTA_DATA (tb[IFLA_IFNAME]);
+
+  if (ifi->ifi_index == IFINDEX_INTERNAL)
+    {
+      zlog_err("%s: reserved ifindex", __func__);
+      return -1;
+    }
 
   /* Add interface. */
-  ifp = if_get_by_name (name);
-  set_ifindex(ifp, ifi->ifi_index);
+  ifp = if_lookup_by_index(ifi->ifi_index);
+  if (!ifp)
+    {
+      ifp = if_create(name, strlen(name));
+      ifp->ifindex = ifi->ifi_index;
+    }
+  strncpy(ifp->name, name, INTERFACE_NAMSIZ);
   ifp->flags = ifi->ifi_flags & 0x0000fffff;
   ifp->mtu6 = ifp->mtu = *(int *) RTA_DATA (tb[IFLA_MTU]);
   ifp->metric = 1;
@@ -905,7 +892,7 @@ netlink_link_change (struct sockaddr_nl 
   struct ifinfomsg *ifi;
   struct rtattr *tb[IFLA_MAX + 1];
   struct interface *ifp;
-  char *name;
+  const char *name;
 
   ifi = NLMSG_DATA (h);
 
@@ -921,7 +908,6 @@ netlink_link_change (struct sockaddr_nl 
   if (len < 0)
     return -1;
 
-  /* Looking up interface name. */
   memset (tb, 0, sizeof tb);
   netlink_parse_rtattr (tb, IFLA_MAX, IFLA_RTA (ifi), len);
 
@@ -934,23 +920,39 @@ netlink_link_change (struct sockaddr_nl 
       return 0;
     }
 #endif /* IFLA_WIRELESS */
-  
   if (tb[IFLA_IFNAME] == NULL)
-    return -1;
-  name = (char *) RTA_DATA (tb[IFLA_IFNAME]);
+    {
+      zlog_err("%s: missing interface name", __func__);
+      return -1;
+    }
+  name = RTA_DATA (tb[IFLA_IFNAME]);
 
   /* Add interface. */
   if (h->nlmsg_type == RTM_NEWLINK)
     {
-      ifp = if_lookup_by_name (name);
+      unsigned long new_flags = ifi->ifi_flags & 0x0000fffff;
+
+      ifp = if_lookup_by_index(ifi->ifi_index);
+      if (ifp && strcmp(ifp->name, name) != 0)
+	{
+	  zlog_info("interface index %d was renamed from %s to %s",
+		    ifi->ifi_index, ifp->name, name);
+
+	  strncpy(ifp->name, name, INTERFACE_NAMSIZ);
+	}
 
       if (ifp == NULL || !CHECK_FLAG (ifp->status, ZEBRA_INTERFACE_ACTIVE))
         {
           if (ifp == NULL)
-            ifp = if_get_by_name (name);
+	    {
+	      ifp = if_create (name, strlen(name));
+	      ifp->ifindex = ifi->ifi_index;
+	    }
+
+	  zlog_info ("interface %s index %d %s added.",
+		     name, ifi->ifi_index, if_flag_dump(new_flags));
 
-          set_ifindex(ifp, ifi->ifi_index);
-          ifp->flags = ifi->ifi_flags & 0x0000fffff;
+          ifp->flags = new_flags;
           ifp->mtu6 = ifp->mtu = *(int *) RTA_DATA (tb[IFLA_MTU]);
           ifp->metric = 1;
 
@@ -960,38 +962,45 @@ netlink_link_change (struct sockaddr_nl 
       else
         {
           /* Interface status change. */
-          set_ifindex(ifp, ifi->ifi_index);
           ifp->mtu6 = ifp->mtu = *(int *) RTA_DATA (tb[IFLA_MTU]);
           ifp->metric = 1;
 
-          if (if_is_operative (ifp))
-            {
-              ifp->flags = ifi->ifi_flags & 0x0000fffff;
-              if (!if_is_operative (ifp))
-                if_down (ifp);
+	  if (new_flags != ifp->flags)
+	    {
+	      zlog_info ("interface %s index %d changed %s.",
+			 name, ifi->ifi_index,  if_flag_dump(new_flags));
+
+	      if (if_is_operative (ifp))
+		{
+		  ifp->flags = new_flags;
+		  if (!if_is_operative (ifp))
+		    if_down (ifp);
+		  else
+		    /* Must notify client daemons of new interface status. */
+		    zebra_interface_up_update (ifp);
+		}
 	      else
-		/* Must notify client daemons of new interface status. */
-	        zebra_interface_up_update (ifp);
-            }
-          else
-            {
-              ifp->flags = ifi->ifi_flags & 0x0000fffff;
-              if (if_is_operative (ifp))
-                if_up (ifp);
-            }
+		{
+		  ifp->flags = new_flags;
+		  if (if_is_operative (ifp))
+		    if_up (ifp);
+		}
+	    }
         }
     }
   else
     {
       /* RTM_DELLINK. */
-      ifp = if_lookup_by_name (name);
-
+      ifp = if_lookup_by_index(ifi->ifi_index);
       if (ifp == NULL)
         {
           zlog (NULL, LOG_WARNING, "interface %s is deleted but can't find",
                 name);
           return 0;
         }
+      else
+	zlog_info ("interface %s index %d deleted.",
+		   name, ifi->ifi_index);
 
       if_delete_update (ifp);
     }



More information about the Quagga-dev mailing list