From 1d21789e169e903b0de56ddec110e573728f7139 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Thu, 6 Dec 2018 21:31:05 +0000 Subject: [PATCH] vrrpd: clean up configuration code, fix skew bug * 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 --- vrrpd/vrrp.c | 83 ++++++++++++++++++++++++++++++++----- vrrpd/vrrp.h | 106 ++++++++++++++++++++++++++++++----------------- vrrpd/vrrp_vty.c | 96 +++++++++++++++++++++++++++++++++--------- 3 files changed, 217 insertions(+), 68 deletions(-) diff --git a/vrrpd/vrrp.c b/vrrpd/vrrp.c index fd0c62cb0d..4d68d617fc 100644 --- a/vrrpd/vrrp.c +++ b/vrrpd/vrrp.c @@ -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; } diff --git a/vrrpd/vrrp.h b/vrrpd/vrrp.h index 240144d3b3..1564cced59 100644 --- a/vrrpd/vrrp.h +++ b/vrrpd/vrrp.h @@ -1,5 +1,5 @@ /* - * VRRPD global definitions + * VRRPD global definitions and state machine * Copyright (C) 2018 Cumulus Networks, Inc. * Quentin Young * @@ -21,11 +21,13 @@ #define _VRRP_H #include -#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 */ diff --git a/vrrpd/vrrp_vty.c b/vrrpd/vrrp_vty.c index 6c7fc5f194..9fda418f75 100644 --- a/vrrpd/vrrp_vty.c +++ b/vrrpd/vrrp_vty.c @@ -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); } -- 2.39.5