bgpd: simplify ebgp-multihop and ttl-security handling
Change to track configured value in ->ttl and ->gtsm_hops;
not the value set to sockopt. Instead, setting of socket's ttl
and minttl options are now merged to one function which calculates
it on demand. This greatly simplifies the code.
diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
index 7da63ea..67c50c4 100644
--- a/bgpd/bgp_fsm.c
+++ b/bgpd/bgp_fsm.c
@@ -716,7 +716,7 @@
}
/* Register to be notified on peer up */
- if ((peer->ttl == 1 || peer->gtsm_hops == 1) &&
+ if ((peer_ttl(peer) == 1 || peer->gtsm_hops == 1) &&
! CHECK_FLAG (peer->flags, PEER_FLAG_DISABLE_CONNECTED_CHECK))
connected = 1;
diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c
index 51a6f60..c7d3389 100644
--- a/bgpd/bgp_network.c
+++ b/bgpd/bgp_network.c
@@ -146,47 +146,39 @@
}
}
-static void
+void
bgp_set_socket_ttl (struct peer *peer, int bgp_sock)
{
char buf[INET_ADDRSTRLEN];
- int ret;
+ int ret, ttl, minttl;
- /* In case of peer is EBGP, we should set TTL for this connection. */
- if (!peer->gtsm_hops && (peer_sort (peer) == BGP_PEER_EBGP))
+ if (bgp_sock < 0)
+ return;
+
+ if (peer->gtsm_hops)
{
- ret = sockopt_ttl (peer->su.sa.sa_family, bgp_sock, peer->ttl);
- if (ret)
- {
- zlog_err ("%s: Can't set TxTTL on peer (rtrid %s) socket, err = %d",
- __func__,
- inet_ntop (AF_INET, &peer->remote_id, buf, sizeof(buf)),
- errno);
- }
+ ttl = 255;
+ minttl = 256 - peer->gtsm_hops;
}
- else if (peer->gtsm_hops)
+ else
{
- /* On Linux, setting minttl without setting ttl seems to mess with the
- outgoing ttl. Therefore setting both.
- */
- ret = sockopt_ttl (peer->su.sa.sa_family, bgp_sock, MAXTTL);
- if (ret)
- {
- zlog_err ("%s: Can't set TxTTL on peer (rtrid %s) socket, err = %d",
- __func__,
- inet_ntop (AF_INET, &peer->remote_id, buf, sizeof(buf)),
- errno);
- }
- ret = sockopt_minttl (peer->su.sa.sa_family, bgp_sock,
- MAXTTL + 1 - peer->gtsm_hops);
- if (ret)
- {
- zlog_err ("%s: Can't set MinTTL on peer (rtrid %s) socket, err = %d",
- __func__,
- inet_ntop (AF_INET, &peer->remote_id, buf, sizeof(buf)),
- errno);
- }
+ ttl = peer_ttl (peer);
+ minttl = 0;
}
+
+ ret = sockopt_ttl (peer->su.sa.sa_family, bgp_sock, ttl);
+ if (ret)
+ zlog_err ("%s: Can't set TxTTL on peer (rtrid %s) socket, err = %d",
+ __func__,
+ inet_ntop (AF_INET, &peer->remote_id, buf, sizeof(buf)),
+ errno);
+
+ ret = sockopt_minttl (peer->su.sa.sa_family, bgp_sock, minttl);
+ if (ret && (errno != ENOTSUP || minttl))
+ zlog_err ("%s: Can't set MinTTL on peer (rtrid %s) socket, err = %d",
+ __func__,
+ inet_ntop (AF_INET, &peer->remote_id, buf, sizeof(buf)),
+ errno);
}
/* Accept bgp connection. */
diff --git a/bgpd/bgp_network.h b/bgpd/bgp_network.h
index 1276843..31995ca 100644
--- a/bgpd/bgp_network.h
+++ b/bgpd/bgp_network.h
@@ -28,6 +28,7 @@
extern int bgp_connect (struct peer *);
extern void bgp_getsockname (struct peer *);
+extern void bgp_set_socket_ttl (struct peer *peer, int bgp_sock);
extern int bgp_md5_set (struct peer *);
#endif /* _QUAGGA_BGP_NETWORK_H */
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 4cb6c14..010fd4d 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -2339,7 +2339,7 @@
/* Nexthop reachability check. */
if ((afi == AFI_IP || afi == AFI_IP6) && safi == SAFI_UNICAST)
{
- if (peer->sort == BGP_PEER_EBGP && peer->ttl == 1 &&
+ if (peer->sort == BGP_PEER_EBGP && peer_ttl (peer) == 1 &&
! CHECK_FLAG (peer->flags, PEER_FLAG_DISABLE_CONNECTED_CHECK))
connected = 1;
else
@@ -2391,7 +2391,7 @@
/* Nexthop reachability check. */
if ((afi == AFI_IP || afi == AFI_IP6) && safi == SAFI_UNICAST)
{
- if (peer->sort == BGP_PEER_EBGP && peer->ttl == 1 &&
+ if (peer->sort == BGP_PEER_EBGP && peer_ttl (peer) == 1 &&
! CHECK_FLAG (peer->flags, PEER_FLAG_DISABLE_CONNECTED_CHECK))
connected = 1;
else
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 7af4e81..8eeaff9 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -3058,7 +3058,7 @@
if (! peer)
return CMD_WARNING;
- return bgp_vty_return (vty, peer_ebgp_multihop_unset (peer));
+ return bgp_vty_return (vty, peer_ebgp_multihop_set (peer, 0));
}
/* neighbor ebgp-multihop. */
@@ -4380,7 +4380,7 @@
if (! peer)
return CMD_WARNING;
- return bgp_vty_return (vty, peer_ttl_security_hops_unset (peer));
+ return bgp_vty_return (vty, peer_ttl_security_hops_set (peer, 0));
}
/* Address family configuration. */
@@ -8364,6 +8364,7 @@
char timebuf[BGP_UPTIME_LEN];
afi_t afi;
safi_t safi;
+ int ttl;
bgp = p->bgp;
@@ -8663,21 +8664,12 @@
}
/* EBGP Multihop and GTSM */
- if (p->sort != BGP_PEER_IBGP)
- {
- if (p->gtsm_hops > 0)
- vty_out (vty, " External BGP neighbor may be up to %d hops away.%s",
- p->gtsm_hops, VTY_NEWLINE);
- else if (p->ttl > 1)
- vty_out (vty, " External BGP neighbor may be up to %d hops away.%s",
- p->ttl, VTY_NEWLINE);
- }
- else
- {
- if (p->gtsm_hops > 0)
- vty_out (vty, " Internal BGP neighbor may be up to %d hops away.%s",
- p->gtsm_hops, VTY_NEWLINE);
- }
+ ttl = p->gtsm_hops;
+ if (! ttl)
+ ttl = peer_ttl (p);
+ vty_out (vty, " %s BGP neighbor may be up to %d hops away.%s",
+ p->sort == BGP_PEER_IBGP ? "Internal" : "External",
+ ttl, VTY_NEWLINE);
/* Local address. */
if (p->su_local)
diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index 45d502a..ba87ad1 100644
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -172,11 +172,10 @@
for (ALL_LIST_ELEMENTS (bgp->peer, node, nnode, peer))
{
- if ((peer->ttl != 1) && (peer->gtsm_hops != 1))
- continue;
-
- if (ifp == peer->nexthop.ifp)
- BGP_EVENT_ADD (peer, BGP_Stop);
+ if (peer->gtsm_hops != 1 && peer_ttl (peer) != 1)
+ continue;
+ if (ifp == peer->nexthop.ifp)
+ BGP_EVENT_ADD (peer, BGP_Stop);
}
}
}
@@ -716,7 +715,7 @@
SET_FLAG (flags, ZEBRA_FLAG_INTERNAL);
}
- if ((peer->sort == BGP_PEER_EBGP && peer->ttl != 1)
+ if ((peer->sort == BGP_PEER_EBGP && peer_ttl (peer) != 1)
|| CHECK_FLAG (peer->flags, PEER_FLAG_DISABLE_CONNECTED_CHECK))
SET_FLAG (flags, ZEBRA_FLAG_INTERNAL);
@@ -991,7 +990,7 @@
SET_FLAG (flags, ZEBRA_FLAG_IBGP);
}
- if ((peer->sort == BGP_PEER_EBGP && peer->ttl != 1)
+ if ((peer->sort == BGP_PEER_EBGP && peer_ttl (peer) != 1)
|| CHECK_FLAG (peer->flags, PEER_FLAG_DISABLE_CONNECTED_CHECK))
SET_FLAG (flags, ZEBRA_FLAG_INTERNAL);
diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 018a599..56e4322 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -646,7 +646,8 @@
{
peer->weight = 0;
peer->change_local_as = 0;
- peer->ttl = (peer_sort (peer) == BGP_PEER_IBGP ? 255 : 1);
+ peer->ttl = 0;
+ peer->gtsm_hops = 0;
if (peer->update_source)
{
sockunion_free (peer->update_source);
@@ -925,9 +926,6 @@
/* Last read and reset time set */
peer->readtime = peer->resettime = bgp_clock ();
- /* Default TTL set. */
- peer->ttl = (peer->sort == BGP_PEER_IBGP) ? 255 : 1;
-
/* Make peer's address string. */
sockunion2str (su, buf, SU_ADDRSTRLEN);
peer->host = XSTRDUP (MTYPE_BGP_PEER_HOST, buf);
@@ -957,7 +955,6 @@
static void
peer_as_change (struct peer *peer, as_t as)
{
- bgp_peer_sort_t type;
struct peer *conf;
/* Stop peer. */
@@ -972,7 +969,6 @@
else
BGP_EVENT_ADD (peer, BGP_Stop);
}
- type = peer_sort (peer);
peer->as = as;
if (bgp_config_check (peer->bgp, BGP_CONFIG_CONFEDERATION)
@@ -995,12 +991,6 @@
else
peer->v_routeadv = BGP_DEFAULT_EBGP_ROUTEADV;
- /* TTL reset */
- if (peer_sort (peer) == BGP_PEER_IBGP)
- peer->ttl = 255;
- else if (type == BGP_PEER_IBGP)
- peer->ttl = 1;
-
/* reflector-client reset */
if (peer_sort (peer) != BGP_PEER_IBGP)
{
@@ -1506,7 +1496,7 @@
group->conf->host = XSTRDUP (MTYPE_BGP_PEER_HOST, name);
group->conf->group = group;
group->conf->as = 0;
- group->conf->ttl = 1;
+ group->conf->ttl = 0;
group->conf->gtsm_hops = 0;
group->conf->v_routeadv = BGP_DEFAULT_EBGP_ROUTEADV;
UNSET_FLAG (group->conf->config, PEER_CONFIG_TIMER);
@@ -1807,6 +1797,16 @@
}
int
+peer_ttl (struct peer *peer)
+{
+ if (peer->ttl)
+ return peer->ttl;
+ if (peer->gtsm_hops || peer->sort == BGP_PEER_IBGP)
+ return 255;
+ return 1;
+}
+
+int
peer_group_delete (struct peer_group *group)
{
struct bgp *bgp;
@@ -1938,10 +1938,6 @@
group->conf->v_routeadv = BGP_DEFAULT_EBGP_ROUTEADV;
}
- /* ebgp-multihop reset */
- if (peer_sort (group->conf) == BGP_PEER_IBGP)
- group->conf->ttl = 255;
-
/* local-as reset */
if (peer_sort (group->conf) != BGP_PEER_EBGP)
{
@@ -2870,29 +2866,17 @@
struct peer *peer1;
if (peer->sort == BGP_PEER_IBGP)
- return 0;
+ return BGP_ERR_NO_IBGP_WITH_TTLHACK;
- /* see comment in peer_ttl_security_hops_set() */
- if (ttl != MAXTTL)
+ if (peer->gtsm_hops != 0)
+ return BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK;
+
+ if (CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP))
{
- if (CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP))
+ group = peer->group;
+ for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer1))
{
- group = peer->group;
- if (group->conf->gtsm_hops != 0)
- return BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK;
-
- for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer1))
- {
- if (peer1->sort == BGP_PEER_IBGP)
- continue;
-
- if (peer1->gtsm_hops != 0)
- return BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK;
- }
- }
- else
- {
- if (peer->gtsm_hops != 0)
+ if (peer1->gtsm_hops != 0)
return BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK;
}
}
@@ -2901,62 +2885,18 @@
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);
+ bgp_set_socket_ttl (peer, peer->fd);
}
else
{
group = peer->group;
for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer))
- {
- if (peer->sort == BGP_PEER_IBGP)
- continue;
-
- peer->ttl = group->conf->ttl;
-
- if (peer->fd >= 0)
- sockopt_ttl (peer->su.sa.sa_family, peer->fd, peer->ttl);
- }
+ {
+ peer->ttl = ttl;
+ bgp_set_socket_ttl (peer, peer->fd);
+ }
}
- return 0;
-}
-int
-peer_ebgp_multihop_unset (struct peer *peer)
-{
- struct peer_group *group;
- struct listnode *node, *nnode;
-
- if (peer->sort == BGP_PEER_IBGP)
- return 0;
-
- if (peer->gtsm_hops != 0 && peer->ttl != MAXTTL)
- return BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK;
-
- if (peer_group_active (peer))
- peer->ttl = peer->group->conf->ttl;
- else
- peer->ttl = 1;
-
- 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);
- }
- else
- {
- group = peer->group;
- for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer))
- {
- if (peer->sort == BGP_PEER_IBGP)
- continue;
-
- peer->ttl = 1;
-
- if (peer->fd >= 0)
- sockopt_ttl (peer->su.sa.sa_family, peer->fd, peer->ttl);
- }
- }
return 0;
}
@@ -4652,78 +4592,41 @@
return 0;
}
-static int is_ebgp_multihop_configured (struct peer *peer)
-{
- struct peer_group *group;
- struct listnode *node, *nnode;
- struct peer *peer1;
-
- if (CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP))
- {
- group = peer->group;
- if ((peer_sort(peer) != BGP_PEER_IBGP) &&
- (group->conf->ttl != 1))
- return 1;
-
- for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer1))
- {
- if ((peer_sort (peer1) != BGP_PEER_IBGP) &&
- (peer1->ttl != 1))
- return 1;
- }
- }
- else
- {
- if ((peer_sort(peer) != BGP_PEER_IBGP) &&
- (peer->ttl != 1))
- return 1;
- }
- return 0;
-}
-
/* Set # of hops between us and BGP peer. */
int
peer_ttl_security_hops_set (struct peer *peer, int gtsm_hops)
{
struct peer_group *group;
struct listnode *node, *nnode;
- int ret;
+ struct peer *peer1;
zlog_debug ("peer_ttl_security_hops_set: set gtsm_hops to %d for %s", gtsm_hops, peer->host);
- /* We cannot configure ttl-security hops when ebgp-multihop is already
- set. For non peer-groups, the check is simple. For peer-groups, it's
- slightly messy, because we need to check both the peer-group structure
- and all peer-group members for any trace of ebgp-multihop configuration
- 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 (is_ebgp_multihop_configured (peer))
- return BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK;
+ if (peer->ttl != 0)
+ 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;
+ if (CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP))
+ {
+ group = peer->group;
+ for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer1))
+ {
+ if (peer1->ttl != 0)
+ return BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK;
+ }
}
-
+
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);
+ bgp_set_socket_ttl (peer, peer->fd);
}
else
{
group = peer->group;
for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer))
{
- peer->gtsm_hops = group->conf->gtsm_hops;
+ peer->gtsm_hops = gtsm_hops;
/* Change setting of existing peer
* established then change value (may break connectivity)
@@ -4732,9 +4635,7 @@
*/
if (peer->status == Established)
{
- if (peer->fd >= 0 && peer->gtsm_hops != 0)
- sockopt_minttl (peer->su.sa.sa_family, peer->fd,
- MAXTTL + 1 - peer->gtsm_hops);
+ bgp_set_socket_ttl (peer, peer->fd);
}
else if (peer->status < Established)
{
@@ -4749,42 +4650,6 @@
}
int
-peer_ttl_security_hops_unset (struct peer *peer)
-{
- struct peer_group *group;
- struct listnode *node, *nnode;
- struct peer *opeer;
-
- zlog_debug ("peer_ttl_security_hops_unset: set gtsm_hops to zero for %s", peer->host);
-
- /* if a peer-group member, then reset to peer-group default rather than 0 */
- if (peer_group_active (peer))
- peer->gtsm_hops = peer->group->conf->gtsm_hops;
- 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);
- }
- else
- {
- group = peer->group;
- 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);
- }
- }
-
- return peer_ebgp_multihop_unset (opeer);
-}
-
-int
peer_clear (struct peer *peer)
{
if (! CHECK_FLAG (peer->flags, PEER_FLAG_SHUTDOWN))
@@ -5094,19 +4959,13 @@
! CHECK_FLAG (g_peer->flags, PEER_FLAG_PASSIVE))
vty_out (vty, " neighbor %s passive%s", addr, VTY_NEWLINE);
- /* EBGP multihop. */
- if (peer->sort != BGP_PEER_IBGP && peer->ttl != 1 &&
- !(peer->gtsm_hops != 0 && peer->ttl == MAXTTL))
- if (! peer_group_active (peer) ||
- g_peer->ttl != peer->ttl)
- vty_out (vty, " neighbor %s ebgp-multihop %d%s", addr, peer->ttl,
- VTY_NEWLINE);
-
- /* ttl-security hops */
- if (peer->gtsm_hops != 0)
- if (! peer_group_active (peer) || g_peer->gtsm_hops != peer->gtsm_hops)
- vty_out (vty, " neighbor %s ttl-security hops %d%s", addr,
- peer->gtsm_hops, VTY_NEWLINE);
+ /* TTL option */
+ if (peer->gtsm_hops && ! peer_group_active (peer))
+ vty_out (vty, " neighbor %s ttl-security hops %d%s", addr,
+ peer->gtsm_hops, VTY_NEWLINE);
+ else if (peer->ttl && ! peer_group_active (peer))
+ vty_out (vty, " neighbor %s ebgp-multihop %d%s", addr, peer->ttl,
+ VTY_NEWLINE);
/* disable-connected-check. */
if (CHECK_FLAG (peer->flags, PEER_FLAG_DISABLE_CONNECTED_CHECK))
diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 0058b58..a4c608d 100644
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -832,8 +832,8 @@
#define BGP_ERR_TCPSIG_FAILED -29
#define BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK -30
#define BGP_ERR_NO_IBGP_WITH_TTLHACK -31
-#define BGP_ERR_MAX -32
-#define BGP_ERR_CANNOT_HAVE_LOCAL_AS_SAME_AS_REMOTE_AS -33
+#define BGP_ERR_CANNOT_HAVE_LOCAL_AS_SAME_AS_REMOTE_AS -32
+#define BGP_ERR_MAX -33
extern struct bgp_master *bm;
@@ -913,6 +913,7 @@
extern int peer_remote_as (struct bgp *, union sockunion *, as_t *, afi_t, safi_t);
extern int peer_group_remote_as (struct bgp *, const char *, as_t *);
+extern int peer_ttl (struct peer *peer);
extern int peer_delete (struct peer *peer);
extern int peer_group_delete (struct peer_group *);
extern int peer_group_remote_as_delete (struct peer_group *);
@@ -934,7 +935,6 @@
extern int peer_af_flag_check (struct peer *, afi_t, safi_t, u_int32_t);
extern int peer_ebgp_multihop_set (struct peer *, int);
-extern int peer_ebgp_multihop_unset (struct peer *);
extern int peer_description_set (struct peer *, const char *);
extern int peer_description_unset (struct peer *);
@@ -996,7 +996,6 @@
extern int peer_clear_soft (struct peer *, afi_t, safi_t, enum bgp_clear_type);
extern int peer_ttl_security_hops_set (struct peer *, int);
-extern int peer_ttl_security_hops_unset (struct peer *);
extern void bgp_scan_finish (void);
#endif /* _QUAGGA_BGPD_H */