BGP: 'neighbor swpX interface peer-group FOO' is needed to simplify swpX

configuration

Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by:   Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by:   Dinesh Dutt <ddutt@cumulusnetworks.com>
Ticket: CM-8321

This allows the user to configure the peer-group as an option for the
"neighbor swpX interface" command.
{code}
!
router bgp 100
 neighbor FOO peer-group
 neighbor FOO remote-as external
 neighbor swp1 interface
 neighbor swp2 interface v6only
 neighbor swp3 interface peer-group FOO
 neighbor swp4 interface v6only peer-group FOO
!
 address-family ipv4 unicast
  neighbor FOO activate
  neighbor swp1 activate
  neighbor swp2 activate
 exit-address-family
!
{code}

Note that if the user configures
{code}
neighbor swp5 interface
neighbor swp5 peer-group FOO
{code}

We will display that as "neighbor swp5 interface peer-group FOO".  It
did not seem worth tracking that the peer-group was entered via two
lines instead of one.

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 24eaf9c47f319995ae8ccd52adca8c236e70604f..1bde623fccf49a77a89509955399ffaf781c1836 100644 (file)
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -223,7 +223,7 @@ bgp_vty_return (struct vty *vty, int ret)
       str = "Cannot change the peer-group. Deconfigure first";
       break;
     case BGP_ERR_PEER_GROUP_MISMATCH:
-      str = "Cannot have different peer-group for the neighbor";
+      str = "Peer is not a member of this peer-group";
       break;
     case BGP_ERR_PEER_FILTER_CONFLICT:
       str = "Prefix/distribute list can not co-exist";
@@ -2677,36 +2677,120 @@ DEFUN (neighbor_remote_as,
   return peer_remote_as_vty (vty, argv[0], argv[1], AFI_IP, SAFI_UNICAST);
 }
 
-DEFUN (neighbor_interface_config,
-       neighbor_interface_config_cmd,
-       "neighbor WORD interface {v6only}",
-       NEIGHBOR_STR
-       "Interface name or neighbor tag\n"
-       "Enable BGP on interface\n"
-       "Enable BGP with v6 link-local only\n")
+static int
+peer_conf_interface_get (struct vty *vty, const char *conf_if, afi_t afi,
+                         safi_t safi, int v6only, const char *peer_group_name)
 {
+  as_t as;
   struct bgp *bgp;
   struct peer *peer;
   struct peer_group *group;
+  int ret = 0;
+  union sockunion su;
 
   bgp = vty->index;
-  group = peer_group_lookup (bgp, argv[0]);
+  group = peer_group_lookup (bgp, conf_if);
+
   if (group)
     {
       vty_out (vty, "%% Name conflict with peer-group %s", VTY_NEWLINE);
       return CMD_WARNING;
     }
 
-  if (argv[1] != NULL)
-    peer = peer_conf_interface_get (bgp, argv[0], AFI_IP, SAFI_UNICAST, 1);
-  else
-    peer = peer_conf_interface_get (bgp, argv[0], AFI_IP, SAFI_UNICAST, 0);
+  peer = peer_lookup_by_conf_if (bgp, conf_if);
+  if (!peer)
+    {
+      if (bgp_flag_check (bgp, BGP_FLAG_NO_DEFAULT_IPV4)
+          && afi == AFI_IP && safi == SAFI_UNICAST)
+        peer = peer_create (NULL, conf_if, bgp, bgp->as, 0, AS_UNSPECIFIED, 0, 0);
+      else
+        peer = peer_create (NULL, conf_if, bgp, bgp->as, 0, AS_UNSPECIFIED, afi, safi);
+
+      if (peer && v6only)
+        SET_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY);
+    }
+  else if ((v6only && !CHECK_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY)) ||
+           (!v6only && CHECK_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY)))
+    {
+      if (v6only)
+        SET_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY);
+      else
+        UNSET_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY);
+
+      /* v6only flag changed. Reset bgp seesion */
+      if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
+        {
+          peer->last_reset = PEER_DOWN_V6ONLY_CHANGE;
+          bgp_notify_send (peer, BGP_NOTIFY_CEASE,
+                           BGP_NOTIFY_CEASE_CONFIG_CHANGE);
+        }
+      else
+        bgp_session_reset(peer);
+    }
+
   if (!peer)
     return CMD_WARNING;
 
-  return CMD_SUCCESS;
+  if (peer_group_name)
+    {
+      group = peer_group_lookup (bgp, peer_group_name);
+      if (! group)
+        {
+          vty_out (vty, "%% Configure the peer-group first%s", VTY_NEWLINE);
+          return CMD_WARNING;
+        }
+
+      ret = peer_group_bind (bgp, &su, peer, group, &as);
+    }
+
+  return bgp_vty_return (vty, ret);
 }
 
+DEFUN (neighbor_interface_config,
+       neighbor_interface_config_cmd,
+       "neighbor WORD interface",
+       NEIGHBOR_STR
+       "Interface name or neighbor tag\n"
+       "Enable BGP on interface\n")
+{
+  if (argc == 2)
+    return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 0, argv[1]);
+  else
+    return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 0, NULL);
+}
+
+ALIAS (neighbor_interface_config,
+       neighbor_interface_config_peergroup_cmd,
+       "neighbor WORD interface peer-group WORD",
+       NEIGHBOR_STR
+       "Interface name or neighbor tag\n"
+       "Enable BGP on interface\n"
+       "Member of the peer-group\n"
+       "peer-group name\n")
+
+DEFUN (neighbor_interface_config_v6only,
+       neighbor_interface_config_v6only_cmd,
+       "neighbor WORD interface v6only",
+       NEIGHBOR_STR
+       "Interface name or neighbor tag\n"
+       "Enable BGP on interface\n"
+       "Enable BGP with v6 link-local only\n")
+{
+  if (argc == 2)
+    return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 1, argv[1]);
+  else
+    return peer_conf_interface_get (vty, argv[0], AFI_IP, SAFI_UNICAST, 1, NULL);
+}
+
+ALIAS (neighbor_interface_config_v6only,
+       neighbor_interface_config_v6only_peergroup_cmd,
+       "neighbor WORD interface v6only peer-group WORD",
+       NEIGHBOR_STR
+       "Interface name or neighbor tag\n"
+       "Enable BGP on interface\n"
+       "Enable BGP with v6 link-local only\n"
+       "Member of the peer-group\n"
+       "peer-group name\n")
 
 DEFUN (neighbor_peer_group,
        neighbor_peer_group_cmd,
@@ -2800,7 +2884,7 @@ ALIAS (no_neighbor,
 
 DEFUN (no_neighbor_interface_config,
        no_neighbor_interface_config_cmd,
-       "no neighbor WORD interface {v6only}",
+       "no neighbor WORD interface",
        NO_STR
        NEIGHBOR_STR
        "Interface name\n"
@@ -2822,6 +2906,37 @@ DEFUN (no_neighbor_interface_config,
   return CMD_SUCCESS;
 }
 
+ALIAS (no_neighbor_interface_config,
+       no_neighbor_interface_config_peergroup_cmd,
+       "no neighbor WORD interface peer-group WORD",
+       NO_STR
+       NEIGHBOR_STR
+       "Interface name\n"
+       "Configure BGP on interface\n"
+       "Member of the peer-group\n"
+       "peer-group name\n")
+
+ALIAS (no_neighbor_interface_config,
+       no_neighbor_interface_config_v6only_cmd,
+       "no neighbor WORD interface v6only",
+       NO_STR
+       NEIGHBOR_STR
+       "Interface name\n"
+       "Configure BGP on interface\n"
+       "Enable BGP with v6 link-local only\n")
+
+ALIAS (no_neighbor_interface_config,
+       no_neighbor_interface_config_v6only_peergroup_cmd,
+       "no neighbor WORD interface v6only peer-group WORD",
+       NO_STR
+       NEIGHBOR_STR
+       "Interface name\n"
+       "Configure BGP on interface\n"
+       "Enable BGP with v6 link-local only\n"
+       "Member of the peer-group\n"
+       "peer-group name\n")
+
+
 DEFUN (no_neighbor_peer_group,
        no_neighbor_peer_group_cmd,
        "no neighbor WORD peer-group",
@@ -12164,9 +12279,15 @@ bgp_vty_init (void)
   /* "neighbor remote-as" commands. */
   install_element (BGP_NODE, &neighbor_remote_as_cmd);
   install_element (BGP_NODE, &neighbor_interface_config_cmd);
+  install_element (BGP_NODE, &neighbor_interface_config_v6only_cmd);
+  install_element (BGP_NODE, &neighbor_interface_config_peergroup_cmd);
+  install_element (BGP_NODE, &neighbor_interface_config_v6only_peergroup_cmd);
   install_element (BGP_NODE, &no_neighbor_cmd);
   install_element (BGP_NODE, &no_neighbor_remote_as_cmd);
   install_element (BGP_NODE, &no_neighbor_interface_config_cmd);
+  install_element (BGP_NODE, &no_neighbor_interface_config_v6only_cmd);
+  install_element (BGP_NODE, &no_neighbor_interface_config_peergroup_cmd);
+  install_element (BGP_NODE, &no_neighbor_interface_config_v6only_peergroup_cmd);
 
   /* "neighbor peer-group" commands. */
   install_element (BGP_NODE, &neighbor_peer_group_cmd);

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index e7b1f20b360a5beae3b0b80c6cb6921bb7c2d20c..ea66dc3b3c646a822375cc72b455410bbb3f6752 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -83,7 +83,7 @@ struct bgp_master *bm;
 struct community_list_handler *bgp_clist;
 
 
-static inline void
+void
 bgp_session_reset(struct peer *peer)
 {
   if (peer->doppelganger && (peer->doppelganger->status != Deleted)
@@ -99,7 +99,7 @@ bgp_session_reset(struct peer *peer)
  * during walk of peer list, we would end up accessing the freed next
  * node. This function moves the next node along.
  */
-static inline void
+static void
 bgp_session_reset_safe(struct peer *peer, struct listnode **nnode)
 {
   struct listnode *n;
@@ -1434,46 +1434,6 @@ peer_create (union sockunion *su, const char *conf_if, struct bgp *bgp,
   return peer;
 }
 
-struct peer *
-peer_conf_interface_get(struct bgp *bgp, const char *conf_if, afi_t afi,
-                        safi_t safi, int v6only)
-{
-  struct peer *peer;
-
-  peer = peer_lookup_by_conf_if (bgp, conf_if);
-  if (!peer)
-    {
-      if (bgp_flag_check (bgp, BGP_FLAG_NO_DEFAULT_IPV4)
-          && afi == AFI_IP && safi == SAFI_UNICAST)
-        peer = peer_create (NULL, conf_if, bgp, bgp->as, 0, AS_UNSPECIFIED, 0, 0);
-      else
-        peer = peer_create (NULL, conf_if, bgp, bgp->as, 0, AS_UNSPECIFIED, afi, safi);
-
-      if (peer && v6only)
-       SET_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY);
-    }
-  else if ((v6only && !CHECK_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY)) ||
-          (!v6only && CHECK_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY)))
-    {
-      if (v6only)
-       SET_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY);
-      else
-       UNSET_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY);
-
-      /* v6only flag changed. Reset bgp seesion */
-      if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
-       {
-         peer->last_reset = PEER_DOWN_V6ONLY_CHANGE;
-         bgp_notify_send (peer, BGP_NOTIFY_CEASE,
-                          BGP_NOTIFY_CEASE_CONFIG_CHANGE);
-       }
-      else
-       bgp_session_reset(peer);
-    }
-
-  return peer;
-}
-
 /* Make accept BGP peer. This function is only called from the test code */
 struct peer *
 peer_create_accept (struct bgp *bgp)
@@ -2540,7 +2500,7 @@ peer_group_bind (struct bgp *bgp, union sockunion *su, struct peer *peer,
     {
       /* When the peer already belongs to peer group, check the consistency.  */
       if (peer_group_active (peer) && strcmp (peer->group->name, group->name) != 0)
-        return BGP_ERR_PEER_GROUP_MISMATCH;
+        return BGP_ERR_PEER_GROUP_CANT_CHANGE;
 
       /* The peer has not specified a remote-as, inherit it from the
        * peer-group */
@@ -6135,9 +6095,14 @@ bgp_config_write_peer_global (struct vty *vty, struct bgp *bgp,
   if (peer->conf_if)
     {
       if (CHECK_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY))
-        vty_out (vty, " neighbor %s interface v6only %s", addr, VTY_NEWLINE);
+        vty_out (vty, " neighbor %s interface v6only", addr);
       else
-        vty_out (vty, " neighbor %s interface%s", addr, VTY_NEWLINE);
+        vty_out (vty, " neighbor %s interface", addr);
+
+      if (peer_group_active (peer))
+         vty_out (vty, " peer-group %s", peer->group->name);
+
+      vty_out (vty, "%s", VTY_NEWLINE);
     }
 
   /* remote-as and peer-group */
@@ -6163,8 +6128,11 @@ bgp_config_write_peer_global (struct vty *vty, struct bgp *bgp,
             }
         }
 
-      vty_out (vty, " neighbor %s peer-group %s%s", addr,
-               peer->group->name, VTY_NEWLINE);
+      /* For swpX peers we displayed the peer-group
+       * via 'neighbor swpX interface peer-group WORD' */
+      if (!peer->conf_if)
+          vty_out (vty, " neighbor %s peer-group %s%s", addr,
+                   peer->group->name, VTY_NEWLINE);
     }
 
   /* peer is NOT a member of a peer-group */

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 8256d2a8932c404dce4347c7b3732adfe1b514e1..c8bdb3f92d2dd8800b384e5acf710feb853eeb6b 100644 (file)
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -1107,8 +1107,6 @@ extern struct bgp *bgp_lookup_by_name (const char *);
 extern struct peer *peer_lookup (struct bgp *, union sockunion *);
 extern struct peer *peer_lookup_by_conf_if (struct bgp *, const char *);
 extern struct peer *peer_lookup_by_hostname(struct bgp *, const char *);
-extern struct peer *peer_conf_interface_get(struct bgp *, const char *, afi_t,
-                                            safi_t, int v6only);
 extern void  bgp_peer_conf_if_to_su_update (struct peer *);
 extern int peer_group_listen_range_del(struct peer_group *, struct prefix *);
 extern struct peer_group *peer_group_lookup (struct bgp *, const char *);
@@ -1150,6 +1148,7 @@ extern void bgp_master_init (void);
 
 extern void bgp_init (void);
 extern void bgp_route_map_init (void);
+extern void bgp_session_reset (struct peer *);
 
 extern int bgp_option_set (int);
 extern int bgp_option_unset (int);