Hi,
when playing with bird I noticed the following problem:

When I add or remove addresses to/from an interface without taking the 
interface down when doing so,
the neighbor cache isn't notified, causing BGP sessions not to start even 
though the neighbor has become reachable,
or BGP sessions that already found their neighbor not to look for it on other 
interfaces after the address has disappeared.

The attached patch fixes this by updating the neighbor cache on every interface 
address update; I've only tested it
with BGP though, I don't know if it has any side effects concerning other 
protocols.

Best regards,
Matthias Schiffer
From a0155e7e243e954adced3b7bca51e507077c8c3f Mon Sep 17 00:00:00 2001
Message-Id: <a0155e7e243e954adced3b7bca51e507077c8c3f.1323303432.git.mschif...@universe-factory.net>
From: Matthias Schiffer <mschif...@universe-factory.net>
Date: Thu, 8 Dec 2011 01:09:26 +0100
Subject: [PATCH] Update the neighbor cache on every address addition or
 removal Only updating the cache when an interface goes up
 or down leaves it unaware of addresses that are added or
 removed without taking the interface down.


Signed-off-by: Matthias Schiffer <mschif...@universe-factory.net>
---
 nest/iface.c    |   11 +++++----
 nest/iface.h    |    3 +-
 nest/neighbor.c |   67 ++++++++++++++++++++++++++----------------------------
 3 files changed, 39 insertions(+), 42 deletions(-)

diff --git a/nest/iface.c b/nest/iface.c
index 2ff2611..2c61680 100644
--- a/nest/iface.c
+++ b/nest/iface.c
@@ -157,8 +157,14 @@ ifa_notify_change(unsigned c, struct ifa *a)
   struct proto *p;
 
   DBG("IFA change notification (%x) for %s:%I\n", c, a->iface->name, a->ip);
+  if (c & IF_CHANGE_UP)
+    neigh_ifa_update(a);
+
   WALK_LIST(p, active_proto_list)
     ifa_send_notify(p, c, a);
+
+  if (c & IF_CHANGE_DOWN)
+    neigh_ifa_update(a);
 }
 
 static inline void
@@ -195,8 +201,6 @@ if_notify_change(unsigned c, struct iface *i)
   if_dump(i);
 #endif
 
-  if (c & IF_CHANGE_UP)
-    neigh_if_up(i);
   if (c & IF_CHANGE_DOWN)
     WALK_LIST(a, i->addrs)
       {
@@ -216,9 +220,6 @@ if_notify_change(unsigned c, struct iface *i)
 
   if ((c & (IF_CHANGE_UP | IF_CHANGE_DOWN | IF_CHANGE_LINK)) == IF_CHANGE_LINK)
     neigh_if_link(i);
-
-  if (c & IF_CHANGE_DOWN)
-    neigh_if_down(i);
 }
 
 static unsigned
diff --git a/nest/iface.h b/nest/iface.h
index a9cf6cd..6d333a1 100644
--- a/nest/iface.h
+++ b/nest/iface.h
@@ -128,8 +128,7 @@ static inline int neigh_connected_to(struct proto *p, ip_addr *a, struct iface *
 void neigh_dump(neighbor *);
 void neigh_dump_all(void);
 void neigh_prune(void);
-void neigh_if_up(struct iface *);
-void neigh_if_down(struct iface *);
+void neigh_ifa_update(struct ifa *);
 void neigh_if_link(struct iface *);
 void neigh_init(struct pool *);
 
diff --git a/nest/neighbor.c b/nest/neighbor.c
index 1a5ac21..cf1b2e9 100644
--- a/nest/neighbor.c
+++ b/nest/neighbor.c
@@ -220,20 +220,47 @@ neigh_dump_all(void)
 }
 
 /**
- * neigh_if_up: notify neighbor cache about interface up event
- * @i: interface in question
+ * neigh_ifa_update: notify neighbor cache about interface address
+ * add or remove event
+ * @ifa: interface address in question
  *
- * Tell the neighbor cache that a new interface became up.
+ * Tell the neighbor cache that a address was added or removed.
  *
  * The neighbor cache wakes up all inactive sticky neighbors with
- * addresses belonging to prefixes of the interface @i.
+ * addresses belonging to prefixes of the interface belonging to @ifa
+ * and causes all unreachable neighbors to be flushed
+ *
+ * A change of the scope of a neighbor is handle as a remove with
+ * a consequent add
  */
 void
-neigh_if_up(struct iface *i)
+neigh_ifa_update(struct ifa *a)
 {
+  node *x, *y;
   neighbor *n, *next;
+  struct iface *i = a->iface;
   int scope;
 
+  /* Remove all neighbors whose scope has changed */
+  WALK_LIST_DELSAFE(x, y, i->neighbors)
+    {
+      n = SKIP_BACK(neighbor, if_n, x);
+      if (if_connected(&n->addr, i) != n->scope)
+	{
+	  DBG("Flushing neighbor %I on %s\n", n->addr, i->name);
+	  rem_node(&n->if_n);
+	  n->iface = NULL;
+	  if (n->proto->neigh_notify && n->proto->core_state != FS_FLUSHING)
+	    n->proto->neigh_notify(n);
+	  rem_node(&n->n);
+	  if (n->flags & NEF_STICKY)
+	    add_tail(&sticky_neigh_list, &n->n);
+	  else
+	    sl_free(neigh_slab, n);
+	}
+    }
+
+  /* Traverse the sticky neighbor list to find neighbors that are reachable now */
   WALK_LIST_DELSAFE(n, next, sticky_neigh_list)
     if ((scope = if_connected(&n->addr, i)) >= 0)
       {
@@ -249,36 +276,6 @@ neigh_if_up(struct iface *i)
 }
 
 /**
- * neigh_if_down - notify neighbor cache about interface down event
- * @i: the interface in question
- *
- * Notify the neighbor cache that an interface has ceased to exist.
- *
- * It causes all entries belonging to neighbors connected to this interface
- * to be flushed.
- */
-void
-neigh_if_down(struct iface *i)
-{
-  node *x, *y;
-
-  WALK_LIST_DELSAFE(x, y, i->neighbors)
-    {
-      neighbor *n = SKIP_BACK(neighbor, if_n, x);
-      DBG("Flushing neighbor %I on %s\n", n->addr, i->name);
-      rem_node(&n->if_n);
-      n->iface = NULL;
-      if (n->proto->neigh_notify && n->proto->core_state != FS_FLUSHING)
-	n->proto->neigh_notify(n);
-      rem_node(&n->n);
-      if (n->flags & NEF_STICKY)
-	add_tail(&sticky_neigh_list, &n->n);
-      else
-	sl_free(neigh_slab, n);
-    }
-}
-
-/**
  * neigh_if_link - notify neighbor cache about interface link change
  * @i: the interface in question
  *
-- 
1.7.8

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to