vrrpd: handle address deletion, don't accept dupes

* Do nothing if user tries to add the same IP twice
* Implement deletion of IPs
* Deactivate virtual router if all IPs are deleted
* Deduplicate add / remove code

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

diff --git a/vrrpd/vrrp.c b/vrrpd/vrrp.c
index 682c85adb5b8e8829c6917877c7dab0a33d08b11..cc93b94a9b1f2959694a061b2dcfba22d2406f32 100644 (file)
--- a/vrrpd/vrrp.c
+++ b/vrrpd/vrrp.c
@@ -143,57 +143,118 @@ void vrrp_set_advertisement_interval(struct vrrp_vrouter *vr,
        vrrp_recalculate_timers(vr->v6);
 }
 
-void vrrp_add_ipv4(struct vrrp_vrouter *vr, struct in_addr v4)
+static bool vrrp_has_ip(struct vrrp_vrouter *vr, struct ipaddr *ip)
 {
-       struct ipaddr *v4_ins = XCALLOC(MTYPE_TMP, sizeof(struct ipaddr));
+       size_t cmpsz = ip->ipa_type == IPADDR_V4 ? sizeof(struct in_addr)
+                                                : sizeof(struct in6_addr);
+       struct vrrp_router *r = ip->ipa_type == IPADDR_V4 ? vr->v4 : vr->v6;
+       struct listnode *ln;
+       struct ipaddr *iter;
+
+       for (ALL_LIST_ELEMENTS_RO(r->addrs, ln, iter))
+               if (!memcmp(&iter->ip, &ip->ip, cmpsz))
+                       return true;
 
-       v4_ins->ipa_type = IPADDR_V4;
-       v4_ins->ipaddr_v4 = v4;
+       return false;
+}
 
-       if (!vrrp_is_owner(vr->ifp, v4_ins) && vr->v4->is_owner) {
+int vrrp_add_ip(struct vrrp_router *r, struct ipaddr *ip, bool activate)
+{
+       if (vrrp_has_ip(r->vr, ip))
+               return 0;
+
+       if (!vrrp_is_owner(r->vr->ifp, ip) && r->is_owner) {
                char ipbuf[INET6_ADDRSTRLEN];
-               ipaddr2str(v4_ins, ipbuf, sizeof(ipbuf));
+               inet_ntop(r->family, &ip->ip, ipbuf, sizeof(ipbuf));
                zlog_err(
                        VRRP_LOGPFX VRRP_LOGPFX_VRID
                        "This VRRP router is not the address owner of %s, but is the address owner of other addresses; this config is unsupported.",
-                       vr->vrid, ipbuf);
-               /* FIXME: indicate failure with rc */
-               return;
+                       r->vr->vrid, ipbuf);
+               return -1;
+       }
+
+       struct ipaddr *new = XCALLOC(MTYPE_TMP, sizeof(struct ipaddr));
+
+       *new = *ip;
+       listnode_add(r->addrs, new);
+
+       bool do_activate = (activate && r->fsm.state == VRRP_STATE_INITIALIZE);
+       int ret = 0;
+
+       if (do_activate)
+               ret = vrrp_event(r, VRRP_EVENT_STARTUP);
+       else if (r->fsm.state == VRRP_STATE_MASTER) {
+               switch (r->family) {
+               case AF_INET:
+                       vrrp_garp_send(r, &new->ipaddr_v4);
+                       break;
+               case AF_INET6:
+                       vrrp_ndisc_una_send(r, new);
+                       break;
+               }
        }
 
-       listnode_add(vr->v4->addrs, v4_ins);
+       return ret;
 }
 
-void vrrp_add_ipv6(struct vrrp_vrouter *vr, struct in6_addr v6)
+int vrrp_add_ipv4(struct vrrp_vrouter *vr, struct in_addr v4, bool activate)
 {
-       struct ipaddr *v6_ins = XCALLOC(MTYPE_TMP, sizeof(struct ipaddr));
+       struct ipaddr ip;
+       ip.ipa_type = IPADDR_V4;
+       ip.ipaddr_v4 = v4;
+       return vrrp_add_ip(vr->v4, &ip, activate);
+}
 
-       v6_ins->ipa_type = IPADDR_V6;
-       memcpy(&v6_ins->ipaddr_v6, &v6, sizeof(struct in6_addr));
+int vrrp_add_ipv6(struct vrrp_vrouter *vr, struct in6_addr v6, bool activate)
+{
+       struct ipaddr ip;
+       ip.ipa_type = IPADDR_V6;
+       ip.ipaddr_v6 = v6;
+       return vrrp_add_ip(vr->v6, &ip, activate);
+}
 
-       if (!vrrp_is_owner(vr->ifp, v6_ins) && vr->v6->is_owner) {
-               char ipbuf[INET6_ADDRSTRLEN];
-               ipaddr2str(v6_ins, ipbuf, sizeof(ipbuf));
-               zlog_err(
-                       VRRP_LOGPFX VRRP_LOGPFX_VRID
-                       "This VRRP router is not the address owner of %s, but is the address owner of other addresses; this config is unsupported.",
-                       vr->vrid, ipbuf);
-               /* FIXME: indicate failure with rc */
-               return;
+int vrrp_del_ip(struct vrrp_router *r, struct ipaddr *ip, bool deactivate)
+{
+       size_t cmpsz = ip->ipa_type == IPADDR_V4 ? sizeof(struct in_addr)
+                                                : sizeof(struct in6_addr);
+       struct listnode *ln, *nn;
+       struct ipaddr *iter;
+       int ret = 0;
+
+       if (!vrrp_has_ip(r->vr, ip))
+               return 0;
+
+       if (deactivate && r->addrs->count == 1
+           && r->fsm.state != VRRP_STATE_INITIALIZE)
+               ret = vrrp_event(r, VRRP_EVENT_SHUTDOWN);
+
+       /*
+        * Don't delete IP if we failed to deactivate, otherwise we'll run into
+        * issues later trying to build a VRRP advertisement with no IPs
+        */
+       if (ret == 0) {
+               for (ALL_LIST_ELEMENTS(r->addrs, ln, nn, iter))
+                       if (!memcmp(&iter->ip, &ip->ip, cmpsz))
+                               list_delete_node(r->addrs, ln);
        }
 
-       listnode_add(vr->v6->addrs, v6_ins);
+       return ret;
+}
 
-       if (vr->v6->fsm.state == VRRP_STATE_MASTER)
-               vrrp_ndisc_una_send(vr->v6, v6_ins);
+int vrrp_del_ipv6(struct vrrp_vrouter *vr, struct in6_addr v6, bool deactivate)
+{
+       struct ipaddr ip;
+       ip.ipa_type = IPADDR_V6;
+       ip.ipaddr_v6 = v6;
+       return vrrp_del_ip(vr->v6, &ip, deactivate);
 }
 
-void vrrp_add_ip(struct vrrp_vrouter *vr, struct ipaddr ip)
+int vrrp_del_ipv4(struct vrrp_vrouter *vr, struct in_addr v4, bool deactivate)
 {
-       if (ip.ipa_type == IPADDR_V4)
-               vrrp_add_ipv4(vr, ip.ipaddr_v4);
-       else if (ip.ipa_type == IPADDR_V6)
-               vrrp_add_ipv6(vr, ip.ipaddr_v6);
+       struct ipaddr ip;
+       ip.ipa_type = IPADDR_V4;
+       ip.ipaddr_v4 = v4;
+       return vrrp_del_ip(vr->v4, &ip, deactivate);
 }
 
 

diff --git a/vrrpd/vrrp.h b/vrrpd/vrrp.h
index e8535cb6918e7d620458bd9c3efdc196530da032..6ee2ad6619271a6092a5f7f2a7c6705c0d088e0d 100644 (file)
--- a/vrrpd/vrrp.h
+++ b/vrrpd/vrrp.h
@@ -260,13 +260,21 @@ void vrrp_set_advertisement_interval(struct vrrp_vrouter *vr,
 /*
  * Add an IPvX address to a VRRP Virtual Router.
  *
- * vr
+ * r
  *    Virtual Router to add IPvx address to
  *
  * ip
  *    Address to add
+ *
+ * activate
+ *    Whether to automatically start the VRRP router if this is the first IP
+ *    address added.
+ *
+ * Returns:
+ *    -1 on error
+ *     0 otherwise
  */
-void vrrp_add_ip(struct vrrp_vrouter *vr, struct ipaddr ip);
+int vrrp_add_ip(struct vrrp_router *r, struct ipaddr *ip, bool activate);
 
 /*
  * Add an IPv4 address to a VRRP Virtual Router.
@@ -276,8 +284,16 @@ void vrrp_add_ip(struct vrrp_vrouter *vr, struct ipaddr ip);
  *
  * v4
  *    Address to add
+ *
+ * activate
+ *    Whether to automatically start the VRRP router if this is the first IP
+ *    address added.
+ *
+ * Returns:
+ *    -1 on error
+ *     0 otherwise
  */
-void vrrp_add_ipv4(struct vrrp_vrouter *vr, struct in_addr v4);
+int vrrp_add_ipv4(struct vrrp_vrouter *vr, struct in_addr v4, bool activate);
 
 /*
  * Add an IPv6 address to a VRRP Virtual Router.
@@ -287,9 +303,79 @@ void vrrp_add_ipv4(struct vrrp_vrouter *vr, struct in_addr v4);
  *
  * v6
  *    Address to add
+ *
+ * activate
+ *    Whether to automatically start the VRRP router if this is the first IP
+ *    address added.
+ *
+ * Returns:
+ *    -1 on error
+ *     0 otherwise
+ */
+int vrrp_add_ipv6(struct vrrp_vrouter *vr, struct in6_addr v6, bool activate);
+
+/*
+ * Remove an IP address from a VRRP Virtual Router.
+ *
+ * r
+ *    Virtual Router to remove IP address from
+ *
+ * ip
+ *    Address to remove
+ *
+ * deactivate
+ *    Whether to automatically stop the VRRP router if removing v4 would leave
+ *    us with an empty address list. If this is not true and ip is the only IP
+ *    address backed up by this virtual router, this function will not remove
+ *    the address and return failure.
+ *
+ * Returns:
+ *    -1 on error
+ *     0 otherwise
  */
-void vrrp_add_ipv6(struct vrrp_vrouter *vr, struct in6_addr v6);
+int vrrp_del_ip(struct vrrp_router *r, struct ipaddr *ip, bool deactivate);
 
+/*
+ * Remove an IPv4 address from a VRRP Virtual Router.
+ *
+ * vr
+ *    Virtual Router to remove IPv4 address from
+ *
+ * v4
+ *    Address to remove
+ *
+ * deactivate
+ *    Whether to automatically stop the VRRP router if removing v4 would leave
+ *    us with an empty address list. If this is not true and v4 is the only
+ *    IPv4 address backed up by this virtual router, this function will not
+ *    remove the address and return failure.
+ *
+ * Returns:
+ *    -1 on error
+ *     0 otherwise
+ */
+int vrrp_del_ipv4(struct vrrp_vrouter *vr, struct in_addr v4, bool deactivate);
+
+/*
+ * Remove an IPv6 address from a VRRP Virtual Router.
+ *
+ * vr
+ *    Virtual Router to remove IPv6 address from
+ *
+ * v6
+ *    Address to remove
+ *
+ * deactivate
+ *    Whether to automatically stop the VRRP router if removing v5 would leave
+ *    us with an empty address list. If this is not true and v4 is the only
+ *    IPv6 address backed up by this virtual router, this function will not
+ *    remove the address and return failure.
+ *
+ * Returns:
+ *    -1 on error
+ *     0 otherwise
+ */
+int vrrp_del_ipv6(struct vrrp_vrouter *vr, struct in6_addr v6, bool deactivate);
 
 /* State machine ----------------------------------------------------------- */
 

diff --git a/vrrpd/vrrp_vty.c b/vrrpd/vrrp_vty.c
index ca2ef1842a65298e4b57e2939a8a3858384ad43c..14693bd0c062c0ebb6512e0e46426982ce153f16 100644 (file)
--- a/vrrpd/vrrp_vty.c
+++ b/vrrpd/vrrp_vty.c
@@ -166,21 +166,40 @@ DEFPY(vrrp_ip,
        VTY_DECLVAR_CONTEXT(interface, ifp);
 
        struct vrrp_vrouter *vr;
-       int ret;
+       bool deactivated = false;
+       bool activated = false;
+       bool failed = false;
+       int ret = CMD_SUCCESS;
 
        VROUTER_GET_VTY(vty, ifp, vrid, vr);
-       vrrp_add_ipv4(vr, ip);
 
-       if (vr->v4->fsm.state == VRRP_STATE_INITIALIZE) {
-               vty_out(vty, "%% Activating Virtual Router %ld\n", vrid);
-               ret = vrrp_event(vr->v4, VRRP_EVENT_STARTUP);
-               ret = ret < 0 ? CMD_WARNING_CONFIG_FAILED : CMD_SUCCESS;
+       bool will_activate = (vr->v4->fsm.state == VRRP_STATE_INITIALIZE);
 
-               if (ret == CMD_WARNING_CONFIG_FAILED)
-                       vty_out(vty, "%% Failed to start Virtual Router %ld\n",
-                               vrid);
+       if (no) {
+               int oldstate = vr->v4->fsm.state;
+               failed = vrrp_del_ipv4(vr, ip, true);
+               deactivated = (vr->v4->fsm.state == VRRP_STATE_INITIALIZE
+                              && oldstate != VRRP_STATE_INITIALIZE);
        } else {
-               ret = CMD_SUCCESS;
+               int oldstate = vr->v4->fsm.state;
+               failed = vrrp_add_ipv4(vr, ip, true);
+               activated = (vr->v4->fsm.state != VRRP_STATE_INITIALIZE
+                            && oldstate == VRRP_STATE_INITIALIZE);
+       }
+
+       if (activated)
+               vty_out(vty, "%% Activated IPv4 Virtual Router %ld\n", vrid);
+       if (deactivated)
+               vty_out(vty, "%% Deactivated IPv4 Virtual Router %ld\n", vrid);
+       if (failed) {
+               vty_out(vty, "%% Failed to %s virtual IP",
+                       no ? "remove" : "add");
+               ret = CMD_WARNING_CONFIG_FAILED;
+               if (will_activate && !activated) {
+                       vty_out(vty,
+                               "%% Failed to activate IPv4 Virtual Router %ld\n",
+                               vrid);
+               }
        }
 
        return ret;
@@ -198,21 +217,40 @@ DEFPY(vrrp_ip6,
        VTY_DECLVAR_CONTEXT(interface, ifp);
 
        struct vrrp_vrouter *vr;
-       int ret;
+       bool deactivated = false;
+       bool activated = false;
+       bool failed = false;
+       int ret = CMD_SUCCESS;
 
        VROUTER_GET_VTY(vty, ifp, vrid, vr);
-       vrrp_add_ipv6(vr, ipv6);
 
-       if (vr->v6->fsm.state == VRRP_STATE_INITIALIZE) {
-               vty_out(vty, "%% Activating Virtual Router %ld\n", vrid);
-               ret = vrrp_event(vr->v6, VRRP_EVENT_STARTUP);
-               ret = ret < 0 ? CMD_WARNING_CONFIG_FAILED : CMD_SUCCESS;
+       bool will_activate = (vr->v6->fsm.state == VRRP_STATE_INITIALIZE);
 
-               if (ret == CMD_WARNING_CONFIG_FAILED)
-                       vty_out(vty, "%% Failed to start Virtual Router %ld\n",
-                               vrid);
+       if (no) {
+               int oldstate = vr->v6->fsm.state;
+               failed = vrrp_del_ipv6(vr, ipv6, true);
+               deactivated = (vr->v6->fsm.state == VRRP_STATE_INITIALIZE
+                              && oldstate != VRRP_STATE_INITIALIZE);
        } else {
-               ret = CMD_SUCCESS;
+               int oldstate = vr->v6->fsm.state;
+               failed = vrrp_add_ipv6(vr, ipv6, true);
+               activated = (vr->v6->fsm.state != VRRP_STATE_INITIALIZE
+                            && oldstate == VRRP_STATE_INITIALIZE);
+       }
+
+       if (activated)
+               vty_out(vty, "%% Activated IPv6 Virtual Router %ld\n", vrid);
+       if (deactivated)
+               vty_out(vty, "%% Deactivated IPv6 Virtual Router %ld\n", vrid);
+       if (failed) {
+               vty_out(vty, "%% Failed to %s virtual IP",
+                       no ? "remove" : "add");
+               ret = CMD_WARNING_CONFIG_FAILED;
+               if (will_activate && !activated) {
+                       vty_out(vty,
+                               "%% Failed to activate IPv4 Virtual Router %ld\n",
+                               vrid);
+               }
        }
 
        return ret;