vtysh: fix exit-vrf printing

Resolves issue with exit-vrf being placed at the end of zebra's portion
of a vrf block, but before other daemons' portions of the same config
block.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

diff --git a/pimd/pim_instance.c b/pimd/pim_instance.c
index 2550651464321d2cf75fbb73d276d5f841e0ab02..b0d7a7b2db07a50b0a1afcc36237de25f17c2511 100644 (file)
--- a/pimd/pim_instance.c
+++ b/pimd/pim_instance.c
@@ -188,7 +188,7 @@ static int pim_vrf_config_write(struct vty *vty)
                pim_global_config_write_worker(pim, vty);
 
                if (vrf->vrf_id != VRF_DEFAULT)
-                       vty_endframe(vty, "!\n");
+                       vty_endframe(vty, " exit-vrf\n!\n");
        }
 
        return 0;

diff --git a/staticd/static_vrf.c b/staticd/static_vrf.c
index ad143209ee0bb70c72147db96ca756fdb738cd3e..9dd25fbdd1316f19685e9f6a35a7cf66006514fc 100644 (file)
--- a/staticd/static_vrf.c
+++ b/staticd/static_vrf.c
@@ -164,7 +164,7 @@ static int static_vrf_config_write(struct vty *vty)
                              SAFI_UNICAST, "ipv6 route");
 
                if (vrf->vrf_id != VRF_DEFAULT)
-                       vty_endframe(vty, "!\n");
+                       vty_endframe(vty, " exit-vrf\n!\n");
        }
 
        return 0;

diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c
index 7a7744f7c05262fbfa0cabc0669e09fa9a6f27b2..541eafcf7fae11d88c1ad960358e87ab00b484a5 100644 (file)
--- a/vtysh/vtysh_config.c
+++ b/vtysh/vtysh_config.c
@@ -132,16 +132,83 @@ static void config_add_line_uniq(struct list *config, const char *line)
 }
 
 /*
- * I want to explicitly move this command to the end of the line
+ * Add a line that should only be shown once, and always show at the end of the
+ * config block.
+ *
+ * If the line already exists, it will be moved to the end of the block. If it
+ * does not exist, it will be added at the end of the block.
+ *
+ * Note that this only makes sense when there is just one such line that should
+ * show up at the very end of a config block. Furthermore, if the same block
+ * can show up from multiple daemons, all of them must make sure to print the
+ * line at the end of their config, otherwise the line will show at the end of
+ * the config for the last daemon that printed it.
+ *
+ * Here is a motivating example with the 'exit-vrf' command. Suppose we receive
+ * a config from Zebra like so:
+ *
+ * vrf BLUE
+ *    ip route A
+ *    ip route B
+ *    exit-vrf
+ *
+ * Then suppose we later receive this config from PIM:
+ *
+ * vrf BLUE
+ *    ip msdp mesh-group MyGroup member 1.2.3.4
+ *    exit-vrf
+ *
+ * Then we will combine them into one config block like so:
+ *
+ * vrf BLUE
+ *    ip route A
+ *    ip route B
+ *    ip msdp mesh-group MyGroup member 1.2.3.4
+ *    exit-vrf
+ *
+ * Because PIM also sent us an 'exit-vrf', we noticed that we already had one
+ * under the 'vrf BLUE' config block and so we moved it to the end of the
+ * config block again. If PIM had neglected to send us 'exit-vrf', the result
+ * would be this:
+ *
+ * vrf BLUE
+ *    ip route A
+ *    ip route B
+ *    exit-vrf
+ *    ip msdp mesh-group MyGroup member 1.2.3.4
+ *
+ * Therefore, daemons that share config blocks must take care to consistently
+ * print the same block terminators.
+ *
+ * Ideally this would be solved by adding a string to struct config that is
+ * always printed at the end when dumping a config. However, this would only
+ * work when the user is using integrated config. In the non-integrated config
+ * case, daemons are responsible for writing their own config files, and so the
+ * must be able to print these blocks correctly independently of vtysh, which
+ * means they are the ones that need to handle printing the block terminators
+ * and VTYSH needs to be smart enough to combine them properly.
+ *
+ * ---
+ *
+ * config
+ *    The config to add the line to
+ *
+ * line
+ *    The line to add to the end of the config
  */
-static void config_add_line_end(struct list *config, const char *line)
+static void config_add_line_uniq_end(struct list *config, const char *line)
 {
        struct listnode *node;
-       void *item = XSTRDUP(MTYPE_VTYSH_CONFIG_LINE, line);
+       char *pnt;
 
-       listnode_add(config, item);
-       node = listnode_lookup(config, item);
-       if (node)
+       for (ALL_LIST_ELEMENTS_RO(config, node, pnt)) {
+               if (strcmp(pnt, line) == 0)
+                       break;
+       }
+
+       if (!node)
+               config_add_line(config, line);
+       else
                listnode_move_to_tail(config, node);
 }
 
@@ -158,8 +225,6 @@ void vtysh_config_parse_line(void *arg, const char *line)
        if (c == '\0')
                return;
 
-       /* printf ("[%s]\n", line); */
-
        switch (c) {
        /* Suppress exclamation points ! and commented lines. The !s are
         * generated
@@ -178,10 +243,10 @@ void vtysh_config_parse_line(void *arg, const char *line)
                        } else if (strncmp(line, " ip multicast boundary",
                                           strlen(" ip multicast boundary"))
                                   == 0) {
-                               config_add_line_end(config->line, line);
+                               config_add_line_uniq_end(config->line, line);
                        } else if (strncmp(line, " ip igmp query-interval",
                                           strlen(" ip igmp query-interval")) == 0) {
-                               config_add_line_end(config->line, line);
+                               config_add_line_uniq_end(config->line, line);
                        } else if (config->index == LINK_PARAMS_NODE
                                   && strncmp(line, "  exit-link-params",
                                              strlen("  exit"))
@@ -192,8 +257,7 @@ void vtysh_config_parse_line(void *arg, const char *line)
                                   && strncmp(line, " exit-vrf",
                                              strlen(" exit-vrf"))
                                              == 0) {
-                               config_add_line(config->line, line);
-                               config->index = CONFIG_NODE;
+                               config_add_line_uniq_end(config->line, line);
                        } else if (config->index == RMAP_NODE
                                   || config->index == INTERFACE_NODE
                                   || config->index == LOGICALROUTER_NODE