From owner-svn-src-all@freebsd.org Wed May 1 21:45:16 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A009B15A1BA7; Wed, 1 May 2019 21:45:16 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 3C1CE8EDDD; Wed, 1 May 2019 21:45:16 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 214AF2777C; Wed, 1 May 2019 21:45:16 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x41LjGCJ074489; Wed, 1 May 2019 21:45:16 GMT (envelope-from jhb@FreeBSD.org) Received: (from jhb@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x41LjFhD074488; Wed, 1 May 2019 21:45:16 GMT (envelope-from jhb@FreeBSD.org) Message-Id: <201905012145.x41LjFhD074488@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: jhb set sender to jhb@FreeBSD.org using -f From: John Baldwin Date: Wed, 1 May 2019 21:45:15 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r347005 - stable/12/sys/net X-SVN-Group: stable-12 X-SVN-Commit-Author: jhb X-SVN-Commit-Paths: stable/12/sys/net X-SVN-Commit-Revision: 347005 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 3C1CE8EDDD X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.95 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_SHORT(-0.96)[-0.956,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 May 2019 21:45:16 -0000 Author: jhb Date: Wed May 1 21:45:15 2019 New Revision: 347005 URL: https://svnweb.freebsd.org/changeset/base/347005 Log: MFC 345655: Remove nested epochs from lagg(4). lagg_bcast_start appeared to have a bug in that was using the last lagg port structure after exiting the epoch that was keeping that structure alive. However, upon further inspection, the epoch was already entered by the caller (lagg_transmit), so the epoch enter/exit in lagg_bcast_start was actually unnecessary. This commit generally removes uses of the net epoch via LAGG_RLOCK to protect the list of ports when the list of ports was already protected by an existing LAGG_RLOCK in a caller, or the LAGG_XLOCK. It also adds a missing epoch enter/exit in lagg_snd_tag_alloc while accessing the lagg port structures. An ifp is still accessed via an unsafe reference after the epoch is exited, but that is true in the current code and will be fixed in a future change. Modified: stable/12/sys/net/if_lagg.c Directory Properties: stable/12/ (props changed) Modified: stable/12/sys/net/if_lagg.c ============================================================================== --- stable/12/sys/net/if_lagg.c Wed May 1 21:26:33 2019 (r347004) +++ stable/12/sys/net/if_lagg.c Wed May 1 21:45:15 2019 (r347005) @@ -750,7 +750,6 @@ lagg_port_create(struct lagg_softc *sc, struct ifnet * * is predictable and `ifconfig laggN create ...` command * will lead to the same result each time. */ - LAGG_RLOCK(); CK_SLIST_FOREACH(tlp, &sc->sc_ports, lp_entries) { if (tlp->lp_ifp->if_index < ifp->if_index && ( CK_SLIST_NEXT(tlp, lp_entries) == NULL || @@ -758,7 +757,6 @@ lagg_port_create(struct lagg_softc *sc, struct ifnet * ifp->if_index)) break; } - LAGG_RUNLOCK(); if (tlp != NULL) CK_SLIST_INSERT_AFTER(tlp, lp, lp_entries); else @@ -1536,14 +1534,17 @@ lagg_snd_tag_alloc(struct ifnet *ifp, struct lagg_lb *lb; uint32_t p; + LAGG_RLOCK(); switch (sc->sc_proto) { case LAGG_PROTO_FAILOVER: lp = lagg_link_active(sc, sc->sc_primary); break; case LAGG_PROTO_LOADBALANCE: if ((sc->sc_opts & LAGG_OPT_USE_FLOWID) == 0 || - params->hdr.flowtype == M_HASHTYPE_NONE) + params->hdr.flowtype == M_HASHTYPE_NONE) { + LAGG_RUNLOCK(); return (EOPNOTSUPP); + } p = params->hdr.flowid >> sc->flowid_shift; p %= sc->sc_count; lb = (struct lagg_lb *)sc->sc_psc; @@ -1552,16 +1553,22 @@ lagg_snd_tag_alloc(struct ifnet *ifp, break; case LAGG_PROTO_LACP: if ((sc->sc_opts & LAGG_OPT_USE_FLOWID) == 0 || - params->hdr.flowtype == M_HASHTYPE_NONE) + params->hdr.flowtype == M_HASHTYPE_NONE) { + LAGG_RUNLOCK(); return (EOPNOTSUPP); + } lp = lacp_select_tx_port_by_hash(sc, params->hdr.flowid); break; default: + LAGG_RUNLOCK(); return (EOPNOTSUPP); } - if (lp == NULL) + if (lp == NULL) { + LAGG_RUNLOCK(); return (EOPNOTSUPP); + } ifp = lp->lp_ifp; + LAGG_RUNLOCK(); if (ifp == NULL || ifp->if_snd_tag_alloc == NULL || (ifp->if_capenable & IFCAP_TXRTLMT) == 0) return (EOPNOTSUPP); @@ -1845,12 +1852,18 @@ struct lagg_port * lagg_link_active(struct lagg_softc *sc, struct lagg_port *lp) { struct lagg_port *lp_next, *rval = NULL; - struct epoch_tracker net_et; /* * Search a port which reports an active link state. */ + /* + * This is called with either LAGG_RLOCK() held or + * LAGG_XLOCK(sc) held. + */ + if (!in_epoch(net_epoch_preempt)) + LAGG_XLOCK_ASSERT(sc); + if (lp == NULL) goto search; if (LAGG_PORTACTIVE(lp)) { @@ -1863,15 +1876,12 @@ lagg_link_active(struct lagg_softc *sc, struct lagg_po goto found; } - search: - epoch_enter_preempt(net_epoch_preempt, &net_et); +search: CK_SLIST_FOREACH(lp_next, &sc->sc_ports, lp_entries) { if (LAGG_PORTACTIVE(lp_next)) { - epoch_exit_preempt(net_epoch_preempt, &net_et); return (lp_next); } } - epoch_exit_preempt(net_epoch_preempt, &net_et); found: return (rval); } @@ -1953,7 +1963,7 @@ lagg_bcast_start(struct lagg_softc *sc, struct mbuf *m struct lagg_port *lp, *last = NULL; struct mbuf *m0; - LAGG_RLOCK(); + LAGG_RLOCK_ASSERT(); CK_SLIST_FOREACH(lp, &sc->sc_ports, lp_entries) { if (!LAGG_PORTACTIVE(lp)) continue; @@ -1974,7 +1984,6 @@ lagg_bcast_start(struct lagg_softc *sc, struct mbuf *m } last = lp; } - LAGG_RUNLOCK(); if (last == NULL) { m_freem(m); @@ -2087,7 +2096,7 @@ lagg_lb_porttable(struct lagg_softc *sc, struct lagg_p rv = 0; bzero(&lb->lb_ports, sizeof(lb->lb_ports)); - LAGG_RLOCK(); + LAGG_XLOCK_ASSERT(sc); CK_SLIST_FOREACH(lp_next, &sc->sc_ports, lp_entries) { if (lp_next == lp) continue; @@ -2100,7 +2109,6 @@ lagg_lb_porttable(struct lagg_softc *sc, struct lagg_p sc->sc_ifname, lp_next->lp_ifp->if_xname, i); lb->lb_ports[i++] = lp_next; } - LAGG_RUNLOCK(); return (rv); }