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"
