Browse Source

bgpd: Send OPEN immediately on inbound connections

* bgpd_fsm.c: (bgp_connect_success) This is the transition function
  called when TCP_connection_open occurs in Connect or Active.  It
  sends OPEN, but only for a !ACCEPT_PEER.  I.e.  only on the local
  bgpd's outbound connection.

  This means OPEN will never be sent on a received connection, until
  OPEN is received on it.  Which means if the remote peer delays
  sending its OPEN on such an inbound connection, the local peer might
  hit a timer (e.g.  connectretry) before then and reset.

  There should be no harm in sending OPEN ASAP on any new connection
  with any conforming implementation, indeed this is supposed to be the
  behaviour.  It should speed up things, decrease the window in which
  collision detection could be hit, and make things more robust.  So do
  so.

* bgp_packet.c: (bgp_open_receive) Update the comment.
  Do not send bgp_open_send on the ACCEPT_PEER connection that has just
  been transferred over, that's now done in bgp_connect_success, as it
  should be.

  The accept peer's output fifo must also be transferred over, to
  ensure the Open gets sent, if not already, and the write thread state
  replicated accordingly.

* bgp_network.c: (bgp_accept) local AS config needs to set, so we can
  send Open early on ACCEPT_PEER connections.

Note: The Cumulus "Fix FSM to handle active/passive connections better"
patch also makes this change, amongst other things.
Paul Jakma 4 years ago
parent
commit
2d81a7a8e4
3 changed files with 13 additions and 3 deletions
  1. 1 2
      bgpd/bgp_fsm.c
  2. 2 0
      bgpd/bgp_network.c
  3. 10 1
      bgpd/bgp_packet.c

+ 1 - 2
bgpd/bgp_fsm.c

@@ -678,8 +678,7 @@ bgp_connect_success (struct peer *peer)
 	zlog_debug ("%s passive open", peer->host);
     }
 
-  if (! CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER))
-    bgp_open_send (peer);
+  bgp_open_send (peer);
 
   return 0;
 }

+ 2 - 0
bgpd/bgp_network.c

@@ -251,6 +251,8 @@ bgp_accept (struct thread *thread)
     peer->local_id = peer1->local_id;
     peer->v_holdtime = peer1->v_holdtime;
     peer->v_keepalive = peer1->v_keepalive;
+    peer->local_as = peer1->local_as;
+    peer->change_local_as = peer1->change_local_as;
 
     /* Make peer's address string. */
     sockunion2str (&su, buf, SU_ADDRSTRLEN);

+ 10 - 1
bgpd/bgp_packet.c

@@ -1423,6 +1423,9 @@ bgp_open_receive (struct peer *peer, bgp_size_t size)
  	   *   Connect                       Connect
  	   *
            *
+           * Note that Active->OpenSent historically in Quagga did not send
+           * OPEN on the accept-peer connection.
+           *
  	   * If both sides are Quagga, they're almost certain to wait for
  	   * the same amount of time of course (which doesn't preclude other
  	   * implementations also waiting for same time). The race is
@@ -1466,6 +1469,11 @@ bgp_open_receive (struct peer *peer, bgp_size_t size)
       realpeer->ibuf = peer->ibuf;
       realpeer->packet_size = peer->packet_size;
       peer->ibuf = NULL;
+    
+      /* Transfer output buffer, there may be an OPEN queued to send */
+      stream_fifo_free (realpeer->obuf);
+      realpeer->obuf = peer->obuf;
+      peer->obuf = NULL;
 
       /* Transfer status. */
       realpeer->status = peer->status;
@@ -1473,7 +1481,6 @@ bgp_open_receive (struct peer *peer, bgp_size_t size)
       
       /* peer pointer change. Open packet send to neighbor. */
       peer = realpeer;
-      bgp_open_send (peer);
       if (peer->fd < 0)
 	{
 	  zlog_err ("bgp_open_receive peer's fd is negative value %d",
@@ -1481,6 +1488,8 @@ bgp_open_receive (struct peer *peer, bgp_size_t size)
 	  return -1;
 	}
       BGP_READ_ON (peer->t_read, bgp_read, peer->fd);
+      if (stream_fifo_head (peer->obuf))
+        BGP_WRITE_ON (peer->t_write, bgp_write, peer->fd);
     }
 
   /* remote router-id check. */