Further refactor IPv4 interface route creation.

* Fix bug with /32 aliases introduced in 81728a538d24.
* Explicitly document business logic for IPv4 ifa routes.
* Remove remnants of rtinit()
* Deduplicate ifa->route prefix code by moving it into ia_getrtprefix()
* Deduplicate conditional check for ifa_maintain_loopback_route()  by
 moving into ia_need_loopback_route()
* Remove now-unused flags argument from in_addprefix().

Reviewed by:		donner
PR:			252883
Differential Revision:	https://reviews.freebsd.org/D28246

3 files changed, 130 insertions, 113 deletions

diff --git a/sys/netinet/in.c b/sys/netinet/in.c
index d6c0c350dec5..eb58c3453cfc 100644
--- a/sys/netinet/in.c
+++ b/sys/netinet/in.c
@@ -79,6 +79,8 @@ static int in_gifaddr_ioctl(u_long, caddr_t, struct ifnet *, struct thread *);
 static void	in_socktrim(struct sockaddr_in *);
 static void	in_purgemaddrs(struct ifnet *);
 
+static bool	ia_need_loopback_route(const struct in_ifaddr *);
+
 VNET_DEFINE_STATIC(int, nosameprefix);
 #define	V_nosameprefix			VNET(nosameprefix)
 SYSCTL_INT(_net_inet_ip, OID_AUTO, no_same_prefix, CTLFLAG_VNET | CTLFLAG_RW,
@@ -445,10 +447,6 @@ in_aifaddr_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, struct thread *td)
 	if (ifp->if_flags & IFF_POINTOPOINT)
 		ia->ia_dstaddr = *dstaddr;
 
-	/* XXXGL: rtinit() needs this strange assignment. */
-	if (ifp->if_flags & IFF_LOOPBACK)
-                ia->ia_dstaddr = ia->ia_addr;
-
 	if (vhid != 0) {
 		error = (*carp_attach_p)(&ia->ia_ifa, vhid);
 		if (error)
@@ -481,12 +479,7 @@ in_aifaddr_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, struct thread *td)
 	 * Add route for the network.
 	 */
 	if (vhid == 0) {
-		int flags = RTF_UP;
-
-		if (ifp->if_flags & (IFF_LOOPBACK|IFF_POINTOPOINT))
-			flags |= RTF_HOST;
-
-		error = in_addprefix(ia, flags);
+		error = in_addprefix(ia);
 		if (error)
 			goto fail1;
 	}
@@ -494,10 +487,7 @@ in_aifaddr_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, struct thread *td)
 	/*
 	 * Add a loopback route to self.
 	 */
-	if (vhid == 0 && (ifp->if_flags & IFF_LOOPBACK) == 0 &&
-	    ia->ia_addr.sin_addr.s_addr != INADDR_ANY &&
-	    !((ifp->if_flags & IFF_POINTOPOINT) &&
-	     ia->ia_dstaddr.sin_addr.s_addr == ia->ia_addr.sin_addr.s_addr)) {
+	if (vhid == 0 && ia_need_loopback_route(ia)) {
 		struct in_ifaddr *eia;
 
 		eia = in_localip_more(ia);
@@ -723,7 +713,8 @@ in_match_ifaddr(const struct rtentry *rt, const struct nhop_object *nh, void *ar
 
 static int
 in_handle_prefix_route(uint32_t fibnum, int cmd,
-    struct sockaddr_in *dst, struct sockaddr_in *netmask, struct ifaddr *ifa)
+    struct sockaddr_in *dst, struct sockaddr_in *netmask, struct ifaddr *ifa,
+    struct ifnet *ifp)
 {
 
 	NET_EPOCH_ASSERT();
@@ -738,6 +729,7 @@ in_handle_prefix_route(uint32_t fibnum, int cmd,
 
 	struct rt_addrinfo info = {
 		.rti_ifa = ifa,
+		.rti_ifp = ifp,
 		.rti_flags = RTF_PINNED | ((netmask != NULL) ? 0 : RTF_HOST),
 		.rti_info = {
 			[RTAX_DST] = (struct sockaddr *)dst,
@@ -753,29 +745,105 @@ in_handle_prefix_route(uint32_t fibnum, int cmd,
 }
 
 /*
- * Adds or delete interface route corresponding to @ifa.
+ * Routing table interaction with interface addresses.
+ *
+ * In general, two types of routes needs to be installed:
+ * a) "interface" or "prefix" route, telling user that the addresses
+ *   behind the ifa prefix are reached directly.
+ * b) "loopback" route installed for the ifa address, telling user that
+ *   the address belongs to local system.
+ *
+ * Handling for (a) and (b) differs in multi-fib aspects, hence they
+ *  are implemented in different functions below.
+ *
+ * The cases above may intersect - /32 interface aliases results in
+ *  the same prefix produced by (a) and (b). This blurs the definition
+ *  of the "loopback" route and complicate interactions. The interaction
+ *  table is defined below. The case numbers are used in the multiple
+ *  functions below to refer to the particular test case.
+ *
  * There can be multiple options:
- * 1) Adding addr with prefix on non-p2p/non-lo interface.
- *  Example: 192.0.2.1/24. Action: add route towards
- *  192.0.2.0/24 via this interface, using ifa as an address source.
- *  Note: route to 192.0.2.1 will be installed separately via
- *  ifa_maintain_loopback_route().
- * 2) Adding addr with "host" mask.
- *  Example: 192.0.2.2/32. In this case no action is performed,
- *  as the route should be installed by ifa_maintain_loopback_route().
- *  Returns 0 to indicate success.
+ * 1) Adding address with prefix on non-p2p/non-loopback interface.
+ *  Example: 192.0.2.1/24. Action:
+ *  * add "prefix" route towards 192.0.2.0/24 via @ia interface,
+ *    using @ia as an address source.
+ *  * add "loopback" route towards 192.0.2.1 via V_loif, saving
+ *   @ia ifp in the gateway and using @ia as an address source.
+ *
+ * 2) Adding address with /32 mask to non-p2p/non-loopback interface.
+ *  Example: 192.0.2.2/32. Action:
+ *  * add "prefix" host route via V_loif, using @ia as an address source.
+ *
  * 3) Adding address with or without prefix to p2p interface.
- *  Example: 10.0.0.1/24->10.0.0.2. In this case, all other addresses
- *  covered by prefix, does not make sense in the context of p2p link.
- *  Action: add route towards 10.0.0.2 via this interface, using ifa as an
- *  address source.
- *  Similar to (1), route to 10.0.0.1 will be installed by
- *  ifa_maintain_loopback_route().
+ *  Example: 10.0.0.1/24->10.0.0.2. Action:
+ *  * add "prefix" host route towards 10.0.0.2 via this interface, using @ia
+ *    as an address source. Note: no sense in installing full /24 as the interface
+ *    is point-to-point.
+ *  * add "loopback" route towards 10.0.9.1 via V_loif, saving
+ *   @ia ifp in the gateway and using @ia as an address source.
+ *
  * 4) Adding address with or without prefix to loopback interface.
- *  Example: 192.0.2.1/24. In this case, trafic to non-host addresses cannot
- *  be forwarded, as it would introduce an infinite cycle.
- *  Similar to (2), perform no action and return 0. Loopback route
- *  will be installed by ifa_maintain_loopback_route().
+ *  Example: 192.0.2.1/24. Action:
+ *  * add "prefix" host route via @ia interface, using @ia as an address source.
+ *    Note: Skip installing /24 prefix as it would introduce TTL loop
+ *    for the traffic destined to these addresses.
+ */
+
+/*
+ * Checks if @ia needs to install loopback route to @ia address via
+ *  ifa_maintain_loopback_route().
+ *
+ * Return true on success.
+ */
+static bool
+ia_need_loopback_route(const struct in_ifaddr *ia)
+{
+	struct ifnet *ifp = ia->ia_ifp;
+
+	/* Case 4: Skip loopback interfaces */
+	if ((ifp->if_flags & IFF_LOOPBACK) ||
+	    (ia->ia_addr.sin_addr.s_addr == INADDR_ANY))
+		return (false);
+
+	/* Clash avoidance: Skip p2p interfaces with both addresses are equal */
+	if ((ifp->if_flags & IFF_POINTOPOINT) &&
+	    ia->ia_dstaddr.sin_addr.s_addr == ia->ia_addr.sin_addr.s_addr)
+		return (false);
+
+	/* Case 2: skip /32 prefixes */
+	if (!(ifp->if_flags & IFF_POINTOPOINT) &&
+	    (ia->ia_sockmask.sin_addr.s_addr == INADDR_BROADCAST))
+		return (false);
+
+	return (true);
+}
+
+/*
+ * Calculate "prefix" route corresponding to @ia.
+ */
+static void
+ia_getrtprefix(const struct in_ifaddr *ia, struct in_addr *prefix, struct in_addr *mask)
+{
+
+	if (ia->ia_ifp->if_flags & IFF_POINTOPOINT) {
+		/* Case 3: return host route for dstaddr */
+		*prefix = ia->ia_dstaddr.sin_addr;
+		mask->s_addr = INADDR_BROADCAST;
+	} else if (ia->ia_ifp->if_flags & IFF_LOOPBACK) {
+		/* Case 4: return host route for ifaddr */
+		*prefix = ia->ia_addr.sin_addr;
+		mask->s_addr = INADDR_BROADCAST;
+	} else {
+		/* Cases 1,2: return actual ia prefix */
+		*prefix = ia->ia_addr.sin_addr;
+		*mask = ia->ia_sockmask.sin_addr;
+		prefix->s_addr &= mask->s_addr;
+	}
+}
+
+/*
+ * Adds or delete interface "prefix" route corresponding to @ifa.
+ *  Returns 0 on success or errno.
  */
 int
 in_handle_ifaddr_route(int cmd, struct in_ifaddr *ia)
@@ -786,25 +854,7 @@ in_handle_ifaddr_route(int cmd, struct in_ifaddr *ia)
 	struct epoch_tracker et;
 	int error;
 
-	/* Case 4: ignore loopbacks */
-	if (ifa->ifa_ifp->if_flags & IFF_LOOPBACK)
-		return (0);
-
-	if (ifa->ifa_ifp->if_flags & IFF_POINTOPOINT) {
-		/* Case 3: install route towards dst addr */
-		daddr = ia->ia_dstaddr.sin_addr;
-		pmask = NULL;
-		maddr.s_addr = INADDR_BROADCAST;
-	} else {
-		daddr = ia->ia_addr.sin_addr;
-		pmask = &ia->ia_sockmask;
-		maddr = pmask->sin_addr;
-
-		if (maddr.s_addr == INADDR_BROADCAST) {
-			/* Case 2: ignore /32 routes */
-			return (0);
-		}
-	}
+	ia_getrtprefix(ia, &daddr, &maddr);
 
 	struct sockaddr_in mask = {
 		.sin_family = AF_INET,
@@ -812,8 +862,7 @@ in_handle_ifaddr_route(int cmd, struct in_ifaddr *ia)
 		.sin_addr = maddr,
 	};
 
-	if (pmask != NULL)
-		pmask = &mask;
+	pmask = (maddr.s_addr != INADDR_BROADCAST) ? &mask : NULL;
 
 	struct sockaddr_in dst = {
 		.sin_family = AF_INET,
@@ -821,56 +870,44 @@ in_handle_ifaddr_route(int cmd, struct in_ifaddr *ia)
 		.sin_addr.s_addr = daddr.s_addr & maddr.s_addr,
 	};
 
+	struct ifnet *ifp = ia->ia_ifp;
+
+	if ((maddr.s_addr == INADDR_BROADCAST) &&
+	    (!(ia->ia_ifp->if_flags & (IFF_POINTOPOINT|IFF_LOOPBACK)))) {
+		/* Case 2: host route on broadcast interface */
+		ifp = V_loif;
+	}
+
 	uint32_t fibnum = ifa->ifa_ifp->if_fib;
 	NET_EPOCH_ENTER(et);
-	error = in_handle_prefix_route(fibnum, cmd, &dst, pmask, ifa);
+	error = in_handle_prefix_route(fibnum, cmd, &dst, pmask, ifa, ifp);
 	NET_EPOCH_EXIT(et);
 
 	return (error);
 }
 
-
-#define rtinitflags(x) \
-	((((x)->ia_ifp->if_flags & (IFF_LOOPBACK | IFF_POINTOPOINT)) != 0) \
-	    ? RTF_HOST : 0)
-
 /*
  * Check if we have a route for the given prefix already.
  */
 static bool
-in_hasrtprefix(struct in_ifaddr *target, int flags)
+in_hasrtprefix(struct in_ifaddr *target)
 {
 	struct rm_priotracker in_ifa_tracker;
 	struct in_ifaddr *ia;
 	struct in_addr prefix, mask, p, m;
 	bool result = false;
 
-	if ((flags & RTF_HOST) != 0) {
-		prefix = target->ia_dstaddr.sin_addr;
-		mask.s_addr = 0;
-	} else {
-		prefix = target->ia_addr.sin_addr;
-		mask = target->ia_sockmask.sin_addr;
-		prefix.s_addr &= mask.s_addr;
-	}
+	ia_getrtprefix(target, &prefix, &mask);
 
 	IN_IFADDR_RLOCK(&in_ifa_tracker);
 	/* Look for an existing address with the same prefix, mask, and fib */
 	CK_STAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) {
-		if (rtinitflags(ia)) {
-			p = ia->ia_dstaddr.sin_addr;
+		ia_getrtprefix(ia, &p, &m);
 
-			if (prefix.s_addr != p.s_addr)
-				continue;
-		} else {
-			p = ia->ia_addr.sin_addr;
-			m = ia->ia_sockmask.sin_addr;
-			p.s_addr &= m.s_addr;
+		if (prefix.s_addr != p.s_addr ||
+		    mask.s_addr != m.s_addr)
+			continue;
 
-			if (prefix.s_addr != p.s_addr ||
-			    mask.s_addr != m.s_addr)
-				continue;
-		}
 		if (target->ia_ifp->if_fib != ia->ia_ifp->if_fib)
 			continue;
 
@@ -889,11 +926,11 @@ in_hasrtprefix(struct in_ifaddr *target, int flags)
 }
 
 int
-in_addprefix(struct in_ifaddr *target, int flags)
+in_addprefix(struct in_ifaddr *target)
 {
 	int error;
 
-	if (in_hasrtprefix(target, flags)) {
+	if (in_hasrtprefix(target)) {
 		if (V_nosameprefix)
 			return (EEXIST);
 		else {
@@ -967,9 +1004,7 @@ in_scrubprefix(struct in_ifaddr *target, u_int flags)
 	/*
 	 * Remove the loopback route to the interface address.
 	 */
-	if ((target->ia_addr.sin_addr.s_addr != INADDR_ANY) &&
-	    !(target->ia_ifp->if_flags & IFF_LOOPBACK) &&
-	    (flags & LLE_STATIC)) {
+	if (ia_need_loopback_route(target) && (flags & LLE_STATIC)) {
 		struct in_ifaddr *eia;
 
 		/*
@@ -989,14 +1024,7 @@ in_scrubprefix(struct in_ifaddr *target, u_int flags)
 		}
 	}
 
-	if (rtinitflags(target)) {
-		prefix = target->ia_dstaddr.sin_addr;
-		mask.s_addr = 0;
-	} else {
-		prefix = target->ia_addr.sin_addr;
-		mask = target->ia_sockmask.sin_addr;
-		prefix.s_addr &= mask.s_addr;
-	}
+	ia_getrtprefix(target, &prefix, &mask);
 
 	if ((target->ia_flags & IFA_ROUTE) == 0) {
 		rt_addrmsg(RTM_DELETE, &target->ia_ifa, target->ia_ifp->if_fib);
@@ -1013,20 +1041,11 @@ in_scrubprefix(struct in_ifaddr *target, u_int flags)
 
 	IN_IFADDR_RLOCK(&in_ifa_tracker);
 	CK_STAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) {
-		if (rtinitflags(ia)) {
-			p = ia->ia_dstaddr.sin_addr;
-
-			if (prefix.s_addr != p.s_addr)
-				continue;
-		} else {
-			p = ia->ia_addr.sin_addr;
-			m = ia->ia_sockmask.sin_addr;
-			p.s_addr &= m.s_addr;
+		ia_getrtprefix(ia, &p, &m);
 
-			if (prefix.s_addr != p.s_addr ||
-			    mask.s_addr != m.s_addr)
-				continue;
-		}
+		if (prefix.s_addr != p.s_addr ||
+		    mask.s_addr != m.s_addr)
+			continue;
 
 		if ((ia->ia_ifp->if_flags & IFF_UP) == 0)
 			continue;
@@ -1077,8 +1096,6 @@ in_scrubprefix(struct in_ifaddr *target, u_int flags)
 	return (error);
 }
 
-#undef rtinitflags
-
 void
 in_ifscrub_all(void)
 {
diff --git a/sys/netinet/in_var.h b/sys/netinet/in_var.h
index 3a83c5e832ab..c7ebff80e56d 100644
--- a/sys/netinet/in_var.h
+++ b/sys/netinet/in_var.h
@@ -461,7 +461,7 @@ int	in_leavegroup_locked(struct in_multi *,
 	    /*const*/ struct in_mfilter *);
 int	in_control(struct socket *, u_long, caddr_t, struct ifnet *,
 	    struct thread *);
-int	in_addprefix(struct in_ifaddr *, int);
+int	in_addprefix(struct in_ifaddr *);
 int	in_scrubprefix(struct in_ifaddr *, u_int);
 void	in_ifscrub_all(void);
 int	in_handle_ifaddr_route(int, struct in_ifaddr *);
diff --git a/sys/netinet/ip_carp.c b/sys/netinet/ip_carp.c
index 1fe3f16a92c8..9a551a70c3ad 100644
--- a/sys/netinet/ip_carp.c
+++ b/sys/netinet/ip_carp.c
@@ -1065,7 +1065,7 @@ carp_ifa_addroute(struct ifaddr *ifa)
 	switch (ifa->ifa_addr->sa_family) {
 #ifdef INET
 	case AF_INET:
-		in_addprefix(ifatoia(ifa), RTF_UP);
+		in_addprefix(ifatoia(ifa));
 		ifa_add_loopback_route(ifa,
 		    (struct sockaddr *)&ifatoia(ifa)->ia_addr);
 		break;