Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused memory allocations associated with bgp (backport #14075) #14077

Merged
merged 5 commits into from
Jul 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 0 additions & 26 deletions bgpd/bgp_advertise.c
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
}
3 changes: 0 additions & 3 deletions bgpd/bgp_advertise.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 0 additions & 2 deletions bgpd/bgp_fsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1524,8 +1524,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);
Expand Down
13 changes: 9 additions & 4 deletions bgpd/bgp_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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)) {
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion bgpd/bgp_updgrp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
6 changes: 2 additions & 4 deletions bgpd/bgp_updgrp.h
Original file line number Diff line number Diff line change
Expand Up @@ -585,11 +585,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;
}
Expand Down
30 changes: 0 additions & 30 deletions bgpd/bgpd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1172,8 +1172,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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -2611,16 +2591,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);
Expand Down
12 changes: 0 additions & 12 deletions bgpd/bgpd.h
Original file line number Diff line number Diff line change
Expand Up @@ -1150,20 +1150,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;

Expand Down Expand Up @@ -1602,8 +1592,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;
Expand Down
4 changes: 0 additions & 4 deletions bgpd/rfapi/rfapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}

Expand Down
4 changes: 0 additions & 4 deletions bgpd/rfapi/vnc_zebra.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}

Expand Down