Source-Changes-HG archive

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

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



details:   https://anonhg.NetBSD.org/src/rev/d3930750dd7c
branches:  netbsd-8
changeset: 851464:d3930750dd7c
user:      martin <martin%NetBSD.org@localhost>
date:      Tue Mar 13 13:27:10 2018 +0000

description:
Pull up following revision(s) (requested by ozaki-r in ticket #622):
        sys/netinet/if_arp.c: revision 1.270
        sys/net/if_llatbl.c: revision 1.24 (patch)
        sys/net/if_llatbl.c: revision 1.25
        sys/net/if_llatbl.c: revision 1.26
        sys/net/route.c: revision 1.204
        sys/netinet6/in6.c: revision 1.261
        sys/netinet6/in6.c: revision 1.262 (patch)
        sys/netinet6/in6.c: revision 1.263
        sys/netinet/in.c: revision 1.216
        sys/netinet6/in6.c: revision 1.264
        sys/netinet6/nd6.c: revision 1.246 (patch)
        sys/netinet/if_arp.c: revision 1.269
        sys/net/if_llatbl.h: revision 1.14
        sys/netinet6/in6.c: revision 1.259
        sys/netinet/in.c: revision 1.220
        sys/netinet/in.c: revision 1.221 (patch)
        sys/netinet/in.c: revision 1.222
        sys/netinet/in.c: revision 1.223

Suppress noisy debugging outputs
Even if DEBUG they are too noisy under load.

Tweak sanity checks

Scheduling a timer of static entries is wrong.

Add assertions

We must not destroy llentries holding mbufs.

Fix reference leaks of llentry
callout_reset and callout_halt can cancel a pending callout without telling us.
Detect a cancel and remove a reference by using callout_pending and
callout_stop (it's a bit tricy though, we can detect it).
While here, we can remove remaining abuses of mutex_owned for softnet_lock.

Fix memory leaks on arp -d and ndp -d for static entries
We have to delete entries on in_lltable_delete and in6_lltable_delete
unconditionally.  Note that we don't need to worry about LLE_IFADDR because
there is no such entries now.

Use pool(9) for llentry allocations
llentry is easy to be leaked and pool suits for it because pool is usable to
detect leaks.

Also sweep unnecessary wrappers for llentry, in_llentry and in6_llentry.

diffstat:

 sys/net/if_llatbl.c  |  62 ++++++++++++++++++++++++++---------------
 sys/net/if_llatbl.h  |   5 ++-
 sys/net/route.c      |   6 ++--
 sys/netinet/if_arp.c |  52 +++++++++++++++++-----------------
 sys/netinet/in.c     |  71 +++++++++++------------------------------------
 sys/netinet6/in6.c   |  76 +++++++++++----------------------------------------
 sys/netinet6/nd6.c   |  38 ++++++++++---------------
 7 files changed, 122 insertions(+), 188 deletions(-)

diffs (truncated from 614 to 300 lines):

diff -r 0daf08a32f3d -r d3930750dd7c sys/net/if_llatbl.c
--- a/sys/net/if_llatbl.c       Tue Mar 13 11:42:59 2018 +0000
+++ b/sys/net/if_llatbl.c       Tue Mar 13 13:27:10 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_llatbl.c,v 1.18.6.2 2017/11/17 20:24:05 snj Exp $   */
+/*     $NetBSD: if_llatbl.c,v 1.18.6.3 2018/03/13 13:27:10 martin Exp $        */
 /*
  * Copyright (c) 2004 Luigi Rizzo, Alessandro Cerri. All rights reserved.
  * Copyright (c) 2004-2008 Qing Li. All rights reserved.
@@ -67,6 +67,7 @@
 
 static SLIST_HEAD(, lltable) lltables;
 krwlock_t lltable_rwlock;
+static struct pool llentry_pool;
 
 static void lltable_unlink(struct lltable *llt);
 static void llentries_unlink(struct lltable *llt, struct llentries *head);
@@ -335,6 +336,24 @@
        return (pkts_dropped);
 }
 
+struct llentry *
+llentry_pool_get(int flags)
+{
+       struct llentry *lle;
+
+       lle = pool_get(&llentry_pool, flags);
+       if (lle != NULL)
+               memset(lle, 0, sizeof(*lle));
+       return lle;
+}
+
+void
+llentry_pool_put(struct llentry *lle)
+{
+
+       pool_put(&llentry_pool, lle);
+}
+
 /*
  * Deletes an address from the address table.
  * This function is called by the timer functions
@@ -351,6 +370,8 @@
 
        LLE_WLOCK_ASSERT(lle);
 
+       lle->la_flags |= LLE_DELETED;
+
        if ((lle->la_flags & LLE_LINKED) != 0) {
                llt = lle->lle_tbl;
 
@@ -358,6 +379,18 @@
                llt->llt_unlink_entry(lle);
        }
 
+       /*
+        * Stop a pending callout if one exists.  If we cancel one, we have to
+        * remove a reference to avoid a leak.  callout_pending is required to
+        * to exclude the case that the callout has never been scheduled.
+        */
+       /* XXX once softnet_lock goes away, we should use callout_halt */
+       if (callout_pending(&lle->la_timer)) {
+               bool expired = callout_stop(&lle->la_timer);
+               if (!expired)
+                       LLE_REMREF(lle);
+       }
+
        pkts_dropped = lltable_drop_entry_queue(lle);
 
        LLE_FREE_LOCKED(lle);
@@ -433,28 +466,8 @@
        llentries_unlink(llt, &dchain);
        IF_AFDATA_WUNLOCK(llt->llt_ifp);
 
-       LIST_FOREACH_SAFE(lle, &dchain, lle_chain, next) {
-               /*
-                * We need to release the lock here to lle_timer proceeds;
-                * lle_timer should stop immediately if LLE_LINKED isn't set.
-                * Note that we cannot pass lle->lle_lock to callout_halt
-                * because it's a rwlock.
-                */
-               LLE_ADDREF(lle);
-               LLE_WUNLOCK(lle);
-#ifdef NET_MPSAFE
-               callout_halt(&lle->la_timer, NULL);
-#else
-               if (mutex_owned(softnet_lock))
-                       callout_halt(&lle->la_timer, softnet_lock);
-               else
-                       callout_halt(&lle->la_timer, NULL);
-#endif
-               LLE_WLOCK(lle);
-               LLE_REMREF(lle);
-               llentry_free(lle);
-       }
-
+       LIST_FOREACH_SAFE(lle, &dchain, lle_chain, next)
+               (void)llentry_free(lle);
 }
 
 /*
@@ -758,6 +771,9 @@
 
        SLIST_INIT(&lltables);
        rw_init(&lltable_rwlock);
+
+       pool_init(&llentry_pool, sizeof(struct llentry), 0, 0, 0, "llentrypl",
+           NULL, IPL_SOFTNET);
 }
 
 #ifdef __FreeBSD__
diff -r 0daf08a32f3d -r d3930750dd7c sys/net/if_llatbl.h
--- a/sys/net/if_llatbl.h       Tue Mar 13 11:42:59 2018 +0000
+++ b/sys/net/if_llatbl.h       Tue Mar 13 13:27:10 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_llatbl.h,v 1.10.8.2 2017/11/17 20:24:05 snj Exp $   */
+/*     $NetBSD: if_llatbl.h,v 1.10.8.3 2018/03/13 13:27:10 martin Exp $        */
 /*
  * Copyright (c) 2004 Luigi Rizzo, Alessandro Cerri. All rights reserved.
  * Copyright (c) 2004-2008 Qing Li. All rights reserved.
@@ -266,6 +266,9 @@
 struct llentry  *llentry_alloc(struct ifnet *, struct lltable *,
                    struct sockaddr_storage *);
 
+struct llentry  *llentry_pool_get(int);
+void           llentry_pool_put(struct llentry *);
+
 /* helper functions */
 size_t lltable_drop_entry_queue(struct llentry *);
 
diff -r 0daf08a32f3d -r d3930750dd7c sys/net/route.c
--- a/sys/net/route.c   Tue Mar 13 11:42:59 2018 +0000
+++ b/sys/net/route.c   Tue Mar 13 13:27:10 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: route.c,v 1.194.6.6 2018/02/05 14:55:16 martin Exp $   */
+/*     $NetBSD: route.c,v 1.194.6.7 2018/03/13 13:27:10 martin 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.6 2018/02/05 14:55:16 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: route.c,v 1.194.6.7 2018/03/13 13:27:10 martin Exp $");
 
 #include <sys/param.h>
 #ifdef RTFLUSH_DEBUG
@@ -143,7 +143,7 @@
 #define RT_REFCNT_TRACE(rt)    do {} while (0)
 #endif
 
-#ifdef DEBUG
+#ifdef RT_DEBUG
 #define dlog(level, fmt, args...)      log(level, fmt, ##args)
 #else
 #define dlog(level, fmt, args...)      do {} while (0)
diff -r 0daf08a32f3d -r d3930750dd7c sys/netinet/if_arp.c
--- a/sys/netinet/if_arp.c      Tue Mar 13 11:42:59 2018 +0000
+++ b/sys/netinet/if_arp.c      Tue Mar 13 13:27:10 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_arp.c,v 1.250.2.6 2018/02/26 13:36:01 martin Exp $  */
+/*     $NetBSD: if_arp.c,v 1.250.2.7 2018/03/13 13:27:10 martin 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.250.2.6 2018/02/26 13:36:01 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.250.2.7 2018/03/13 13:27:10 martin Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -314,36 +314,21 @@
        struct llentry *lle = arg;
        struct ifnet *ifp;
 
-       if (lle == NULL)
-               return;
-
-       if (lle->la_flags & LLE_STATIC)
-               return;
+       KASSERT((lle->la_flags & LLE_STATIC) == 0);
 
        LLE_WLOCK(lle);
-       if (callout_pending(&lle->la_timer)) {
-               /*
-                * Here we are a bit odd here in the treatment of
-                * active/pending. If the pending bit is set, it got
-                * rescheduled before I ran. The active
-                * bit we ignore, since if it was stopped
-                * in ll_tablefree() and was currently running
-                * it would have return 0 so the code would
-                * not have deleted it since the callout could
-                * not be stopped so we want to go through
-                * with the delete here now. If the callout
-                * was restarted, the pending bit will be back on and
-                * we just want to bail since the callout_reset would
-                * return 1 and our reference would have been removed
-                * by arpresolve() below.
-                */
-               LLE_WUNLOCK(lle);
+
+       /*
+        * This shortcut is required to avoid trying to touch ifp that may be
+        * being destroyed.
+        */
+       if ((lle->la_flags & LLE_LINKED) == 0) {
+               LLE_FREE_LOCKED(lle);
                return;
        }
+
        ifp = lle->lle_tbl->llt_ifp;
 
-       callout_stop(&lle->la_timer);
-
        /* XXX: LOR avoidance. We still have ref on lle. */
        LLE_WUNLOCK(lle);
 
@@ -370,6 +355,21 @@
 {
 
        LLE_WLOCK_ASSERT(la);
+       KASSERT((la->la_flags & LLE_STATIC) == 0);
+
+       /*
+        * We have to take care of a reference leak which occurs if
+        * callout_reset overwrites a pending callout schedule.  Unfortunately
+        * we don't have a mean to know the overwrite, so we need to know it
+        * using callout_stop.  We need to call callout_pending first to exclude
+        * the case that the callout has never been scheduled.
+        */
+       if (callout_pending(&la->la_timer)) {
+               bool expired = callout_stop(&la->la_timer);
+               if (!expired)
+                       /* A pending callout schedule is canceled. */
+                       LLE_REMREF(la);
+       }
        LLE_ADDREF(la);
        callout_reset(&la->la_timer, hz * sec, arptimer, la);
 }
diff -r 0daf08a32f3d -r d3930750dd7c sys/netinet/in.c
--- a/sys/netinet/in.c  Tue Mar 13 11:42:59 2018 +0000
+++ b/sys/netinet/in.c  Tue Mar 13 13:27:10 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: in.c,v 1.203.2.10 2018/02/26 13:32:01 martin Exp $     */
+/*     $NetBSD: in.c,v 1.203.2.11 2018/03/13 13:27:10 martin Exp $     */
 
 /*
  * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@@ -91,7 +91,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: in.c,v 1.203.2.10 2018/02/26 13:32:01 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: in.c,v 1.203.2.11 2018/03/13 13:27:10 martin Exp $");
 
 #include "arp.h"
 
@@ -1918,10 +1918,6 @@
 
 #if NARP > 0
 
-struct in_llentry {
-       struct llentry          base;
-};
-
 #define        IN_LLTBL_DEFAULT_HSIZE  32
 #define        IN_LLTBL_HASH(k, h) \
        (((((((k >> 8) ^ k) >> 8) ^ k) >> 8) ^ k) & ((h) - 1))
@@ -1935,17 +1931,19 @@
 in_lltable_destroy_lle(struct llentry *lle)
 {
 
+       KASSERT(lle->la_numheld == 0);
+
        LLE_WUNLOCK(lle);
        LLE_LOCK_DESTROY(lle);
-       kmem_intr_free(lle, sizeof(*lle));
+       llentry_pool_put(lle);
 }
 
 static struct llentry *
 in_lltable_new(struct in_addr addr4, u_int flags)
 {
-       struct in_llentry *lle;
+       struct llentry *lle;
 
-       lle = kmem_intr_zalloc(sizeof(*lle), KM_NOSLEEP);
+       lle = llentry_pool_get(PR_NOWAIT);
        if (lle == NULL)                /* NB: caller generates msg */
                return NULL;
 
@@ -1953,14 +1951,14 @@
         * For IPv4 this will trigger "arpresolve" to generate
         * an ARP request.
         */
-       lle->base.la_expire = time_uptime; /* mark expired */



Home | Main Index | Thread Index | Old Index