Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/net Resolve tangled lock dependencies in route.c
details: https://anonhg.NetBSD.org/src/rev/cfdd4a694063
branches: trunk
changeset: 321971:cfdd4a694063
user: ozaki-r <ozaki-r%NetBSD.org@localhost>
date: Thu Apr 12 04:38:13 2018 +0000
description:
Resolve tangled lock dependencies in route.c
This change sweeps remaining lock decisions based on if locked or not by moving
utility functions of rtentry updates from rtsock.c and ensuring holding the
rt_lock. It also improves the atomicity of a update of a rtentry.
diffstat:
sys/net/if.h | 4 +-
sys/net/route.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++++-----
sys/net/route.h | 9 +-
sys/net/rtsock.c | 175 +++++---------------------------------------------
4 files changed, 193 insertions(+), 181 deletions(-)
diffs (truncated from 588 to 300 lines):
diff -r 9a3ff763fc80 -r cfdd4a694063 sys/net/if.h
--- a/sys/net/if.h Thu Apr 12 03:25:08 2018 +0000
+++ b/sys/net/if.h Thu Apr 12 04:38:13 2018 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if.h,v 1.258 2018/01/15 13:05:40 maxv Exp $ */
+/* $NetBSD: if.h,v 1.259 2018/04/12 04:38:13 ozaki-r Exp $ */
/*-
* Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc.
@@ -1146,8 +1146,6 @@
struct ifaddr *ifa_ifwithnet_psref(const struct sockaddr *, struct psref *);
struct ifaddr *ifa_ifwithladdr(const struct sockaddr *);
struct ifaddr *ifa_ifwithladdr_psref(const struct sockaddr *, struct psref *);
-struct ifaddr *ifa_ifwithroute_psref(int, const struct sockaddr *,
- const struct sockaddr *, struct psref *);
struct ifaddr *ifaof_ifpforaddr(const struct sockaddr *, struct ifnet *);
struct ifaddr *ifaof_ifpforaddr_psref(const struct sockaddr *, struct ifnet *,
struct psref *);
diff -r 9a3ff763fc80 -r cfdd4a694063 sys/net/route.c
--- a/sys/net/route.c Thu Apr 12 03:25:08 2018 +0000
+++ b/sys/net/route.c Thu Apr 12 04:38:13 2018 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: route.c,v 1.208 2018/04/05 03:39:14 ozaki-r Exp $ */
+/* $NetBSD: route.c,v 1.209 2018/04/12 04:38:13 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.208 2018/04/05 03:39:14 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: route.c,v 1.209 2018/04/12 04:38:13 ozaki-r Exp $");
#include <sys/param.h>
#ifdef RTFLUSH_DEBUG
@@ -130,6 +130,8 @@
#include <netinet/in.h>
#include <netinet/in_var.h>
+#define PRESERVED_RTF (RTF_UP | RTF_GATEWAY | RTF_HOST | RTF_DONE | RTF_MASK)
+
#ifdef RTFLUSH_DEBUG
#define rtcache_debug() __predict_false(_rtcache_debug)
#else /* RTFLUSH_DEBUG */
@@ -224,13 +226,13 @@
#define RT_RLOCK() rw_enter(&rt_lock, RW_READER)
#define RT_WLOCK() rw_enter(&rt_lock, RW_WRITER)
#define RT_UNLOCK() rw_exit(&rt_lock)
-#define RT_LOCKED() rw_lock_held(&rt_lock)
+#define RT_WLOCKED() rw_write_held(&rt_lock)
#define RT_ASSERT_WLOCK() KASSERT(rw_write_held(&rt_lock))
#else
#define RT_RLOCK() do {} while (0)
#define RT_WLOCK() do {} while (0)
#define RT_UNLOCK() do {} while (0)
-#define RT_LOCKED() false
+#define RT_WLOCKED() true
#define RT_ASSERT_WLOCK() do {} while (0)
#endif
@@ -280,6 +282,11 @@
static struct rtentry *
rtalloc1_locked(const struct sockaddr *, int, bool, bool);
+static struct ifaddr *rt_getifa(struct rt_addrinfo *, struct psref *);
+static struct ifnet *rt_getifp(struct rt_addrinfo *, struct psref *);
+static struct ifaddr *ifa_ifwithroute_psref(int, const struct sockaddr *,
+ const struct sockaddr *, struct psref *);
+
static void rtcache_ref(struct rtentry *, struct route *);
#ifdef NET_MPSAFE
@@ -884,11 +891,13 @@
error = rt_update_prepare(rt);
if (error == 0) {
#endif
+ RT_WLOCK();
error = rt_setgate(rt, gateway);
if (error == 0) {
rt->rt_flags |= RTF_MODIFIED;
flags |= RTF_MODIFIED;
}
+ RT_UNLOCK();
#ifdef NET_MPSAFE
rt_update_finish(rt);
} else {
@@ -952,9 +961,9 @@
return error;
}
-struct ifaddr *
+static struct ifaddr *
ifa_ifwithroute_psref(int flags, const struct sockaddr *dst,
- const struct sockaddr *gateway, struct psref *psref)
+ const struct sockaddr *gateway, struct psref *psref)
{
struct ifaddr *ifa = NULL;
@@ -984,11 +993,7 @@
int s;
struct rtentry *rt;
- /* XXX we cannot call rtalloc1 if holding the rt lock */
- if (RT_LOCKED())
- rt = rtalloc1_locked(gateway, 0, true, true);
- else
- rt = rtalloc1(gateway, 0);
+ rt = rtalloc1_locked(gateway, 0, true, true);
if (rt == NULL)
return NULL;
if (rt->rt_flags & RTF_GATEWAY) {
@@ -1074,7 +1079,7 @@
return 0;
}
-struct ifnet *
+static struct ifnet *
rt_getifp(struct rt_addrinfo *info, struct psref *psref)
{
const struct sockaddr *ifpaddr = info->rti_info[RTAX_IFP];
@@ -1099,7 +1104,7 @@
return info->rti_ifp;
}
-struct ifaddr *
+static struct ifaddr *
rt_getifa(struct rt_addrinfo *info, struct psref *psref)
{
struct ifaddr *ifa = NULL;
@@ -1331,6 +1336,7 @@
{
struct sockaddr *new, *old;
+ KASSERT(RT_WLOCKED());
KASSERT(rt->_rt_key != NULL);
RT_DPRINTF("rt->_rt_key = %p\n", (void *)rt->_rt_key);
@@ -1349,11 +1355,7 @@
if (rt->rt_flags & RTF_GATEWAY) {
struct rtentry *gwrt;
- /* XXX we cannot call rtalloc1 if holding the rt lock */
- if (RT_LOCKED())
- gwrt = rtalloc1_locked(gate, 1, false, true);
- else
- gwrt = rtalloc1(gate, 1);
+ gwrt = rtalloc1_locked(gate, 1, false, true);
/*
* If we switched gateways, grab the MTU from the new
* gateway route if the current MTU, if the current MTU is
@@ -1377,6 +1379,154 @@
return 0;
}
+static struct ifaddr *
+rt_update_get_ifa(const struct rt_addrinfo info, const struct rtentry *rt,
+ struct ifnet **ifp, struct psref *psref_ifp, struct psref *psref)
+{
+ struct ifaddr *ifa = NULL;
+
+ *ifp = NULL;
+ if (info.rti_info[RTAX_IFP] != NULL) {
+ ifa = ifa_ifwithnet_psref(info.rti_info[RTAX_IFP], psref);
+ if (ifa == NULL)
+ goto next;
+ *ifp = ifa->ifa_ifp;
+ if_acquire(*ifp, psref_ifp);
+ if (info.rti_info[RTAX_IFA] == NULL &&
+ info.rti_info[RTAX_GATEWAY] == NULL)
+ goto next;
+ ifa_release(ifa, psref);
+ if (info.rti_info[RTAX_IFA] == NULL) {
+ /* route change <dst> <gw> -ifp <if> */
+ ifa = ifaof_ifpforaddr_psref(info.rti_info[RTAX_GATEWAY],
+ *ifp, psref);
+ } else {
+ /* route change <dst> -ifp <if> -ifa <addr> */
+ ifa = ifa_ifwithaddr_psref(info.rti_info[RTAX_IFA], psref);
+ if (ifa != NULL)
+ goto out;
+ ifa = ifaof_ifpforaddr_psref(info.rti_info[RTAX_IFA],
+ *ifp, psref);
+ }
+ goto out;
+ }
+next:
+ if (info.rti_info[RTAX_IFA] != NULL) {
+ /* route change <dst> <gw> -ifa <addr> */
+ ifa = ifa_ifwithaddr_psref(info.rti_info[RTAX_IFA], psref);
+ if (ifa != NULL)
+ goto out;
+ }
+ if (info.rti_info[RTAX_GATEWAY] != NULL) {
+ /* route change <dst> <gw> */
+ ifa = ifa_ifwithroute_psref(rt->rt_flags, rt_getkey(rt),
+ info.rti_info[RTAX_GATEWAY], psref);
+ }
+out:
+ if (ifa != NULL && *ifp == NULL) {
+ *ifp = ifa->ifa_ifp;
+ if_acquire(*ifp, psref_ifp);
+ }
+ if (ifa == NULL && *ifp != NULL) {
+ if_put(*ifp, psref_ifp);
+ *ifp = NULL;
+ }
+ return ifa;
+}
+
+int
+rt_update(struct rtentry *rt, struct rt_addrinfo *info, void *rtm)
+{
+ int error = 0;
+ struct ifnet *ifp = NULL, *new_ifp = NULL;
+ struct ifaddr *ifa = NULL, *new_ifa;
+ struct psref psref_ifa, psref_new_ifa, psref_ifp, psref_new_ifp;
+ bool newgw, ifp_changed = false;
+
+ RT_WLOCK();
+ /*
+ * New gateway could require new ifaddr, ifp;
+ * flags may also be different; ifp may be specified
+ * by ll sockaddr when protocol address is ambiguous
+ */
+ newgw = info->rti_info[RTAX_GATEWAY] != NULL &&
+ sockaddr_cmp(info->rti_info[RTAX_GATEWAY], rt->rt_gateway) != 0;
+
+ if (newgw || info->rti_info[RTAX_IFP] != NULL ||
+ info->rti_info[RTAX_IFA] != NULL) {
+ ifp = rt_getifp(info, &psref_ifp);
+ /* info refers ifp so we need to keep a reference */
+ ifa = rt_getifa(info, &psref_ifa);
+ if (ifa == NULL) {
+ error = ENETUNREACH;
+ goto out;
+ }
+ }
+ if (newgw) {
+ error = rt_setgate(rt, info->rti_info[RTAX_GATEWAY]);
+ if (error != 0)
+ goto out;
+ }
+ if (info->rti_info[RTAX_TAG]) {
+ const struct sockaddr *tag;
+ tag = rt_settag(rt, info->rti_info[RTAX_TAG]);
+ if (tag == NULL) {
+ error = ENOBUFS;
+ goto out;
+ }
+ }
+ /*
+ * New gateway could require new ifaddr, ifp;
+ * flags may also be different; ifp may be specified
+ * by ll sockaddr when protocol address is ambiguous
+ */
+ new_ifa = rt_update_get_ifa(*info, rt, &new_ifp, &psref_new_ifp,
+ &psref_new_ifa);
+ if (new_ifa != NULL) {
+ ifa_release(ifa, &psref_ifa);
+ ifa = new_ifa;
+ }
+ if (ifa) {
+ struct ifaddr *oifa = rt->rt_ifa;
+ if (oifa != ifa && !ifa_is_destroying(ifa) &&
+ new_ifp != NULL && !if_is_deactivated(new_ifp)) {
+ if (oifa && oifa->ifa_rtrequest)
+ oifa->ifa_rtrequest(RTM_DELETE, rt, info);
+ rt_replace_ifa(rt, ifa);
+ rt->rt_ifp = new_ifp;
+ ifp_changed = true;
+ }
+ if (new_ifa == NULL)
+ ifa_release(ifa, &psref_ifa);
+ }
+ ifa_release(new_ifa, &psref_new_ifa);
+ if (new_ifp && rt->rt_ifp != new_ifp && !if_is_deactivated(new_ifp)) {
+ rt->rt_ifp = new_ifp;
+ ifp_changed = true;
+ }
+ rt_setmetrics(rtm, rt);
+ if (rt->rt_flags != info->rti_flags) {
+ rt->rt_flags = (info->rti_flags & ~PRESERVED_RTF) |
+ (rt->rt_flags & PRESERVED_RTF);
+ }
+ if (rt->rt_ifa && rt->rt_ifa->ifa_rtrequest)
+ rt->rt_ifa->ifa_rtrequest(RTM_ADD, rt, info);
+#if defined(INET) || defined(INET6)
+ if (ifp_changed && rt_mask(rt) != NULL)
+ lltable_prefix_free(rt_getkey(rt)->sa_family, rt_getkey(rt),
+ rt_mask(rt), 0);
+#else
+ (void)ifp_changed; /* XXX gcc */
+#endif
+out:
+ if_put(new_ifp, &psref_new_ifp);
+ if_put(ifp, &psref_ifp);
+
+ RT_UNLOCK();
+
Home |
Main Index |
Thread Index |
Old Index