bgp/bgp.c, bgp/packets.c, io-loop.c: solving slow shutdown when bird is busy

proto/bgp/bgp.c

@@ -862,7 +862,7 @@ bgp_conn_enter_close_state(struct bgp_conn *conn)
 
   bgp_conn_set_state(conn, BS_CLOSE);
   tm_stop(conn->keepalive_timer);
-  conn->sk->rx_hook = NULL;
+  //conn->sk->rx_hook = NULL;  // deleting rx_hook here may cause problems with pool listening
 
   /* Timeout for CLOSE state, if we cannot send notification soon then we just hangup */
   bgp_start_timer(p, conn->hold_timer, 10);

proto/bgp/packets.c

@@ -3094,6 +3094,8 @@ bgp_send(struct bgp_conn *conn, uint type, uint len)
   int success = sk_send(sk, len);
   if (success && ((conn->state == BS_ESTABLISHED) || (conn->state == BS_OPENCONFIRM)))
     bgp_start_timer(conn->bgp, conn->send_hold_timer, conn->send_hold_time);
+  struct bgp_proto *p = conn->bgp;
+  BGP_TRACE(D_PACKETS, "Scheduled packet type %d %s", type, success ? "succesfuly" : "unsuccesfully");
 
   return success;
 }
@@ -3234,12 +3236,11 @@ bgp_schedule_packet(struct bgp_conn *conn, struct bgp_channel *c, int type)
   ASSERT(conn->sk);
 
   struct bgp_proto *p = conn->bgp;
+  bool wont_ping = conn->channels_to_send || conn->packets_to_send || (conn->sk->tpos != conn->sk->tbuf);
   if (c)
-    BGP_TRACE(D_PACKETS, "Scheduling packet type %d for channel %s", type, c->c.name);
+    BGP_TRACE(D_PACKETS, "Scheduling packet type %d for channel %s%s", type, c->c.name, wont_ping ? " (TX already scheduled)" : "");
   else
-    BGP_TRACE(D_PACKETS, "Scheduling packet type %d", type);
+    BGP_TRACE(D_PACKETS, "Scheduling packet type %d %s", type, wont_ping ? " (TX already scheduled)" : "");
-
-  bool was_active = conn->channels_to_send || conn->packets_to_send;
 
   if (c)
   {
@@ -3255,7 +3256,7 @@ bgp_schedule_packet(struct bgp_conn *conn, struct bgp_channel *c, int type)
   else
     conn->packets_to_send |= 1 << type;
 
-  if (was_active || (conn->sk->tpos != conn->sk->tbuf))
+  if (wont_ping && (type != PKT_SCHEDULE_CLOSE))
     return;
   else if ((type == PKT_KEEPALIVE) || (this_birdloop != p->p.loop))
     while (bgp_fire_tx(conn) > 0)
@@ -3572,7 +3573,10 @@ bgp_rx(sock *sk, uint size)
   while (end >= pkt_start + BGP_HEADER_LENGTH)
     {
       if ((conn->state == BS_CLOSE) || (conn->sk != sk))
-	return 0;
+      {
+        BGP_TRACE(D_PACKETS,"Packet arrived after closing");
+        return 0;
+      }
       if ((conn->state == BS_ESTABLISHED) && rt_cork_check(&conn->bgp->uncork))
       {
 	sk_pause_rx(p->p.loop, sk);

sysdep/unix/io-loop.c

@@ -535,8 +535,16 @@ sockets_fire(struct birdloop *loop, bool read, bool write)
       if (write && (rev & POLLOUT))
       {
 	/* Write until task limit is up */
-	while ((s == loop->sock_active) && (e = sk_write(s)) && task_still_in_limit())
+       while (s == loop->sock_active)
-	  ;
+       {
+         int fd = s->fd; /* The socket may disappear after sk_write! */
+         e = sk_write(s);
+         bool allowed = task_still_in_limit();
+         LOOP_TRACE(loop, DL_SOCKETS, "Writing to fd %d: %s%s", fd, e ? "again" : "done", (e && !allowed) ? " (out of time)" : "");
+
+         if (!e || !allowed)
+           break;
+       }
 
 	if (s != loop->sock_active)
 	  continue;
@@ -547,11 +555,21 @@ sockets_fire(struct birdloop *loop, bool read, bool write)
 
       /* Read until task limit is up */
       if (read && (rev & POLLIN))
-	while ((s == loop->sock_active) && s->rx_hook && sk_read(s, rev) && (s->fast_rx || task_still_in_limit()))
+      {
-	  ;
 
-      if (s != loop->sock_active)
+        while (s == loop->sock_active && s->rx_hook )
-	continue;
+	{
+	  int fd = s->fd; /* The socket may disappear after sk_write! */
+          e = sk_read(s, rev);
+          bool allowed = task_still_in_limit();
+          LOOP_TRACE(loop, DL_SOCKETS, "Read from fd %d: %s%s", fd, e ? "again" : "done", (e && !allowed) ? " (out of time)" : "");
+
+          if (!e || !allowed)
+            break;
+	}
+        if (s != loop->sock_active)
+	  continue;
+      }
 
       if (!(rev & (POLLOUT | POLLIN)) && (rev & POLLERR))
 	sk_err(s, rev);
@@ -914,7 +932,15 @@ bird_thread_main(void *arg)
     account_to(&this_thread->idle);
     birdloop_leave(thr->meta);
 poll_retry:;
+    for (uint i=0; i<pfd.pfd.used; i++)
+    {
+      if (pfd.loop.data[i])
+        LOOP_TRACE(pfd.loop.data[i], DL_SOCKETS, "Polling fd %d with events 0x%x (timeout %i)",
+	  pfd.pfd.data[i].fd, pfd.pfd.data[i].events, timeout);
+    }
+
     int rv = poll(pfd.pfd.data, pfd.pfd.used, timeout);
+    THREAD_TRACE(DL_SOCKETS, "Returned %d from poll", rv);
     if (rv < 0)
     {
       if (errno == EINTR || errno == EAGAIN)