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 <ddutt@cumulusnetworks.com>

diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c
index 899d565adca76f98b18c11eb0bdac720d57b76bb..68934655f09948631e5397c493b9b48f7dacd8ef 100644 (file)
--- 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 403393e84fdfadfe75bfca102ab43a204688b85e..4216967fbf9a3a92a6ad95d62b186a74e087f602 100644 (file)
--- 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 14aa909cbb90db3911e26b44747608490ccbdba1..0f99fcc4555a2d380fde6914efc2dab557f4e60f 100644 (file)
--- 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 04f876d2053e5203ad3f8b6747bf8ba977ca21dd..a59bb77f38929765d6989fba5fdee051aa24ad3c 100644 (file)
--- 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 7d9c958f6424b5108d81aa4ef944a1c98aada486..16f5dac40ec0dbccbe94616388bd950714123ead 100644 (file)
--- 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)
     {