]> git.puffer.fish Git - matthieu/frr.git/commitdiff
vrrpd: clean up configuration code, fix skew bug
authorQuentin Young <qlyoung@cumulusnetworks.com>
Thu, 6 Dec 2018 21:31:05 +0000 (21:31 +0000)
committerQuentin Young <qlyoung@cumulusnetworks.com>
Fri, 17 May 2019 00:27:08 +0000 (00:27 +0000)
* Update vrrp.[ch] file header to be more accurate
* Make vrrp_update_times() private again
* Add times reset function and use it
* Add priority and advertisement interval setter functions and use them
* Add command to change advertisement interval
* Allow showing all VRRP instances
* Improve doc comments on functions
* Add ability to shutdown router
* Reorganize vrrp.h
* Add doc comments to vrrp.h

* Fix bug where Skew_time was not used to compute Master_Down_Interval

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
vrrpd/vrrp.c
vrrpd/vrrp.h
vrrpd/vrrp_vty.c

index fd0c62cb0d7b203f515319b9067d6df84714a4d2..4d68d617fca75308a05a7203f53fe3e450b85fbd 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * VRRPD global definitions
+ * VRRPD global definitions and state machine
  * Copyright (C) 2018 Cumulus Networks, Inc.
  *               Quentin Young
  *
@@ -58,18 +58,42 @@ static void vrrp_mac_set(struct ethaddr *mac, bool v6, uint8_t vrid)
        mac->octet[5] = vrid;
 }
 
-void vrrp_update_times(struct vrrp_vrouter *vr, uint16_t advertisement_interval,
-                      uint16_t master_adver_interval)
+/*
+ * Sets advertisement_interval and master_adver_interval on a Virtual Router,
+ * then recalculates and sets skew_time and master_down_interval based on these
+ * values.
+ *
+ * vr
+ *    Virtual Router to operate on
+ *
+ * advertisement_interval
+ *    Advertisement_Interval to set
+ *
+ * master_adver_interval
+ *    Master_Adver_Interval to set
+ */
+static void vrrp_update_times(struct vrrp_vrouter *vr,
+                             uint16_t advertisement_interval,
+                             uint16_t master_adver_interval)
 {
        vr->advertisement_interval = advertisement_interval;
        vr->master_adver_interval = master_adver_interval;
        vr->skew_time = (256 - vr->priority) * vr->master_adver_interval;
        vr->skew_time /= 256;
        vr->master_down_interval = (3 * vr->master_adver_interval);
-       vr->master_down_interval /= 256;
+       vr->master_down_interval /= 256 + vr->skew_time;
 }
 
-void vrrp_update_priority(struct vrrp_vrouter *vr, uint8_t priority)
+/*
+ */
+static void vrrp_reset_times(struct vrrp_vrouter *vr)
+{
+       vrrp_update_times(vr, vr->advertisement_interval, 0);
+}
+
+/* Configuration controllers ----------------------------------------------- */
+
+void vrrp_set_priority(struct vrrp_vrouter *vr, uint8_t priority)
 {
        if (vr->priority == priority)
                return;
@@ -80,6 +104,15 @@ void vrrp_update_priority(struct vrrp_vrouter *vr, uint8_t priority)
                          vr->master_adver_interval);
 }
 
+void vrrp_set_advertisement_interval(struct vrrp_vrouter *vr,
+                                    uint16_t advertisement_interval)
+{
+       if (vr->advertisement_interval == advertisement_interval)
+               return;
+
+       vrrp_update_times(vr, advertisement_interval, vr->master_adver_interval);
+}
+
 void vrrp_add_ip(struct vrrp_vrouter *vr, struct in_addr v4)
 {
        struct in_addr *v4_ins = XCALLOC(MTYPE_TMP, sizeof(struct in_addr));
@@ -88,6 +121,9 @@ void vrrp_add_ip(struct vrrp_vrouter *vr, struct in_addr v4)
        listnode_add(vr->v4, v4_ins);
 }
 
+
+/* Creation and destruction ------------------------------------------------ */
+
 struct vrrp_vrouter *vrrp_vrouter_create(struct interface *ifp, uint8_t vrid)
 {
        struct vrrp_vrouter *vr =
@@ -100,15 +136,12 @@ struct vrrp_vrouter *vrrp_vrouter_create(struct interface *ifp, uint8_t vrid)
        vr->v6 = list_new();
        vr->is_master = false;
        vr->priority = VRRP_DEFAULT_PRIORITY;
-       vr->advertisement_interval = VRRP_DEFAULT_ADVINT;
-       vr->master_adver_interval = 0;
-       vr->skew_time = 0;
-       vr->master_down_interval = 0;
        vr->preempt_mode = true;
        vr->accept_mode = false;
        vrrp_mac_set(&vr->vr_mac_v4, false, vrid);
        vrrp_mac_set(&vr->vr_mac_v6, true, vrid);
        vr->fsm.state = VRRP_STATE_INITIALIZE;
+       vrrp_reset_times(vr);
 
        hash_get(vrrp_vrouters_hash, vr, hash_alloc_intern);
 
@@ -236,6 +269,8 @@ static void vrrp_change_state_backup(struct vrrp_vrouter *vr)
  */
 static void vrrp_change_state_initialize(struct vrrp_vrouter *vr)
 {
+       /* Reset timers */
+       vrrp_reset_times(vr);
 }
 
 void (*vrrp_change_state_handlers[])(struct vrrp_vrouter *vr) = {
@@ -294,13 +329,25 @@ static int vrrp_master_down_timer_expire(struct thread *thread)
  * Event handler for Startup event.
  *
  * Creates sockets, sends advertisements and ARP requests, starts timers,
- * updates state machine.
+ * and transitions the Virtual Router to either Master or Backup states.
+ *
+ * This function will also initialize the program's global ARP subsystem if it
+ * has not yet been initialized.
  *
  * vr
  *    Virtual Router on which to apply Startup event
+ *
+ * Returns:
+ *    < 0 if the session socket could not be created, or the state is not
+ *        Initialize
+ *      0 on success
  */
 static int vrrp_startup(struct vrrp_vrouter *vr)
 {
+       /* May only be called when the state is Initialize */
+       if (vr->fsm.state != VRRP_STATE_INITIALIZE)
+               return -1;
+
        /* Initialize global gratuitous ARP socket if necessary */
        if (!vrrp_garp_is_init())
                vrrp_garp_init();
@@ -335,9 +382,23 @@ static int vrrp_startup(struct vrrp_vrouter *vr)
        return 0;
 }
 
+/*
+ * Shuts down a Virtual Router and transitions it to Initialize.
+ *
+ * This call must be idempotent; it is safe to call multiple times on the same
+ * Virtual Router.
+ */
 static int vrrp_shutdown(struct vrrp_vrouter *vr)
 {
-       /* NOTHING */
+       /* close socket */
+       if (vr->sock >= 0)
+               close(vr->sock);
+
+       /* cancel all threads */
+       /* ... */
+
+       vrrp_change_state(vr, VRRP_STATE_INITIALIZE);
+
        return 0;
 }
 
index 240144d3b39eb155eb03fc99d56cee5110074545..1564cced59419479ee1d2337de0a4cb350881aa5 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * VRRPD global definitions
+ * VRRPD global definitions and state machine
  * Copyright (C) 2018 Cumulus Networks, Inc.
  *               Quentin Young
  *
 #define _VRRP_H
 
 #include <zebra.h>
-#include "linklist.h"
-#include "hash.h"
-#include "if.h"
-#include "thread.h"
-#include "hook.h"
+
+#include "lib/hash.h"
+#include "lib/hook.h"
+#include "lib/if.h"
+#include "lib/linklist.h"
+#include "lib/privs.h"
+#include "lib/thread.h"
 
 /* Global definitions */
 #define VRRP_DEFAULT_ADVINT 100
@@ -127,24 +129,22 @@ struct vrrp_vrouter {
        } fsm;
 };
 
-/* State machine */
-#define VRRP_STATE_INITIALIZE 1
-#define VRRP_STATE_MASTER 2
-#define VRRP_STATE_BACKUP 3
-#define VRRP_EVENT_STARTUP 1
-#define VRRP_EVENT_SHUTDOWN 2
-
-DECLARE_HOOK(vrrp_change_state_hook, (struct vrrp_vrouter *vr, int to), (vr, to));
-/* End state machine */
-
-
 /*
  * Initialize VRRP global datastructures.
  */
 void vrrp_init(void);
 
+
+/* Creation and destruction ------------------------------------------------ */
+
 /*
  * Create and register a new VRRP Virtual Router.
+ *
+ * ifp
+ *    Base interface to configure VRRP on
+ *
+ * vrid
+ *    Virtual Router Identifier
  */
 struct vrrp_vrouter *vrrp_vrouter_create(struct interface *ifp, uint8_t vrid);
 
@@ -153,38 +153,36 @@ struct vrrp_vrouter *vrrp_vrouter_create(struct interface *ifp, uint8_t vrid);
  */
 void vrrp_vrouter_destroy(struct vrrp_vrouter *vr);
 
+
+/* Configuration controllers ----------------------------------------------- */
+
 /*
- * Sets advertisement_interval and master_adver_interval on a Virtual Router,
- * then recalculates and sets skew_time and master_down_interval based on these
- * values.
+ * Change the priority of a VRRP Virtual Router.
  *
- * vr
- *    Virtual Router to operate on
+ * Also recalculates timers using new priority.
  *
- * advertisement_interval
- *    Advertisement_Interval to set
+ * vr
+ *    Virtual Router to change priority of
  *
- * master_adver_interval
- *    Master_Adver_Interval to set
+ * priority
+ *    New priority
  */
-void vrrp_update_times(struct vrrp_vrouter *vr, uint16_t advertisement_interval,
-                      uint16_t master_adver_interval);
+void vrrp_set_priority(struct vrrp_vrouter *vr, uint8_t priority);
 
 /*
- * Change the priority of a VRRP Virtual Router.
- *
- * Recalculates timers using new priority.
+ * Set Advertisement Interval on this Virtual Router.
  *
  * vr
  *    Virtual Router to change priority of
  *
- * priority
- *    New priority
+ * advertisement_interval
+ *    New advertisement interval
  */
-void vrrp_update_priority(struct vrrp_vrouter *vr, uint8_t priority);
+void vrrp_set_advertisement_interval(struct vrrp_vrouter *vr,
+                                    uint16_t advertisement_interval);
 
 /*
- * Add IPv4 address to a VRRP Virtual Router
+ * Add IPv4 address to a VRRP Virtual Router.
  *
  * vr
  *    Virtual Router to add IPv4 address to
@@ -194,14 +192,46 @@ void vrrp_update_priority(struct vrrp_vrouter *vr, uint8_t priority);
  */
 void vrrp_add_ip(struct vrrp_vrouter *vr, struct in_addr v4);
 
+
+/* State machine ----------------------------------------------------------- */
+
+#define VRRP_STATE_INITIALIZE 1
+#define VRRP_STATE_MASTER 2
+#define VRRP_STATE_BACKUP 3
+#define VRRP_EVENT_STARTUP 1
+#define VRRP_EVENT_SHUTDOWN 2
+
 /*
- * Find VRRP Virtual Router by Virtual Router ID
+ * This hook called whenever the state of a Virtual Router changes, after the
+ * specific internal state handlers have run.
+ *
+ * Use this if you need to react to state changes to perform non-critical
+ * tasks. Critical tasks should go in the internal state change handlers.
  */
-struct vrrp_vrouter *vrrp_lookup(uint8_t vrid);
+DECLARE_HOOK(vrrp_change_state_hook, (struct vrrp_vrouter *vr, int to), (vr, to));
 
 /*
- * Trigger VRRP event
+ * Trigger a VRRP event on a given Virtual Router..
+ *
+ * vr
+ *    Virtual Router to operate on
+ *
+ * event
+ *    Event to kick off. All event related processing will have completed upon
+ *    return of this function.
+ *
+ * Returns:
+ *    < 0 if the event created an error
+ *      0 otherwise
  */
 int vrrp_event(struct vrrp_vrouter *vr, int event);
 
+
+/* Other ------------------------------------------------------------------- */
+
+/*
+ * Find VRRP Virtual Router by Virtual Router ID
+ */
+struct vrrp_vrouter *vrrp_lookup(uint8_t vrid);
+
 #endif /* _VRRP_H */
index 6c7fc5f1947962377ce557b165a9744f97de9bf7..9fda418f75ee1f0880142ea690d39679dbac12aa 100644 (file)
@@ -70,29 +70,60 @@ DEFPY(vrrp_vrid,
 {
        VTY_DECLVAR_CONTEXT(interface, ifp);
 
-       struct vrrp_vrouter *vr = vrrp_vrouter_create(ifp, vrid);
-       int ret = vrrp_event(vr, VRRP_EVENT_STARTUP);
-       if (ret < 0) {
-               vty_out(vty, "%% Failed to start VRRP instance\n");
-               return CMD_WARNING_CONFIG_FAILED;
-       }
+       vrrp_vrouter_create(ifp, vrid);
 
        return CMD_SUCCESS;
 }
 
 DEFPY(vrrp_priority,
       vrrp_priority_cmd,
-      "[no] vrrp (1-255)$vrid priority (1-254)",
+      "[no] vrrp (1-255)$vrid priority (1-255)",
       NO_STR
       VRRP_STR
       VRRP_VRID_STR
       VRRP_PRIORITY_STR
-      "Priority value\n")
+      "Priority value; set 255 to designate this Virtual Router as Master\n")
 {
        struct vrrp_vrouter *vr;
+       bool need_restart = false;
+       int ret = CMD_SUCCESS;
 
        VROUTER_GET_VTY(vty, vrid, vr);
-       vrrp_update_priority(vr, priority);
+
+       need_restart = (vr->fsm.state != VRRP_STATE_INITIALIZE);
+
+       if (need_restart) {
+               vty_out(vty,
+                       "%% WARNING: Restarting Virtual Router %ld to update priority\n",
+                       vrid);
+               (void) vrrp_event(vr, VRRP_EVENT_SHUTDOWN);
+       }
+
+       vrrp_set_priority(vr, priority);
+
+       if (need_restart) {
+               ret = vrrp_event(vr, VRRP_EVENT_STARTUP);
+               if (ret < 0)
+                       vty_out(vty, "%% Failed to start Virtual Router %ld\n",
+                               vrid);
+       }
+
+       return CMD_SUCCESS;
+}
+
+DEFPY(vrrp_advertisement_interval,
+      vrrp_advertisement_interval_cmd,
+      "[no] vrrp (1-255)$vrid advertisement-interval (1-4096)",
+      NO_STR
+      VRRP_STR
+      VRRP_VRID_STR
+      VRRP_PRIORITY_STR
+      "Priority value; set 255 to designate this Virtual Router as Master\n")
+{
+       struct vrrp_vrouter *vr;
+
+       VROUTER_GET_VTY(vty, vrid, vr);
+       vrrp_set_advertisement_interval(vr, advertisement_interval);
 
        return CMD_SUCCESS;
 }
@@ -107,27 +138,32 @@ DEFPY(vrrp_ip,
       VRRP_IP_STR)
 {
        struct vrrp_vrouter *vr;
+       int ret;
 
        VROUTER_GET_VTY(vty, vrid, vr);
        vrrp_add_ip(vr, ip);
 
-       return CMD_SUCCESS;
+       if (vr->fsm.state == VRRP_STATE_INITIALIZE) {
+               vty_out(vty, "%% Activating Virtual Router %ld\n", vrid);
+               ret = vrrp_event(vr, VRRP_EVENT_STARTUP);
+               ret = ret < 0 ? CMD_WARNING_CONFIG_FAILED : CMD_SUCCESS;
+
+               if (ret == CMD_WARNING_CONFIG_FAILED)
+                       vty_out(vty, "%% Failed to start Virtual Router %ld\n",
+                               vrid);
+       } else {
+               ret = CMD_SUCCESS;
+       }
+
+       return ret;
 }
 
-DEFPY(vrrp_vrid_show,
-      vrrp_vrid_show_cmd,
-      "show vrrp [(1-255)$vrid]",
-      SHOW_STR
-      VRRP_STR
-      VRRP_VRID_STR)
+static void vrrp_show(struct vty *vty, struct vrrp_vrouter *vr)
 {
-       struct vrrp_vrouter *vr;
        char ethstr[ETHER_ADDR_STRLEN];
        char ipstr[INET_ADDRSTRLEN];
        const char *stastr;
 
-       VROUTER_GET_VTY(vty, vrid, vr);
-
        switch (vr->fsm.state) {
        case VRRP_STATE_INITIALIZE:
                stastr = "Initialize";
@@ -177,6 +213,27 @@ DEFPY(vrrp_vrid_show,
                }
                vty_out(vty, "\n");
        }
+}
+
+DEFPY(vrrp_vrid_show,
+      vrrp_vrid_show_cmd,
+      "show vrrp [(1-255)$vrid]",
+      SHOW_STR
+      VRRP_STR
+      VRRP_VRID_STR)
+{
+       struct vrrp_vrouter *vr;
+
+       if (vrid) {
+               VROUTER_GET_VTY(vty, vrid, vr);
+               vrrp_show(vty, vr);
+       } else {
+               struct list *ll = hash_to_list(vrrp_vrouters_hash);
+               struct listnode *ln;
+
+               for (ALL_LIST_ELEMENTS_RO(ll, ln, vr))
+                       vrrp_show(vty, vr);
+       }
 
        return CMD_SUCCESS;
 }
@@ -194,5 +251,6 @@ void vrrp_vty_init(void)
        install_element(VIEW_NODE, &vrrp_vrid_show_cmd);
        install_element(INTERFACE_NODE, &vrrp_vrid_cmd);
        install_element(INTERFACE_NODE, &vrrp_priority_cmd);
+       install_element(INTERFACE_NODE, &vrrp_advertisement_interval_cmd);
        install_element(INTERFACE_NODE, &vrrp_ip_cmd);
 }