Fixes some BFD bugs and makes logging thread-safe.

5 files changed, 88 insertions, 40 deletions

diff --git a/proto/bfd/bfd.c b/proto/bfd/bfd.c
index 6e38102b..3e2af9d5 100644
--- a/proto/bfd/bfd.c
+++ b/proto/bfd/bfd.c
@@ -17,6 +17,9 @@
 #define HASH_IP_EQ(a,b)		ipa_equal(a,b)
 #define HASH_IP_FN(k)		ipa_hash(k)
 
+
+const char *bfd_state_names[] = { "AdminDown", "Down", "Init", "Up" };
+
 static inline void bfd_notify_kick(struct bfd_proto *p);
 
 static void 
@@ -28,7 +31,7 @@ bfd_session_update_state(struct bfd_session *s, uint state, uint diag)
   if (s->loc_state == state)
     return;
 
-  //TRACE(D_EVENTS, "Session changed %I %d %d", s->addr, state, diag);
+  TRACE(D_EVENTS, "Session changed %I %d %d", s->addr, state, diag);
   debug("STATE %I %d %d %d\n", s->addr, s->loc_state, state, diag);
     
   bfd_lock_sessions(p);
@@ -547,8 +550,6 @@ bfd_start(struct proto *P)
   init_list(&p->sock_list);
 
 
-  birdloop_mask_wakeups(p->loop);
-
   init_list(&p->notify_list);
   bfd_notify_init(p);
 
@@ -561,7 +562,7 @@ bfd_start(struct proto *P)
   WALK_LIST(n, cf->neigh_list)
     bfd_start_neighbor(p, n);
 
-  birdloop_unmask_wakeups(p->loop);
+  birdloop_start(p->loop);
 
   return PS_UP;
 }
@@ -572,6 +573,13 @@ bfd_shutdown(struct proto *P)
 {
   struct bfd_proto *p = (struct bfd_proto *) P;
 
+  birdloop_stop(p->loop);
+
+  /* FIXME: This is hack */
+  birdloop_enter(p->loop);
+  rfree(p->tpool);
+  birdloop_leave(p->loop);
+
   return PS_DOWN;
 }
 
@@ -661,8 +669,8 @@ bfd_show_sessions(struct proto *P)
     tx_int = (MAX(s->des_min_tx_int, s->rem_min_rx_int) TO_MS);
     timeout = (MAX(s->req_min_rx_int, s->rem_min_tx_int) TO_MS) * s->rem_detect_mult;
 
-    cli_msg(-1013, "%I\t%s\t%d %d\t%u\t%u",
-	    s->addr, ifname, state, diag, tx_int, timeout);
+    cli_msg(-1013, "%I\t%s\t%s %d\t%u\t%u",
+	    s->addr, ifname, bfd_state_names[state], diag, tx_int, timeout);
   }
   HASH_WALK_END;
 
diff --git a/proto/bfd/bfd.h b/proto/bfd/bfd.h
index 97ccb507..66c6ed17 100644
--- a/proto/bfd/bfd.h
+++ b/proto/bfd/bfd.h
@@ -27,9 +27,9 @@
 #define BFD_ECHO_PORT		3785
 #define BFD_MULTI_CTL_PORT	4784
 
-#define BFD_DEFAULT_MIN_RX_INT	(10 MS)
-#define BFD_DEFAULT_MIN_TX_INT	(100 MS)
-#define BFD_DEFAULT_IDLE_TX_INT	(1 S)
+#define BFD_DEFAULT_MIN_RX_INT	(10 _MS)
+#define BFD_DEFAULT_MIN_TX_INT	(100 _MS)
+#define BFD_DEFAULT_IDLE_TX_INT	(1 _S)
 #define BFD_DEFAULT_MULTIPLIER	5
 
 
@@ -127,6 +127,7 @@ struct bfd_session
 };
 
 
+extern const char *bfd_state_names[];
 
 #define BFD_STATE_ADMIN_DOWN	0
 #define BFD_STATE_DOWN		1
@@ -146,6 +147,7 @@ struct bfd_session
 #define BFD_POLL_TX		1
 #define BFD_POLL_RX		2
 
+#define BFD_FLAGS		0x3f
 #define BFD_FLAG_POLL		(1 << 5)
 #define BFD_FLAG_FINAL		(1 << 4)
 #define BFD_FLAG_CPI		(1 << 3)
diff --git a/proto/bfd/io.c b/proto/bfd/io.c
index c5f2d1b0..2c1f7b03 100644
--- a/proto/bfd/io.c
+++ b/proto/bfd/io.c
@@ -35,6 +35,7 @@ struct birdloop
   btime real_time;
   u8 use_monotonic_clock;
 
+  u8 stop_called;
   u8 poll_active;
   u8 wakeup_masked;
   int wakeup_fds[2];
@@ -85,7 +86,7 @@ times_init(struct birdloop *loop)
   rv = clock_gettime(CLOCK_MONOTONIC, &ts);
   if (rv < 0)
   {
-    // log(L_WARN "Monotonic clock is missing");
+    log(L_WARN "Monotonic clock is missing");
 
     loop->use_monotonic_clock = 0;
     loop->last_time = 0;
@@ -94,13 +95,11 @@ times_init(struct birdloop *loop)
     return;
   }
 
-  /*
   if ((ts.tv_sec < 0) || (((s64) ts.tv_sec) > ((s64) 1 << 40)))
     log(L_WARN "Monotonic clock is crazy");
-  */
 
   loop->use_monotonic_clock = 1;
-  loop->last_time = (ts.tv_sec S) + (ts.tv_nsec / 1000);
+  loop->last_time = ((s64) ts.tv_sec S) + (ts.tv_nsec / 1000);
   loop->real_time = 0;
 }
 
@@ -114,12 +113,10 @@ times_update_pri(struct birdloop *loop)
   if (rv < 0)
     die("clock_gettime: %m");
 
-  btime new_time = (ts.tv_sec S) + (ts.tv_nsec / 1000);
+  btime new_time = ((s64) ts.tv_sec S) + (ts.tv_nsec / 1000);
 
-  /*
   if (new_time < loop->last_time)
     log(L_ERR "Monotonic clock is broken");
-  */
 
   loop->last_time = new_time;
   loop->real_time = 0;
@@ -135,15 +132,13 @@ times_update_alt(struct birdloop *loop)
   if (rv < 0)
     die("gettimeofday: %m");
 
-  btime new_time = (tv.tv_sec S) + tv.tv_usec;
+  btime new_time = ((s64) tv.tv_sec S) + tv.tv_usec;
   btime delta = new_time - loop->real_time;
 
   if ((delta < 0) || (delta > (60 S)))
   {
-    /*
     if (loop->real_time)
       log(L_WARN "Time jump, delta %d us", (int) delta);
-    */
 
     delta = 100 MS;
   }
@@ -621,14 +616,31 @@ birdloop_new(pool *p)
   timers_init(loop);
   sockets_init(loop);
 
+  return loop;
+}
 
+void
+birdloop_start(struct birdloop *loop)
+{
   int rv = pthread_create(&loop->thread, NULL, birdloop_main, loop);
-  if (rv < 0)
-    die("pthread_create(): %m");
+  if (rv)
+    die("pthread_create(): %M", rv);
+}
 
-  return loop;
+void
+birdloop_stop(struct birdloop *loop)
+{
+  pthread_mutex_lock(&loop->mutex);
+  loop->stop_called = 1;
+  wakeup_do_kick(loop);
+  pthread_mutex_unlock(&loop->mutex);
+
+  int rv = pthread_join(loop->thread, NULL);
+  if (rv)
+    die("pthread_join(): %M", rv);
 }
 
+
 void
 birdloop_enter(struct birdloop *loop)
 {
@@ -707,12 +719,18 @@ birdloop_main(void *arg)
     if (loop->close_scheduled)
       sockets_close_fds(loop);
 
+    if (loop->stop_called)
+      break;
+
     if (rv)
       sockets_fire(loop);
 
     timers_fire(loop);
   }
 
+  loop->stop_called = 0;
+  pthread_mutex_unlock(&loop->mutex);
+
   return NULL;
 }
 
diff --git a/proto/bfd/io.h b/proto/bfd/io.h
index c186ba2b..4f7c678d 100644
--- a/proto/bfd/io.h
+++ b/proto/bfd/io.h
@@ -15,16 +15,6 @@
 // #include "lib/timer.h"
 
 
-#define S	*1000000
-#define MS	*1000
-#define US	*1
-#define TO_S	/1000000
-#define TO_MS	/1000
-#define TO_US	/1
-
-
-typedef s64 btime;
-
 typedef struct timer2
 {
   resource r;
@@ -89,6 +79,8 @@ void sk_stop(sock *s);
 
 
 struct birdloop *birdloop_new(pool *p);
+void birdloop_start(struct birdloop *loop);
+void birdloop_stop(struct birdloop *loop);
 void birdloop_enter(struct birdloop *loop);
 void birdloop_leave(struct birdloop *loop);
 void birdloop_mask_wakeups(struct birdloop *loop);
diff --git a/proto/bfd/packets.c b/proto/bfd/packets.c
index 8f7a1f65..0e24114b 100644
--- a/proto/bfd/packets.c
+++ b/proto/bfd/packets.c
@@ -20,8 +20,8 @@ struct bfd_ctl_packet
   u32 req_min_echo_rx_int;
 };
 
-#define BFD_BASE_LEN sizeof(struct bfd_ctl_packet)
-
+#define BFD_BASE_LEN	sizeof(struct bfd_ctl_packet)
+#define BFD_MAX_LEN	64
 
 static inline u8 bfd_pack_vdiag(u8 version, u8 diag)
 { return (version << 5) | diag; }
@@ -43,11 +43,28 @@ static inline void bfd_pkt_set_state(struct bfd_ctl_packet *pkt, u8 val)
 { pkt->flags = val << 6; }
 
 
+char *
+bfd_format_flags(u8 flags, char *buf)
+{
+  char *bp = buf;
+  if (flags & BFD_FLAGS)	*bp++ = ' ';
+  if (flags & BFD_FLAG_POLL)	*bp++ = 'P';
+  if (flags & BFD_FLAG_FINAL)	*bp++ = 'F';
+  if (flags & BFD_FLAG_CPI)	*bp++ = 'C';
+  if (flags & BFD_FLAG_AP)	*bp++ = 'A';
+  if (flags & BFD_FLAG_DEMAND)	*bp++ = 'D';
+  if (flags & BFD_FLAG_MULTIPOINT) *bp++ = 'M';
+  *bp = 0;
+
+  return buf;
+}
+
 void
 bfd_send_ctl(struct bfd_proto *p, struct bfd_session *s, int final)
 {
   sock *sk = s->bsock->sk;
   struct bfd_ctl_packet *pkt = (struct bfd_ctl_packet *) sk->tbuf;
+  char fb[8];
 
   pkt->vdiag = bfd_pack_vdiag(1, s->loc_diag);
   pkt->flags = bfd_pack_flags(s->loc_state, 0);
@@ -65,7 +82,10 @@ bfd_send_ctl(struct bfd_proto *p, struct bfd_session *s, int final)
     pkt->flags |= BFD_FLAG_POLL;
 
   if (sk->tbuf != sk->tpos)
-    log(L_ERR "%s: old packet was overwritten in TX buffer", p->p.name);
+    log(L_WARN "%s: Old packet overwritten in TX buffer", p->p.name);
+
+  TRACE(D_PACKETS, "Sending CTL to %I [%s%s]", s->addr,
+	bfd_state_names[s->loc_state], bfd_format_flags(pkt->flags, fb));
 
   sk_send_to(sk, pkt->length, s->addr, sk->dport);
 }
@@ -79,6 +99,10 @@ bfd_rx_hook(sock *sk, int len)
   struct bfd_ctl_packet *pkt = (struct bfd_ctl_packet *) sk->rbuf;
   const char *err_dsc = NULL;
   uint err_val = 0;
+  char fb[8];
+
+  if ((sk->sport == BFD_CONTROL_PORT) && (sk->ttl < 255))
+    DROP("wrong TTL", sk->ttl);
 
   if (len < BFD_BASE_LEN)
     DROP("too short", len);
@@ -93,7 +117,8 @@ bfd_rx_hook(sock *sk, int len)
   if (pkt->detect_mult == 0)
     DROP("invalid detect mult", 0);
 
-  if (pkt->flags & BFD_FLAG_MULTIPOINT)
+  if ((pkt->flags & BFD_FLAG_MULTIPOINT) ||
+      ((pkt->flags & BFD_FLAG_POLL) && (pkt->flags & BFD_FLAG_FINAL)))
     DROP("invalid flags", pkt->flags);
 
   if (pkt->snd_id == 0)
@@ -107,7 +132,7 @@ bfd_rx_hook(sock *sk, int len)
     s = bfd_find_session_by_id(p, id);
 
     if (!s)
-      DROP("unknown session", id);
+      DROP("unknown session id", id);
   }
   else
   {
@@ -138,11 +163,14 @@ bfd_rx_hook(sock *sk, int len)
   s->rem_min_rx_int = ntohl(pkt->req_min_rx_int);
   s->rem_detect_mult = pkt->detect_mult;
 
+  TRACE(D_PACKETS, "CTL received from %I [%s%s]", sk->faddr,
+	bfd_state_names[s->rem_state], bfd_format_flags(pkt->flags, fb));
+
   bfd_session_process_ctl(s, pkt->flags, old_tx_int, old_rx_int);
   return 1;
 
  drop:
-  // log(L_WARN "%s: Bad packet from %I - %s (%u)", p->p.name, sk->faddr, err_dsc, err_val);
+  log(L_REMOTE "%s: Bad packet from %I - %s (%u)", p->p.name, sk->faddr, err_dsc, err_val);
   return 1;
 }
 
@@ -161,7 +189,7 @@ bfd_open_rx_sk(struct bfd_proto *p, int multihop)
   sk->sport = !multihop ? BFD_CONTROL_PORT : BFD_MULTI_CTL_PORT;
   sk->data = p;
 
-  sk->rbsize = 64; // XXX
+  sk->rbsize = BFD_MAX_LEN;
   sk->rx_hook = bfd_rx_hook;
   sk->err_hook = bfd_err_hook;
 
@@ -195,7 +223,7 @@ bfd_open_tx_sk(struct bfd_proto *p, ip_addr local, struct iface *ifa)
   sk->iface = ifa;
   sk->data = p;
 
-  sk->tbsize = 64; // XXX
+  sk->tbsize = BFD_MAX_LEN;
   sk->err_hook = bfd_err_hook;
 
   /* TODO: configurable ToS, priority and TTL security */