From 2cd909020d730e5457ea211a21242b5181deb334 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Wed, 30 Jan 2019 21:47:55 +0000 Subject: [PATCH] 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 --- vrrpd/vrrp.c | 123 +++++++++++++++++++++++++++++++++++------------ vrrpd/vrrp.h | 94 ++++++++++++++++++++++++++++++++++-- vrrpd/vrrp_vty.c | 78 ++++++++++++++++++++++-------- 3 files changed, 240 insertions(+), 55 deletions(-) diff --git a/vrrpd/vrrp.c b/vrrpd/vrrp.c index 682c85adb5..cc93b94a9b 100644 --- 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 e8535cb691..6ee2ad6619 100644 --- 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 ca2ef1842a..14693bd0c0 100644 --- 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; -- 2.39.5