Skip site navigation (1)Skip section navigation (2) 
Date:      Sun, 19 May 2019 21:49:57 +0000 (UTC)
From:      "Alexander V. Chernikov" <melifaro@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r347982 - head/sys/net
Message-ID:  <201905192149.x4JLnvt5028495@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: melifaro
Date: Sun May 19 21:49:56 2019
New Revision: 347982
URL: https://svnweb.freebsd.org/changeset/base/347982

Log:
  Fix rt_ifa selection during loopback route insertion process.
    Currently such routes are added with a link-level IFA, which is
    plain wrong. Only after the insertion they get fixed by the special
    link_rtrequest() ifa handler. This behaviour complicates routing code
    and makes ifa selection more complex.
  Streamline this process by explicitly moving link_rtrequest() logic
    to the pre-insertion rt_getifa_fib() ifa selector. Avoid calling all
    this logic in the loopback route case by explicitly specifying
    proper rt_ifa inside the ifa_maintain_loopback_route().ยง
  
  MFC after:	2 weeks
  Differential Revision:	https://reviews.freebsd.org/D20076

Modified:
  head/sys/net/if.c
  head/sys/net/route.c

Modified: head/sys/net/if.c
==============================================================================
--- head/sys/net/if.c	Sun May 19 20:28:49 2019	(r347981)
+++ head/sys/net/if.c	Sun May 19 21:49:56 2019	(r347982)
@@ -264,7 +264,6 @@ static void	if_route(struct ifnet *, int flag, int fam
 static int	if_setflag(struct ifnet *, int, int, int *, int);
 static int	if_transmit(struct ifnet *ifp, struct mbuf *m);
 static void	if_unroute(struct ifnet *, int flag, int fam);
-static void	link_rtrequest(int, struct rtentry *, struct rt_addrinfo *);
 static int	if_delmulti_locked(struct ifnet *, struct ifmultiaddr *, int);
 static void	do_link_state_change(void *, int);
 static int	if_getgroup(struct ifgroupreq *, struct ifnet *);
@@ -862,7 +861,6 @@ if_attach_internal(struct ifnet *ifp, int vmove, struc
 		sdl->sdl_type = ifp->if_type;
 		ifp->if_addr = ifa;
 		ifa->ifa_ifp = ifp;
-		ifa->ifa_rtrequest = link_rtrequest;
 		ifa->ifa_addr = (struct sockaddr *)sdl;
 		sdl = (struct sockaddr_dl *)(socksize + (caddr_t)sdl);
 		ifa->ifa_netmask = (struct sockaddr *)sdl;
@@ -1892,6 +1890,7 @@ static int
 ifa_maintain_loopback_route(int cmd, const char *otype, struct ifaddr *ifa,
     struct sockaddr *ia)
 {
+	struct epoch_tracker et;
 	int error;
 	struct rt_addrinfo info;
 	struct sockaddr_dl null_sdl;
@@ -1902,6 +1901,16 @@ ifa_maintain_loopback_route(int cmd, const char *otype
 	bzero(&info, sizeof(info));
 	if (cmd != RTM_DELETE)
 		info.rti_ifp = V_loif;
+	if (cmd == RTM_ADD) {
+		/* explicitly specify (loopback) ifa */
+		if (info.rti_ifp != NULL) {
+			NET_EPOCH_ENTER(et);
+			info.rti_ifa = ifaof_ifpforaddr(ifa->ifa_addr, info.rti_ifp);
+			if (info.rti_ifa != NULL)
+				ifa_ref(info.rti_ifa);
+			NET_EPOCH_EXIT(et);
+		}
+	}
 	info.rti_flags = ifa->ifa_flags | RTF_HOST | RTF_STATIC | RTF_PINNED;
 	info.rti_info[RTAX_DST] = ia;
 	info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)&null_sdl;
@@ -2208,39 +2217,6 @@ ifa_preferred(struct ifaddr *cur, struct ifaddr *next)
 
 	return (cur->ifa_carp && (!next->ifa_carp ||
 	    ((*carp_master_p)(next) && !(*carp_master_p)(cur))));
-}
-
-#include <net/if_llatbl.h>
-
-/*
- * Default action when installing a route with a Link Level gateway.
- * Lookup an appropriate real ifa to point to.
- * This should be moved to /sys/net/link.c eventually.
- */
-static void
-link_rtrequest(int cmd, struct rtentry *rt, struct rt_addrinfo *info)
-{
-	struct epoch_tracker et;
-	struct ifaddr *ifa, *oifa;
-	struct sockaddr *dst;
-	struct ifnet *ifp;
-
-	if (cmd != RTM_ADD || ((ifa = rt->rt_ifa) == NULL) ||
-	    ((ifp = ifa->ifa_ifp) == NULL) || ((dst = rt_key(rt)) == NULL))
-		return;
-	NET_EPOCH_ENTER(et);
-	ifa = ifaof_ifpforaddr(dst, ifp);
-	if (ifa) {
-		oifa = rt->rt_ifa;
-		if (oifa != ifa) {
-			ifa_free(oifa);
-			ifa_ref(ifa);
-		}
-		rt->rt_ifa = ifa;
-		if (ifa->ifa_rtrequest && ifa->ifa_rtrequest != link_rtrequest)
-			ifa->ifa_rtrequest(cmd, rt, info);
-	}
-	NET_EPOCH_EXIT(et);
 }
 
 struct sockaddr_dl *

Modified: head/sys/net/route.c
==============================================================================
--- head/sys/net/route.c	Sun May 19 20:28:49 2019	(r347981)
+++ head/sys/net/route.c	Sun May 19 21:49:56 2019	(r347982)
@@ -1276,12 +1276,14 @@ rt_notifydelete(struct rtentry *rt, struct rt_addrinfo
 /*
  * Look up rt_addrinfo for a specific fib.  Note that if rti_ifa is defined,
  * it will be referenced so the caller must free it.
+ *
+ * Assume basic consistency checks are executed by callers:
+ * RTAX_DST exists, if RTF_GATEWAY is set, RTAX_GATEWAY exists as well.
  */
 int
 rt_getifa_fib(struct rt_addrinfo *info, u_int fibnum)
 {
 	struct epoch_tracker et;
-	struct ifaddr *ifa;
 	int needref, error;
 
 	/*
@@ -1291,21 +1293,54 @@ rt_getifa_fib(struct rt_addrinfo *info, u_int fibnum)
 	error = 0;
 	needref = (info->rti_ifa == NULL);
 	NET_EPOCH_ENTER(et);
+
+	/* If we have interface specified by the ifindex in the address, use it */
 	if (info->rti_ifp == NULL && ifpaddr != NULL &&
-	    ifpaddr->sa_family == AF_LINK &&
-	    (ifa = ifa_ifwithnet(ifpaddr, 0, fibnum)) != NULL) {
-		info->rti_ifp = ifa->ifa_ifp;
+	    ifpaddr->sa_family == AF_LINK) {
+	    const struct sockaddr_dl *sdl = (const struct sockaddr_dl *)ifpaddr;
+	    if (sdl->sdl_index != 0)
+		    info->rti_ifp = ifnet_byindex_locked(sdl->sdl_index);
 	}
+	/*
+	 * If we have source address specified, try to find it
+	 * TODO: avoid enumerating all ifas on all interfaces.
+	 */
 	if (info->rti_ifa == NULL && ifaaddr != NULL)
 		info->rti_ifa = ifa_ifwithaddr(ifaaddr);
 	if (info->rti_ifa == NULL) {
 		struct sockaddr *sa;
 
-		sa = ifaaddr != NULL ? ifaaddr :
-		    (gateway != NULL ? gateway : dst);
-		if (sa != NULL && info->rti_ifp != NULL)
+		/*
+		 * Most common use case for the userland-supplied routes.
+		 *
+		 * Choose sockaddr to select ifa.
+		 * -- if ifp is set --
+		 * Order of preference:
+		 * 1) IFA address
+		 * 2) gateway address
+		 *   Note: for interface routes link-level gateway address 
+		 *     is specified to indicate the interface index without
+		 *     specifying RTF_GATEWAY. In this case, ignore gateway
+		 *   Note: gateway AF may be different from dst AF. In this case,
+		 *   ignore gateway
+		 * 3) final destination.
+		 * 4) if all of these fails, try to get at least link-level ifa.
+		 * -- else --
+		 * try to lookup gateway or dst in the routing table to get ifa
+		 */
+		if (info->rti_info[RTAX_IFA] != NULL)
+			sa = info->rti_info[RTAX_IFA];
+		else if ((info->rti_flags & RTF_GATEWAY) != 0 &&
+		    gateway->sa_family == dst->sa_family)
+			sa = gateway;
+		else
+			sa = dst;
+		if (info->rti_ifp != NULL) {
 			info->rti_ifa = ifaof_ifpforaddr(sa, info->rti_ifp);
-		else if (dst != NULL && gateway != NULL)
+			/* Case 4 */
+			if (info->rti_ifa == NULL && gateway != NULL)
+				info->rti_ifa = ifaof_ifpforaddr(gateway, info->rti_ifp);
+		} else if (dst != NULL && gateway != NULL)
 			info->rti_ifa = ifa_ifwithroute(flags, dst, gateway,
 							fibnum);
 		else if (sa != NULL)

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201905192149.x4JLnvt5028495>