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

diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index 408053d5cfe07b36cfe403cbc9fb372b24b6256e..bb1ff09943263b66061fea580da6dce9343b583f 100644 (file)
--- a/proto/bgp/bgp.c
+++ b/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);

diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
index 8a61a1c5424f92d6e77bb72b2e77554748ec24ec..5069bdf203cd5ef607ebd5e701f88ca91f23ee41 100644 (file)
--- a/proto/bgp/packets.c
+++ b/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);
-
-  bool was_active = conn->channels_to_send || conn->packets_to_send;
+    BGP_TRACE(D_PACKETS, "Scheduling packet type %d %s", type, wont_ping ? " (TX already scheduled)" : "");
 
   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);

diff --git a/sysdep/unix/io-loop.c b/sysdep/unix/io-loop.c
index 6dca5f3c9fa7b6f24fcbb08ca9ba193b17bde2d2..9210efbab5d9292150b1542160426a82644d5648 100644 (file)
--- a/sysdep/unix/io-loop.c
+++ b/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)
-       continue;
+        while (s == loop->sock_active && s->rx_hook )
+       {
+         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)