Merge pull request #14075 from donaldsharp/bgp_memory_fun

Remove unused memory allocations associated with bgp

bgpd/bgp_advertise.c

@@ -219,29 +219,3 @@ bool bgp_adj_in_unset(struct bgp_dest *dest, struct peer *peer,
 
 	return true;
 }
-
-void bgp_sync_init(struct peer *peer)
-{
-	afi_t afi;
-	safi_t safi;
-	struct bgp_synchronize *sync;
-
-	FOREACH_AFI_SAFI (afi, safi) {
-		sync = XCALLOC(MTYPE_BGP_SYNCHRONISE,
-			       sizeof(struct bgp_synchronize));
-		bgp_adv_fifo_init(&sync->update);
-		bgp_adv_fifo_init(&sync->withdraw);
-		bgp_adv_fifo_init(&sync->withdraw_low);
-		peer->sync[afi][safi] = sync;
-	}
-}
-
-void bgp_sync_delete(struct peer *peer)
-{
-	afi_t afi;
-	safi_t safi;
-
-	FOREACH_AFI_SAFI (afi, safi) {
-		XFREE(MTYPE_BGP_SYNCHRONISE, peer->sync[afi][safi]);
-	}
-}

bgpd/bgp_advertise.h

@@ -100,7 +100,6 @@ struct bgp_adj_in {
 struct bgp_synchronize {
 	struct bgp_adv_fifo_head update;
 	struct bgp_adv_fifo_head withdraw;
-	struct bgp_adv_fifo_head withdraw_low;
 };
 
 /* BGP adjacency linked list.  */
@@ -135,8 +134,6 @@ extern bool bgp_adj_in_unset(struct bgp_dest *dest, struct peer *peer,
 			     uint32_t addpath_id);
 extern void bgp_adj_in_remove(struct bgp_dest *dest, struct bgp_adj_in *bai);
 
-extern void bgp_sync_init(struct peer *peer);
-extern void bgp_sync_delete(struct peer *peer);
 extern unsigned int bgp_advertise_attr_hash_key(const void *p);
 extern bool bgp_advertise_attr_hash_cmp(const void *p1, const void *p2);
 extern void bgp_advertise_add(struct bgp_advertise_attr *baa,

bgpd/bgp_fsm.c

@@ -1517,8 +1517,6 @@ enum bgp_fsm_state_progress bgp_stop(struct peer *peer)
 
 		if (peer->ibuf_work)
 			ringbuf_wipe(peer->ibuf_work);
-		if (peer->obuf_work)
-			stream_reset(peer->obuf_work);
 
 		if (peer->curr) {
 			stream_free(peer->curr);

bgpd/bgp_io.c

@@ -480,13 +480,18 @@ done : {
 	return status;
 }
 
+uint8_t ibuf_scratch[BGP_EXTENDED_MESSAGE_MAX_PACKET_SIZE * BGP_READ_PACKET_MAX];
 /*
  * Reads a chunk of data from peer->fd into peer->ibuf_work.
  *
  * code_p
  *    Pointer to location to store FSM event code in case of fatal error.
  *
  * @return status flag (see top-of-file)
+ *
+ * PLEASE NOTE:  If we ever transform the bgp_read to be a pthread
+ * per peer then we need to rethink the global ibuf_scratch
+ * data structure above.
  */
 static uint16_t bgp_read(struct peer *peer, int *code_p)
 {
@@ -502,9 +507,9 @@ static uint16_t bgp_read(struct peer *peer, int *code_p)
 		return status;
 	}
 
-	readsize = MIN(ibuf_work_space, sizeof(peer->ibuf_scratch));
+	readsize = MIN(ibuf_work_space, sizeof(ibuf_scratch));
 
-	nbytes = read(peer->fd, peer->ibuf_scratch, readsize);
+	nbytes = read(peer->fd, ibuf_scratch, readsize);
 
 	/* EAGAIN or EWOULDBLOCK; come back later */
 	if (nbytes < 0 && ERRNO_IO_RETRY(errno)) {
@@ -533,8 +538,8 @@ static uint16_t bgp_read(struct peer *peer, int *code_p)
 
 		SET_FLAG(status, BGP_IO_FATAL_ERR);
 	} else {
-		assert(ringbuf_put(peer->ibuf_work, peer->ibuf_scratch, nbytes)
-		       == (size_t)nbytes);
+		assert(ringbuf_put(peer->ibuf_work, ibuf_scratch, nbytes) ==
+		       (size_t)nbytes);
 	}
 
 	return status;

bgpd/bgp_updgrp.c

@@ -74,7 +74,7 @@ static void sync_init(struct update_subgroup *subgrp,
 		XCALLOC(MTYPE_BGP_SYNCHRONISE, sizeof(struct bgp_synchronize));
 	bgp_adv_fifo_init(&subgrp->sync->update);
 	bgp_adv_fifo_init(&subgrp->sync->withdraw);
-	bgp_adv_fifo_init(&subgrp->sync->withdraw_low);
+
 	subgrp->hash =
 		hash_create(bgp_advertise_attr_hash_key,
 			    bgp_advertise_attr_hash_cmp, "BGP SubGroup Hash");

bgpd/bgp_updgrp.h

@@ -584,11 +584,9 @@ static inline void bgp_announce_peer(struct peer *peer)
  */
 static inline int advertise_list_is_empty(struct update_subgroup *subgrp)
 {
-	if (bgp_adv_fifo_count(&subgrp->sync->update)
-	    || bgp_adv_fifo_count(&subgrp->sync->withdraw)
-	    || bgp_adv_fifo_count(&subgrp->sync->withdraw_low)) {
+	if (bgp_adv_fifo_count(&subgrp->sync->update) ||
+	    bgp_adv_fifo_count(&subgrp->sync->withdraw))
 		return 0;
-	}
 
 	return 1;
 }

bgpd/bgpd.c

@@ -1171,8 +1171,6 @@ static void peer_free(struct peer *peer)
 	if (peer->clear_node_queue)
 		work_queue_free_and_null(&peer->clear_node_queue);
 
-	bgp_sync_delete(peer);
-
 	XFREE(MTYPE_PEER_CONF_IF, peer->conf_if);
 
 	XFREE(MTYPE_BGP_SOFT_VERSION, peer->soft_version);
@@ -1415,27 +1413,9 @@ struct peer *peer_new(struct bgp *bgp)
 	peer->obuf = stream_fifo_new();
 	pthread_mutex_init(&peer->io_mtx, NULL);
 
-	/* We use a larger buffer for peer->obuf_work in the event that:
-	 * - We RX a BGP_UPDATE where the attributes alone are just
-	 *   under BGP_EXTENDED_MESSAGE_MAX_PACKET_SIZE.
-	 * - The user configures an outbound route-map that does many as-path
-	 *   prepends or adds many communities. At most they can have
-	 *   CMD_ARGC_MAX args in a route-map so there is a finite limit on how
-	 *   large they can make the attributes.
-	 *
-	 * Having a buffer with BGP_MAX_PACKET_SIZE_OVERFLOW allows us to avoid
-	 * bounds checking for every single attribute as we construct an
-	 * UPDATE.
-	 */
-	peer->obuf_work =
-		stream_new(BGP_MAX_PACKET_SIZE + BGP_MAX_PACKET_SIZE_OVERFLOW);
 	peer->ibuf_work =
 		ringbuf_new(BGP_MAX_PACKET_SIZE * BGP_READ_PACKET_MAX);
 
-	peer->scratch = stream_new(BGP_MAX_PACKET_SIZE);
-
-	bgp_sync_init(peer);
-
 	/* Get service port number.  */
 	sp = getservbyname("bgp", "tcp");
 	peer->port = (sp == NULL) ? BGP_PORT_DEFAULT : ntohs(sp->s_port);
@@ -2624,16 +2604,6 @@ int peer_delete(struct peer *peer)
 		peer->ibuf_work = NULL;
 	}
 
-	if (peer->obuf_work) {
-		stream_free(peer->obuf_work);
-		peer->obuf_work = NULL;
-	}
-
-	if (peer->scratch) {
-		stream_free(peer->scratch);
-		peer->scratch = NULL;
-	}
-
 	/* Local and remote addresses. */
 	if (peer->su_local) {
 		sockunion_free(peer->su_local);

bgpd/bgpd.h

@@ -1163,20 +1163,10 @@ struct peer {
 	struct stream_fifo *ibuf; // packets waiting to be processed
 	struct stream_fifo *obuf; // packets waiting to be written
 
-	/* used as a block to deposit raw wire data to */
-	uint8_t ibuf_scratch[BGP_EXTENDED_MESSAGE_MAX_PACKET_SIZE
-			     * BGP_READ_PACKET_MAX];
 	struct ringbuf *ibuf_work; // WiP buffer used by bgp_read() only
-	struct stream *obuf_work;  // WiP buffer used to construct packets
 
 	struct stream *curr; // the current packet being parsed
 
-	/* We use a separate stream to encode MP_REACH_NLRI for efficient
-	 * NLRI packing. peer->obuf_work stores all the other attributes. The
-	 * actual packet is then constructed by concatenating the two.
-	 */
-	struct stream *scratch;
-
 	/* the doppelganger peer structure, due to dual TCP conn setup */
 	struct peer *doppelganger;
 
@@ -1612,8 +1602,6 @@ struct peer {
 	uint8_t update_delay_over; /* When this is set, BGP is no more waiting
 				     for EOR */
 
-	/* Syncronization list and time.  */
-	struct bgp_synchronize *sync[AFI_MAX][SAFI_MAX];
 	time_t synctime;
 	/* timestamp when the last UPDATE msg was written */
 	_Atomic time_t last_write;

bgpd/rfapi/rfapi.c

@@ -1237,7 +1237,6 @@ static int rfapi_open_inner(struct rfapi_descriptor *rfd, struct bgp *bgp,
 	 */
 	rfd->peer = peer_new(bgp);
 	rfd->peer->status = Established; /* keep bgp core happy */
-	bgp_sync_delete(rfd->peer);      /* don't need these */
 
 	/*
 	 * since this peer is not on the I/O thread, this lock is not strictly
@@ -1252,12 +1251,9 @@ static int rfapi_open_inner(struct rfapi_descriptor *rfd, struct bgp *bgp,
 
 		if (rfd->peer->ibuf_work)
 			ringbuf_del(rfd->peer->ibuf_work);
-		if (rfd->peer->obuf_work)
-			stream_free(rfd->peer->obuf_work);
 
 		rfd->peer->ibuf = NULL;
 		rfd->peer->obuf = NULL;
-		rfd->peer->obuf_work = NULL;
 		rfd->peer->ibuf_work = NULL;
 	}
 

bgpd/rfapi/vnc_zebra.c

@@ -173,7 +173,6 @@ static void vnc_redistribute_add(struct prefix *p, uint32_t metric,
 			vncHD1VR.peer = peer_new(bgp);
 			vncHD1VR.peer->status =
 				Established; /* keep bgp core happy */
-			bgp_sync_delete(vncHD1VR.peer); /* don't need these */
 
 			/*
 			 * since this peer is not on the I/O thread, this lock
@@ -189,12 +188,9 @@ static void vnc_redistribute_add(struct prefix *p, uint32_t metric,
 
 				if (vncHD1VR.peer->ibuf_work)
 					ringbuf_del(vncHD1VR.peer->ibuf_work);
-				if (vncHD1VR.peer->obuf_work)
-					stream_free(vncHD1VR.peer->obuf_work);
 
 				vncHD1VR.peer->ibuf = NULL;
 				vncHD1VR.peer->obuf = NULL;
-				vncHD1VR.peer->obuf_work = NULL;
 				vncHD1VR.peer->ibuf_work = NULL;
 			}