Module Name:    src
Committed By:   ozaki-r
Date:           Tue Oct 30 05:54:42 UTC 2018

Modified Files:
        src/sys/net: if.c route.c route.h
        src/sys/netinet: if_arp.c
        src/sys/netinet6: nd6.c

Log Message:
Avoid double rt_replace_ifa on rtrequest1(RTM_ADD)

Some callers of rtrequest1(RTM_ADD) adjust rt_ifa of an rtentry created by
rtrequest1 that may change rt_ifa (in ifa_rtrequest) with another ifa that is
different from requested one.  It's wasteful and even worse introduces a race
condition.  rtrequest1 should just use a passed ifa as is if a caller hopes so.


To generate a diff of this commit:
cvs rdiff -u -r1.439 -r1.440 src/sys/net/if.c
cvs rdiff -u -r1.214 -r1.215 src/sys/net/route.c
cvs rdiff -u -r1.119 -r1.120 src/sys/net/route.h
cvs rdiff -u -r1.275 -r1.276 src/sys/netinet/if_arp.c
cvs rdiff -u -r1.250 -r1.251 src/sys/netinet6/nd6.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/net/if.c
diff -u src/sys/net/if.c:1.439 src/sys/net/if.c:1.440
--- src/sys/net/if.c:1.439	Tue Oct 30 05:29:21 2018
+++ src/sys/net/if.c	Tue Oct 30 05:54:42 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: if.c,v 1.439 2018/10/30 05:29:21 ozaki-r Exp $	*/
+/*	$NetBSD: if.c,v 1.440 2018/10/30 05:54:42 ozaki-r Exp $	*/
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc.
@@ -90,7 +90,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.439 2018/10/30 05:29:21 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.440 2018/10/30 05:54:42 ozaki-r Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_inet.h"
@@ -2183,7 +2183,8 @@ link_rtrequest(int cmd, struct rtentry *
 	struct psref psref;
 
 	if (cmd != RTM_ADD || (ifa = rt->rt_ifa) == NULL ||
-	    (ifp = ifa->ifa_ifp) == NULL || (dst = rt_getkey(rt)) == NULL)
+	    (ifp = ifa->ifa_ifp) == NULL || (dst = rt_getkey(rt)) == NULL ||
+	    ISSET(info->rti_flags, RTF_DONTCHANGEIFA))
 		return;
 	if ((ifa = ifaof_ifpforaddr_psref(dst, ifp, &psref)) != NULL) {
 		rt_replace_ifa(rt, ifa);
@@ -2437,6 +2438,9 @@ p2p_rtrequest(int req, struct rtentry *r
 
 		rt->rt_ifp = lo0ifp;
 
+		if (ISSET(info->rti_flags, RTF_DONTCHANGEIFA))
+			break;
+
 		IFADDR_READER_FOREACH(ifa, ifp) {
 			if (equal(rt_getkey(rt), ifa->ifa_addr))
 				break;

Index: src/sys/net/route.c
diff -u src/sys/net/route.c:1.214 src/sys/net/route.c:1.215
--- src/sys/net/route.c:1.214	Tue Oct 30 05:30:31 2018
+++ src/sys/net/route.c	Tue Oct 30 05:54:42 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: route.c,v 1.214 2018/10/30 05:30:31 ozaki-r Exp $	*/
+/*	$NetBSD: route.c,v 1.215 2018/10/30 05:54:42 ozaki-r Exp $	*/
 
 /*-
  * Copyright (c) 1998, 2008 The NetBSD Foundation, Inc.
@@ -97,7 +97,7 @@
 #endif
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: route.c,v 1.214 2018/10/30 05:30:31 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: route.c,v 1.215 2018/10/30 05:54:42 ozaki-r Exp $");
 
 #include <sys/param.h>
 #ifdef RTFLUSH_DEBUG
@@ -1242,7 +1242,7 @@ rtrequest1(int req, struct rt_addrinfo *
 		if (rt == NULL)
 			senderr(ENOBUFS);
 		memset(rt, 0, sizeof(*rt));
-		rt->rt_flags = RTF_UP | flags;
+		rt->rt_flags = RTF_UP | (flags & ~RTF_DONTCHANGEIFA);
 		LIST_INIT(&rt->rt_timer);
 
 		RT_DPRINTF("rt->_rt_key = %p\n", (void *)rt->_rt_key);
@@ -1605,7 +1605,7 @@ rtinit(struct ifaddr *ifa, int cmd, int 
 	}
 	memset(&info, 0, sizeof(info));
 	info.rti_ifa = ifa;
-	info.rti_flags = flags | ifa->ifa_flags;
+	info.rti_flags = flags | ifa->ifa_flags | RTF_DONTCHANGEIFA;
 	info.rti_info[RTAX_DST] = dst;
 	info.rti_info[RTAX_GATEWAY] = ifa->ifa_addr;
 
@@ -1636,40 +1636,7 @@ rtinit(struct ifaddr *ifa, int cmd, int 
 		rt_unref(rt);
 		break;
 	case RTM_ADD:
-		/*
-		 * XXX it looks just reverting rt_ifa replaced by ifa_rtrequest
-		 * called via rtrequest1. Can we just prevent the replacement
-		 * somehow and remove the following code? And also doesn't
-		 * calling ifa_rtrequest(RTM_ADD) replace rt_ifa again?
-		 */
-		if (rt->rt_ifa != ifa) {
-			printf("rtinit: wrong ifa (%p) was (%p)\n", ifa,
-				rt->rt_ifa);
-#ifdef NET_MPSAFE
-			KASSERT(!cpu_softintr_p());
-
-			error = rt_update_prepare(rt);
-			if (error == 0) {
-#endif
-				if (rt->rt_ifa->ifa_rtrequest != NULL) {
-					rt->rt_ifa->ifa_rtrequest(RTM_DELETE,
-					    rt, &info);
-				}
-				rt_replace_ifa(rt, ifa);
-				rt->rt_ifp = ifa->ifa_ifp;
-				if (ifa->ifa_rtrequest != NULL)
-					ifa->ifa_rtrequest(RTM_ADD, rt, &info);
-#ifdef NET_MPSAFE
-				rt_update_finish(rt);
-			} else {
-				/*
-				 * If error != 0, the rtentry is being
-				 * destroyed, so doing nothing doesn't
-				 * matter.
-				 */
-			}
-#endif
-		}
+		KASSERT(rt->rt_ifa == ifa);
 		rt_newmsg(cmd, rt);
 		rt_unref(rt);
 		RT_REFCNT_TRACE(rt);
@@ -1701,17 +1668,16 @@ rt_ifa_addlocal(struct ifaddr *ifa)
 		struct rtentry *nrt;
 
 		memset(&info, 0, sizeof(info));
-		info.rti_flags = RTF_HOST | RTF_LOCAL;
+		info.rti_flags = RTF_HOST | RTF_LOCAL | RTF_DONTCHANGEIFA;
 		info.rti_info[RTAX_DST] = ifa->ifa_addr;
 		info.rti_info[RTAX_GATEWAY] =
 		    (const struct sockaddr *)ifa->ifa_ifp->if_sadl;
 		info.rti_ifa = ifa;
 		nrt = NULL;
 		e = rtrequest1(RTM_ADD, &info, &nrt);
-		if (nrt && ifa != nrt->rt_ifa)
-			rt_replace_ifa(nrt, ifa);
 		rt_newaddrmsg(RTM_ADD, ifa, e, nrt);
 		if (nrt != NULL) {
+			KASSERT(nrt->rt_ifa == ifa);
 #ifdef RT_DEBUG
 			dump_rt(nrt);
 #endif

Index: src/sys/net/route.h
diff -u src/sys/net/route.h:1.119 src/sys/net/route.h:1.120
--- src/sys/net/route.h:1.119	Thu Apr 19 21:20:43 2018
+++ src/sys/net/route.h	Tue Oct 30 05:54:42 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: route.h,v 1.119 2018/04/19 21:20:43 christos Exp $	*/
+/*	$NetBSD: route.h,v 1.120 2018/10/30 05:54:42 ozaki-r Exp $	*/
 
 /*
  * Copyright (c) 1980, 1986, 1993
@@ -172,6 +172,11 @@ struct ortentry {
 #define RTF_LOCAL	0x40000		/* route represents a local address */
 #define RTF_BROADCAST	0x80000		/* route represents a bcast address */
 #define RTF_UPDATING	0x100000	/* route is updating */
+/*
+ * The flag is nevert set to rt_flags.  It just tells rtrequest1 to set a passed
+ * ifa to rt_ifa (via rti_ifa) and not replace rt_ifa in ifa_rtrequest.
+ */
+#define RTF_DONTCHANGEIFA	0x200000	/* suppress rt_ifa replacement */
 
 /*
  * 0x400 is exposed to userland just for backward compatibility. For that

Index: src/sys/netinet/if_arp.c
diff -u src/sys/netinet/if_arp.c:1.275 src/sys/netinet/if_arp.c:1.276
--- src/sys/netinet/if_arp.c:1.275	Fri May 11 13:56:43 2018
+++ src/sys/netinet/if_arp.c	Tue Oct 30 05:54:41 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_arp.c,v 1.275 2018/05/11 13:56:43 maxv Exp $	*/
+/*	$NetBSD: if_arp.c,v 1.276 2018/10/30 05:54:41 ozaki-r Exp $	*/
 
 /*
  * Copyright (c) 1998, 2000, 2008 The NetBSD Foundation, Inc.
@@ -68,7 +68,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.275 2018/05/11 13:56:43 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.276 2018/10/30 05:54:41 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -605,6 +605,11 @@ arp_rtrequest(int req, struct rtentry *r
 			rt->rt_rmx.rmx_mtu = 0;
 		}
 		rt->rt_flags |= RTF_LOCAL;
+
+		if (ISSET(info->rti_flags, RTF_DONTCHANGEIFA)) {
+			pserialize_read_exit(s);
+			goto out;
+		}
 		/*
 		 * make sure to set rt->rt_ifa to the interface
 		 * address we are using, otherwise we will have trouble

Index: src/sys/netinet6/nd6.c
diff -u src/sys/netinet6/nd6.c:1.250 src/sys/netinet6/nd6.c:1.251
--- src/sys/netinet6/nd6.c:1.250	Mon Sep  3 16:29:36 2018
+++ src/sys/netinet6/nd6.c	Tue Oct 30 05:54:41 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: nd6.c,v 1.250 2018/09/03 16:29:36 riastradh Exp $	*/
+/*	$NetBSD: nd6.c,v 1.251 2018/10/30 05:54:41 ozaki-r Exp $	*/
 /*	$KAME: nd6.c,v 1.279 2002/06/08 11:16:51 itojun Exp $	*/
 
 /*
@@ -31,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nd6.c,v 1.250 2018/09/03 16:29:36 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nd6.c,v 1.251 2018/10/30 05:54:41 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_net_mpsafe.h"
@@ -1566,7 +1566,8 @@ nd6_rtrequest(int req, struct rtentry *r
 				 * that the rt_ifa points to the address instead
 				 * of the loopback address.
 				 */
-				if (ifa != rt->rt_ifa)
+				if (!ISSET(info->rti_flags, RTF_DONTCHANGEIFA)
+				    && ifa != rt->rt_ifa)
 					rt_replace_ifa(rt, ifa);
 			}
 		} else if (rt->rt_flags & RTF_ANNOUNCE) {

Reply via email to