diff options
| -rw-r--r-- | bgpd/bgp_fsm.c | 24 | ||||
| -rw-r--r-- | bgpd/bgp_fsm.h | 6 | ||||
| -rw-r--r-- | bgpd/bgp_io.c | 7 | ||||
| -rw-r--r-- | bgpd/bgp_packet.c | 619 |
4 files changed, 397 insertions, 259 deletions
diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index d18c5cedc5..3f90d4f660 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -1619,11 +1619,6 @@ static int bgp_establish(struct peer *peer) /* Keepalive packet is received. */ static int bgp_fsm_keepalive(struct peer *peer) { - bgp_update_implicit_eors(peer); - - /* peer count update */ - peer->keepalive_in++; - BGP_TIMER_OFF(peer->t_holdtime); return 0; } @@ -1650,6 +1645,7 @@ static int bgp_ignore(struct peer *peer) /* This is to handle unexpected events.. */ static int bgp_fsm_exeption(struct peer *peer) { + zlog_err("%s [FSM] cur_event: %d", peer->host, peer->cur_event); zlog_err( "%s [FSM] Unexpected event %s in state %s, prior events %s, %s, fd %d", peer->host, bgp_event_str[peer->cur_event], @@ -1864,6 +1860,9 @@ int bgp_event_update(struct peer *peer, int event) int passive_conn = 0; int dyn_nbr; + /* default return code */ + ret = FSM_PEER_NOOP; + other = peer->doppelganger; passive_conn = (CHECK_FLAG(peer->sflags, PEER_STATUS_ACCEPT_PEER)) ? 1 : 0; @@ -1885,19 +1884,30 @@ int bgp_event_update(struct peer *peer, int event) if (FSM[peer->status - 1][event - 1].func) ret = (*(FSM[peer->status - 1][event - 1].func))(peer); - /* When function do not want proceed next job return -1. */ if (ret >= 0) { if (ret == 1 && next == Established) { /* The case when doppelganger swap accurred in bgp_establish. Update the peer pointer accordingly */ + ret = FSM_PEER_TRANSFERRED; peer = other; } /* If status is changed. */ - if (next != peer->status) + 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 (ret != FSM_PEER_TRANSFERRED) + ret = FSM_PEER_TRANSITIONED; + } + /* Make sure timer is set. */ bgp_timer_set(peer); diff --git a/bgpd/bgp_fsm.h b/bgpd/bgp_fsm.h index 131de40b5b..d021c9884a 100644 --- a/bgpd/bgp_fsm.h +++ b/bgpd/bgp_fsm.h @@ -50,6 +50,12 @@ #define BGP_MSEC_JITTER 10 +/* Status codes for bgp_event_update() */ +#define FSM_PEER_NOOP 0 +#define FSM_PEER_STOPPED 1 +#define FSM_PEER_TRANSFERRED 2 +#define FSM_PEER_TRANSITIONED 3 + /* Prototypes. */ extern void bgp_fsm_nht_update(struct peer *, int valid); extern int bgp_event(struct thread *); diff --git a/bgpd/bgp_io.c b/bgpd/bgp_io.c index 4369ef7a84..0e557ec205 100644 --- a/bgpd/bgp_io.c +++ b/bgpd/bgp_io.c @@ -327,13 +327,6 @@ static int bgp_process_reads(struct thread *thread) /* handle invalid header */ if (fatal) { - if (!header_valid) { - bgp_size_t pktsize = BGP_HEADER_SIZE; - stream_get(peer->last_reset_cause, peer->ibuf_work, - pktsize); - peer->last_reset_cause_size = pktsize; - } - /* wipe buffer just in case someone screwed up */ stream_reset(peer->ibuf_work); } else { diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index 7c924fd733..750c0119c9 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -59,7 +59,13 @@ #include "bgpd/bgp_label.h" #include "bgpd/bgp_io.h" -/* Set up BGP packet marker and packet type. */ +/** + * Sets marker and type fields for a BGP message. + * + * @param s the stream containing the packet + * @param type the packet type + * @return the size of the stream + */ int bgp_packet_set_marker(struct stream *s, u_char type) { int i; @@ -78,8 +84,14 @@ int bgp_packet_set_marker(struct stream *s, u_char type) return stream_get_endp(s); } -/* Set BGP packet header size entry. If size is zero then use current - stream size. */ +/** + * Sets size field for a BGP message. + * + * Size field is set to the size of the stream passed. + * + * @param s the stream containing the packet + * @return the size of the stream + */ int bgp_packet_set_size(struct stream *s) { int cp; @@ -144,6 +156,149 @@ static struct stream *bgp_update_packet_eor(struct peer *peer, afi_t afi, return s; } +/* Called when there is a change in the EOR(implicit or explicit) status of a + * peer. Ends the update-delay if all expected peers are done with EORs. */ +void bgp_check_update_delay(struct bgp *bgp) +{ + struct listnode *node, *nnode; + struct peer *peer = NULL; + + if (bgp_debug_neighbor_events(peer)) + zlog_debug("Checking update delay, T: %d R: %d I:%d E: %d", + bgp->established, bgp->restarted_peers, + bgp->implicit_eors, bgp->explicit_eors); + + 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 */ + if (bgp->t_establish_wait + || (bgp->v_establish_wait == bgp->v_update_delay)) + for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { + if (CHECK_FLAG(peer->flags, + PEER_FLAG_CONFIG_NODE) + && !CHECK_FLAG(peer->flags, + PEER_FLAG_SHUTDOWN) + && !peer->update_delay_over) { + if (bgp_debug_neighbor_events(peer)) + zlog_debug( + " Peer %s pending, continuing read-only mode", + peer->host); + return; + } + } + + zlog_info( + "Update delay ended, restarted: %d, EORs implicit: %d, explicit: %d", + bgp->restarted_peers, bgp->implicit_eors, + bgp->explicit_eors); + bgp_update_delay_end(bgp); + } +} + +/* 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)) + return; /* BGP update delay has ended */ + if (peer->update_delay_over) + return; /* This peer has already been considered */ + + if (bgp_debug_neighbor_events(peer)) + zlog_debug("Peer %s: Checking restarted", peer->host); + + if (peer->status == Established) { + peer->update_delay_over = 1; + peer->bgp->restarted_peers++; + bgp_check_update_delay(peer->bgp); + } +} + +/* 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)) + return; /* BGP update delay has ended */ + if (peer->update_delay_over) + return; /* This peer has already been considered */ + + if (bgp_debug_neighbor_events(peer)) + zlog_debug("Peer %s: Checking implicit EORs", peer->host); + + if (peer->status == Established) { + peer->update_delay_over = 1; + peer->bgp->implicit_eors++; + bgp_check_update_delay(peer->bgp); + } +} + +/* 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; + safi_t safi; + + if (!bgp_update_delay_active(peer->bgp)) + return; /* BGP update delay has ended */ + if (peer->update_delay_over) + return; /* This peer has already been considered */ + + if (bgp_debug_neighbor_events(peer)) + zlog_debug("Peer %s: Checking explicit EORs", peer->host); + + for (afi = AFI_IP; afi < AFI_MAX; afi++) + for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) { + if (peer->afc_nego[afi][safi] + && !CHECK_FLAG(peer->af_sflags[afi][safi], + PEER_STATUS_EOR_RECEIVED)) { + if (bgp_debug_neighbor_events(peer)) + zlog_debug( + " afi %d safi %d didnt receive EOR", + afi, safi); + return; + } + } + + peer->update_delay_over = 1; + peer->bgp->explicit_eors++; + bgp_check_update_delay(peer->bgp); +} + +/** + * Frontend for NLRI parsing, to fan-out to AFI/SAFI specific parsers. + * + * mp_withdraw, if set, is used to nullify attr structure on most of the + * calling safi function and for evpn, passed as parameter + */ +int bgp_nlri_parse(struct peer *peer, struct attr *attr, + struct bgp_nlri *packet, int mp_withdraw) +{ + switch (packet->safi) { + case SAFI_UNICAST: + case SAFI_MULTICAST: + return bgp_nlri_parse_ip(peer, mp_withdraw ? NULL : attr, + packet); + case SAFI_LABELED_UNICAST: + return bgp_nlri_parse_label(peer, mp_withdraw ? NULL : attr, + packet); + case SAFI_MPLS_VPN: + return bgp_nlri_parse_vpn(peer, mp_withdraw ? NULL : attr, + packet); + case SAFI_EVPN: + return bgp_nlri_parse_evpn(peer, attr, packet, mp_withdraw); + } + return -1; +} + /* * Enqueue onto the peer's output buffer any packets which are pending for the * update group it is a member of. @@ -433,6 +588,18 @@ 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 (peer->curr && peer->last_reset_cause_size) { + size_t packetsize = stream_get_endp(peer->curr); + assert(packetsize <= peer->last_reset_cause_size); + memcpy(peer->last_reset_cause, peer->curr->data, packetsize); + peer->last_reset_cause_size = packetsize; + } + /* For debug */ { struct bgp_notify bgp_notify; @@ -739,6 +906,42 @@ static int bgp_collision_detect(struct peer *new, struct in_addr remote_id) return 0; } +/* Packet processing routines ---------------------------------------------- */ +/* + * This is a family of functions designed to be called from + * bgp_process_packet(). These functions all share similar behavior and should + * adhere to the following invariants and restrictions: + * + * Return codes + * ------------ + * The return code of any one of those functions should be one of the FSM event + * codes specified in bgpd.h. If a NOTIFY was sent, this event code MUST be + * BGP_Stop. Otherwise, the code SHOULD correspond to the function's expected + * packet type. For example, bgp_open_receive() should return BGP_Stop upon + * error and Receive_OPEN_message otherwise. + * + * If no action is necessary, the correct return code is BGP_PACKET_NOOP as + * defined below. + * + * Side effects + * ------------ + * - May send NOTIFY messages + * - May not modify peer->status + * - May not call bgp_event_update() + */ + +#define BGP_PACKET_NOOP 0 + +/** + * Process BGP OPEN message for peer. + * + * If any errors are encountered in the OPEN message, immediately sends NOTIFY + * and returns BGP_Stop. + * + * @param peer + * @param size size of the packet + * @return as in summary + */ static int bgp_open_receive(struct peer *peer, bgp_size_t size) { int ret; @@ -781,7 +984,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) if (STREAM_READABLE(peer->curr) < optlen) { bgp_notify_send(peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_MALFORMED_ATTR); - return -1; + return BGP_Stop; } /* We need the as4 capability value *right now* because @@ -801,7 +1004,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) bgp_notify_send_with_data(peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_BAD_PEER_AS, notify_data_remote_as4, 4); - return -1; + return BGP_Stop; } if (remote_as == BGP_AS_TRANS) { @@ -816,7 +1019,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) bgp_notify_send_with_data(peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_BAD_PEER_AS, notify_data_remote_as4, 4); - return -1; + return BGP_Stop; } if (!as4 && BGP_DEBUG(as4, AS4)) @@ -846,7 +1049,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) bgp_notify_send_with_data(peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_BAD_PEER_AS, notify_data_remote_as4, 4); - return -1; + return BGP_Stop; } } @@ -859,7 +1062,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) bgp_notify_send_with_data(peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_BAD_BGP_IDENT, notify_data_remote_id, 4); - return -1; + return BGP_Stop; } /* Set remote router-id */ @@ -877,7 +1080,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) bgp_notify_send_with_data(peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_UNSUP_VERSION, (u_int8_t *)&maxver, 2); - return -1; + return BGP_Stop; } /* Check neighbor as number. */ @@ -889,7 +1092,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) bgp_notify_send_with_data(peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_BAD_PEER_AS, notify_data_remote_as, 2); - return -1; + return BGP_Stop; } else if (peer->as_type == AS_INTERNAL) { if (remote_as != peer->bgp->as) { if (bgp_debug_neighbor_events(peer)) @@ -899,7 +1102,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) bgp_notify_send_with_data(peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_BAD_PEER_AS, notify_data_remote_as, 2); - return -1; + return BGP_Stop; } peer->as = peer->local_as; } else if (peer->as_type == AS_EXTERNAL) { @@ -911,7 +1114,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) bgp_notify_send_with_data(peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_BAD_PEER_AS, notify_data_remote_as, 2); - return -1; + return BGP_Stop; } peer->as = remote_as; } else if ((peer->as_type == AS_SPECIFIED) && (remote_as != peer->as)) { @@ -921,7 +1124,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) bgp_notify_send_with_data(peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_BAD_PEER_AS, notify_data_remote_as, 2); - return -1; + return BGP_Stop; } /* From the rfc: Upon receipt of an OPEN message, a BGP speaker MUST @@ -935,7 +1138,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) bgp_notify_send_with_data(peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_UNACEP_HOLDTIME, (u_char *)holdtime_ptr, 2); - return -1; + return BGP_Stop; } /* From the rfc: A reasonable maximum time between KEEPALIVE messages @@ -964,7 +1167,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) if (optlen != 0) { if ((ret = bgp_open_option_parse(peer, optlen, &mp_capability)) < 0) - return ret; + return BGP_Stop; } else { if (bgp_debug_neighbor_events(peer)) zlog_debug("%s rcvd OPEN w/ OPTION parameter len: 0", @@ -998,13 +1201,13 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) immidiately. */ ret = bgp_collision_detect(peer, remote_id); if (ret < 0) - return ret; + return BGP_Stop; /* Get sockname. */ if ((ret = bgp_getsockname(peer)) < 0) { zlog_err("%s: bgp_getsockname() failed for peer: %s", __FUNCTION__, peer->host); - return (ret); + return BGP_Stop; } /* Verify valid local address present based on negotiated @@ -1021,7 +1224,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) peer->host, peer->fd); bgp_notify_send(peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_SUBCODE_UNSPECIFIC); - return -1; + return BGP_Stop; #endif } } @@ -1037,166 +1240,42 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) peer->host, peer->fd); bgp_notify_send(peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_SUBCODE_UNSPECIFIC); - return -1; + return BGP_Stop; #endif } } peer->rtt = sockopt_tcp_rtt(peer->fd); - if ((ret = bgp_event_update(peer, Receive_OPEN_message)) < 0) { - zlog_err("%s: BGP event update failed for peer: %s", - __FUNCTION__, peer->host); - /* DD: bgp send notify and reset state */ - return (ret); - } - - return 0; + return Receive_OPEN_message; } -/* Called when there is a change in the EOR(implicit or explicit) status of a - peer. - Ends the update-delay if all expected peers are done with EORs. */ -void bgp_check_update_delay(struct bgp *bgp) -{ - struct listnode *node, *nnode; - struct peer *peer = NULL; - - if (bgp_debug_neighbor_events(peer)) - zlog_debug("Checking update delay, T: %d R: %d I:%d E: %d", - bgp->established, bgp->restarted_peers, - bgp->implicit_eors, bgp->explicit_eors); - - 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 */ - if (bgp->t_establish_wait - || (bgp->v_establish_wait == bgp->v_update_delay)) - for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { - if (CHECK_FLAG(peer->flags, - PEER_FLAG_CONFIG_NODE) - && !CHECK_FLAG(peer->flags, - PEER_FLAG_SHUTDOWN) - && !peer->update_delay_over) { - if (bgp_debug_neighbor_events(peer)) - zlog_debug( - " Peer %s pending, continuing read-only mode", - peer->host); - return; - } - } - - zlog_info( - "Update delay ended, restarted: %d, EORs implicit: %d, explicit: %d", - bgp->restarted_peers, bgp->implicit_eors, - bgp->explicit_eors); - bgp_update_delay_end(bgp); - } -} - -/* 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)) - return; /* BGP update delay has ended */ - if (peer->update_delay_over) - return; /* This peer has already been considered */ - - if (bgp_debug_neighbor_events(peer)) - zlog_debug("Peer %s: Checking restarted", peer->host); - - if (peer->status == Established) { - peer->update_delay_over = 1; - peer->bgp->restarted_peers++; - bgp_check_update_delay(peer->bgp); - } -} - -/* 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) +/** + * Process BGP KEEPALIVE message for peer. + * + * @param peer + * @param size size of the packet + * @return as in summary + */ +static int bgp_keepalive_receive(struct peer *peer, bgp_size_t size) { - if (!bgp_update_delay_active(peer->bgp)) - return; /* BGP update delay has ended */ - if (peer->update_delay_over) - return; /* This peer has already been considered */ + if (bgp_debug_keepalive(peer)) + zlog_debug("%s KEEPALIVE rcvd", peer->host); - if (bgp_debug_neighbor_events(peer)) - zlog_debug("Peer %s: Checking implicit EORs", peer->host); + bgp_update_implicit_eors(peer); - if (peer->status == Established) { - peer->update_delay_over = 1; - peer->bgp->implicit_eors++; - bgp_check_update_delay(peer->bgp); - } + return Receive_KEEPALIVE_message; } -/* 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; - safi_t safi; - - if (!bgp_update_delay_active(peer->bgp)) - return; /* BGP update delay has ended */ - if (peer->update_delay_over) - return; /* This peer has already been considered */ - - if (bgp_debug_neighbor_events(peer)) - zlog_debug("Peer %s: Checking explicit EORs", peer->host); - FOREACH_AFI_SAFI (afi, safi) { - if (peer->afc_nego[afi][safi] - && !CHECK_FLAG(peer->af_sflags[afi][safi], - PEER_STATUS_EOR_RECEIVED)) { - if (bgp_debug_neighbor_events(peer)) - zlog_debug( - " afi %d safi %d didnt receive EOR", - afi, safi); - return; - } - } - - peer->update_delay_over = 1; - peer->bgp->explicit_eors++; - bgp_check_update_delay(peer->bgp); -} - -/* Frontend for NLRI parsing, to fan-out to AFI/SAFI specific parsers - * mp_withdraw, if set, is used to nullify attr structure on most of the calling - * safi function - * and for evpn, passed as parameter +/** + * Process BGP UPDATE message for peer. + * + * Parses UPDATE and creates attribute object. + * + * @param peer + * @param size size of the packet + * @return as in summary */ -int bgp_nlri_parse(struct peer *peer, struct attr *attr, - struct bgp_nlri *packet, int mp_withdraw) -{ - switch (packet->safi) { - case SAFI_UNICAST: - case SAFI_MULTICAST: - return bgp_nlri_parse_ip(peer, mp_withdraw ? NULL : attr, - packet); - case SAFI_LABELED_UNICAST: - return bgp_nlri_parse_label(peer, mp_withdraw ? NULL : attr, - packet); - case SAFI_MPLS_VPN: - return bgp_nlri_parse_vpn(peer, mp_withdraw ? NULL : attr, - packet); - case SAFI_EVPN: - return bgp_nlri_parse_evpn(peer, attr, packet, mp_withdraw); - default: - return -1; - } -} - -/* Parse BGP Update packet and make attribute object. */ static int bgp_update_receive(struct peer *peer, bgp_size_t size) { int ret, nlri_ret; @@ -1222,7 +1301,7 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size) peer->host, lookup_msg(bgp_status_msg, peer->status, NULL)); bgp_notify_send(peer, BGP_NOTIFY_FSM_ERR, 0); - return -1; + return BGP_Stop; } /* Set initial values. */ @@ -1247,7 +1326,7 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size) peer->host); bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_MAL_ATTR); - return -1; + return BGP_Stop; } /* Unfeasible Route Length. */ @@ -1261,7 +1340,7 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size) peer->host, withdraw_len); bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_MAL_ATTR); - return -1; + return BGP_Stop; } /* Unfeasible Route packet format check. */ @@ -1281,7 +1360,7 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size) peer->host); bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_MAL_ATTR); - return -1; + return BGP_Stop; } /* Fetch attribute total length. */ @@ -1295,7 +1374,7 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size) peer->host, attribute_len); bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_MAL_ATTR); - return -1; + return BGP_Stop; } /* Certain attribute parsing errors should not be considered bad enough @@ -1318,7 +1397,7 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size) &nlris[NLRI_MP_WITHDRAW]); if (attr_parse_ret == BGP_ATTR_PARSE_ERROR) { bgp_attr_unintern_sub(&attr); - return -1; + return BGP_Stop; } } @@ -1397,7 +1476,7 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size) ? BGP_NOTIFY_UPDATE_INVAL_NETWORK : BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); bgp_attr_unintern_sub(&attr); - return -1; + return BGP_Stop; } } @@ -1453,24 +1532,23 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size) interned in bgp_attr_parse(). */ bgp_attr_unintern_sub(&attr); - /* If peering is stopped due to some reason, do not generate BGP - event. */ - if (peer->status != Established) - return 0; - - /* Increment packet counter. */ - peer->update_in++; peer->update_time = bgp_clock(); /* Rearm holdtime timer */ BGP_TIMER_OFF(peer->t_holdtime); bgp_timer_set(peer); - return 0; + return Receive_UPDATE_message; } -/* Notify message treatment function. */ -static void bgp_notify_receive(struct peer *peer, bgp_size_t size) +/** + * Process BGP NOTIFY message for peer. + * + * @param peer + * @param size size of the packet + * @return as in summary + */ +static int bgp_notify_receive(struct peer *peer, bgp_size_t size) { struct bgp_notify bgp_notify; @@ -1539,20 +1617,17 @@ static void bgp_notify_receive(struct peer *peer, bgp_size_t size) && bgp_notify.subcode == BGP_NOTIFY_OPEN_UNSUP_PARAM) UNSET_FLAG(peer->sflags, PEER_STATUS_CAPABILITY_OPEN); - BGP_EVENT_ADD(peer, Receive_NOTIFICATION_message); -} - -/* Keepalive treatment function -- get keepalive send keepalive */ -static void bgp_keepalive_receive(struct peer *peer, bgp_size_t size) -{ - if (bgp_debug_keepalive(peer)) - zlog_debug("%s KEEPALIVE rcvd", peer->host); - - BGP_EVENT_ADD(peer, Receive_KEEPALIVE_message); + return Receive_NOTIFICATION_message; } -/* Route refresh message is received. */ -static void bgp_route_refresh_receive(struct peer *peer, bgp_size_t size) +/** + * Process BGP ROUTEREFRESH message for peer. + * + * @param peer + * @param size size of the packet + * @return as in summary + */ +static int bgp_route_refresh_receive(struct peer *peer, bgp_size_t size) { iana_afi_t pkt_afi; afi_t afi; @@ -1569,7 +1644,7 @@ static void bgp_route_refresh_receive(struct peer *peer, bgp_size_t size) peer->host); bgp_notify_send(peer, BGP_NOTIFY_HEADER_ERR, BGP_NOTIFY_HEADER_BAD_MESTYPE); - return; + return BGP_Stop; } /* Status must be Established. */ @@ -1579,7 +1654,7 @@ static void bgp_route_refresh_receive(struct peer *peer, bgp_size_t size) peer->host, lookup_msg(bgp_status_msg, peer->status, NULL)); bgp_notify_send(peer, BGP_NOTIFY_FSM_ERR, 0); - return; + return BGP_Stop; } s = peer->curr; @@ -1598,7 +1673,7 @@ static void bgp_route_refresh_receive(struct peer *peer, bgp_size_t size) zlog_info( "%s REFRESH_REQ for unrecognized afi/safi: %d/%d - ignored", peer->host, pkt_afi, pkt_safi); - return; + return BGP_PACKET_NOOP; } if (size != BGP_MSG_ROUTE_REFRESH_MIN_SIZE - BGP_HEADER_SIZE) { @@ -1612,7 +1687,7 @@ static void bgp_route_refresh_receive(struct peer *peer, bgp_size_t size) zlog_info("%s ORF route refresh length error", peer->host); bgp_notify_send(peer, BGP_NOTIFY_CEASE, 0); - return; + return BGP_Stop; } when_to_refresh = stream_getc(s); @@ -1783,7 +1858,7 @@ static void bgp_route_refresh_receive(struct peer *peer, bgp_size_t size) ? "Defer" : "Immediate"); if (when_to_refresh == REFRESH_DEFER) - return; + return BGP_PACKET_NOOP; } /* First update is deferred until ORF or ROUTE-REFRESH is received */ @@ -1814,8 +1889,18 @@ static void bgp_route_refresh_receive(struct peer *peer, bgp_size_t size) /* Perform route refreshment to the peer */ bgp_announce_route(peer, afi, safi); + + /* No FSM action necessary */ + return BGP_PACKET_NOOP; } +/** + * Parse BGP CAPABILITY message for peer. + * + * @param peer + * @param size size of the packet + * @return as in summary + */ static int bgp_capability_msg_parse(struct peer *peer, u_char *pnt, bgp_size_t length) { @@ -1836,7 +1921,7 @@ static int bgp_capability_msg_parse(struct peer *peer, u_char *pnt, if (pnt + 3 > end) { zlog_info("%s Capability length error", peer->host); bgp_notify_send(peer, BGP_NOTIFY_CEASE, 0); - return -1; + return BGP_Stop; } action = *pnt; hdr = (struct capability_header *)(pnt + 1); @@ -1847,7 +1932,7 @@ static int bgp_capability_msg_parse(struct peer *peer, u_char *pnt, zlog_info("%s Capability Action Value error %d", peer->host, action); bgp_notify_send(peer, BGP_NOTIFY_CEASE, 0); - return -1; + return BGP_Stop; } if (bgp_debug_neighbor_events(peer)) @@ -1859,7 +1944,7 @@ static int bgp_capability_msg_parse(struct peer *peer, u_char *pnt, if ((pnt + hdr->length + 3) > end) { zlog_info("%s Capability length error", peer->host); bgp_notify_send(peer, BGP_NOTIFY_CEASE, 0); - return -1; + return BGP_Stop; } /* Fetch structure to the byte stream. */ @@ -1910,7 +1995,7 @@ static int bgp_capability_msg_parse(struct peer *peer, u_char *pnt, if (peer_active_nego(peer)) bgp_clear_route(peer, afi, safi); else - BGP_EVENT_ADD(peer, BGP_Stop); + return BGP_Stop; } } else { zlog_warn( @@ -1918,12 +2003,19 @@ static int bgp_capability_msg_parse(struct peer *peer, u_char *pnt, peer->host, hdr->code); } } - return 0; + + /* No FSM action necessary */ + return BGP_PACKET_NOOP; } -/* Dynamic Capability is received. +/** + * Parse BGP CAPABILITY message for peer. * - * This is exported for unit-test purposes + * Exported for unit testing. + * + * @param peer + * @param size size of the packet + * @return as in summary */ int bgp_capability_receive(struct peer *peer, bgp_size_t size) { @@ -1941,7 +2033,7 @@ int bgp_capability_receive(struct peer *peer, bgp_size_t size) peer->host); bgp_notify_send(peer, BGP_NOTIFY_HEADER_ERR, BGP_NOTIFY_HEADER_BAD_MESTYPE); - return -1; + return BGP_Stop; } /* Status must be Established. */ @@ -1951,19 +2043,34 @@ int bgp_capability_receive(struct peer *peer, bgp_size_t size) peer->host, lookup_msg(bgp_status_msg, peer->status, NULL)); bgp_notify_send(peer, BGP_NOTIFY_FSM_ERR, 0); - return -1; + return BGP_Stop; } /* Parse packet. */ return bgp_capability_msg_parse(peer, pnt, size); } -/* Starting point of packet process function. */ +/** + * Processes a peer's input buffer. + * + * This function sidesteps the event loop and directly calls bgp_event_update() + * after processing each BGP message. This is necessary to ensure proper + * ordering of FSM events and unifies the behavior that was present previously, + * whereby some of the packet handling functions would update the FSM and some + * would not, making event flow difficult to understand. Please think twice + * before hacking this. + * + * Thread type: THREAD_EVENT + * @param thread + * @return 0 + */ int bgp_process_packet(struct thread *thread) { /* Yes first of all get peer pointer. */ - struct peer *peer; - uint32_t rpkt_quanta_old; + struct peer *peer; // peer + uint32_t rpkt_quanta_old; // how many packets to read + int fsm_update_result; // return code of bgp_event_update() + int mprc; // message processing return code peer = THREAD_ARG(thread); rpkt_quanta_old = atomic_load_explicit(&peer->bgp->rpkt_quanta, @@ -1995,11 +2102,6 @@ int bgp_process_packet(struct thread *thread) u_char type = 0; bgp_size_t size; char notify_data_length[2]; - u_int32_t notify_out; - - /* Note notify_out so we can check later to see if we sent - * another one */ - notify_out = peer->notify_out; pthread_mutex_lock(&peer->io_mtx); { @@ -2010,8 +2112,6 @@ int bgp_process_packet(struct thread *thread) if (peer->curr == NULL) // no packets to process, hmm... return 0; - bgp_size_t actual_size = stream_get_endp(peer->curr); - /* skip the marker and copy the packet length */ stream_forward_getp(peer->curr, BGP_MARKER_SIZE); memcpy(notify_data_length, stream_pnt(peer->curr), 2); @@ -2031,56 +2131,85 @@ int bgp_process_packet(struct thread *thread) switch (type) { case BGP_MSG_OPEN: peer->open_in++; - bgp_open_receive(peer, - size); /* XXX return value ignored! */ + mprc = bgp_open_receive(peer, size); + if (mprc == BGP_Stop) + zlog_err( + "%s: BGP OPEN receipt failed for peer: %s", + __FUNCTION__, peer->host); break; case BGP_MSG_UPDATE: + peer->update_in++; peer->readtime = monotime(NULL); - bgp_update_receive(peer, size); + mprc = bgp_update_receive(peer, size); + if (mprc == BGP_Stop) + zlog_err( + "%s: BGP UPDATE receipt failed for peer: %s", + __FUNCTION__, peer->host); break; case BGP_MSG_NOTIFY: - bgp_notify_receive(peer, size); + peer->notify_in++; + mprc = bgp_notify_receive(peer, size); + if (mprc == BGP_Stop) + zlog_err( + "%s: BGP NOTIFY receipt failed for peer: %s", + __FUNCTION__, peer->host); break; case BGP_MSG_KEEPALIVE: peer->readtime = monotime(NULL); - bgp_keepalive_receive(peer, size); + peer->keepalive_in++; + mprc = bgp_keepalive_receive(peer, size); + if (mprc == BGP_Stop) + zlog_err( + "%s: BGP KEEPALIVE receipt failed for peer: %s", + __FUNCTION__, peer->host); break; case BGP_MSG_ROUTE_REFRESH_NEW: case BGP_MSG_ROUTE_REFRESH_OLD: peer->refresh_in++; - bgp_route_refresh_receive(peer, size); + mprc = bgp_route_refresh_receive(peer, size); + if (mprc == BGP_Stop) + zlog_err( + "%s: BGP ROUTEREFRESH receipt failed for peer: %s", + __FUNCTION__, peer->host); break; case BGP_MSG_CAPABILITY: peer->dynamic_cap_in++; - bgp_capability_receive(peer, size); + mprc = bgp_capability_receive(peer, size); + if (mprc == BGP_Stop) + zlog_err( + "%s: BGP CAPABILITY receipt failed for peer: %s", + __FUNCTION__, peer->host); break; } - /* If reading this packet caused us to send a NOTIFICATION then - * store a copy - * of the packet for troubleshooting purposes - */ - if (notify_out < peer->notify_out) { - memcpy(peer->last_reset_cause, peer->curr->data, - actual_size); - peer->last_reset_cause_size = actual_size; - } + /* delete processed packet */ + stream_free(peer->curr); + peer->curr = NULL; + processed++; - /* Delete packet and carry on. */ - if (peer->curr) { - stream_free(peer->curr); - peer->curr = NULL; - processed++; - } + /* Update FSM */ + if (mprc != BGP_PACKET_NOOP) + fsm_update_result = bgp_event_update(peer, mprc); + + /* 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; } - pthread_mutex_lock(&peer->io_mtx); - { - if (peer->ibuf->count > 0) // more work to do, come back later - thread_add_event(bm->master, bgp_process_packet, peer, - 0, NULL); + if (fsm_update_result != FSM_PEER_TRANSFERRED + && fsm_update_result != FSM_PEER_STOPPED) { + pthread_mutex_lock(&peer->io_mtx); + { + if (peer->ibuf->count + > 0) // more work to do, come back later + thread_add_event(bm->master, bgp_process_packet, + peer, 0, NULL); + } + pthread_mutex_unlock(&peer->io_mtx); } - pthread_mutex_unlock(&peer->io_mtx); return 0; } |