From: Donald Sharp Date: Wed, 20 May 2015 01:45:53 +0000 (-0700) Subject: bgpd-ebgp-multihop-fix.patch X-Git-Tag: frr-2.0-rc1~1358 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=e5cc509c3401b94e95cbfeeb3628c83269221a4e;p=matthieu%2Ffrr.git bgpd-ebgp-multihop-fix.patch BGP: Fix EBGP multihop transitions correctly Since BGP connection setup has migrated to using NHT to decide when to bring a session up, we have to handle ebgp multihop transitions correctly to ensure NHT registrations are correctly handled. Signed-off-by: Dinesh G Dutt --- diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index 899d565adc..68934655f0 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -148,7 +148,7 @@ bgp_update_sock_send_buffer_size (int fd) } } -static void +int bgp_set_socket_ttl (struct peer *peer, int bgp_sock) { char buf[INET_ADDRSTRLEN]; @@ -164,6 +164,7 @@ bgp_set_socket_ttl (struct peer *peer, int bgp_sock) __func__, inet_ntop (AF_INET, &peer->remote_id, buf, sizeof(buf)), errno); + return ret; } } else if (peer->gtsm_hops) @@ -178,6 +179,7 @@ bgp_set_socket_ttl (struct peer *peer, int bgp_sock) __func__, inet_ntop (AF_INET, &peer->remote_id, buf, sizeof(buf)), errno); + return ret; } ret = sockopt_minttl (peer->su.sa.sa_family, bgp_sock, MAXTTL + 1 - peer->gtsm_hops); @@ -187,8 +189,11 @@ bgp_set_socket_ttl (struct peer *peer, int bgp_sock) __func__, inet_ntop (AF_INET, &peer->remote_id, buf, sizeof(buf)), errno); + return ret; } } + + return ret; } /* Accept bgp connection. */ @@ -305,7 +310,10 @@ bgp_accept (struct thread *thread) peer_delete(peer1->doppelganger); } - bgp_set_socket_ttl (peer1, bgp_sock); + if (bgp_set_socket_ttl (peer1, bgp_sock) < 0) + if (bgp_debug_neighbor_events(peer1)) + zlog_debug ("[Event] Unable to set min/max TTL on peer %s, Continuing", + peer1->host); peer = peer_create (&su, peer1->conf_if, peer1->bgp, peer1->local_as, peer1->as, peer1->as_type, 0, 0); @@ -453,7 +461,8 @@ bgp_connect (struct peer *peer) /* Set socket send buffer size */ bgp_update_sock_send_buffer_size(peer->fd); - bgp_set_socket_ttl (peer, peer->fd); + if (bgp_set_socket_ttl (peer, peer->fd) < 0) + return -1; sockopt_reuseaddr (peer->fd); sockopt_reuseport (peer->fd); diff --git a/bgpd/bgp_network.h b/bgpd/bgp_network.h index 403393e84f..4216967fbf 100644 --- a/bgpd/bgp_network.h +++ b/bgpd/bgp_network.h @@ -29,5 +29,6 @@ extern int bgp_connect (struct peer *); extern int bgp_getsockname (struct peer *); extern int bgp_md5_set (struct peer *); +extern int bgp_set_socket_ttl(struct peer *, int fd); #endif /* _QUAGGA_BGP_NETWORK_H */ diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 14aa909cbb..0f99fcc455 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3568,7 +3568,13 @@ peer_ebgp_multihop_set (struct peer *peer, int ttl) if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP)) { if (peer->fd >= 0 && peer->sort != BGP_PEER_IBGP) - sockopt_ttl (peer->su.sa.sa_family, peer->fd, peer->ttl); + { + if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) + bgp_notify_send (peer, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_CONFIG_CHANGE); + else + bgp_session_reset(peer); + } } else { @@ -3580,8 +3586,11 @@ peer_ebgp_multihop_set (struct peer *peer, int ttl) peer->ttl = group->conf->ttl; - if (peer->fd >= 0) - sockopt_ttl (peer->su.sa.sa_family, peer->fd, peer->ttl); + if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) + bgp_notify_send (peer, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_CONFIG_CHANGE); + else + bgp_session_reset(peer); } } return 0; @@ -3606,8 +3615,11 @@ peer_ebgp_multihop_unset (struct peer *peer) if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP)) { - if (peer->fd >= 0 && peer->sort != BGP_PEER_IBGP) - sockopt_ttl (peer->su.sa.sa_family, peer->fd, peer->ttl); + if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) + bgp_notify_send (peer, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_CONFIG_CHANGE); + else + bgp_session_reset(peer); } else { @@ -3618,9 +3630,15 @@ peer_ebgp_multihop_unset (struct peer *peer) continue; peer->ttl = 1; - + if (peer->fd >= 0) - sockopt_ttl (peer->su.sa.sa_family, peer->fd, peer->ttl); + { + if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) + bgp_notify_send (peer, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_CONFIG_CHANGE); + else + bgp_session_reset(peer); + } } } return 0; @@ -5505,53 +5523,79 @@ peer_ttl_security_hops_set (struct peer *peer, int gtsm_hops) before actually applying the ttl-security rules. Cisco really made a mess of this configuration parameter, and OpenBGPD got it right. */ - - if (peer->gtsm_hops == 0) + + if ((peer->gtsm_hops == 0) && (peer->sort != BGP_PEER_IBGP)) { if (is_ebgp_multihop_configured (peer)) return BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK; - /* specify MAXTTL on outgoing packets */ - /* Routine handles iBGP peers correctly */ - ret = peer_ebgp_multihop_set (peer, MAXTTL); - if (ret != 0) - return ret; - } - - peer->gtsm_hops = gtsm_hops; + if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP)) + { + peer->gtsm_hops = gtsm_hops; - if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP)) - { - if (peer->fd >= 0) - sockopt_minttl (peer->su.sa.sa_family, peer->fd, - MAXTTL + 1 - gtsm_hops); - if (peer->status != Established && peer->doppelganger) - sockopt_minttl (peer->su.sa.sa_family, peer->doppelganger->fd, - MAXTTL + 1 - gtsm_hops); + /* Calling ebgp multihop also resets the session. + * On restart, NHT will get setup correctly as will the + * min & max ttls on the socket. The return value is + * irrelevant. + */ + ret = peer_ebgp_multihop_set (peer, MAXTTL); + + if (ret != 0) + return ret; + } + else + { + group = peer->group; + for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer)) + { + peer->gtsm_hops = group->conf->gtsm_hops; + + /* Calling ebgp multihop also resets the session. + * On restart, NHT will get setup correctly as will the + * min & max ttls on the socket. The return value is + * irrelevant. + */ + ret = peer_ebgp_multihop_set (peer, MAXTTL); + } + } } else { - group = peer->group; - for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer)) + /* Post the first gtsm setup or if its ibgp, maxttl setting isn't + * necessary, just set the minttl. + */ + if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP)) { - peer->gtsm_hops = group->conf->gtsm_hops; + peer->gtsm_hops = gtsm_hops; - /* Change setting of existing peer - * established then change value (may break connectivity) - * not established yet (teardown session and restart) - * no session then do nothing (will get handled by next connection) - */ - if (peer->status == Established) + if (peer->fd >= 0) + sockopt_minttl (peer->su.sa.sa_family, peer->fd, + MAXTTL + 1 - gtsm_hops); + if ((peer->status < Established) && peer->doppelganger && + (peer->doppelganger->fd >= 0)) + sockopt_minttl (peer->su.sa.sa_family, peer->doppelganger->fd, + MAXTTL + 1 - gtsm_hops); + } + else + { + group = peer->group; + for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer)) { + peer->gtsm_hops = group->conf->gtsm_hops; + + /* Change setting of existing peer + * established then change value (may break connectivity) + * not established yet (teardown session and restart) + * no session then do nothing (will get handled by next connection) + */ if (peer->fd >= 0 && peer->gtsm_hops != 0) sockopt_minttl (peer->su.sa.sa_family, peer->fd, MAXTTL + 1 - peer->gtsm_hops); - } - else if (peer->status < Established) - { - if (bgp_debug_neighbor_events(peer)) - zlog_debug ("%s Min-ttl changed", peer->host); - bgp_session_reset(peer); + if ((peer->status < Established) && peer->doppelganger && + (peer->doppelganger->fd >= 0)) + sockopt_minttl (peer->su.sa.sa_family, peer->doppelganger->fd, + MAXTTL + 1 - gtsm_hops); + } } } @@ -5564,7 +5608,7 @@ peer_ttl_security_hops_unset (struct peer *peer) { struct peer_group *group; struct listnode *node, *nnode; - struct peer *opeer; + int ret = 0; zlog_debug ("peer_ttl_security_hops_unset: set gtsm_hops to zero for %s", peer->host); @@ -5574,14 +5618,23 @@ peer_ttl_security_hops_unset (struct peer *peer) else peer->gtsm_hops = 0; - opeer = peer; if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP)) { - if (peer->fd >= 0) - sockopt_minttl (peer->su.sa.sa_family, peer->fd, 0); + /* Invoking ebgp_multihop_set will set the TTL back to the original + * value as well as restting the NHT and such. The session is reset. + */ + if (peer->sort == BGP_PEER_EBGP) + ret = peer_ebgp_multihop_unset (peer); + else + { + if (peer->fd >= 0) + sockopt_minttl (peer->su.sa.sa_family, peer->fd, 0); - if (peer->status != Established && peer->doppelganger) - sockopt_minttl (peer->su.sa.sa_family, peer->doppelganger->fd, 0); + if ((peer->status < Established) && peer->doppelganger && + (peer->doppelganger->fd >= 0)) + sockopt_minttl (peer->su.sa.sa_family, + peer->doppelganger->fd, 0); + } } else { @@ -5589,16 +5642,22 @@ peer_ttl_security_hops_unset (struct peer *peer) for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer)) { peer->gtsm_hops = 0; - - if (peer->fd >= 0) - sockopt_minttl (peer->su.sa.sa_family, peer->fd, 0); + if (peer->sort == BGP_PEER_EBGP) + ret = peer_ebgp_multihop_unset (peer); + else + { + if (peer->fd >= 0) + sockopt_minttl (peer->su.sa.sa_family, peer->fd, 0); - if (peer->status != Established && peer->doppelganger) - sockopt_minttl (peer->su.sa.sa_family, peer->doppelganger->fd, 0); + if ((peer->status < Established) && peer->doppelganger && + (peer->doppelganger->fd >= 0)) + sockopt_minttl (peer->su.sa.sa_family, + peer->doppelganger->fd, 0); + } } } - return peer_ebgp_multihop_unset (opeer); + return 0; } /* diff --git a/lib/sockunion.c b/lib/sockunion.c index 04f876d205..a59bb77f38 100644 --- a/lib/sockunion.c +++ b/lib/sockunion.c @@ -496,6 +496,16 @@ sockopt_cork (int sock, int onoff) #endif } +/* For some crazy reason, our build doesn't seem to pick this up */ +#ifdef GNU_LINUX +#ifndef IP_MINTTL +#define IP_MINTTL 21 +#endif +#ifndef IPV6_MINHOPCNT +#define IPV6_MINHOPCNT 73 +#endif +#endif + int sockopt_minttl (int family, int sock, int minttl) { diff --git a/zebra/zebra_rnh.c b/zebra/zebra_rnh.c index 7d9c958f64..16f5dac40e 100644 --- a/zebra/zebra_rnh.c +++ b/zebra/zebra_rnh.c @@ -860,7 +860,8 @@ print_rnh (struct route_node *rn, struct vty *vty) char buf[BUFSIZ]; rnh = rn->info; - vty_out(vty, "%s%s", inet_ntop(rn->p.family, &rn->p.u.prefix, buf, BUFSIZ), + vty_out(vty, "%s%s%s", inet_ntop(rn->p.family, &rn->p.u.prefix, buf, BUFSIZ), + CHECK_FLAG(rnh->flags, ZEBRA_NHT_CONNECTED) ? "(Connected)" : "", VTY_NEWLINE); if (rnh->state) {