Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/netbsd-8]: src Pull up following revision(s) (requested by ozaki-r in ti...



details:   https://anonhg.NetBSD.org/src/rev/dfb3c56e53be
branches:  netbsd-8
changeset: 851073:dfb3c56e53be
user:      snj <snj%NetBSD.org@localhost>
date:      Tue Oct 24 08:55:55 2017 +0000

description:
Pull up following revision(s) (requested by ozaki-r in ticket #305):
        distrib/sets/lists/tests/mi: revision 1.762
        sys/net/route.c: revision 1.198-1.201
        sys/net/route.h: revision 1.114
        sys/netatalk/at_proto.c: revision 1.22
        sys/netinet/in_proto.c: revision 1.124
        sys/netinet6/in6_proto.c: revision 1.118
        sys/netmpls/mpls_proto.c: revision 1.31
        sys/netnatm/natm_proto.c: revision 1.18
        sys/rump/net/lib/libsockin/sockin.c: revision 1.65
        sys/sys/domain.h: revision 1.33
        tests/net/route/Makefile: revision 1.6
        tests/net/route/t_rtcache.sh: revision 1.1
Add tests of rtcache invalidation
Remove unnecessary NULL check of rt_ifp
It's always non-NULL.
Invalidate rtcache based on a global generation counter
The change introduces a global generation counter that is incremented when any
routes have been added or deleted. When a rtcache caches a rtentry into itself,
it also stores a snapshot of the generation counter. If the snapshot equals to
the global counter, the cache is still valid, otherwise invalidated.
One drawback of the change is that all rtcaches of all protocol families are
invalidated when any routes of any protocol families are added or deleted.
If that matters, we should have separate generation counters based on
protocol families.
This change removes LIST_ENTRY from struct route, which fixes a part of
PR kern/52515.
Remove the global lock for rtcache
Thanks to removal of LIST_ENTRY of struct route, rtcaches are accessed only by
their users. And in existing usages a rtcache is guranteed to be not accessed
simultaneously. So the rtcache framework doesn't need any exclusion controls
in itself.
Synchronize on rtcache_generation with rtlock
It's racy if NET_MPSAFE is enabled.
Pointed out by joerg@

diffstat:

 distrib/sets/lists/tests/mi         |    3 +-
 sys/net/route.c                     |  327 +++++++++--------------------------
 sys/net/route.h                     |    7 +-
 sys/netatalk/at_proto.c             |    5 +-
 sys/netinet/in_proto.c              |    5 +-
 sys/netinet6/in6_proto.c            |    5 +-
 sys/netmpls/mpls_proto.c            |    5 +-
 sys/netnatm/natm_proto.c            |    5 +-
 sys/rump/net/lib/libsockin/sockin.c |    6 +-
 sys/sys/domain.h                    |    3 +-
 tests/net/route/Makefile            |    4 +-
 tests/net/route/t_rtcache.sh        |  167 ++++++++++++++++++
 12 files changed, 274 insertions(+), 268 deletions(-)

diffs (truncated from 991 to 300 lines):

diff -r f5c410d6862e -r dfb3c56e53be distrib/sets/lists/tests/mi
--- a/distrib/sets/lists/tests/mi       Tue Oct 24 08:50:44 2017 +0000
+++ b/distrib/sets/lists/tests/mi       Tue Oct 24 08:55:55 2017 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: mi,v 1.752.2.4 2017/10/21 19:43:53 snj Exp $
+# $NetBSD: mi,v 1.752.2.5 2017/10/24 08:55:55 snj Exp $
 #
 # Note: don't delete entries from here - mark them as "obsolete" instead.
 #
@@ -3376,6 +3376,7 @@
 ./usr/tests/net/route/t_flags                  tests-net-tests         atf,rump
 ./usr/tests/net/route/t_flags6                 tests-net-tests         atf,rump
 ./usr/tests/net/route/t_route                  tests-net-tests         atf,rump
+./usr/tests/net/route/t_rtcache                        tests-net-tests         atf,rump
 ./usr/tests/net/sys                            tests-net-tests         compattestfile,atf
 ./usr/tests/net/sys/Atffile                    tests-net-tests         compattestfile,atf
 ./usr/tests/net/sys/Kyuafile                   tests-net-tests         compattestfile,atf,kyua
diff -r f5c410d6862e -r dfb3c56e53be sys/net/route.c
--- a/sys/net/route.c   Tue Oct 24 08:50:44 2017 +0000
+++ b/sys/net/route.c   Tue Oct 24 08:55:55 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: route.c,v 1.194.6.2 2017/07/07 13:57:26 martin Exp $   */
+/*     $NetBSD: route.c,v 1.194.6.3 2017/10/24 08:55:55 snj 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.194.6.2 2017/07/07 13:57:26 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: route.c,v 1.194.6.3 2017/10/24 08:55:55 snj Exp $");
 
 #include <sys/param.h>
 #ifdef RTFLUSH_DEBUG
@@ -169,8 +169,12 @@
  * Locking notes:
  * - The routing table is protected by a global rwlock
  *   - API: RT_RLOCK and friends
- * - rtcaches are protected by a global rwlock
- *   - API: RTCACHE_RLOCK and friends
+ * - rtcaches are NOT protected by the framework
+ *   - Callers must guarantee a rtcache isn't accessed simultaneously
+ *   - How the constraint is guranteed in the wild
+ *     - Protect a rtcache by a mutex (e.g., inp_route)
+ *     - Make rtcache per-CPU and allow only accesses from softint
+ *       (e.g., ipforward_rt_percpu)
  * - References to a rtentry is managed by reference counting and psref
  *   - Reference couting is used for temporal reference when a rtentry
  *     is fetched from the routing table
@@ -203,11 +207,17 @@
  *     - if the caller runs in softint, the caller fails to fetch
  *     - otherwise, the caller waits for the update completed and retries
  *       to fetch (probably succeed to fetch for the second time)
+ * - rtcache invalidation
+ *   - There is a global generation counter that is incremented when
+ *     any routes have been added or deleted
+ *   - When a rtcache caches a rtentry into itself, it also stores
+ *     a snapshot of the generation counter
+ *   - If the snapshot equals to the global counter, the cache is valid,
+ *     otherwise the cache is invalidated
  */
 
 /*
- * Global locks for the routing table and rtcaches.
- * Locking order: rtcache_lock => rt_lock
+ * Global lock for the routing table.
  */
 static krwlock_t               rt_lock __cacheline_aligned;
 #ifdef NET_MPSAFE
@@ -224,20 +234,7 @@
 #define        RT_ASSERT_WLOCK()       do {} while (0)
 #endif
 
-static krwlock_t               rtcache_lock __cacheline_aligned;
-#ifdef NET_MPSAFE
-#define RTCACHE_RLOCK()                rw_enter(&rtcache_lock, RW_READER)
-#define RTCACHE_WLOCK()                rw_enter(&rtcache_lock, RW_WRITER)
-#define RTCACHE_UNLOCK()       rw_exit(&rtcache_lock)
-#define        RTCACHE_ASSERT_WLOCK()  KASSERT(rw_write_held(&rtcache_lock))
-#define        RTCACHE_WLOCKED()       rw_write_held(&rtcache_lock)
-#else
-#define RTCACHE_RLOCK()                do {} while (0)
-#define RTCACHE_WLOCK()                do {} while (0)
-#define RTCACHE_UNLOCK()       do {} while (0)
-#define        RTCACHE_ASSERT_WLOCK()  do {} while (0)
-#define        RTCACHE_WLOCKED()       false
-#endif
+static uint64_t rtcache_generation;
 
 /*
  * mutex and cv that are used to wait for references to a rtentry left
@@ -271,23 +268,16 @@
 static kauth_listener_t route_listener;
 
 static int rtdeletemsg(struct rtentry *);
-static void rtflushall(int);
 
 static void rt_maskedcopy(const struct sockaddr *,
     struct sockaddr *, const struct sockaddr *);
 
-static void rtcache_clear(struct route *);
-static void rtcache_clear_rtentry(int, struct rtentry *);
-static void rtcache_invalidate(struct dom_rtlist *);
+static void rtcache_invalidate(void);
 
 static void rt_ref(struct rtentry *);
 
 static struct rtentry *
     rtalloc1_locked(const struct sockaddr *, int, bool, bool);
-static struct rtentry *
-    rtcache_validate_locked(struct route *);
-static void rtcache_free_locked(struct route *);
-static int rtcache_setdst_locked(struct route *, const struct sockaddr *);
 
 static void rtcache_ref(struct rtentry *, struct route *);
 
@@ -491,38 +481,15 @@
 }
 
 static void
-rtflushall(int family)
+rtcache_invalidate(void)
 {
-       struct domain *dom;
+
+       RT_ASSERT_WLOCK();
 
        if (rtcache_debug())
                printf("%s: enter\n", __func__);
 
-       if ((dom = pffinddomain(family)) == NULL)
-               return;
-
-       RTCACHE_WLOCK();
-       rtcache_invalidate(&dom->dom_rtcache);
-       RTCACHE_UNLOCK();
-}
-
-static void
-rtcache(struct route *ro)
-{
-       struct domain *dom;
-
-       RTCACHE_ASSERT_WLOCK();
-
-       rtcache_invariants(ro);
-       KASSERT(ro->_ro_rt != NULL);
-       KASSERT(ro->ro_invalid == false);
-       KASSERT(rtcache_getdst(ro) != NULL);
-
-       if ((dom = pffinddomain(rtcache_getdst(ro)->sa_family)) == NULL)
-               return;
-
-       LIST_INSERT_HEAD(&dom->dom_rtcache, ro, ro_rtcache_next);
-       rtcache_invariants(ro);
+       rtcache_generation++;
 }
 
 #ifdef RT_DEBUG
@@ -586,23 +553,14 @@
        if (ISSET(rt->rt_flags, RTF_UPDATING) &&
            /* XXX updater should be always able to acquire */
            curlwp != rt_update_global.lwp) {
-               bool need_lock = false;
                if (!wait_ok || !rt_wait_ok())
                        goto miss;
                RT_UNLOCK();
                splx(s);
 
-               /* XXX need more proper solution */
-               if (RTCACHE_WLOCKED()) {
-                       RTCACHE_UNLOCK();
-                       need_lock = true;
-               }
-
                /* We can wait until the update is complete */
                rt_update_wait();
 
-               if (need_lock)
-                       RTCACHE_WLOCK();
                if (wlock)
                        RT_WLOCK();
                else
@@ -792,17 +750,14 @@
 
        dlog(LOG_DEBUG, "%s: updating rt=%p lwp=%p\n", __func__, rt, curlwp);
 
-       RTCACHE_WLOCK();
        RT_WLOCK();
        /* If the entry is being destroyed, don't proceed the update. */
        if (!ISSET(rt->rt_flags, RTF_UP)) {
                RT_UNLOCK();
-               RTCACHE_UNLOCK();
                return -1;
        }
        rt->rt_flags |= RTF_UPDATING;
        RT_UNLOCK();
-       RTCACHE_UNLOCK();
 
        mutex_enter(&rt_update_global.lock);
        while (rt_update_global.ongoing) {
@@ -827,11 +782,9 @@
 rt_update_finish(struct rtentry *rt)
 {
 
-       RTCACHE_WLOCK();
        RT_WLOCK();
        rt->rt_flags &= ~RTF_UPDATING;
        RT_UNLOCK();
-       RTCACHE_UNLOCK();
 
        mutex_enter(&rt_update_global.lock);
        rt_update_global.ongoing = false;
@@ -1248,10 +1201,10 @@
                        rt_ref(rt);
                        RT_REFCNT_TRACE(rt);
                }
+               rtcache_invalidate();
                RT_UNLOCK();
                need_unlock = false;
                rt_timer_remove_all(rt);
-               rtcache_clear_rtentry(dst->sa_family, rt);
 #if defined(INET) || defined(INET6)
                if (netmask != NULL)
                        lltable_prefix_free(dst->sa_family, dst, netmask, 0);
@@ -1344,9 +1297,9 @@
                        rt_ref(rt);
                        RT_REFCNT_TRACE(rt);
                }
+               rtcache_invalidate();
                RT_UNLOCK();
                need_unlock = false;
-               rtflushall(dst->sa_family);
                break;
        case RTM_GET:
                if (netmask != NULL) {
@@ -1696,7 +1649,6 @@
 
        /* XXX should be in rt_init */
        rw_init(&rt_lock);
-       rw_init(&rtcache_lock);
 
        LIST_INIT(&rttimer_queue_head);
        callout_init(&rt_timer_ch, CALLOUT_MPSAFE);
@@ -1889,20 +1841,20 @@
 
        rtcache_invariants(ro);
        KASSERT(ro->_ro_rt == NULL);
-       RTCACHE_ASSERT_WLOCK();
 
        if (rtcache_getdst(ro) == NULL)
                return NULL;
-       ro->ro_invalid = false;
        rt = rtalloc1(rtcache_getdst(ro), flag);
-       if (rt != NULL && ISSET(rt->rt_flags, RTF_UP)) {
-               ro->_ro_rt = rt;
-               KASSERT(!ISSET(rt->rt_flags, RTF_UPDATING));
-               rtcache_ref(rt, ro);
+       if (rt != NULL) {
+               RT_RLOCK();
+               if (ISSET(rt->rt_flags, RTF_UP)) {
+                       ro->_ro_rt = rt;
+                       ro->ro_rtcache_generation = rtcache_generation;
+                       rtcache_ref(rt, ro);
+               }
+               RT_UNLOCK();
                rt_unref(rt);
-               rtcache(ro);
-       } else if (rt != NULL)
-               rt_unref(rt);
+       }
 
        rtcache_invariants(ro);
        return ro->_ro_rt;
@@ -1911,32 +1863,23 @@
 struct rtentry *
 rtcache_init(struct route *ro)
 {
-       struct rtentry *rt;
-       RTCACHE_WLOCK();
-       rt = _rtcache_init(ro, 1);
-       RTCACHE_UNLOCK();
-       return rt;
+
+       return _rtcache_init(ro, 1);
 }
 
 struct rtentry *
 rtcache_init_noclone(struct route *ro)
 {
-       struct rtentry *rt;
-       RTCACHE_WLOCK();
-       rt = _rtcache_init(ro, 0);
-       RTCACHE_UNLOCK();
-       return rt;
+
+       return _rtcache_init(ro, 0);
 }
 



Home | Main Index | Thread Index | Old Index