BGP: Fix update-groups commands to match neighbors

show update-groups summary was mislabeled. What it displays is not a summary
at all, but the detailed info about all update-groups. Furthermore, there
was no way to get detailed info about a specific subgroup.

This patch renames "show * update-groups summary" to "show * update-groups"
and adds an option to see the info specific to a subgroup only. It also
validates the subgroup-id.

show * update-groups summary will be added separately.

bgpd/bgp_updgrp.c

@@ -521,10 +521,42 @@ subgroup_total_packets_enqueued (struct update_subgroup *subgrp)
 static int
 update_group_show_walkcb (struct update_group *updgrp, void *arg)
 {
-  struct vty *vty = arg;
+  struct updwalk_context *ctx = arg;
+  struct vty *vty;
   struct update_subgroup *subgrp;
   struct peer_af *paf;
   struct bgp_filter *filter;
+  int match = 0;
+
+  if (!ctx)
+    return;
+
+  if (ctx->subgrp_id)
+    {
+      UPDGRP_FOREACH_SUBGRP (updgrp, subgrp)
+	{
+	  if (ctx->subgrp_id && (ctx->subgrp_id != subgrp->id))
+	    continue;
+	  else
+	    {
+	      match = 1;
+	      break;
+	    }
+	}
+    }
+  else
+    {
+      match = 1;
+    }
+
+  if (!match)
+    {
+      /* Since this routine is invoked from a walk, we cannot signal any */
+      /* error here, can only return. */
+      return CMD_SUCCESS;
+    }
+
+  vty = ctx->vty;
 
   vty_out (vty, "Update-group %llu:%s", updgrp->id, VTY_NEWLINE);
   vty_out (vty, "  Created: %s", timestamp_string (updgrp->uptime));
@@ -546,6 +578,8 @@ update_group_show_walkcb (struct update_group *updgrp, void *arg)
 
   UPDGRP_FOREACH_SUBGRP (updgrp, subgrp)
   {
+    if (ctx->subgrp_id && (ctx->subgrp_id != subgrp->id))
+      continue;
     vty_out (vty, "%s", VTY_NEWLINE);
     vty_out (vty, "  Update-subgroup %llu:%s", subgrp->id, VTY_NEWLINE);
     vty_out (vty, "    Created: %s", timestamp_string (subgrp->uptime));
@@ -1526,9 +1560,15 @@ update_group_init (struct bgp *bgp)
 }
 
 void
-update_group_show (struct bgp *bgp, afi_t afi, safi_t safi, struct vty *vty)
+update_group_show (struct bgp *bgp, afi_t afi, safi_t safi, struct vty *vty,
+		   u_int64_t subgrp_id)
 {
-  update_group_af_walk (bgp, afi, safi, update_group_show_walkcb, vty);
+  struct updwalk_context ctx;
+  memset (&ctx, 0, sizeof (ctx));
+  ctx.vty = vty;
+  ctx.subgrp_id = subgrp_id;
+
+  update_group_af_walk (bgp, afi, safi, update_group_show_walkcb, &ctx);
 }
 
 /*

bgpd/bgp_updgrp.h

@@ -355,7 +355,7 @@ struct updwalk_context
 /* bgp_updgrp.c */
 extern void update_group_init (struct bgp *);
 extern void
-update_group_show (struct bgp *bgp, afi_t afi, safi_t safi, struct vty *vty);
+update_group_show (struct bgp *bgp, afi_t afi, safi_t safi, struct vty *vty, u_int64_t subgrp_id);
 extern void update_group_show_stats (struct bgp *bgp, struct vty *vty);
 extern void update_group_adjust_peer (struct peer_af *paf);
 extern int update_group_adjust_soloness (struct peer *peer, int set);

bgpd/bgp_vty.c

@@ -10352,61 +10352,107 @@ ALIAS (show_bgp_instance_ipv6_safi_rsclient_summary,
 
 #endif /* HAVE IPV6 */
 
-DEFUN (show_ip_bgp_updgrps,
+static int bgp_show_update_groups(int afi, int safi, struct vty *vty,
-       show_ip_bgp_updgrps_cmd,
+				  u_int64_t subgrp_id)
-       "show ip bgp update-groups summary",
-       SHOW_STR
-       IP_STR
-       BGP_STR
-       "BGP update groups\n"
-       "Summary information\n")
 {
   struct bgp *bgp;
 
   bgp = bgp_get_default();
   if (bgp)
-    update_group_show(bgp, AFI_IP, SAFI_UNICAST, vty);
+    update_group_show(bgp, afi, safi, vty, subgrp_id);
   return CMD_SUCCESS;
 }
 
+DEFUN (show_ip_bgp_updgrps,
+       show_ip_bgp_updgrps_cmd,
+       "show ip bgp update-groups",
+       SHOW_STR
+       IP_STR
+       BGP_STR
+       "Detailed info about dynamic update groups\n")
+{
+  return (bgp_show_update_groups(AFI_IP, SAFI_UNICAST, vty, 0));
+}
+
 DEFUN (show_bgp_ipv6_updgrps,
        show_bgp_ipv6_updgrps_cmd,
-       "show bgp update-groups summary",
+       "show bgp update-groups",
        SHOW_STR
        BGP_STR
-       "BGP update groups\n"
+       "Detailed info about v6 dynamic update groups\n")
-       "Summary information\n")
 {
-  struct bgp *bgp;
+  return (bgp_show_update_groups(AFI_IP6, SAFI_UNICAST, vty, 0));
-
-  bgp = bgp_get_default();
-  if (bgp)
-    update_group_show(bgp, AFI_IP6, SAFI_UNICAST, vty);
-  return CMD_SUCCESS;
 }
 
 DEFUN (show_bgp_updgrps,
        show_bgp_updgrps_cmd,
-       "show bgp (ipv4|ipv6) (unicast|multicast) update-groups summary",
+       "show bgp (ipv4|ipv6) (unicast|multicast) update-groups",
        SHOW_STR
        BGP_STR
        "Address family\n"
        "Address family\n"
        "Address Family modifier\n"
        "Address Family modifier\n"
-       "BGP update groups\n"
+       "Detailed info about dynamic update groups\n")
-       "Summary information\n")
 {
-  struct bgp *bgp;
   afi_t afi;
   safi_t safi;
 
   afi = (strcmp(argv[0], "ipv4") == 0) ? AFI_IP : AFI_IP6;
   safi = (strncmp (argv[1], "m", 1) == 0) ? SAFI_MULTICAST : SAFI_UNICAST;
-  bgp = bgp_get_default();
+  return (bgp_show_update_groups(afi, safi, vty, 0));
-  if (bgp)
+}
-    update_group_show(bgp, afi, safi, vty);
+
-  return CMD_SUCCESS;
+DEFUN (show_ip_bgp_updgrps_s,
+       show_ip_bgp_updgrps_s_cmd,
+       "show ip bgp update-groups SUBGROUP-ID",
+       SHOW_STR
+       IP_STR
+       BGP_STR
+       "Detailed info about dynamic update groups\n"
+       "Specific subgroup to display detailed info for\n")
+{
+  u_int64_t subgrp_id;
+
+  VTY_GET_ULL("subgroup-id", subgrp_id, argv[0]);
+  return (bgp_show_update_groups(AFI_IP, SAFI_UNICAST, vty, subgrp_id));
+}
+
+DEFUN (show_bgp_ipv6_updgrps_s,
+       show_bgp_ipv6_updgrps_s_cmd,
+       "show bgp update-groups SUBGROUP-ID",
+       SHOW_STR
+       BGP_STR
+       "Detailed info about v6 dynamic update groups\n"
+       "Specific subgroup to display detailed info for\n")
+{
+  u_int64_t subgrp_id;
+
+  VTY_GET_ULL("subgroup-id", subgrp_id, argv[0]);
+  return(bgp_show_update_groups(AFI_IP6, SAFI_UNICAST, vty, subgrp_id));
+}
+
+DEFUN (show_bgp_updgrps_s,
+       show_bgp_updgrps_s_cmd,
+       "show bgp (ipv4|ipv6) (unicast|multicast) update-groups SUBGROUP-ID",
+       SHOW_STR
+       BGP_STR
+       "Address family\n"
+       "Address family\n"
+       "Address Family modifier\n"
+       "Address Family modifier\n"
+       "Detailed info about v6 dynamic update groups\n"
+       "Specific subgroup to display detailed info for")
+{
+  afi_t afi;
+  safi_t safi;
+  u_int64_t subgrp_id;
+
+  afi = (strcmp(argv[0], "ipv4") == 0) ? AFI_IP : AFI_IP6;
+  safi = (strncmp (argv[1], "m", 1) == 0) ? SAFI_MULTICAST : SAFI_UNICAST;
+
+  VTY_GET_ULL("subgroup-id", subgrp_id, argv[2]);
+  return(bgp_show_update_groups(afi, safi, vty, subgrp_id));
 }
 
 DEFUN (show_bgp_updgrps_stats,
@@ -10453,6 +10499,7 @@ DEFUN (show_ip_bgp_updgrps_adj,
        "Advertisement queue\n"
        "Announced routes\n"
        "Packet queue\n")
+
 {
   show_bgp_updgrps_adj_info_aux(vty, AFI_IP, SAFI_UNICAST, argv[0], 0);
   return CMD_SUCCESS;
@@ -10470,7 +10517,8 @@ DEFUN (show_bgp_updgrps_afi_adj,
        "BGP update groups\n"
        "Advertisement queue\n"
        "Announced routes\n"
-       "Packet queue\n")
+       "Packet queue\n"
+       "Specific subgroup info wanted for\n")
 {
   afi_t afi;
   safi_t safi;
@@ -10478,6 +10526,7 @@ DEFUN (show_bgp_updgrps_afi_adj,
   afi = (strcmp(argv[0], "ipv4") == 0) ? AFI_IP : AFI_IP6;
   safi = (strncmp (argv[1], "m", 1) == 0) ? SAFI_MULTICAST : SAFI_UNICAST;
   show_bgp_updgrps_adj_info_aux(vty, afi, safi, argv[2], 0);
+  return CMD_SUCCESS;
 }
 
 DEFUN (show_bgp_updgrps_adj,
@@ -10495,40 +10544,28 @@ DEFUN (show_bgp_updgrps_adj,
 }
 
 DEFUN (show_ip_bgp_updgrps_adj_s,
-       show_ip_bgp_updgrps_adj_subgroup_cmd,
+       show_ip_bgp_updgrps_adj_s_cmd,
        "show ip bgp update-groups SUBGROUP-ID (advertise-queue|advertised-routes|packet-queue)",
        SHOW_STR
        IP_STR
        BGP_STR
        "BGP update groups\n"
-       "64-bit subgroup id\n"
+       "Specific subgroup to display info for\n"
        "Advertisement queue\n"
        "Announced routes\n"
        "Packet queue\n")
+
 {
-  show_bgp_updgrps_adj_info_aux(vty, AFI_IP, SAFI_UNICAST, argv[1],
+  u_int64_t subgrp_id;
-				atoll(argv[0]));
+
+  VTY_GET_ULL("subgroup-id", subgrp_id, argv[0]);
+
+  show_bgp_updgrps_adj_info_aux(vty, AFI_IP, SAFI_UNICAST, argv[1], subgrp_id);
   return CMD_SUCCESS;
 }
 
-DEFUN (show_bgp_updgrps_adj_s,
+DEFUN (show_bgp_updgrps_afi_adj_s,
-       show_bgp_updgrps_adj_subgroup_cmd,
+       show_bgp_updgrps_afi_adj_s_cmd,
-       "show bgp update-groups SUBGROUP-ID (advertise-queue|advertised-routes|packet-queue)",
-       SHOW_STR
-       BGP_STR
-       "BGP update groups\n"
-       "64-bit subgroup id\n"
-       "Advertisement queue\n"
-       "Announced routes\n"
-       "Packet queue\n")
-{
-  show_bgp_updgrps_adj_info_aux(vty, AFI_IP6, SAFI_UNICAST, argv[1],
-				atoll(argv[0]));
-  return CMD_SUCCESS;
-}
-
-DEFUN (show_bgp_updgrps_afi_adj_subgroup,
-       show_bgp_updgrps_afi_adj_subgroup_cmd,
        "show bgp (ipv4|ipv6) (unicast|multicast) update-groups SUBGROUP-ID (advertise-queue|advertised-routes|packet-queue)",
        SHOW_STR
        BGP_STR
@@ -10537,19 +10574,44 @@ DEFUN (show_bgp_updgrps_afi_adj_subgroup,
        "Address Family modifier\n"
        "Address Family modifier\n"
        "BGP update groups\n"
-       "64-bit subgroup id\n"
+       "Specific subgroup to display info for\n"
+       "Advertisement queue\n"
+       "Announced routes\n"
+       "Packet queue\n"
+       "Specific subgroup info wanted for\n")
+{
+  afi_t afi;
+  safi_t safi;
+  u_int64_t subgrp_id;
+
+  afi = (strcmp(argv[0], "ipv4") == 0) ? AFI_IP : AFI_IP6;
+  safi = (strncmp (argv[1], "m", 1) == 0) ? SAFI_MULTICAST : SAFI_UNICAST;
+  VTY_GET_ULL("subgroup-id", subgrp_id, argv[2]);
+
+  show_bgp_updgrps_adj_info_aux(vty, afi, safi, argv[3], subgrp_id);
+  return CMD_SUCCESS;
+}
+
+DEFUN (show_bgp_updgrps_adj_s,
+       show_bgp_updgrps_adj_s_cmd,
+       "show bgp update-groups SUBGROUP-ID (advertise-queue|advertised-routes|packet-queue)",
+       SHOW_STR
+       BGP_STR
+       "BGP update groups\n"
+       "Specific subgroup to display info for\n"
        "Advertisement queue\n"
        "Announced routes\n"
        "Packet queue\n")
 {
-  afi_t afi;
+  u_int64_t subgrp_id;
-  safi_t safi;
 
-  afi = (strcmp(argv[0], "ipv4") == 0) ? AFI_IP : AFI_IP6;
+  VTY_GET_ULL("subgroup-id", subgrp_id, argv[0]);
-  safi = (strncmp (argv[1], "m", 1) == 0) ? SAFI_MULTICAST : SAFI_UNICAST;
+
-  show_bgp_updgrps_adj_info_aux(vty, afi, safi, argv[3], atoll(argv[2]));
+  show_bgp_updgrps_adj_info_aux(vty, AFI_IP6, SAFI_UNICAST, argv[1], subgrp_id);
+  return CMD_SUCCESS;
 }
 
+
 static int
 bgp_show_one_peer_group (struct vty *vty, struct peer_group *group)
 {
@@ -12468,12 +12530,15 @@ bgp_vty_init (void)
   install_element (VIEW_NODE, &show_ip_bgp_updgrps_cmd);
   install_element (VIEW_NODE, &show_bgp_updgrps_cmd);
   install_element (VIEW_NODE, &show_bgp_ipv6_updgrps_cmd);
+  install_element (VIEW_NODE, &show_ip_bgp_updgrps_s_cmd);
+  install_element (VIEW_NODE, &show_bgp_updgrps_s_cmd);
+  install_element (VIEW_NODE, &show_bgp_ipv6_updgrps_s_cmd);
   install_element (VIEW_NODE, &show_ip_bgp_updgrps_adj_cmd);
   install_element (VIEW_NODE, &show_bgp_updgrps_adj_cmd);
   install_element (VIEW_NODE, &show_bgp_updgrps_afi_adj_cmd);
-  install_element (VIEW_NODE, &show_ip_bgp_updgrps_adj_subgroup_cmd);
+  install_element (VIEW_NODE, &show_ip_bgp_updgrps_adj_s_cmd);
-  install_element (VIEW_NODE, &show_bgp_updgrps_adj_subgroup_cmd);
+  install_element (VIEW_NODE, &show_bgp_updgrps_adj_s_cmd);
-  install_element (VIEW_NODE, &show_bgp_updgrps_afi_adj_subgroup_cmd);
+  install_element (VIEW_NODE, &show_bgp_updgrps_afi_adj_s_cmd);
   install_element (VIEW_NODE, &show_ip_bgp_instance_summary_cmd);
   install_element (VIEW_NODE, &show_ip_bgp_ipv4_summary_cmd);
   install_element (VIEW_NODE, &show_bgp_ipv4_safi_summary_cmd);
@@ -12493,12 +12558,15 @@ bgp_vty_init (void)
   install_element (RESTRICTED_NODE, &show_ip_bgp_updgrps_cmd);
   install_element (RESTRICTED_NODE, &show_bgp_updgrps_cmd);
   install_element (RESTRICTED_NODE, &show_bgp_ipv6_updgrps_cmd);
+  install_element (RESTRICTED_NODE, &show_ip_bgp_updgrps_s_cmd);
+  install_element (RESTRICTED_NODE, &show_bgp_updgrps_s_cmd);
+  install_element (RESTRICTED_NODE, &show_bgp_ipv6_updgrps_s_cmd);
   install_element (RESTRICTED_NODE, &show_ip_bgp_updgrps_adj_cmd);
   install_element (RESTRICTED_NODE, &show_bgp_updgrps_adj_cmd);
   install_element (RESTRICTED_NODE, &show_bgp_updgrps_afi_adj_cmd);
-  install_element (RESTRICTED_NODE, &show_ip_bgp_updgrps_adj_subgroup_cmd);
+  install_element (RESTRICTED_NODE, &show_ip_bgp_updgrps_adj_s_cmd);
-  install_element (RESTRICTED_NODE, &show_bgp_updgrps_adj_subgroup_cmd);
+  install_element (RESTRICTED_NODE, &show_bgp_updgrps_adj_s_cmd);
-  install_element (RESTRICTED_NODE, &show_bgp_updgrps_afi_adj_subgroup_cmd);
+  install_element (RESTRICTED_NODE, &show_bgp_updgrps_afi_adj_s_cmd);
   install_element (RESTRICTED_NODE, &show_ip_bgp_instance_summary_cmd);
   install_element (RESTRICTED_NODE, &show_ip_bgp_ipv4_summary_cmd);
   install_element (RESTRICTED_NODE, &show_bgp_ipv4_safi_summary_cmd);
@@ -12518,13 +12586,15 @@ bgp_vty_init (void)
   install_element (ENABLE_NODE, &show_ip_bgp_updgrps_cmd);
   install_element (ENABLE_NODE, &show_bgp_updgrps_cmd);
   install_element (ENABLE_NODE, &show_bgp_ipv6_updgrps_cmd);
-  install_element (ENABLE_NODE, &show_bgp_updgrps_stats_cmd);
+  install_element (ENABLE_NODE, &show_ip_bgp_updgrps_s_cmd);
+  install_element (ENABLE_NODE, &show_bgp_updgrps_s_cmd);
+  install_element (ENABLE_NODE, &show_bgp_ipv6_updgrps_s_cmd);
   install_element (ENABLE_NODE, &show_ip_bgp_updgrps_adj_cmd);
   install_element (ENABLE_NODE, &show_bgp_updgrps_adj_cmd);
   install_element (ENABLE_NODE, &show_bgp_updgrps_afi_adj_cmd);
-  install_element (ENABLE_NODE, &show_ip_bgp_updgrps_adj_subgroup_cmd);
+  install_element (ENABLE_NODE, &show_ip_bgp_updgrps_adj_s_cmd);
-  install_element (ENABLE_NODE, &show_bgp_updgrps_adj_subgroup_cmd);
+  install_element (ENABLE_NODE, &show_bgp_updgrps_adj_s_cmd);
-  install_element (ENABLE_NODE, &show_bgp_updgrps_afi_adj_subgroup_cmd);
+  install_element (ENABLE_NODE, &show_bgp_updgrps_afi_adj_s_cmd);
   install_element (ENABLE_NODE, &show_ip_bgp_instance_summary_cmd);
   install_element (ENABLE_NODE, &show_ip_bgp_ipv4_summary_cmd);
   install_element (ENABLE_NODE, &show_bgp_ipv4_safi_summary_cmd);

lib/vty.h

@@ -162,6 +162,19 @@ do { \
     } \
 } while (0)
 
+/* Utility macros to convert VTY argument to unsigned long long */
+#define VTY_GET_ULL(NAME,V,STR) \
+do { \
+  char *endptr = NULL; \
+  errno = 0; \
+  (V) = strtoull ((STR), &endptr, 10); \
+  if (*(STR) == '-' || *endptr != '\0' || errno) \
+    { \
+      vty_out (vty, "%% Invalid %s value%s", NAME, VTY_NEWLINE); \
+      return CMD_WARNING; \
+    } \
+} while (0)
+
 /*
  * The logic below ((TMPL) <= ((MIN) && (TMPL) != (MIN)) is
  * done to circumvent the compiler complaining about