From owner-svn-src-head@FreeBSD.ORG Tue Apr 21 20:25:13 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 378A190D; Tue, 21 Apr 2015 20:25:13 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 259571852; Tue, 21 Apr 2015 20:25:13 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id t3LKPD04073560; Tue, 21 Apr 2015 20:25:13 GMT (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id t3LKPDj7073559; Tue, 21 Apr 2015 20:25:13 GMT (envelope-from glebius@FreeBSD.org) Message-Id: <201504212025.t3LKPDj7073559@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: glebius set sender to glebius@FreeBSD.org using -f From: Gleb Smirnoff Date: Tue, 21 Apr 2015 20:25:12 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r281839 - head/sys/netinet X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Apr 2015 20:25:13 -0000 Author: glebius Date: Tue Apr 21 20:25:12 2015 New Revision: 281839 URL: https://svnweb.freebsd.org/changeset/base/281839 Log: Improve carp(4) locking: - Use the carp_sx to serialize not only CARP ioctls, but also carp_attach() and carp_detach(). - Use cif_mtx to lock only access to those the linked list. - These locking changes allow us to do some memory allocations with M_WAITOK and also properly call callout_drain() in carp_destroy(). - In carp_attach() assert that ifaddr isn't attached. We always come here with a pristine address from in[6]_control(). Reviewed by: oleg Sponsored by: Nginx, Inc. Modified: head/sys/netinet/ip_carp.c Modified: head/sys/netinet/ip_carp.c ============================================================================== --- head/sys/netinet/ip_carp.c Tue Apr 21 20:24:15 2015 (r281838) +++ head/sys/netinet/ip_carp.c Tue Apr 21 20:25:12 2015 (r281839) @@ -256,7 +256,7 @@ SYSCTL_VNET_PCPUSTAT(_net_inet_carp, OID #define CIF_LOCK(cif) mtx_lock(&(cif)->cif_mtx) #define CIF_UNLOCK(cif) mtx_unlock(&(cif)->cif_mtx) #define CIF_FREE(cif) do { \ - CIF_LOCK_ASSERT(cif); \ + CIF_LOCK(cif); \ if (TAILQ_EMPTY(&(cif)->cif_vrs)) \ carp_free_if(cif); \ else \ @@ -296,7 +296,6 @@ SYSCTL_VNET_PCPUSTAT(_net_inet_carp, OID static void carp_input_c(struct mbuf *, struct carp_header *, sa_family_t); static struct carp_softc *carp_alloc(struct ifnet *); -static void carp_detach_locked(struct ifaddr *); static void carp_destroy(struct carp_softc *); static struct carp_if *carp_alloc_if(struct ifnet *); @@ -1250,8 +1249,6 @@ carp_multicast_setup(struct carp_if *cif struct ifnet *ifp = cif->cif_ifp; int error = 0; - CIF_LOCK_ASSERT(cif); - switch (sa) { #ifdef INET case AF_INET: @@ -1264,9 +1261,7 @@ carp_multicast_setup(struct carp_if *cif imo->imo_membership = (struct in_multi **)malloc( (sizeof(struct in_multi *) * IP_MIN_MEMBERSHIPS), M_CARP, - M_NOWAIT); - if (imo->imo_membership == NULL) - return (ENOMEM); + M_WAITOK); imo->imo_mfilters = NULL; imo->imo_max_memberships = IP_MIN_MEMBERSHIPS; imo->imo_multicast_vif = -1; @@ -1296,9 +1291,7 @@ carp_multicast_setup(struct carp_if *cif im6o->im6o_membership = (struct in6_multi **)malloc( (sizeof(struct in6_multi *) * IPV6_MIN_MEMBERSHIPS), M_CARP, - M_ZERO | M_NOWAIT); - if (im6o->im6o_membership == NULL) - return (ENOMEM); + M_ZERO | M_WAITOK); im6o->im6o_mfilters = NULL; im6o->im6o_max_memberships = IPV6_MIN_MEMBERSHIPS; im6o->im6o_multicast_hlim = CARP_DFLTTL; @@ -1355,7 +1348,8 @@ static void carp_multicast_cleanup(struct carp_if *cif, sa_family_t sa) { - CIF_LOCK_ASSERT(cif); + sx_assert(&carp_sx, SA_XLOCKED); + switch (sa) { #ifdef INET case AF_INET: @@ -1504,22 +1498,18 @@ carp_alloc(struct ifnet *ifp) return (sc); } -static int +static void carp_grow_ifas(struct carp_softc *sc) { struct ifaddr **new; - CARP_LOCK_ASSERT(sc); - - new = malloc(sc->sc_ifasiz * 2, M_CARP, M_NOWAIT|M_ZERO); - if (new == NULL) - return (ENOMEM); + new = malloc(sc->sc_ifasiz * 2, M_CARP, M_WAITOK | M_ZERO); + CARP_LOCK(sc); bcopy(sc->sc_ifas, new, sc->sc_ifasiz); free(sc->sc_ifas, M_CARP); sc->sc_ifas = new; sc->sc_ifasiz *= 2; - - return (0); + CARP_UNLOCK(sc); } static void @@ -1528,17 +1518,20 @@ carp_destroy(struct carp_softc *sc) struct ifnet *ifp = sc->sc_carpdev; struct carp_if *cif = ifp->if_carp; - CIF_LOCK_ASSERT(cif); + sx_assert(&carp_sx, SA_XLOCKED); + + if (sc->sc_suppress) + carp_demote_adj(-V_carp_ifdown_adj, "vhid removed"); + CARP_UNLOCK(sc); + CIF_LOCK(cif); TAILQ_REMOVE(&cif->cif_vrs, sc, sc_list); + CIF_UNLOCK(cif); mtx_lock(&carp_mtx); LIST_REMOVE(sc, sc_next); mtx_unlock(&carp_mtx); - CARP_LOCK(sc); - if (sc->sc_suppress) - carp_demote_adj(-V_carp_ifdown_adj, "vhid removed"); callout_drain(&sc->sc_ad_tmo); #ifdef INET callout_drain(&sc->sc_md_tmo); @@ -1807,8 +1800,7 @@ carp_attach(struct ifaddr *ifa, int vhid struct carp_softc *sc; int index, error; - if (ifp->if_carp == NULL) - return (ENOPROTOOPT); + KASSERT(ifa->ifa_carp == NULL, ("%s: ifa %p attached", __func__, ifa)); switch (ifa->ifa_addr->sa_family) { #ifdef INET @@ -1822,40 +1814,32 @@ carp_attach(struct ifaddr *ifa, int vhid return (EPROTOTYPE); } + sx_xlock(&carp_sx); + if (ifp->if_carp == NULL) { + sx_xunlock(&carp_sx); + return (ENOPROTOOPT); + } + CIF_LOCK(cif); IFNET_FOREACH_CARP(ifp, sc) if (sc->sc_vhid == vhid) break; + CIF_UNLOCK(cif); if (sc == NULL) { - CIF_UNLOCK(cif); + sx_xunlock(&carp_sx); return (ENOENT); } - if (ifa->ifa_carp) { - if (ifa->ifa_carp->sc_vhid != vhid) - carp_detach_locked(ifa); - else { - CIF_UNLOCK(cif); - return (0); - } - } - error = carp_multicast_setup(cif, ifa->ifa_addr->sa_family); if (error) { CIF_FREE(cif); + sx_xunlock(&carp_sx); return (error); } - CARP_LOCK(sc); index = sc->sc_naddrs + sc->sc_naddrs6 + 1; if (index > sc->sc_ifasiz / sizeof(struct ifaddr *)) - if ((error = carp_grow_ifas(sc)) != 0) { - carp_multicast_cleanup(cif, - ifa->ifa_addr->sa_family); - CARP_UNLOCK(sc); - CIF_FREE(cif); - return (error); - } + carp_grow_ifas(sc); switch (ifa->ifa_addr->sa_family) { #ifdef INET @@ -1873,14 +1857,15 @@ carp_attach(struct ifaddr *ifa, int vhid } ifa_ref(ifa); + + CARP_LOCK(sc); sc->sc_ifas[index - 1] = ifa; ifa->ifa_carp = sc; - carp_hmac_prepare(sc); carp_sc_state(sc); - CARP_UNLOCK(sc); - CIF_UNLOCK(cif); + + sx_xunlock(&carp_sx); return (0); } @@ -1890,25 +1875,14 @@ carp_detach(struct ifaddr *ifa) { struct ifnet *ifp = ifa->ifa_ifp; struct carp_if *cif = ifp->if_carp; - - CIF_LOCK(cif); - carp_detach_locked(ifa); - CIF_FREE(cif); -} - -static void -carp_detach_locked(struct ifaddr *ifa) -{ - struct ifnet *ifp = ifa->ifa_ifp; - struct carp_if *cif = ifp->if_carp; struct carp_softc *sc = ifa->ifa_carp; int i, index; KASSERT(sc != NULL, ("%s: %p not attached", __func__, ifa)); - CIF_LOCK_ASSERT(cif); - CARP_LOCK(sc); + sx_xlock(&carp_sx); + CARP_LOCK(sc); /* Shift array. */ index = sc->sc_naddrs + sc->sc_naddrs6; for (i = 0; i < index; i++) @@ -1943,11 +1917,14 @@ carp_detach_locked(struct ifaddr *ifa) carp_hmac_prepare(sc); carp_sc_state(sc); - if (sc->sc_naddrs == 0 && sc->sc_naddrs6 == 0) { - CARP_UNLOCK(sc); + if (sc->sc_naddrs == 0 && sc->sc_naddrs6 == 0) carp_destroy(sc); - } else + else CARP_UNLOCK(sc); + + CIF_FREE(cif); + + sx_xunlock(&carp_sx); } static void