BMP: Use OPEN messages stored in BGP

The BMP protocol needs OPEN messages of established BGP sessions to
construct appropriate Peer Up messages. Instead of saving them internally
we use OPEN messages stored in BGP instances. This allows BMP instances
to be restarted or enabled later.

Because of this change, we can simplify BMP data structures. No need to
keep track of BGP sessions when we are not started. We have to iterate
over all (established) BGP sessions when the BMP session is established.
This is just a scaffolding now, but some kind of iteration would be
necessary anyway.

Also, the commit cleans up handling of msg/msg_length arguments to be
body/body_length consistently in both rx/tx and peer_up/peer_down calls.

proto/bgp/bgp.c

@@ -689,6 +689,8 @@ bgp_conn_enter_established_state(struct bgp_conn *conn)
 
   bgp_conn_set_state(conn, BS_ESTABLISHED);
   proto_notify_state(&p->p, PS_UP);
+  bmp_peer_up(p, conn->local_open_msg, conn->local_open_length,
+	      conn->remote_open_msg, conn->remote_open_length);
 }
 
 static void

proto/bgp/packets.c

@@ -167,7 +167,6 @@ bgp_create_notification(struct bgp_conn *conn, byte *buf)
   buf[0] = conn->notify_code;
   buf[1] = conn->notify_subcode;
   memcpy(buf+2, conn->notify_data, conn->notify_size);
-  bmp_peer_down(p, BE_NONE, buf, conn->notify_size + 2);
   return buf + 2 + conn->notify_size;
 }
 
@@ -988,7 +987,6 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, uint len)
   bgp_schedule_packet(conn, NULL, PKT_KEEPALIVE);
   bgp_start_timer(conn->hold_timer, conn->hold_time);
   bgp_conn_enter_openconfirm_state(conn);
-  bmp_put_recv_bgp_open_msg(p, pkt, len);
 }
 
 
@@ -3037,6 +3035,7 @@ bgp_fire_tx(struct bgp_conn *conn)
   {
     conn->packets_to_send = 1 << PKT_SCHEDULE_CLOSE;
     end = bgp_create_notification(conn, pkt);
+    bmp_peer_down(p, BE_BGP_TX, pkt, end - pkt);
     return bgp_send(conn, PKT_NOTIFICATION, end - buf);
   }
   else if (s & (1 << PKT_OPEN))
@@ -3047,12 +3046,7 @@ bgp_fire_tx(struct bgp_conn *conn)
     conn->local_open_msg = bgp_copy_open(p, buf, end - buf);
     conn->local_open_length = end - buf - BGP_HEADER_LENGTH;
 
-    int rv = bgp_send(conn, PKT_OPEN, end - buf);
-    if (rv >= 0)
-    {
-      bmp_put_sent_bgp_open_msg(p, pkt, end - buf);
-    }
-    return rv;
+    return bgp_send(conn, PKT_OPEN, end - buf);
   }
   else if (s & (1 << PKT_KEEPALIVE))
   {
@@ -3352,7 +3346,7 @@ bgp_rx_notification(struct bgp_conn *conn, byte *pkt, uint len)
     }
   }
 
-  bmp_peer_down(p, BE_NONE, pkt, len);
+  bmp_peer_down(p, BE_BGP_RX, pkt + BGP_HEADER_LENGTH, len - BGP_HEADER_LENGTH);
 }
 
 static void

proto/bmp/bmp.c

@@ -214,7 +214,7 @@ struct bmp_data_node {
 };
 
 static void
-bmp_route_monitor_pre_policy_table_in_snapshot(struct channel *C);
+bmp_route_monitor_pre_policy_table_in_snapshot(struct bgp_channel *c);
 
 static void
 bmp_common_hdr_serialize(buffer *stream, const enum bmp_message_type type, const u32 data_size)
@@ -336,12 +336,14 @@ bmp_put_ipa(buffer *stream, const ip_addr addr)
 }
 
 static void
-bmp_set_initial_bgp_hdr(buffer *stream, const u16 msg_size, const u8 msg_type)
+bmp_put_bgp_hdr(buffer *stream, const u8 msg_type, const u16 msg_length)
 {
-  byte marker[BGP_MSG_HDR_MARKER_SIZE];
-  memset(marker, 0xff, BGP_MSG_HDR_MARKER_SIZE);
-  bmp_put_data(stream, marker, BGP_MSG_HDR_MARKER_SIZE);
-  bmp_put_u16(stream, msg_size);
+  bmp_buffer_need(stream, BGP_HEADER_LENGTH);
+
+  memset(stream->pos, 0xff, BGP_HDR_MARKER_LENGTH);
+  stream->pos += BGP_HDR_MARKER_LENGTH;
+
+  bmp_put_u16(stream, msg_length);
   bmp_put_u8(stream, msg_type);
 }
 
@@ -413,8 +415,10 @@ bmp_peer_up_notif_msg_serialize(buffer *stream, const bool is_peer_global,
   const u16 remote_port, const byte *sent_msg, const size_t sent_msg_size,
   const byte *recv_msg, const size_t recv_msg_size)
 {
-  const size_t data_size = BMP_PER_PEER_HDR_SIZE + BMP_PEER_UP_NOTIF_MSG_FIX_SIZE
-                             + sent_msg_size + recv_msg_size;
+  const size_t data_size =
+    BMP_PER_PEER_HDR_SIZE + BMP_PEER_UP_NOTIF_MSG_FIX_SIZE +
+    BGP_HEADER_LENGTH + sent_msg_size + BGP_HEADER_LENGTH + recv_msg_size;
+
   bmp_buffer_need(stream, BMP_COMMON_HDR_SIZE + data_size);
   bmp_common_hdr_serialize(stream, BMP_PEER_UP_NOTIF, data_size);
   bmp_per_peer_hdr_serialize(stream, is_peer_global,
@@ -423,11 +427,9 @@ bmp_peer_up_notif_msg_serialize(buffer *stream, const bool is_peer_global,
   bmp_put_ipa(stream, local_addr);
   bmp_put_u16(stream, local_port);
   bmp_put_u16(stream, remote_port);
-  bmp_set_initial_bgp_hdr(stream, sent_msg_size, PKT_OPEN);
-  const size_t missing_bgp_hdr_size = BGP_MSG_HDR_MARKER_SIZE
-                                        + BGP_MSG_HDR_LENGTH_SIZE
-                                        + BGP_MSG_HDR_TYPE_SIZE;
-  bmp_put_data(stream, sent_msg, sent_msg_size - missing_bgp_hdr_size);
+  bmp_put_bgp_hdr(stream, PKT_OPEN, BGP_HEADER_LENGTH + sent_msg_size);
+  bmp_put_data(stream, sent_msg, sent_msg_size);
+  bmp_put_bgp_hdr(stream, PKT_OPEN, BGP_HEADER_LENGTH + recv_msg_size);
   bmp_put_data(stream, recv_msg, recv_msg_size);
 }
 
@@ -445,43 +447,37 @@ bmp_peer_down_notif_msg_serialize(buffer *stream, const bool is_peer_global,
   bmp_put_data(stream, data, data_size);
 }
 
-static void
-bmp_peer_map_walk_tx_open_msg_and_send_peer_up_notif(
-  const struct bmp_peer_map_key key, const byte *tx_msg,
-  const size_t tx_msg_size, void *bmp_)
+void
+bmp_peer_up(const struct bgp_proto *bgp,
+	    const byte *tx_open_msg, uint tx_open_length,
+	    const byte *rx_open_msg, uint rx_open_length)
 {
-  struct bmp_proto *p = bmp_;
-  ASSERT(p->started);
+  struct bmp_proto *p = g_bmp;
 
-  const struct bmp_peer_map_entry *map_rx_msg = bmp_peer_map_get(&p->peer_open_msg.rx_msg, key);
-  IF_PTR_IS_NULL_PRINT_ERR_MSG_AND_RETURN_OPT_VAL(
-    map_rx_msg,
-    "Processing TX BGP OPEN MSG but there is not corresponding received MSG"
-  );
+  if (!p || !p->started)
+    return;
 
-  const struct bmp_peer_map_entry *map_bgp_proto = bmp_peer_map_get(&p->bgp_peers, key);
-  IF_PTR_IS_NULL_PRINT_ERR_MSG_AND_RETURN_OPT_VAL(
-    map_bgp_proto,
-    "There is not BGP proto related with stored TX/RX OPEN MSG"
-  );
+  // struct bmp_peer_map_key key = bmp_peer_map_key_create(bgp->remote_ip, bgp->remote_as);
+  // bmp_peer_map_insert(&p->bgp_peers, key, (const byte *) &bgp, sizeof (bgp));
 
-  const struct bgp_proto *bgp;
-  memcpy(&bgp, map_bgp_proto->data.buf, sizeof (bgp));
-  if (bgp->p.proto_state == PS_UP)
-  {
-    bmp_send_peer_up_notif_msg(p, bgp, tx_msg, tx_msg_size,
-			       map_rx_msg->data.buf, map_rx_msg->data.buf_size);
-  }
+  bmp_send_peer_up_notif_msg(p, bgp, tx_open_msg, tx_open_length, rx_open_msg, rx_open_length);
+
+  struct bgp_channel *c;
+  BGP_WALK_CHANNELS(bgp, c)
+    bmp_route_monitor_pre_policy_table_in_snapshot(c);
 }
 
 static void
-bmp_peer_up(const struct bgp_proto *bgp)
+bmp_peer_init(const struct bgp_proto *bgp)
 {
-  struct bgp_channel *c;
-  WALK_LIST(c, bgp->p.channels)
-  {
-    bmp_route_monitor_pre_policy_table_in_snapshot((struct channel *) c);
-  }
+  struct bgp_conn *conn = bgp->conn;
+
+  if (!conn || (conn->state != BS_ESTABLISHED) ||
+      !conn->local_open_msg || !conn->remote_open_msg)
+    return;
+
+  bmp_peer_up(bgp, conn->local_open_msg, conn->local_open_length,
+	      conn->remote_open_msg, conn->remote_open_length);
 }
 
 static const struct birdsock *
@@ -579,60 +575,6 @@ bmp_send_peer_up_notif_msg(struct bmp_proto *p, const struct bgp_proto *bgp,
     rx_data, rx_data_size);
   bmp_schedule_tx_packet(p, bmp_buffer_data(&payload), bmp_buffer_pos(&payload));
   bmp_buffer_free(&payload);
-
-  bmp_peer_up(bgp);
-}
-
-void
-bmp_put_sent_bgp_open_msg(const struct bgp_proto *bgp, const byte* pkt,
-  const size_t pkt_size)
-{
-  struct bmp_proto *p = g_bmp;
-
-  if (!p)
-  {
-    return;
-  }
-
-  struct bmp_peer_map_key key
-    = bmp_peer_map_key_create(bgp->remote_ip, bgp->remote_as);
-  const struct bmp_peer_map_entry *rx_msg
-    = bmp_peer_map_get(&p->peer_open_msg.rx_msg, key);
-
-  bmp_peer_map_insert(&p->peer_open_msg.tx_msg, key, pkt, pkt_size);
-
-  if (!rx_msg)
-    bmp_peer_map_insert(&p->bgp_peers, key, (const byte *) &bgp, sizeof (bgp));
-
-  if (rx_msg && p->started)
-    bmp_send_peer_up_notif_msg(p, bgp, pkt, pkt_size, rx_msg->data.buf,
-			       rx_msg->data.buf_size);
-}
-
-void
-bmp_put_recv_bgp_open_msg(const struct bgp_proto *bgp, const byte* pkt,
-  const size_t pkt_size)
-{
-  struct bmp_proto *p = g_bmp;
-
-  if (!p)
-  {
-    return;
-  }
-
-  struct bmp_peer_map_key key
-    = bmp_peer_map_key_create(bgp->remote_ip, bgp->remote_as);
-  const struct bmp_peer_map_entry *tx_msg
-    = bmp_peer_map_get(&p->peer_open_msg.tx_msg, key);
-
-  bmp_peer_map_insert(&p->peer_open_msg.rx_msg, key, pkt, pkt_size);
-
-  if (!tx_msg)
-    bmp_peer_map_insert(&p->bgp_peers, key, (const byte *) &bgp, sizeof (bgp));
-
-  if (tx_msg && p->started)
-    bmp_send_peer_up_notif_msg(p, bgp, tx_msg->data.buf, tx_msg->data.buf_size,
-			       pkt, pkt_size);
 }
 
 void
@@ -775,7 +717,7 @@ bmp_route_monitor_update_in_pre_end()
 }
 
 static void
-bmp_route_monitor_pre_policy_table_in_snapshot(struct channel *C)
+bmp_route_monitor_pre_policy_table_in_snapshot(struct bgp_channel *c)
 {
   struct bmp_proto *p = g_bmp;
 
@@ -784,7 +726,7 @@ bmp_route_monitor_pre_policy_table_in_snapshot(struct channel *C)
     return;
   }
 
-  struct rtable *tab = C->in_table;
+  struct rtable *tab = c->c.in_table;
   if (!tab)
   {
     return;
@@ -808,7 +750,7 @@ bmp_route_monitor_pre_policy_table_in_snapshot(struct channel *C)
     rte *e;
     for (e = n->routes; e; e = e->next)
     {
-      bgp_rte_update_in_notify(C, n->n.addr, e, e->src);
+      bgp_rte_update_in_notify(&c->c, n->n.addr, e, e->src);
     }
 
     bmp_route_monitor_update_in_pre_commit((struct bgp_proto *) P);
@@ -821,15 +763,13 @@ bmp_route_monitor_pre_policy_table_in_snapshot(struct channel *C)
   {
     bmp_route_monitor_update_in_pre_begin();
     byte rx_end_payload[DEFAULT_MEM_BLOCK_SIZE];
-    byte *pos
-      = bgp_create_end_mark((struct bgp_channel *) C, rx_end_payload
-                                                        + BGP_HEADER_LENGTH);
+    byte *pos = bgp_create_end_mark(c, rx_end_payload + BGP_HEADER_LENGTH);
     memset(rx_end_payload + BGP_MSG_HDR_MARKER_POS, 0xff,
              BGP_MSG_HDR_MARKER_SIZE); // BGP UPDATE MSG marker
     put_u16(rx_end_payload + BGP_MSG_HDR_LENGTH_POS, pos - rx_end_payload);
     put_u8(rx_end_payload + BGP_MSG_HDR_TYPE_POS, PKT_UPDATE);
     bmp_route_monitor_put_update_in_pre_msg(rx_end_payload, pos - rx_end_payload);
-    bmp_route_monitor_update_in_pre_commit((struct bgp_proto *) C->proto);
+    bmp_route_monitor_update_in_pre_commit((struct bgp_proto *) c->c.proto);
     bmp_route_monitor_update_in_pre_end();
   }
 }
@@ -854,49 +794,28 @@ bmp_send_peer_down_notif_msg(struct bmp_proto *p, const struct bgp_proto *bgp,
 }
 
 void
-bmp_peer_down(const struct bgp_proto *bgp, const int err_class, const byte *pkt,
-  size_t pkt_size)
+bmp_peer_down(const struct bgp_proto *bgp, const int err_class,
+	      const byte *msg, size_t msg_length)
 {
   struct bmp_proto *p = g_bmp;
 
-  if (!p)
-  {
-    return;
-  }
-
-  struct bmp_peer_map_key key
-    = bmp_peer_map_key_create(bgp->remote_ip, bgp->remote_as);
-
-  bmp_peer_map_remove(&p->peer_open_msg.tx_msg, key);
-  bmp_peer_map_remove(&p->peer_open_msg.rx_msg, key);
-  bmp_peer_map_remove(&p->bgp_peers, key);
-  const size_t missing_bgp_hdr_size = BGP_MSG_HDR_MARKER_SIZE
-                                        + BGP_MSG_HDR_LENGTH_SIZE
-                                        + BGP_MSG_HDR_TYPE_SIZE;
-
-  if (!p->started)
+  if (!p || !p->started)
     return;
 
-  buffer payload
-    = bmp_buffer_alloc(p->buffer_mpool, pkt_size + missing_bgp_hdr_size + 1);
-  if (pkt != NULL && pkt_size > 0)
+  // struct bmp_peer_map_key key = bmp_peer_map_key_create(bgp->remote_ip, bgp->remote_as);
+  // bmp_peer_map_remove(&p->bgp_peers, key);
+
+  buffer payload = bmp_buffer_alloc(p->buffer_mpool, 1 + BGP_HEADER_LENGTH + msg_length);
+
+  if (msg)
   {
-    byte marker[BGP_MSG_HDR_MARKER_SIZE];
-    memset(marker, 0xff, BGP_MSG_HDR_MARKER_SIZE); // NOTIF MSG marker
-    if (!memcmp(pkt, marker, BGP_MSG_HDR_MARKER_SIZE))
-    {
-      // So it is received BGP PDU
-      bmp_put_u8(&payload, BMP_PEER_DOWN_REASON_REMOTE_BGP_NOTIFICATION);
-      bmp_put_data(&payload, pkt, pkt_size);
-    }
-    else
-    {
+    if (err_class == BE_BGP_TX)
       bmp_put_u8(&payload, BMP_PEER_DOWN_REASON_LOCAL_BGP_NOTIFICATION);
-      bmp_put_data(&payload, marker, BGP_MSG_HDR_MARKER_SIZE);
-      bmp_put_u16(&payload, pkt_size);
-      bmp_put_u8(&payload, PKT_NOTIFICATION);
-      bmp_put_data(&payload, pkt, pkt_size);
-    }
+    else
+      bmp_put_u8(&payload, BMP_PEER_DOWN_REASON_REMOTE_BGP_NOTIFICATION);
+
+    bmp_put_bgp_hdr(&payload, BGP_HEADER_LENGTH + msg_length, PKT_NOTIFICATION);
+    bmp_put_data(&payload, msg, msg_length);
   }
   else
   {
@@ -962,8 +881,10 @@ bmp_startup(struct bmp_proto *p)
   bmp_buffer_free(&payload);
 
   /* Send Peer Up messages */
-  bmp_peer_map_walk(&p->peer_open_msg.tx_msg,
-		    bmp_peer_map_walk_tx_open_msg_and_send_peer_up_notif, p);
+  struct proto *peer;
+  WALK_LIST(peer, proto_list)
+    if ((peer->proto->class == PROTOCOL_BGP) && (peer->proto_state == PS_UP))
+      bmp_peer_init((struct bgp_proto *) peer);
 
   proto_notify_state(&p->p, PS_UP);
 }
@@ -1105,9 +1026,7 @@ bmp_start(struct proto *P)
   p->connect_retry_timer = tm_new_init(p->p.pool, bmp_connection_retry, p, 0, 0);
   p->sk = NULL;
 
-  bmp_peer_map_init(&p->peer_open_msg.tx_msg, p->map_mem_pool);
-  bmp_peer_map_init(&p->peer_open_msg.rx_msg, p->map_mem_pool);
-  bmp_peer_map_init(&p->bgp_peers, p->map_mem_pool);
+  // bmp_peer_map_init(&p->bgp_peers, p->map_mem_pool);
 
   init_list(&p->tx_queue);
   init_list(&p->rt_table_in_pre_policy.update_msg_queue);

proto/bmp/bmp.h

@@ -46,12 +46,6 @@ struct bmp_config {
 struct bgp_proto;
 struct bmp_proto;
 
-// Stores sent and received BGP OPEN MSGs
-struct bmp_peer_open_msg {
-  struct bmp_peer_map tx_msg;
-  struct bmp_peer_map rx_msg;
-};
-
 // Keeps necessary information during composing BGP UPDATE MSG which is going
 // to be sent to the BMP collector
 struct rt_table_info {
@@ -72,8 +66,7 @@ struct bmp_proto {
   u16 station_port;                // Monitoring station TCP port
   struct monitoring_rib monitoring_rib;
   // Below fields are for internal use
-  struct bmp_peer_map bgp_peers;   // Stores 'bgp_proto' structure per BGP peer
-  struct bmp_peer_open_msg peer_open_msg; // Stores sent and received BGP OPEN MSG per BGP peer
+  // struct bmp_peer_map bgp_peers;   // Stores 'bgp_proto' structure per BGP peer
   pool *buffer_mpool;              // Memory pool used for BMP buffer allocations
   pool *map_mem_pool;              // Memory pool used for BMP map allocations
   pool *tx_mem_pool;               // Memory pool used for packet allocations designated to BMP collector
@@ -88,24 +81,12 @@ struct bmp_proto {
 #ifdef CONFIG_BMP
 
 /**
- * bmp_put_sent_bgp_open_msg - save sent BGP OPEN msg packet in BMP implementation.
- * NOTE: If there has been passed sent and received BGP OPEN MSGs to the BMP
- *       implementation, then there is going to be send BMP Peer Up Notification
- *       message to the BMP collector.
+ * bmp_peer_up - send notification that BGP peer connection is established
  */
 void
-bmp_put_sent_bgp_open_msg(const struct bgp_proto *bgp, const byte* pkt,
-  const size_t pkt_size);
-
-/**
- * bmp_put_recv_bgp_open_msg - save received BGP OPEN msg packet in BMP implementation.
- * NOTE: If there has been passed sent and received BGP OPEN MSGs to the BMP
- *       implementation, then there is going to be send BMP Peer Up Notification
- *       message to the BMP collector.
- */
-void
-bmp_put_recv_bgp_open_msg(const struct bgp_proto *bgp, const byte* pkt,
-  const size_t pkt_size);
+bmp_peer_up(const struct bgp_proto *bgp,
+	    const byte *tx_open_msg, uint tx_open_length,
+	    const byte *rx_open_msg, uint rx_open_length);
 
 /**
  * The following 4 functions create BMP Route Monitoring message based on
@@ -140,8 +121,7 @@ bmp_peer_down(const struct bgp_proto *bgp, const int err_class, const byte *pkt,
 
 #else /* BMP build disabled */
 
-static inline void bmp_put_sent_bgp_open_msg(const struct bgp_proto *bgp UNUSED, const byte* pkt UNUSED, const size_t pkt_size UNUSED) { }
-static inline void bmp_put_recv_bgp_open_msg(const struct bgp_proto *bgp UNUSED, const byte* pkt UNUSED, const size_t pkt_size UNUSED) { }
+static inline void bmp_peer_up(const struct bgp_proto *bgp, const byte *tx_open_msg, uint tx_open_length, const byte *rx_open_msg, uint rx_open_length) { }
 static inline void bmp_route_monitor_update_in_pre_begin(void) { }
 static inline void bmp_route_monitor_put_update_in_pre_msg(const byte *data UNUSED, const size_t data_size UNUSED) { }
 static inline void bmp_route_monitor_update_in_pre_commit(const struct bgp_proto *bgp UNUSED) { }