Use neighbor cache to track direct route to the peer or multihop destination.

Calculate next_hop properly based on the local address we get from the
neighbor entry.

4 files changed, 167 insertions, 108 deletions

diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index 0fdc3b6c..841fe103 100644
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -23,16 +23,20 @@ void
 bgp_rt_notify(struct proto *P, net *n, rte *new, rte *old, ea_list *tmpa)
 {
   DBG("BGP: Got route %I/%d\n", n->n.prefix, n->n.pxlen);
+  /* FIXME: Normalize attributes */
+  /* FIXME: Check next hop */
+  /* FIXME: Someone might have undefined the mandatory attributes */
 }
 
-static ea_list *
-bgp_create_attrs(struct bgp_proto *p, rte *e, ea_list *old, struct linpool *pool)
+static int
+bgp_create_attrs(struct bgp_proto *p, rte *e, ea_list **attrs, struct linpool *pool)
 {
   ea_list *ea = lp_alloc(pool, sizeof(ea_list) + 3*sizeof(eattr));
   eattr *a = ea->attrs;
   rta *rta = e->attrs;
 
-  ea->next = old;
+  ea->next = *attrs;
+  *attrs = ea;
   ea->flags = EALF_SORTED;
   ea->count = 3;
 
@@ -68,17 +72,14 @@ bgp_create_attrs(struct bgp_proto *p, rte *e, ea_list *old, struct linpool *pool
   a->type = EAF_TYPE_IP_ADDRESS;
   a->u.ptr = lp_alloc(pool, sizeof(struct adata) + sizeof(ip_addr));
   a->u.ptr->length = sizeof(ip_addr);
-
-  /* FIXME: These rules are bogus!!! */
-  if (rta->dest == RTD_ROUTER)
-    *(ip_addr *)a->u.ptr->data = e->attrs->gw;
+  if (p->cf->next_hop_self ||
+      !p->is_internal ||
+      rta->dest != RTD_ROUTER)
+    *(ip_addr *)a->u.ptr->data = p->local_addr;
   else
-    {
-      /* FIXME: Next hop == self ... how to do that? */
-      *(ip_addr *)a->u.ptr->data = IPA_NONE;
-    }
+    *(ip_addr *)a->u.ptr->data = e->attrs->gw;
 
-  return ea;
+  return 0;				/* Leave decision to the filters */
 }
 
 ea_list *
@@ -115,15 +116,37 @@ bgp_path_prepend(struct linpool *pool, eattr *a, ea_list *old, int as)
   return e;
 }
 
-static ea_list *
-bgp_update_attrs(struct bgp_proto *p, rte *e, ea_list *old, struct linpool *pool)
+static int
+bgp_update_attrs(struct bgp_proto *p, rte *e, ea_list **attrs, struct linpool *pool)
 {
+  eattr *a;
+
   if (!p->is_internal)
-    old = bgp_path_prepend(pool, ea_find(e->attrs->eattrs, EA_CODE(EAP_BGP, BA_AS_PATH)), old, p->local_as);
+    *attrs = bgp_path_prepend(pool, ea_find(e->attrs->eattrs, EA_CODE(EAP_BGP, BA_AS_PATH)), *attrs, p->local_as);
 
-  /* FIXME: Set NEXT_HOP to self */
+  a = ea_find(e->attrs->eattrs, EA_CODE(EAP_BGP, BA_NEXT_HOP));
+  if (a && (p->is_internal || (!p->is_internal && e->attrs->iface == p->neigh->iface)))
+    {
+      /* Leave the original next hop attribute, will check later where does it point */
+    }
+  else
+    {
+      /* Need to create new one */
+      ea_list *ea = lp_alloc(pool, sizeof(ea_list) + sizeof(eattr));
+      ea->next = *attrs;
+      *attrs = ea;
+      ea->flags = EALF_SORTED;
+      ea->count = 1;
+      a = ea->attrs;
+      a->id = EA_CODE(EAP_BGP, BA_NEXT_HOP);
+      a->flags = BAF_TRANSITIVE;
+      a->type = EAF_TYPE_IP_ADDRESS;
+      a->u.ptr = lp_alloc(pool, sizeof(struct adata) + sizeof(ip_addr));
+      a->u.ptr->length = sizeof(ip_addr);
+      *(ip_addr *)a->u.ptr->data = p->local_addr;
+    }
 
-  return old;
+  return 0;				/* Leave decision to the filters */
 }
 
 int
@@ -133,19 +156,16 @@ bgp_import_control(struct proto *P, rte **new, ea_list **attrs, struct linpool *
   struct bgp_proto *p = (struct bgp_proto *) P;
   struct bgp_proto *new_bgp = (e->attrs->proto->proto == &proto_bgp) ? (struct bgp_proto *) e->attrs->proto : NULL;
 
-  if (e->attrs->dest != RTD_ROUTER)	/* FIXME: This is a debugging kludge, remove some day */
+  if (p == new_bgp)			/* Poison reverse updates */
     return -1;
   if (new_bgp)
     {
       if (p->local_as == new_bgp->local_as && p->is_internal && new_bgp->is_internal)
 	return -1;			/* Don't redistribute internal routes with IBGP */
-      *attrs = bgp_update_attrs(p, e, *attrs, pool);
+      return bgp_update_attrs(p, e, attrs, pool);
     }
   else
-    *attrs = bgp_create_attrs(p, e, *attrs, pool);
-  if (p == new_bgp)			/* FIXME: Use a more realistic check based on the NEXT_HOP attribute */
-    return 1;
-  return 0;				/* Leave the decision to the filter */
+    return bgp_create_attrs(p, e, attrs, pool);
 }
 
 int
@@ -284,7 +304,7 @@ static struct attr_desc bgp_attr_table[] = {
   { "aggregator", 6, BAF_OPTIONAL, EAF_TYPE_OPAQUE,		/* BA_AGGREGATOR */
     NULL, NULL },
 #if 0
-  /* FIXME: Handle community lists */
+  /* FIXME: Handle community lists and remember to convert their endianity and normalize them */
   { 0, 0 },									/* BA_COMMUNITY */
   { 0, 0 },									/* BA_ORIGINATOR_ID */
   { 0, 0 },									/* BA_CLUSTER_LIST */
@@ -443,19 +463,12 @@ bgp_decode_attrs(struct bgp_conn *conn, byte *attr, unsigned int len, struct lin
   e = ea_find(a->eattrs, EA_CODE(EAP_BGP, BA_NEXT_HOP));
   ASSERT(e);
   nexthop = *(ip_addr *) e->u.ptr->data;
-  neigh = neigh_find(&bgp->p, &nexthop, 0);
-  if (!neigh)
-    {
-      if (bgp->cf->multihop)
-	neigh = neigh_find(&bgp->p, &bgp->cf->multihop_via, 0);
-      else
-	neigh = neigh_find(&bgp->p, &bgp->cf->remote_ip, 0);
-    }
-  if (!neigh || !neigh->iface)
+  if (ipa_equal(nexthop, bgp->local_addr))
     {
-      DBG("BGP: No route to peer!\n");	/* FIXME */
+      DBG("BGP: Loop!\n");		/* FIXME */
       return NULL;
     }
+  neigh = neigh_find(&bgp->p, &nexthop, 0) ? : bgp->neigh;
   a->gw = neigh->addr;
   a->iface = neigh->iface;
   return rta_lookup(a);
diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index 11729db4..d9eddbd5 100644
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -26,25 +26,6 @@ static list bgp_list;			/* List of active BGP instances */
 static char *bgp_state_names[] = { "Idle", "Connect", "Active", "OpenSent", "OpenConfirm", "Established" };
 
 static void bgp_connect(struct bgp_proto *p);
-static void bgp_setup_sk(struct bgp_proto *p, struct bgp_conn *conn, sock *s);
-
-static struct proto *
-bgp_init(struct proto_config *C)
-{
-  struct bgp_config *c = (struct bgp_config *) C;
-  struct proto *P = proto_new(C, sizeof(struct bgp_proto));
-  struct bgp_proto *p = (struct bgp_proto *) P;
-
-  P->rt_notify = bgp_rt_notify;
-  P->rte_better = bgp_rte_better;
-  P->import_control = bgp_import_control;
-  p->cf = c;
-  p->local_as = c->local_as;
-  p->remote_as = c->remote_as;
-  p->is_internal = (c->local_as == c->remote_as);
-  p->local_id = C->global->router_id;
-  return P;
-}
 
 void
 bgp_close(struct bgp_proto *p)
@@ -99,6 +80,27 @@ bgp_close_conn(struct bgp_conn *conn)
     }
 }
 
+static int
+bgp_graceful_close_conn(struct bgp_conn *c)
+{
+  switch (c->state)
+    {
+    case BS_IDLE:
+      return 0;
+    case BS_CONNECT:
+    case BS_ACTIVE:
+      bgp_close_conn(c);
+      return 1;
+    case BS_OPENSENT:
+    case BS_OPENCONFIRM:
+    case BS_ESTABLISHED:
+      bgp_error(c, 6, 0, 0, 0);
+      return 1;
+    default:
+      bug("bgp_graceful_close_conn: Unknown state %d", c->state);
+    }
+}
+
 static void
 bgp_send_open(struct bgp_conn *conn)
 {
@@ -152,33 +154,6 @@ bgp_sock_err(sock *sk, int err)
   bgp_close_conn(conn);
 }
 
-static int
-bgp_incoming_connection(sock *sk, int dummy)
-{
-  node *n;
-
-  DBG("BGP: Incoming connection from %I port %d\n", sk->daddr, sk->dport);
-  WALK_LIST(n, bgp_list)
-    {
-      struct bgp_proto *p = SKIP_BACK(struct bgp_proto, bgp_node, n);
-      if (ipa_equal(p->cf->remote_ip, sk->daddr) && sk->dport == BGP_PORT)
-	{
-	  DBG("BGP: Authorized\n");
-	  if (p->incoming_conn.sk)
-	    {
-	      DBG("BGP: But one incoming connection already exists, how is that possible?\n");
-	      break;
-	    }
-	  bgp_setup_sk(p, &p->incoming_conn, sk);
-	  bgp_send_open(&p->incoming_conn);
-	  return 0;
-	}
-    }
-  DBG("BGP: Unauthorized\n");
-  rfree(sk);
-  return 0;
-}
-
 static void
 bgp_hold_timeout(timer *t)
 {
@@ -253,12 +228,38 @@ bgp_connect(struct bgp_proto *p)	/* Enter Connect state and start establishing c
   bgp_start_timer(conn->connect_retry_timer, p->cf->connect_retry_time);
 }
 
-static void
-bgp_start_locked(struct object_lock *lock)
+static int
+bgp_incoming_connection(sock *sk, int dummy)
 {
-  struct bgp_proto *p = lock->data;
+  node *n;
 
-  DBG("BGP: Got lock\n");
+  DBG("BGP: Incoming connection from %I port %d\n", sk->daddr, sk->dport);
+  WALK_LIST(n, bgp_list)
+    {
+      struct bgp_proto *p = SKIP_BACK(struct bgp_proto, bgp_node, n);
+      if (ipa_equal(p->cf->remote_ip, sk->daddr) && sk->dport == BGP_PORT)
+	{
+	  DBG("BGP: Authorized\n");
+	  if (p->incoming_conn.sk)
+	    {
+	      DBG("BGP: But one incoming connection already exists, how is that possible?\n");
+	      break;
+	    }
+	  bgp_setup_sk(p, &p->incoming_conn, sk);
+	  bgp_send_open(&p->incoming_conn);
+	  return 0;
+	}
+    }
+  DBG("BGP: Unauthorized\n");
+  rfree(sk);
+  return 0;
+}
+
+static void
+bgp_start_neighbor(struct bgp_proto *p)
+{
+  p->local_addr = p->neigh->iface->addr->ip;
+  DBG("BGP: local=%I remote=%I\n", p->local_addr, p->next_hop);
   if (!bgp_counter++)
     init_list(&bgp_list);
   if (!bgp_listen_sk)
@@ -282,7 +283,49 @@ bgp_start_locked(struct object_lock *lock)
   if (!bgp_linpool)
     bgp_linpool = lp_new(&root_pool, 4080);
   add_tail(&bgp_list, &p->bgp_node);
-  bgp_connect(p);			/* FIXME: Use neighbor cache for fast up/down transitions? */
+  bgp_connect(p);
+}
+
+static void
+bgp_neigh_notify(neighbor *n)
+{
+  struct bgp_proto *p = (struct bgp_proto *) n->proto;
+
+  if (n->iface)
+    {
+      DBG("BGP: Neighbor is reachable\n");
+      bgp_start_neighbor(p);
+    }
+  else
+    {
+      DBG("BGP: Neighbor is unreachable\n");
+      /* Send cease packets, but don't wait for them to be delivered */
+      bgp_graceful_close_conn(&p->outgoing_conn);
+      bgp_graceful_close_conn(&p->incoming_conn);
+      proto_notify_state(&p->p, PS_DOWN);
+      /* FIXME: Remember to delay protocol startup here! */
+    }
+}
+
+static void
+bgp_start_locked(struct object_lock *lock)
+{
+  struct bgp_proto *p = lock->data;
+  struct bgp_config *cf = p->cf;
+
+  DBG("BGP: Got lock\n");
+  p->next_hop = cf->multihop ? cf->multihop_via : cf->remote_ip;
+  p->neigh = neigh_find(&p->p, &p->next_hop, NEF_STICKY);
+  if (!p->neigh)
+    {
+      log(L_ERR "%s: Invalid next hop %I", p->p.name, p->next_hop);
+      p->p.disabled = 1;
+      proto_notify_state(&p->p, PS_DOWN);
+    }
+  else if (p->neigh->iface)
+    bgp_start_neighbor(p);
+  else
+    DBG("BGP: Waiting for neighbor\n");
 }
 
 static int
@@ -313,27 +356,6 @@ bgp_start(struct proto *P)
 }
 
 static int
-bgp_graceful_close(struct bgp_conn *c)
-{
-  switch (c->state)
-    {
-    case BS_IDLE:
-      return 0;
-    case BS_CONNECT:
-    case BS_ACTIVE:
-      bgp_close_conn(c);
-      return 1;
-    case BS_OPENSENT:
-    case BS_OPENCONFIRM:
-    case BS_ESTABLISHED:
-      bgp_error(c, 6, 0, 0, 0);
-      return 1;
-    default:
-      bug("bgp_graceful_close: Unknown state %d", c->state);
-    }
-}
-
-static int
 bgp_shutdown(struct proto *P)
 {
   struct bgp_proto *p = (struct bgp_proto *) P;
@@ -356,7 +378,7 @@ bgp_shutdown(struct proto *P)
       else if (p->incoming_conn.state != BS_IDLE)
 	p->incoming_conn.primary = 1;
     }
-  if (bgp_graceful_close(&p->outgoing_conn) || bgp_graceful_close(&p->incoming_conn))
+  if (bgp_graceful_close_conn(&p->outgoing_conn) || bgp_graceful_close_conn(&p->incoming_conn))
     return p->p.proto_state;
   else
     {
@@ -366,6 +388,25 @@ bgp_shutdown(struct proto *P)
     }
 }
 
+static struct proto *
+bgp_init(struct proto_config *C)
+{
+  struct bgp_config *c = (struct bgp_config *) C;
+  struct proto *P = proto_new(C, sizeof(struct bgp_proto));
+  struct bgp_proto *p = (struct bgp_proto *) P;
+
+  P->rt_notify = bgp_rt_notify;
+  P->rte_better = bgp_rte_better;
+  P->import_control = bgp_import_control;
+  P->neigh_notify = bgp_neigh_notify;
+  p->cf = c;
+  p->local_as = c->local_as;
+  p->remote_as = c->remote_as;
+  p->is_internal = (c->local_as == c->remote_as);
+  p->local_id = C->global->router_id;
+  return P;
+}
+
 void
 bgp_error(struct bgp_conn *c, unsigned code, unsigned subcode, unsigned data, unsigned len)
 {
diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index 45f7e0b1..6ae594a6 100644
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -18,6 +18,7 @@ struct bgp_config {
   ip_addr remote_ip;
   int multihop;				/* Number of hops if multihop */
   ip_addr multihop_via;			/* Multihop: address to route to */
+  int next_hop_self;			/* Always set next hop to local IP address */
   unsigned connect_retry_time;
   unsigned hold_time, initial_hold_time;
   unsigned keepalive_time;
@@ -49,6 +50,9 @@ struct bgp_proto {
   struct bgp_conn outgoing_conn;	/* Outgoing connection we're working with */
   struct bgp_conn incoming_conn;	/* Incoming connection we have neither accepted nor rejected yet */
   struct object_lock *lock;		/* Lock for neighbor connection */
+  ip_addr next_hop;			/* Either the peer or multihop_via */
+  struct neighbor *neigh;		/* Neighbor entry corresponding to next_hop */
+  ip_addr local_addr;			/* Address of the local end of the link to next_hop */
 };
 
 #define BGP_PORT		179
diff --git a/proto/bgp/config.Y b/proto/bgp/config.Y
index f83191ad..f18f2bdc 100644
--- a/proto/bgp/config.Y
+++ b/proto/bgp/config.Y
@@ -15,7 +15,7 @@ CF_HDR
 CF_DECLS
 
 CF_KEYWORDS(BGP, LOCAL, NEIGHBOR, AS, HOLD, TIME, CONNECT, RETRY, KEEPALIVE,
-	MULTIHOP, STARTUP, VIA)
+	MULTIHOP, STARTUP, VIA, NEXT, HOP, SELF)
 
 CF_GRAMMAR
 
@@ -47,6 +47,7 @@ bgp_proto:
  | bgp_proto CONNECT RETRY TIME NUM ';' { BGP_CFG->connect_retry_time = $5; }
  | bgp_proto KEEPALIVE TIME NUM ';' { BGP_CFG->connect_retry_time = $4; }
  | bgp_proto MULTIHOP NUM VIA IPA ';' { BGP_CFG->multihop = $3; BGP_CFG->multihop_via = $5; }
+ | bgp_proto NEXT HOP SELF ';' { BGP_CFG->next_hop_self = 1; }
  ;
 
 CF_CODE