From becedef6c3d836f7981369db90d3f31b1f235f5d Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Fri, 8 Sep 2017 11:51:12 -0400 Subject: [PATCH] bgpd, tests: comment formatting Signed-off-by: Quentin Young --- bgpd/bgp_fsm.c | 54 +++++++-------- bgpd/bgp_io.c | 11 ++-- bgpd/bgp_keepalives.h | 21 +++--- bgpd/bgp_packet.c | 123 +++++++++++++++++++---------------- bgpd/bgp_packet.h | 3 - bgpd/bgp_updgrp.c | 9 +-- bgpd/rfapi/rfapi.c | 6 +- bgpd/rfapi/vnc_zebra.c | 9 +-- tests/bgpd/test_capability.c | 10 +-- 9 files changed, 132 insertions(+), 114 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index e37e16886a..7282d81f67 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -138,11 +138,11 @@ static struct peer *peer_xfer_conn(struct peer *from_peer) BGP_TIMER_OFF(from_peer->t_connect); BGP_TIMER_OFF(from_peer->t_connect_check); - // At this point in time, it is possible that there are packets pending - // on - // various buffers. Those need to be transferred or dropped, otherwise - // we'll - // get spurious failures during session establishment. + /* + * At this point in time, it is possible that there are packets pending + * on various buffers. Those need to be transferred or dropped, + * otherwise we'll get spurious failures during session establishment. + */ pthread_mutex_lock(&peer->io_mtx); pthread_mutex_lock(&from_peer->io_mtx); { @@ -154,10 +154,11 @@ static struct peer *peer_xfer_conn(struct peer *from_peer) stream_fifo_clean(peer->obuf); stream_reset(peer->ibuf_work); - // this should never happen, since bgp_process_packet() is the - // only task - // that sets and unsets the current packet and it runs in our - // pthread. + /* + * this should never happen, since bgp_process_packet() is the + * only task that sets and unsets the current packet and it + * runs in our pthread. + */ if (peer->curr) { zlog_err( "[%s] Dropping pending packet on connection transfer:", @@ -1407,9 +1408,10 @@ int bgp_start(struct peer *peer) peer->fd); return -1; } - // when the socket becomes ready (or fails to connect), - // bgp_connect_check - // will be called. + /* + * when the socket becomes ready (or fails to connect), + * bgp_connect_check will be called. + */ thread_add_read(bm->master, bgp_connect_check, peer, peer->fd, &peer->t_connect_check); break; @@ -1898,13 +1900,13 @@ int bgp_event_update(struct peer *peer, int event) if (next != peer->status) { bgp_fsm_change_status(peer, next); - /* If we're going to ESTABLISHED then we executed a peer - * transfer. In - * this case we can either return FSM_PEER_TRANSITIONED - * or - * FSM_PEER_TRANSFERRED. Opting for TRANSFERRED since - * transfer implies - * session establishment. */ + /* + * If we're going to ESTABLISHED then we executed a + * peer transfer. In this case we can either return + * FSM_PEER_TRANSITIONED or FSM_PEER_TRANSFERRED. + * Opting for TRANSFERRED since transfer implies + * session establishment. + */ if (ret != FSM_PEER_TRANSFERRED) ret = FSM_PEER_TRANSITIONED; } @@ -1913,13 +1915,13 @@ int bgp_event_update(struct peer *peer, int event) bgp_timer_set(peer); } else { - /* If we got a return value of -1, that means there was an - * error, restart - * the FSM. Since bgp_stop() was called on the peer. only a few - * fields - * are safe to access here. In any case we need to indicate that - * the peer - * was stopped in the return code. */ + /* + * If we got a return value of -1, that means there was an + * error, restart the FSM. Since bgp_stop() was called on the + * peer. only a few fields are safe to access here. In any case + * we need to indicate that the peer was stopped in the return + * code. + */ if (!dyn_nbr && !passive_conn && peer->bgp) { zlog_err( "%s [FSM] Failure handling event %s in state %s, " diff --git a/bgpd/bgp_io.c b/bgpd/bgp_io.c index 0529cd5c19..fc8a474ba2 100644 --- a/bgpd/bgp_io.c +++ b/bgpd/bgp_io.c @@ -185,7 +185,6 @@ void bgp_reads_off(struct peer *peer) /** * Called from I/O pthread when a file descriptor has become ready for writing. - * HUP is not handled here, */ static int bgp_process_writes(struct thread *thread) { @@ -318,7 +317,8 @@ static int bgp_process_reads(struct thread *thread) break; } - /* After reading: + /* + * After reading: * 1. Move unread data to stream start to make room for more. * 2. Reschedule and return when we have additional data. * @@ -421,9 +421,7 @@ static uint16_t bgp_write(struct peer *peer) peer->v_start = (60 * 2); /* Handle Graceful Restart case where the state changes - to - Connect instead of Idle */ - /* Flush any existing events */ + * to Connect instead of Idle */ BGP_EVENT_ADD(peer, BGP_Stop); goto done; @@ -531,7 +529,7 @@ static uint16_t bgp_read(struct peer *peer) */ static bool validate_header(struct peer *peer) { - u_int16_t size, type; + uint16_t size, type; struct stream *pkt = peer->ibuf_work; size_t getp = stream_get_getp(pkt); @@ -580,7 +578,6 @@ static bool validate_header(struct peer *peer) || (type == BGP_MSG_CAPABILITY && size < BGP_MSG_CAPABILITY_MIN_SIZE)) { if (bgp_debug_neighbor_events(peer)) { - // XXX: zlog is not MT-safe zlog_debug("%s bad message length - %d for %s", peer->host, size, type == 128 ? "ROUTE-REFRESH" diff --git a/bgpd/bgp_keepalives.h b/bgpd/bgp_keepalives.h index 2096079454..1fbd035b9e 100644 --- a/bgpd/bgp_keepalives.h +++ b/bgpd/bgp_keepalives.h @@ -20,13 +20,14 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ -#ifndef _BGP_KEEPALIVES_H_ -#define _BGP_KEEPALIVES_H_ +#ifndef _FRR_BGP_KEEPALIVES_H +#define _FRR_BGP_KEEPALIVES_H #include "frr_pthread.h" #include "bgpd.h" -/* Turns on keepalives for a peer. +/** + * Turns on keepalives for a peer. * * This function adds the peer to an internal list of peers to generate * keepalives for. @@ -43,7 +44,8 @@ */ extern void bgp_keepalives_on(struct peer *); -/* Turns off keepalives for a peer. +/** + * Turns off keepalives for a peer. * * Removes the peer from the internal list of peers to generate keepalives for. * @@ -51,14 +53,16 @@ extern void bgp_keepalives_on(struct peer *); */ extern void bgp_keepalives_off(struct peer *); -/* Pre-run initialization function for keepalives pthread. +/** + * Pre-run initialization function for keepalives pthread. * * Initializes synchronization primitives. This should be called before * anything else to avoid race conditions. */ extern void bgp_keepalives_init(void); -/* Entry function for keepalives pthread. +/** + * Entry function for keepalives pthread. * * This function loops over an internal list of peers, generating keepalives at * regular intervals as determined by each peer's keepalive timer. @@ -69,7 +73,8 @@ extern void bgp_keepalives_init(void); */ extern void *bgp_keepalives_start(void *arg); -/* Poking function for keepalives pthread. +/** + * Poking function for keepalives pthread. * * Under normal circumstances the pthread will automatically wake itself * whenever it is necessary to do work. This function may be used to force the @@ -85,4 +90,4 @@ extern void bgp_keepalives_wake(void); */ int bgp_keepalives_stop(void **result, struct frr_pthread *fpt); -#endif /* _BGP_KEEPALIVES_H */ +#endif /* _FRR_BGP_KEEPALIVES_H */ diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index 41f0cac6d5..31918236ef 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -170,12 +170,12 @@ void bgp_check_update_delay(struct bgp *bgp) if (bgp->established <= bgp->restarted_peers + bgp->implicit_eors + bgp->explicit_eors) { - /* This is an extra sanity check to make sure we wait for all - the - eligible configured peers. This check is performed if - establish wait - timer is on, or establish wait option is not given with the - update-delay command */ + /* + * This is an extra sanity check to make sure we wait for all + * the eligible configured peers. This check is performed if + * establish wait timer is on, or establish wait option is not + * given with the update-delay command + */ if (bgp->t_establish_wait || (bgp->v_establish_wait == bgp->v_update_delay)) for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { @@ -200,8 +200,10 @@ void bgp_check_update_delay(struct bgp *bgp) } } -/* Called if peer is known to have restarted. The restart-state bit in - Graceful-Restart capability is used for that */ +/* + * Called if peer is known to have restarted. The restart-state bit in + * Graceful-Restart capability is used for that + */ void bgp_update_restarted_peers(struct peer *peer) { if (!bgp_update_delay_active(peer->bgp)) @@ -219,10 +221,12 @@ void bgp_update_restarted_peers(struct peer *peer) } } -/* Called as peer receives a keep-alive. Determines if this occurence can be - taken as an implicit EOR for this peer. - NOTE: The very first keep-alive after the Established state of a peer is - considered implicit EOR for the update-delay purposes */ +/* + * Called as peer receives a keep-alive. Determines if this occurence can be + * taken as an implicit EOR for this peer. + * NOTE: The very first keep-alive after the Established state of a peer is + * considered implicit EOR for the update-delay purposes + */ void bgp_update_implicit_eors(struct peer *peer) { if (!bgp_update_delay_active(peer->bgp)) @@ -240,8 +244,10 @@ void bgp_update_implicit_eors(struct peer *peer) } } -/* Should be called only when there is a change in the EOR_RECEIVED status - for any afi/safi on a peer */ +/* + * Should be called only when there is a change in the EOR_RECEIVED status + * for any afi/safi on a peer. + */ static void bgp_update_explicit_eors(struct peer *peer) { afi_t afi; @@ -299,7 +305,7 @@ int bgp_nlri_parse(struct peer *peer, struct attr *attr, return -1; } -/* +/** * Enqueue onto the peer's output buffer any packets which are pending for the * update group it is a member of. * @@ -335,10 +341,11 @@ int bgp_generate_updgrp_packets(struct thread *thread) continue; next_pkt = paf->next_pkt_to_send; - /* Try to generate a packet for the peer if we - * are at the end of - * the list. Always try to push out WITHDRAWs - * first. */ + /* + * Try to generate a packet for the peer if we + * are at the end of the list. Always try to + * push out WITHDRAWs first. + */ if (!next_pkt || !next_pkt->buffer) { next_pkt = subgroup_withdraw_packet( PAF_SUBGRP(paf)); @@ -348,15 +355,13 @@ int bgp_generate_updgrp_packets(struct thread *thread) next_pkt = paf->next_pkt_to_send; } - /* If we still don't have a packet to send to - * the peer, then - * try to find out out if we have to send eor or - * if not, skip to - * the next AFI, SAFI. - * Don't send the EOR prematurely... if the - * subgroup's coalesce - * timer is running, the adjacency-out structure - * is not created + /* + * If we still don't have a packet to send to + * the peer, then try to find out out if we + * have to send eor or if not, skip to the next + * AFI, SAFI. Don't send the EOR prematurely... + * if the subgroup's coalesce timer is running, + * the adjacency-out structure is not created * yet. */ if (!next_pkt || !next_pkt->buffer) { @@ -393,8 +398,8 @@ int bgp_generate_updgrp_packets(struct thread *thread) /* Found a packet template to send, overwrite - * packet with appropriate - * attributes from peer and advance peer */ + * packet with appropriate attributes from peer + * and advance peer */ s = bpacket_reformat_for_peer(next_pkt, paf); bgp_packet_add(peer, s); bgp_writes_on(peer); @@ -507,12 +512,16 @@ static int bgp_write_notify(struct peer *peer) /* Stop collecting data within the socket */ sockopt_cork(peer->fd, 0); - /* socket is in nonblocking mode, if we can't deliver the NOTIFY, well, - * we only care about getting a clean shutdown at this point. */ + /* + * socket is in nonblocking mode, if we can't deliver the NOTIFY, well, + * we only care about getting a clean shutdown at this point. + */ ret = write(peer->fd, STREAM_DATA(s), stream_get_endp(s)); - /* only connection reset/close gets counted as TCP_fatal_error, failure - * to write the entire NOTIFY doesn't get different FSM treatment */ + /* + * only connection reset/close gets counted as TCP_fatal_error, failure + * to write the entire NOTIFY doesn't get different FSM treatment + */ if (ret <= 0) { stream_free(s); BGP_EVENT_ADD(peer, TCP_fatal_error); @@ -540,8 +549,10 @@ static int bgp_write_notify(struct peer *peer) if (peer->v_start >= (60 * 2)) peer->v_start = (60 * 2); - /* Handle Graceful Restart case where the state changes to - Connect instead of Idle */ + /* + * Handle Graceful Restart case where the state changes to + * Connect instead of Idle + */ BGP_EVENT_ADD(peer, BGP_Stop); stream_free(s); @@ -552,8 +563,8 @@ static int bgp_write_notify(struct peer *peer) /* * Creates a BGP Notify and appends it to the peer's output queue. * - * This function awakens the write thread to ensure the packet - * gets out ASAP. + * This function attempts to write the packet from the thread it is called + * from, to ensure the packet gets out ASAP. * * @param peer * @param code BGP error code @@ -591,11 +602,11 @@ void bgp_notify_send_with_data(struct peer *peer, u_char code, u_char sub_code, } pthread_mutex_unlock(&peer->io_mtx); - /* If possible, store last packet for debugging purposes. This check is - * in - * place because we are sometimes called with a doppelganger peer, who - * tends - * to have a plethora of fields nulled out. */ + /* + * If possible, store last packet for debugging purposes. This check is + * in place because we are sometimes called with a doppelganger peer, + * who tends to have a plethora of fields nulled out. + */ if (peer->curr && peer->last_reset_cause_size) { size_t packetsize = stream_get_endp(peer->curr); assert(packetsize <= peer->last_reset_cause_size); @@ -661,8 +672,8 @@ void bgp_notify_send_with_data(struct peer *peer, u_char code, u_char sub_code, /* * Creates a BGP Notify and appends it to the peer's output queue. * - * This function awakens the write thread to ensure the packet - * gets out ASAP. + * This function attempts to write the packet from the thread it is called + * from, to ensure the packet gets out ASAP. * * @param peer * @param code BGP error code @@ -2169,11 +2180,12 @@ int bgp_process_packet(struct thread *thread) __FUNCTION__, peer->host); break; default: - /* The message type should have been sanitized before we - * ever got - * here. Receipt of a message with an invalid header at - * this point is - * indicative of a security issue. */ + /* + * The message type should have been sanitized before + * we ever got here. Receipt of a message with an + * invalid header at this point is indicative of a + * security issue. + */ assert (!"Message of invalid type received during input processing"); } @@ -2188,9 +2200,10 @@ int bgp_process_packet(struct thread *thread) else continue; - /* If peer was deleted, do not process any more packets. This is - * usually - * due to executing BGP_Stop or a stub deletion. */ + /* + * If peer was deleted, do not process any more packets. This + * is usually due to executing BGP_Stop or a stub deletion. + */ if (fsm_update_result == FSM_PEER_TRANSFERRED || fsm_update_result == FSM_PEER_STOPPED) break; @@ -2200,8 +2213,8 @@ int bgp_process_packet(struct thread *thread) && fsm_update_result != FSM_PEER_STOPPED) { pthread_mutex_lock(&peer->io_mtx); { - if (peer->ibuf->count - > 0) // more work to do, come back later + // more work to do, come back later + if (peer->ibuf->count > 0) thread_add_event(bm->master, bgp_process_packet, peer, 0, NULL); } diff --git a/bgpd/bgp_packet.h b/bgpd/bgp_packet.h index 62c3fe671b..008f2b814b 100644 --- a/bgpd/bgp_packet.h +++ b/bgpd/bgp_packet.h @@ -61,9 +61,6 @@ extern void bgp_check_update_delay(struct bgp *); extern int bgp_packet_set_marker(struct stream *s, u_char type); extern int bgp_packet_set_size(struct stream *s); -/* Control variable for write thread. */ -extern bool bgp_packet_writes_thread_run; - extern int bgp_generate_updgrp_packets(struct thread *); extern int bgp_process_packet(struct thread *); diff --git a/bgpd/bgp_updgrp.c b/bgpd/bgp_updgrp.c index a45353b375..1c589f7960 100644 --- a/bgpd/bgp_updgrp.c +++ b/bgpd/bgp_updgrp.c @@ -1872,10 +1872,11 @@ void subgroup_trigger_write(struct update_subgroup *subgrp) { struct peer_af *paf; - /* For each peer in the subgroup, schedule a job to pull packets from - * the - * subgroup output queue into their own output queue. This action will - * trigger a write job on the I/O thread. */ + /* + * For each peer in the subgroup, schedule a job to pull packets from + * the subgroup output queue into their own output queue. This action + * will trigger a write job on the I/O thread. + */ SUBGRP_FOREACH_PEER(subgrp, paf) if (paf->peer->status == Established) thread_add_timer_msec(bm->master, bgp_generate_updgrp_packets, diff --git a/bgpd/rfapi/rfapi.c b/bgpd/rfapi/rfapi.c index 1c342a2ff8..fa3da9c283 100644 --- a/bgpd/rfapi/rfapi.c +++ b/bgpd/rfapi/rfapi.c @@ -1305,8 +1305,10 @@ static int rfapi_open_inner(struct rfapi_descriptor *rfd, struct bgp *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 - // necessary, but serves as a reminder to those who may meddle... + /* + * since this peer is not on the I/O thread, this lock is not strictly + * necessary, but serves as a reminder to those who may meddle... + */ pthread_mutex_lock(&rfd->peer->io_mtx); { // we don't need any I/O related facilities diff --git a/bgpd/rfapi/vnc_zebra.c b/bgpd/rfapi/vnc_zebra.c index 5f79892381..07be7833b6 100644 --- a/bgpd/rfapi/vnc_zebra.c +++ b/bgpd/rfapi/vnc_zebra.c @@ -184,10 +184,11 @@ static void vnc_redistribute_add(struct prefix *p, u_int32_t metric, 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 - // is not strictly - // necessary, but serves as a reminder to those who may - // meddle... + /* + * since this peer is not on the I/O thread, this lock + * is not strictly necessary, but serves as a reminder + * to those who may meddle... + */ pthread_mutex_lock(&vncHD1VR.peer->io_mtx); { // we don't need any I/O related facilities diff --git a/tests/bgpd/test_capability.c b/tests/bgpd/test_capability.c index cf3c36804b..a5092708e2 100644 --- a/tests/bgpd/test_capability.c +++ b/tests/bgpd/test_capability.c @@ -866,11 +866,11 @@ static void parse_test(struct peer *peer, struct test_segment *t, int type) failed++; } - /* Some of the functions used return BGP_Stop on error and some return - * -1. If - * we have -1, keep it; if we have BGP_Stop, transform it to the correct - * pass/fail code */ - + /* + * Some of the functions used return BGP_Stop on error and some return + * -1. If we have -1, keep it; if we have BGP_Stop, transform it to the + * correct pass/fail code + */ if (ret != -1) ret = (ret == BGP_Stop) ? -1 : 0; -- 2.39.5