zebra: don't change connected state from zebra/interface.c

Try to avoid changing connected state from zebra/interface.c as this
means making assumptions about kernel behaviour which may be or may
become wrong. This state should rather be updated by events from the
kernel.

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
diff --git a/zebra/interface.c b/zebra/interface.c
index 470df0c..51798ca 100644
--- a/zebra/interface.c
+++ b/zebra/interface.c
@@ -193,6 +193,9 @@
 	  ifc = listgetdata (listhead (addr_list));
 	  zebra_interface_address_delete_update (ifp, ifc);
 	  UNSET_FLAG (ifc->flags, ZEBRA_IFA_SECONDARY);
+	  /* XXX: Linux kernel removes all the secondary addresses when the primary
+	   * address is removed. We could try to work around that, though this is
+	   * non-trivial. */
 	  zebra_interface_address_add_update (ifp, ifc);
 	}
       
@@ -296,16 +299,23 @@
 	    {
 	      if (! if_is_up (ifp))
 		{
-		  /* XXX: WTF is it trying to set flags here?
-		   * caller has just gotten a new interface, has been
-                   * handed the flags already. This code has no business
-                   * trying to override administrative status of the interface.
-                   * The only call path to here which doesn't originate from
-                   * kernel event is irdp - what on earth is it trying to do?
-                   *
-                   * further RUNNING is not a settable flag on any system
-                   * I (paulj) am aware of.
-                   */
+		  /* Assume zebra is configured like following:
+		   *
+		   *   interface gre0
+		   *    ip addr 192.0.2.1/24
+		   *   !
+		   *
+		   * As soon as zebra becomes first aware that gre0 exists in the
+		   * kernel, it will set gre0 up and configure its addresses.
+		   *
+		   * (This may happen at startup when the interface already exists
+		   * or during runtime when the interface is added to the kernel)
+		   *
+		   * XXX: IRDP code is calling here via if_add_update - this seems
+		   * somewhat weird.
+		   * XXX: RUNNING is not a settable flag on any system
+		   * I (paulj) am aware of.
+		  */
 		  if_set_flags (ifp, IFF_UP | IFF_RUNNING);
 		  if_refresh (ifp);
 		}
@@ -318,23 +328,17 @@
 		  continue;
 		}
 
-	      /* Add to subnet chain list. */
-	      if_subnet_add (ifp, ifc);
-
 	      SET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED);
-	      SET_FLAG (ifc->conf, ZEBRA_IFC_REAL);
-
-	      zebra_interface_address_add_update (ifp, ifc);
-
-	      if (if_is_operative(ifp))
-		connected_up_ipv4 (ifp, ifc);
+	      /* The address will be advertised to zebra clients when the notification
+	       * from the kernel has been received.
+	       * It will also be added to the interface's subnet list then. */
 	    }
 #ifdef HAVE_IPV6
 	  if (p->family == AF_INET6)
 	    {
 	      if (! if_is_up (ifp))
 		{
-		  /* XXX: See long comment above */
+		  /* See long comment above */
 		  if_set_flags (ifp, IFF_UP | IFF_RUNNING);
 		  if_refresh (ifp);
 		}
@@ -346,13 +350,10 @@
 			     safe_strerror(errno));
 		  continue;
 		}
+
 	      SET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED);
-	      SET_FLAG (ifc->conf, ZEBRA_IFC_REAL);
-
-	      zebra_interface_address_add_update (ifp, ifc);
-
-	      if (if_is_operative(ifp))
-		connected_up_ipv6 (ifp, ifc);
+	      /* The address will be advertised to zebra clients when the notification
+	       * from the kernel has been received. */
 	    }
 #endif /* HAVE_IPV6 */
 	}
@@ -450,9 +451,11 @@
 
 		  ifc = listgetdata (anode);
 		  p = ifc->address;
-
 		  connected_down_ipv4 (ifp, ifc);
 
+		  /* XXX: We have to send notifications here explicitly, because we destroy
+		   * the ifc before receiving the notification about the address being deleted.
+		   */
 		  zebra_interface_address_delete_update (ifp, ifc);
 
 		  UNSET_FLAG (ifc->conf, ZEBRA_IFC_REAL);
@@ -1251,19 +1254,10 @@
 	  return CMD_WARNING;
 	}
 
-      /* Add to subnet chain list (while marking secondary attribute). */
-      if_subnet_add (ifp, ifc);
-
-      /* IP address propery set. */
       SET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED);
-      SET_FLAG (ifc->conf, ZEBRA_IFC_REAL);
-
-      /* Update interface address information to protocol daemon. */
-      zebra_interface_address_add_update (ifp, ifc);
-
-      /* If interface is up register connected route. */
-      if (if_is_operative(ifp))
-	connected_up_ipv4 (ifp, ifc);
+      /* The address will be advertised to zebra clients when the notification
+       * from the kernel has been received.
+       * It will also be added to the subnet chain list, then. */
     }
 
   return CMD_SUCCESS;
@@ -1317,35 +1311,9 @@
 	       safe_strerror(errno), VTY_NEWLINE);
       return CMD_WARNING;
     }
-  /* success! call returned that the address deletion went through.
-   * this is a synchronous operation, so we know it succeeded and can
-   * now update all internal state. */
-
-  /* the HAVE_NETLINK check is only here because, on BSD, although the
-   * call above is still synchronous, we get a second confirmation later
-   * through the route socket, and we don't want to touch that behaviour
-   * for now.  It should work without the #ifdef, but why take the risk...
-   * -- equinox 2012-07-13 */
   UNSET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED);
-#ifdef HAVE_NETLINK
-
-  /* Remove connected route. */
-  connected_down_ipv4 (ifp, ifc);
-
-  /* Redistribute this information. */
-  zebra_interface_address_delete_update (ifp, ifc);
-
-  /* IP address propery set. */
-  UNSET_FLAG (ifc->conf, ZEBRA_IFC_REAL);
-
-  /* remove from interface, remark secondaries */
-  if_subnet_delete (ifp, ifc);
-
-  /* Free address information. */
-  listnode_delete (ifp->connected, ifc);
-  connected_free (ifc);
-#endif
-
+  /* we will receive a kernel notification about this route being removed.
+   * this will trigger its removal from the connected list. */
   return CMD_SUCCESS;
 }
 
@@ -1462,16 +1430,9 @@
 	  return CMD_WARNING;
 	}
 
-      /* IP address propery set. */
       SET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED);
-      SET_FLAG (ifc->conf, ZEBRA_IFC_REAL);
-
-      /* Update interface address information to protocol daemon. */
-      zebra_interface_address_add_update (ifp, ifc);
-
-      /* If interface is up register connected route. */
-      if (if_is_operative(ifp))
-	connected_up_ipv6 (ifp, ifc);
+      /* The address will be advertised to zebra clients when the notification
+       * from the kernel has been received. */
     }
 
   return CMD_SUCCESS;
@@ -1527,17 +1488,8 @@
     }
 
   UNSET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED);
-
-  /* Redistribute this information. */
-  zebra_interface_address_delete_update (ifp, ifc);
-
-  /* Remove connected route. */
-  connected_down_ipv6 (ifp, ifc);
-
-  /* Free address information. */
-  listnode_delete (ifp->connected, ifc);
-  connected_free (ifc);
-
+  /* This information will be propagated to the zclients when the
+   * kernel notification is received. */
   return CMD_SUCCESS;
 }