More BGP. This time it connects, but the state machine still isn't complete.

proto/bgp/bgp.c

@@ -22,7 +22,6 @@ static sock *bgp_listen_sk;		/* Global listening socket */
 static int bgp_counter;			/* Number of protocol instances using the listening socket */
 static list bgp_list;			/* List of active BGP instances */
 
-static void bgp_close_conn(struct bgp_conn *conn);
 static void bgp_connect(struct bgp_proto *p);
 static void bgp_setup_sk(struct bgp_proto *p, struct bgp_conn *conn, sock *s);
 
@@ -63,19 +62,20 @@ bgp_close(struct bgp_proto *p)
   /* FIXME: Automatic restart after errors? */
 }
 
-static void
+static void				/* FIXME: Nobody uses */
 bgp_reset(struct bgp_proto *p)
 {
   bgp_close(p);
   proto_notify_state(&p->p, PS_DOWN);
 }
 
-static void
+void
 bgp_start_timer(timer *t, int value)
 {
   /* FIXME: Randomize properly */
   /* FIXME: Check if anybody uses tm_start directly */
-  tm_start(t, value);
+  if (value)
+    tm_start(t, value);
 }
 
 static void
@@ -86,36 +86,34 @@ bgp_send_open(struct bgp_conn *conn)
   tm_stop(conn->connect_retry_timer);
   bgp_schedule_packet(conn, PKT_OPEN);
   conn->state = BS_OPENSENT;
+  bgp_start_timer(conn->hold_timer, conn->bgp->cf->initial_hold_time);
 }
 
-static int
+static void
-bgp_connected(sock *sk, int dummy)
+bgp_connected(sock *sk)
 {
   struct bgp_conn *conn = sk->data;
 
   DBG("BGP: Connected\n");
   bgp_send_open(conn);
-  return 0;
 }
 
 static void
 bgp_connect_timeout(timer *t)
 {
-  struct bgp_proto *p = t->data;
+  struct bgp_conn *conn = t->data;
-  struct bgp_conn *conn = &p->conn;
 
   DBG("BGP: Connect timeout, retrying\n");
   bgp_close_conn(conn);
-  bgp_connect(p);
+  bgp_connect(conn->bgp);
 }
 
 static void
-bgp_err(sock *sk, int err)
+bgp_sock_err(sock *sk, int err)
 {
   struct bgp_conn *conn = sk->data;
 
   DBG("BGP: Socket error %d in state %d\n", err, conn->state);
-  sk->type = SK_DELETED;		/* FIXME: Need to do this always! */
   switch (conn->state)
     {
     case BS_CONNECT:
@@ -125,10 +123,11 @@ bgp_err(sock *sk, int err)
       break;
     case BS_OPENCONFIRM:
     case BS_ESTABLISHED:
-      /* FIXME: Should close the connection and go to Idle state */
+      /* FIXME */
     default:
-      bug("bgp_err called in invalid state %d", conn->state);
+      bug("bgp_sock_err called in invalid state %d", conn->state);
     }
+  bgp_close_conn(conn);
 }
 
 static int
@@ -158,6 +157,24 @@ bgp_incoming_connection(sock *sk, int dummy)
   return 0;
 }
 
+static void
+bgp_hold_timeout(timer *t)
+{
+  struct bgp_conn *conn = t->data;
+
+  DBG("BGP: Hold timeout, closing connection\n"); /* FIXME: Check states? */
+  bgp_error(conn, 4, 0, 0, 0);
+}
+
+static void
+bgp_keepalive_timeout(timer *t)
+{
+  struct bgp_conn *conn = t->data;
+
+  DBG("BGP: Keepalive timer\n");
+  bgp_schedule_packet(conn, PKT_KEEPALIVE);
+}
+
 static void
 bgp_setup_sk(struct bgp_proto *p, struct bgp_conn *conn, sock *s)
 {
@@ -168,7 +185,7 @@ bgp_setup_sk(struct bgp_proto *p, struct bgp_conn *conn, sock *s)
   s->rbsize = BGP_RX_BUFFER_SIZE;
   s->tbsize = BGP_TX_BUFFER_SIZE;
   s->tx_hook = bgp_tx;
-  s->err_hook = bgp_err;
+  s->err_hook = bgp_sock_err;
   s->tos = IP_PREC_INTERNET_CONTROL;
 
   conn->bgp = p;
@@ -177,27 +194,27 @@ bgp_setup_sk(struct bgp_proto *p, struct bgp_conn *conn, sock *s)
 
   t = conn->connect_retry_timer = tm_new(p->p.pool);
   t->hook = bgp_connect_timeout;
-  t->data = p;
+  t->data = conn;
-#if 0
+  t = conn->hold_timer = tm_new(p->p.pool);
-  t = p->hold_timer = tm_new(p->p.pool);
   t->hook = bgp_hold_timeout;
-  t->data = p;
+  t->data = conn;
-  t = p->keepalive_timer = tm_new(p->p.pool);
+  t = conn->keepalive_timer = tm_new(p->p.pool);
   t->hook = bgp_keepalive_timeout;
-  t->data = p;
+  t->data = conn;
-#endif
 }
 
-static void
+void
 bgp_close_conn(struct bgp_conn *conn)
 {
+  DBG("BGP: Closing connection\n");
+  conn->packets_to_send = 0;
   rfree(conn->connect_retry_timer);
   conn->connect_retry_timer = NULL;
   rfree(conn->keepalive_timer);
   conn->keepalive_timer = NULL;
   rfree(conn->hold_timer);
   conn->hold_timer = NULL;
-  rfree(conn->sk);
+  sk_close(conn->sk);
   conn->sk = NULL;
   conn->state = BS_IDLE;
 }
@@ -217,12 +234,12 @@ bgp_connect(struct bgp_proto *p)	/* Enter Connect state and start establishing c
   s->sport =				/* FIXME */
 #endif
   s->dport = BGP_PORT;
-  s->rx_hook = bgp_connected;
   bgp_setup_sk(p, conn, s);
+  s->tx_hook = bgp_connected;
   conn->state = BS_CONNECT;
   if (sk_open(s))
     {
-      bgp_err(s, 0);
+      bgp_sock_err(s, 0);
       return;
     }
   DBG("BGP: Waiting for connect success\n");
@@ -294,6 +311,21 @@ bgp_shutdown(struct proto *P)
   return PS_DOWN;
 }
 
+void
+bgp_error(struct bgp_conn *c, unsigned code, unsigned subcode, unsigned data, unsigned len)
+{
+  DBG("BGP: Error %d,%d,%d,%d\n", code, subcode, data, len); /* FIXME: Better messages */
+  if (c->error_flag)
+    return;
+  c->error_flag = 1;
+  c->notify_code = code;
+  c->notify_subcode = subcode;
+  c->notify_arg = data;
+  c->notify_arg_size = len;
+  proto_notify_state(&c->bgp->p, PS_STOP);
+  bgp_schedule_packet(c, PKT_NOTIFICATION);
+}
+
 void
 bgp_check(struct bgp_config *c)
 {

proto/bgp/bgp.h

@@ -14,9 +14,9 @@ struct bgp_config {
   unsigned int local_as, remote_as;
   ip_addr remote_ip;
   int multihop;				/* Number of hops if multihop */
-  int connect_retry_time;
+  unsigned connect_retry_time;
-  int hold_time;
+  unsigned hold_time, initial_hold_time;
-  int keepalive_time;
+  unsigned keepalive_time;
 };
 
 struct bgp_conn {
@@ -26,20 +26,20 @@ struct bgp_conn {
   struct timer *connect_retry_timer;
   struct timer *hold_timer;
   struct timer *keepalive_timer;
-  unsigned int packets_to_send;		/* Bitmap of packet types to be sent */
+  int packets_to_send;			/* Bitmap of packet types to be sent */
-  unsigned int notify_code, notify_subcode, notify_arg, notify_arg_size;
+  int notify_code, notify_subcode, notify_arg, notify_arg_size;
-  unsigned int error_flag;		/* Error state, ignore all input */
+  int error_flag;			/* Error state, ignore all input */
+  unsigned hold_time, keepalive_time;	/* Times calculated from my and neighbor's requirements */
 };
 
 struct bgp_proto {
   struct proto p;
   struct bgp_config *cf;		/* Shortcut to BGP configuration */
   node bgp_node;			/* Node in global BGP protocol list */
-  int local_as, remote_as;
+  unsigned local_as, remote_as;
   int is_internal;			/* Internal BGP connection (local_as == remote_as) */
   u32 local_id;				/* BGP identifier of this router */
   u32 remote_id;			/* BGP identifier of the neighbor */
-  int hold_time;			/* Hold time calculated from my and neighbor's requirements */
   struct bgp_conn conn;			/* Our primary connection */
   struct bgp_conn incoming_conn;	/* Incoming connection we have neither accepted nor rejected yet */
   struct object_lock *lock;		/* Lock for neighbor connection */
@@ -52,7 +52,10 @@ struct bgp_proto {
 #define BGP_RX_BUFFER_SIZE	4096
 #define BGP_TX_BUFFER_SIZE	BGP_MAX_PACKET_LENGTH
 
+void bgp_start_timer(struct timer *t, int value);
 void bgp_check(struct bgp_config *c);
+void bgp_error(struct bgp_conn *c, unsigned code, unsigned subcode, unsigned data, unsigned len);
+void bgp_close_conn(struct bgp_conn *conn);
 
 /* attrs.c */
 

proto/bgp/config.Y

@@ -15,7 +15,7 @@ CF_HDR
 CF_DECLS
 
 CF_KEYWORDS(BGP, LOCAL, NEIGHBOR, AS, HOLD, TIME, CONNECT, RETRY, KEEPALIVE,
-	MULTIHOP)
+	MULTIHOP, STARTUP)
 
 CF_GRAMMAR
 
@@ -26,7 +26,7 @@ bgp_proto_start: proto_start BGP {
      this_proto->preference = DEF_PREF_BGP;
      BGP_CFG->hold_time = 240;
      BGP_CFG->connect_retry_time = 120;
-     BGP_CFG->keepalive_time = 30;
+     BGP_CFG->initial_hold_time = 300;
   }
  ;
 
@@ -43,6 +43,7 @@ bgp_proto:
      BGP_CFG->remote_as = $5;
    }
  | bgp_proto HOLD TIME NUM ';' { BGP_CFG->hold_time = $4; }
+ | bgp_proto STARTUP HOLD TIME NUM ';' { BGP_CFG->initial_hold_time = $5; }
  | 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 ';' { BGP_CFG->multihop = $3; }

proto/bgp/packets.c

@@ -53,6 +53,7 @@ static byte *
 bgp_create_update(struct bgp_conn *conn, byte *buf)
 {
   DBG("BGP: Sending update\n");
+
   bug("Don't know how to create updates");
 }
 
@@ -64,7 +65,7 @@ bgp_create_header(byte *buf, unsigned int len, unsigned int type)
   buf[18] = type;
 }
 
-static void
+static int
 bgp_fire_tx(struct bgp_conn *conn)
 {
   unsigned int s = conn->packets_to_send;
@@ -76,8 +77,8 @@ bgp_fire_tx(struct bgp_conn *conn)
 
   if (s & (1 << PKT_SCHEDULE_CLOSE))
     {
-      conn->packets_to_send = 0;
+      bgp_close_conn(conn);
-      bug("Scheduled close");		/* FIXME */
+      return 0;
     }
   if (s & (1 << PKT_NOTIFICATION))
     {
@@ -91,6 +92,7 @@ bgp_fire_tx(struct bgp_conn *conn)
       type = PKT_KEEPALIVE;
       end = pkt;			/* Keepalives carry no data */
       DBG("BGP: Sending keepalive\n");
+      bgp_start_timer(conn->keepalive_timer, conn->keepalive_time);
     }
   else if (s & (1 << PKT_OPEN))
     {
@@ -105,14 +107,14 @@ bgp_fire_tx(struct bgp_conn *conn)
       if (!end)
 	{
 	  conn->packets_to_send = 0;
-	  return;
+	  return 0;
 	}
     }
   else
-    return;
+    return 0;
   conn->packets_to_send = s;
   bgp_create_header(buf, end - buf, type);
-  sk_send(sk, end - buf);
+  return sk_send(sk, end - buf);
 }
 
 void
@@ -120,8 +122,9 @@ bgp_schedule_packet(struct bgp_conn *conn, int type)
 {
   DBG("BGP: Scheduling packet type %d\n", type);
   conn->packets_to_send |= 1 << type;
-  if (conn->sk->tpos != conn->sk->tbuf)
+  if (conn->sk->tpos == conn->sk->tbuf)
-    bgp_fire_tx(conn);
+    while (bgp_fire_tx(conn))
+      ;
 }
 
 void
@@ -130,7 +133,124 @@ bgp_tx(sock *sk)
   struct bgp_conn *conn = sk->data;
 
   DBG("BGP: TX hook\n");
-  bgp_fire_tx(conn);
+  while (bgp_fire_tx(conn))
+    ;
+}
+
+static void
+bgp_rx_open(struct bgp_conn *conn, byte *pkt, int len)
+{
+  struct bgp_proto *p = conn->bgp;
+  struct bgp_config *cf = p->cf;
+  unsigned as, hold;
+  u32 id;
+
+  /* Check state */
+  if (conn->state != BS_OPENSENT)
+    { bgp_error(conn, 5, 0, conn->state, 0); }
+
+  /* Check message contents */
+  if (len < 29 || len != 29 + pkt[28])
+    { bgp_error(conn, 1, 2, len, 2); return; }
+  if (pkt[19] != BGP_VERSION)
+    { bgp_error(conn, 2, 1, pkt[19], 2); return; }
+  as = get_u16(pkt+20);
+  hold = get_u16(pkt+22);
+  id = get_u32(pkt+24);
+  DBG("BGP: OPEN as=%d hold=%d id=%08x\n", as, hold, id);
+  if (cf->remote_as && as != p->remote_as)
+    { bgp_error(conn, 2, 2, as, 0); return; }
+  if (hold > 0 && hold < 3)
+    { bgp_error(conn, 2, 6, hold, 0); return; }
+  p->remote_id = id;
+  if (pkt[28])				/* Currently we support no optional parameters */
+    { bgp_error(conn, 2, 4, pkt[28], 0); return; }
+  if (!id || id == 0xffffffff || id == p->local_id)
+    { bgp_error(conn, 2, 3, id, 0); return; }
+
+  /* FIXME: What to do with the other connection??? */
+  ASSERT(conn == &p->conn);
+
+  /* Update our local variables */
+  if (hold < p->cf->hold_time)
+    conn->hold_time = hold;
+  else
+    conn->hold_time = p->cf->hold_time;
+  conn->keepalive_time = p->cf->keepalive_time ? : conn->hold_time / 3;
+  p->remote_as = as;
+  p->remote_id = id;
+  DBG("BGP: Hold timer set to %d, keepalive to %d, AS to %d, ID to %x\n", conn->hold_time, conn->keepalive_time, p->remote_as, p->remote_id);
+
+  bgp_schedule_packet(conn, PKT_KEEPALIVE);
+  bgp_start_timer(conn->hold_timer, conn->hold_time);
+  conn->state = BS_OPENCONFIRM;
+}
+
+static void
+bgp_rx_update(struct bgp_conn *conn, byte *pkt, int len)
+{
+  if (conn->state != BS_ESTABLISHED)
+    { bgp_error(conn, 5, 0, conn->state, 0); return; }
+  bgp_start_timer(conn->hold_timer, conn->hold_time);
+
+  DBG("BGP: UPDATE (ignored)\n");
+}
+
+static void
+bgp_rx_notification(struct bgp_conn *conn, byte *pkt, int len)
+{
+  unsigned arg;
+
+  if (len < 21)
+    {
+      bgp_error(conn, 1, 2, len, 2);
+      return;
+    }
+  switch (len)
+    {
+    case 21: arg = 0; break;
+    case 22: arg = pkt[21]; break;
+    case 23: arg = get_u16(pkt+21); break;
+    case 25: arg = get_u32(pkt+23); break;
+    default: DBG("BGP: NOTIFICATION with too much data\n"); /* FIXME */ arg = 0;
+    }
+  DBG("BGP: NOTIFICATION %d.%d %08x\n", pkt[19], pkt[20], arg);	/* FIXME: Better reporting */
+  conn->error_flag = 1;
+  proto_notify_state(&conn->bgp->p, PS_STOP);
+  bgp_schedule_packet(conn, PKT_SCHEDULE_CLOSE);
+}
+
+static void
+bgp_rx_keepalive(struct bgp_conn *conn, byte *pkt, unsigned len)
+{
+  DBG("BGP: KEEPALIVE\n");
+  bgp_start_timer(conn->hold_timer, conn->hold_time);
+  switch (conn->state)
+    {
+    case BS_OPENCONFIRM:
+      DBG("BGP: UP!!!\n");
+      conn->state = BS_ESTABLISHED;
+      proto_notify_state(&conn->bgp->p, PS_UP);
+      break;
+    case BS_ESTABLISHED:
+      break;
+    default:
+      bgp_error(conn, 5, 0, conn->state, 0);
+    }
+}
+
+static void
+bgp_rx_packet(struct bgp_conn *conn, byte *pkt, unsigned len)
+{
+  DBG("BGP: Got packet %02x (%d bytes)\n", pkt[18], len);
+  switch (pkt[18])
+    {
+    case PKT_OPEN:		return bgp_rx_open(conn, pkt, len);
+    case PKT_UPDATE:		return bgp_rx_update(conn, pkt, len);
+    case PKT_NOTIFICATION:      return bgp_rx_notification(conn, pkt, len);
+    case PKT_KEEPALIVE:		return bgp_rx_keepalive(conn, pkt, len);
+    default:			bgp_error(conn, 1, 3, pkt[18], 1);
+    }
 }
 
 int
@@ -139,13 +259,33 @@ bgp_rx(sock *sk, int size)
   struct bgp_conn *conn = sk->data;
   byte *pkt_start = sk->rbuf;
   byte *end = pkt_start + size;
+  unsigned i, len;
 
   DBG("BGP: RX hook: Got %d bytes\n", size);
   while (end >= pkt_start + BGP_HEADER_LENGTH)
     {
       if (conn->error_flag)
-	return 1;
+	{
-      bug("Incoming packets not handled"); /* FIXME */
+	  DBG("BGP: Error, dropping input\n");
+	  return 1;
+	}
+      for(i=0; i<16; i++)
+	if (pkt_start[i] != 0xff)
+	  {
+	    bgp_error(conn, 1, 1, 0, 0);
+	    break;
+	  }
+      len = get_u16(pkt_start+16);
+      if (len < BGP_HEADER_LENGTH || len > BGP_MAX_PACKET_LENGTH)
+	{
+	  bgp_error(conn, 1, 2, len, 2);
+	  break;
+	}
+      if (end >= pkt_start + len)
+	{
+	  bgp_rx_packet(conn, pkt_start, len);
+	  pkt_start += len;
+	}
     }
   if (pkt_start != sk->rbuf)
     {