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) {