Author: melifaro Date: Fri Jan 8 16:25:11 2016 New Revision: 293424 URL: https://svnweb.freebsd.org/changeset/base/293424
Log: Do more fine-grained locking in rtrequest1_fib(). Last consumer using RTF_RNH_LOCKED flag was eliminated in r291643. Restrict passing RTF_RNH_LOCKED to rtrequest1_fib() and do better locking for RTM_ADD / RTM_DELETE cases. Modified: head/sys/net/route.c Modified: head/sys/net/route.c ============================================================================== --- head/sys/net/route.c Fri Jan 8 15:53:48 2016 (r293423) +++ head/sys/net/route.c Fri Jan 8 16:25:11 2016 (r293424) @@ -754,7 +754,7 @@ ifa_ifwithroute(int flags, const struct if (ifa == NULL) ifa = ifa_ifwithnet(gateway, 0, fibnum); if (ifa == NULL) { - struct rtentry *rt = rtalloc1_fib(gateway, 0, RTF_RNH_LOCKED, fibnum); + struct rtentry *rt = rtalloc1_fib(gateway, 0, 0, fibnum); if (rt == NULL) return (NULL); /* @@ -1575,7 +1575,7 @@ int rtrequest1_fib(int req, struct rt_addrinfo *info, struct rtentry **ret_nrt, u_int fibnum) { - int error = 0, needlock = 0; + int error = 0; struct rtentry *rt, *rt_old; #ifdef FLOWTABLE struct rtentry *rt0; @@ -1585,9 +1585,9 @@ rtrequest1_fib(int req, struct rt_addrin struct ifaddr *ifa; struct sockaddr *ndst; struct sockaddr_storage mdst; -#define senderr(x) { error = x ; goto bad; } KASSERT((fibnum < rt_numfibs), ("rtrequest1_fib: bad fibnum")); + KASSERT((flags & RTF_RNH_LOCKED) == 0, ("rtrequest1_fib: locked")); switch (dst->sa_family) { case AF_INET6: case AF_INET: @@ -1604,12 +1604,7 @@ rtrequest1_fib(int req, struct rt_addrin rnh = rt_tables_get_rnh(fibnum, dst->sa_family); if (rnh == NULL) return (EAFNOSUPPORT); - needlock = ((flags & RTF_RNH_LOCKED) == 0); - flags &= ~RTF_RNH_LOCKED; - if (needlock) - RADIX_NODE_HEAD_LOCK(rnh); - else - RADIX_NODE_HEAD_LOCK_ASSERT(rnh); + /* * If we are adding a host route then we don't want to put * a netmask in the tree, nor do we want to clone it. @@ -1624,9 +1619,11 @@ rtrequest1_fib(int req, struct rt_addrin dst = (struct sockaddr *)&mdst; } + RADIX_NODE_HEAD_LOCK(rnh); rt = rt_unlinkrte(rnh, info, &error); + RADIX_NODE_HEAD_UNLOCK(rnh); if (error != 0) - goto bad; + return (error); rt_notifydelete(rt, info); @@ -1649,33 +1646,32 @@ rtrequest1_fib(int req, struct rt_addrin break; case RTM_ADD: if ((flags & RTF_GATEWAY) && !gateway) - senderr(EINVAL); + return (EINVAL); if (dst && gateway && (dst->sa_family != gateway->sa_family) && (gateway->sa_family != AF_UNSPEC) && (gateway->sa_family != AF_LINK)) - senderr(EINVAL); + return (EINVAL); if (info->rti_ifa == NULL) { error = rt_getifa_fib(info, fibnum); if (error) - senderr(error); + return (error); } else ifa_ref(info->rti_ifa); ifa = info->rti_ifa; rt = uma_zalloc(V_rtzone, M_NOWAIT); if (rt == NULL) { ifa_free(ifa); - senderr(ENOBUFS); + return (ENOBUFS); } rt->rt_flags = RTF_UP | flags; rt->rt_fibnum = fibnum; /* * Add the gateway. Possibly re-malloc-ing the storage for it. */ - RT_LOCK(rt); if ((error = rt_setgate(rt, dst, gateway)) != 0) { ifa_free(ifa); uma_zfree(V_rtzone, rt); - senderr(error); + return (error); } /* @@ -1702,14 +1698,18 @@ rtrequest1_fib(int req, struct rt_addrin rt_setmetrics(info, rt); + RADIX_NODE_HEAD_LOCK(rnh); + RT_LOCK(rt); #ifdef RADIX_MPATH /* do not permit exactly the same dst/mask/gw pair */ if (rn_mpath_capable(rnh) && rt_mpath_conflict(rnh, rt, netmask)) { + RADIX_NODE_HEAD_UNLOCK(rnh); + ifa_free(rt->rt_ifa); R_Free(rt_key(rt)); uma_zfree(V_rtzone, rt); - senderr(EEXIST); + return (EEXIST); } #endif @@ -1738,6 +1738,7 @@ rtrequest1_fib(int req, struct rt_addrin rn = rnh->rnh_addaddr(ndst, netmask, rnh, rt->rt_nodes); } + RADIX_NODE_HEAD_UNLOCK(rnh); if (rt_old != NULL) RT_UNLOCK(rt_old); @@ -1754,7 +1755,7 @@ rtrequest1_fib(int req, struct rt_addrin if (rt0 != NULL) RTFREE(rt0); #endif - senderr(EEXIST); + return (EEXIST); } #ifdef FLOWTABLE else if (rt0 != NULL) { @@ -1786,16 +1787,15 @@ rtrequest1_fib(int req, struct rt_addrin RT_UNLOCK(rt); break; case RTM_CHANGE: + RADIX_NODE_HEAD_LOCK(rnh); error = rtrequest1_fib_change(rnh, info, ret_nrt, fibnum); + RADIX_NODE_HEAD_UNLOCK(rnh); break; default: error = EOPNOTSUPP; } -bad: - if (needlock) - RADIX_NODE_HEAD_UNLOCK(rnh); + return (error); -#undef senderr } #undef dst @@ -1945,15 +1945,7 @@ rt_setgate(struct rtentry *rt, struct so { /* XXX dst may be overwritten, can we move this to below */ int dlen = SA_SIZE(dst), glen = SA_SIZE(gate); -#ifdef INVARIANTS - struct radix_node_head *rnh; - rnh = rt_tables_get_rnh(rt->rt_fibnum, dst->sa_family); -#endif - - RT_LOCK_ASSERT(rt); - RADIX_NODE_HEAD_LOCK_ASSERT(rnh); - /* * Prepare to store the gateway in rt->rt_gateway. * Both dst and gateway are stored one after the other in the same _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"