From: Quentin Young <qlyoung@cumulusnetworks.com>
Date: Mon, 4 Mar 2019 17:27:55 +0000 (+0000)
Subject: vrrpd: allow user to set priority = 255
X-Git-Tag: base_7.2~330^2~45
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=2f1fc30fd25aeff4ff88efbab31cf31d366793ef;p=matthieu%2Ffrr.git

vrrpd: allow user to set priority = 255

Too many problems with implicit ownership determination via duplicate
address assignment. Will revisit that in the future. For now, allow user
to specify 255 as a priority value. This is functionally no different
than any other priority value; it just serves as a self-documenting way
of saying you want one router to always be master.

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

diff --git a/vrrpd/vrrp.c b/vrrpd/vrrp.c
index fbc738e308..fc0a57b71c 100644
--- a/vrrpd/vrrp.c
+++ b/vrrpd/vrrp.c
@@ -107,12 +107,16 @@ static void vrrp_recalculate_timers(struct vrrp_router *r)
  * Determines if a VRRP router is the owner of the specified address.
  *
  * The determining factor for whether an interface is the address owner is
- * simply whether the address is assigned to the VRRP subinterface by someone
+ * simply whether the address is assigned to the VRRP base interface by someone
  * other than vrrpd.
  *
  * This function should always return the correct answer regardless of
  * master/backup status.
  *
+ * ifp
+ *    The interface to check owernship of. This should be the base interface of
+ *    a VRRP router.
+ *
  * vr
  *    Virtual Router
  *
@@ -121,6 +125,23 @@ static void vrrp_recalculate_timers(struct vrrp_router *r)
  */
 static bool vrrp_is_owner(struct interface *ifp, struct ipaddr *addr)
 {
+	/*
+	 * This code sanity checks implicit ownership configuration. Ideally,
+	 * the way we determine address ownership status for this VRRP router
+	 * is by looking at whether our VIPs are also assigned to the base
+	 * interface, and therefore count as "real" addresses. This frees the
+	 * user from having to manually configure priority 255 to indicate
+	 * address ownership. However, this means one of the VIPs will be used
+	 * as the source address for VRRP advertisements, which in turn means
+	 * that other VRRP routers will be receiving packets with a source
+	 * address they themselves have. This causes lots of different issues
+	 * so for now we're disabling this and forcing the user to configure
+	 * priority 255 to indicate ownership.
+	 */
+
+	return false;
+
+#if 0
 	struct prefix p;
 
 	p.family = IS_IPADDR_V4(addr) ? AF_INET : AF_INET6;
@@ -128,6 +149,7 @@ static bool vrrp_is_owner(struct interface *ifp, struct ipaddr *addr)
 	memcpy(&p.u, &addr->ip, sizeof(addr->ip));
 
 	return !!connected_lookup_prefix_exact(ifp, &p);
+#endif
 }
 
 /*
@@ -1411,13 +1433,14 @@ static int vrrp_startup(struct vrrp_router *r)
 	char ipbuf[INET6_ADDRSTRLEN];
 	inet_ntop(r->family, &primary->ip.addr, ipbuf, sizeof(ipbuf));
 
-	if (vrrp_is_owner(r->vr->ifp, primary)) {
+	if (r->vr->priority == VRRP_PRIO_MASTER
+	    || vrrp_is_owner(r->vr->ifp, primary)) {
 		r->priority = VRRP_PRIO_MASTER;
 		vrrp_recalculate_timers(r);
 
 		zlog_info(
 			VRRP_LOGPFX VRRP_LOGPFX_VRID
-			"%s owns primary Virtual Router IP %s; electing self as Master",
+			"%s has priority set to 255 or owns primary Virtual Router IP %s; electing self as Master",
 			r->vr->vrid, r->vr->ifp->name, ipbuf);
 	}
 
diff --git a/vrrpd/vrrp_vty.c b/vrrpd/vrrp_vty.c
index 0498d3c260..ec3a70dadb 100644
--- a/vrrpd/vrrp_vty.c
+++ b/vrrpd/vrrp_vty.c
@@ -116,7 +116,7 @@ DEFPY(vrrp_shutdown,
 
 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