Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 26 Jul 2020 15:21:40 -0400
From:      mike tancsa <mike@sentex.net>
To:        Kristof Provost <kp@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org
Subject:   Re: svn commit: r363568 - stable/12/sys/net
Message-ID:  <9e260355-3bba-a615-8536-da17c3158141@sentex.net>
In-Reply-To: <202007261744.06QHi3iB011159@repo.freebsd.org>
References:  <202007261744.06QHi3iB011159@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Hi Kristof,

    First off, thank you for all your efforts in pf and if_bridge.  I
have trying to track down a problem with a golang app (sysutils/zrepl)
that started acting up around the time the if_bridge stuff was commited
(june 26th).  The problem would manifest in stalls of the daemon and am
wondering this might have played a role.  The june 10th kernel I had
seemed to work just fine with the app, although I just rebooted to that
to confirm as around that time we added more RAM to the server in
question and put the app under slightly higher load too. I have yet to
boot to a kernel post this being reverted.  But apart from the panics
some people saw could other 'odd' things pop up as well if traffic was
coming in a bridge interface using an igb0 nic ?

    ---Mike

On 7/26/2020 1:44 PM, Kristof Provost wrote:
> Author: kp
> Date: Sun Jul 26 17:44:03 2020
> New Revision: 363568
> URL: https://svnweb.freebsd.org/changeset/base/363568
>
> Log:
>   Revert bridge epochification
>   
>   Revert r363492, r363491, r363430, r363429 and r362650.
>   
>   The introduction of epoch in the network stack is incomplete in stable/12, and
>   there are simply too many limitations to make the bridge epoch code work there.
>   
>   The final problem is capability configuration of the bridge member interfaces.
>   if_bridge needs to enable promiscuous mode, which for certain drivers (e1000
>   for example) can sleep. In stable/12 we may not sleep within epoch.
>
> Modified:
>   stable/12/sys/net/if_bridge.c
>
> Modified: stable/12/sys/net/if_bridge.c
> ==============================================================================
> --- stable/12/sys/net/if_bridge.c	Sun Jul 26 17:21:24 2020	(r363567)
> +++ stable/12/sys/net/if_bridge.c	Sun Jul 26 17:44:03 2020	(r363568)
> @@ -189,14 +189,41 @@ extern void	nd6_setmtu(struct ifnet *);
>   */
>  #define BRIDGE_LOCK_INIT(_sc)		do {			\
>  	mtx_init(&(_sc)->sc_mtx, "if_bridge", NULL, MTX_DEF);	\
> +	cv_init(&(_sc)->sc_cv, "if_bridge_cv");			\
>  } while (0)
>  #define BRIDGE_LOCK_DESTROY(_sc)	do {	\
>  	mtx_destroy(&(_sc)->sc_mtx);		\
> +	cv_destroy(&(_sc)->sc_cv);		\
>  } while (0)
>  #define BRIDGE_LOCK(_sc)		mtx_lock(&(_sc)->sc_mtx)
>  #define BRIDGE_UNLOCK(_sc)		mtx_unlock(&(_sc)->sc_mtx)
>  #define BRIDGE_LOCK_ASSERT(_sc)		mtx_assert(&(_sc)->sc_mtx, MA_OWNED)
>  #define BRIDGE_UNLOCK_ASSERT(_sc)	mtx_assert(&(_sc)->sc_mtx, MA_NOTOWNED)
> +#define	BRIDGE_LOCK2REF(_sc, _err)	do {	\
> +	mtx_assert(&(_sc)->sc_mtx, MA_OWNED);	\
> +	if ((_sc)->sc_iflist_xcnt > 0)		\
> +		(_err) = EBUSY;			\
> +	else					\
> +		(_sc)->sc_iflist_ref++;		\
> +	mtx_unlock(&(_sc)->sc_mtx);		\
> +} while (0)
> +#define	BRIDGE_UNREF(_sc)		do {				\
> +	mtx_lock(&(_sc)->sc_mtx);					\
> +	(_sc)->sc_iflist_ref--;						\
> +	if (((_sc)->sc_iflist_xcnt > 0) && ((_sc)->sc_iflist_ref == 0))	\
> +		cv_broadcast(&(_sc)->sc_cv);				\
> +	mtx_unlock(&(_sc)->sc_mtx);					\
> +} while (0)
> +#define	BRIDGE_XLOCK(_sc)		do {		\
> +	mtx_assert(&(_sc)->sc_mtx, MA_OWNED);		\
> +	(_sc)->sc_iflist_xcnt++;			\
> +	while ((_sc)->sc_iflist_ref > 0)		\
> +		cv_wait(&(_sc)->sc_cv, &(_sc)->sc_mtx);	\
> +} while (0)
> +#define	BRIDGE_XDROP(_sc)		do {	\
> +	mtx_assert(&(_sc)->sc_mtx, MA_OWNED);	\
> +	(_sc)->sc_iflist_xcnt--;		\
> +} while (0)
>  
>  /*
>   * Bridge interface list entry.
> @@ -210,8 +237,6 @@ struct bridge_iflist {
>  	uint32_t		bif_addrmax;	/* max # of addresses */
>  	uint32_t		bif_addrcnt;	/* cur. # of addresses */
>  	uint32_t		bif_addrexceeded;/* # of address violations */
> -
> -	struct epoch_context	bif_epoch_ctx;
>  };
>  
>  /*
> @@ -225,9 +250,6 @@ struct bridge_rtnode {
>  	uint8_t			brt_flags;	/* address flags */
>  	uint8_t			brt_addr[ETHER_ADDR_LEN];
>  	uint16_t		brt_vlan;	/* vlan id */
> -
> -	struct	vnet		*brt_vnet;
> -	struct	epoch_context	brt_epoch_ctx;
>  };
>  #define	brt_ifp			brt_dst->bif_ifp
>  
> @@ -238,10 +260,13 @@ struct bridge_softc {
>  	struct ifnet		*sc_ifp;	/* make this an interface */
>  	LIST_ENTRY(bridge_softc) sc_list;
>  	struct mtx		sc_mtx;
> +	struct cv		sc_cv;
>  	uint32_t		sc_brtmax;	/* max # of addresses */
>  	uint32_t		sc_brtcnt;	/* cur. # of addresses */
>  	uint32_t		sc_brttimeout;	/* rt timeout in seconds */
>  	struct callout		sc_brcallout;	/* bridge callout */
> +	uint32_t		sc_iflist_ref;	/* refcount for sc_iflist */
> +	uint32_t		sc_iflist_xcnt;	/* refcount for sc_iflist */
>  	CK_LIST_HEAD(, bridge_iflist) sc_iflist;	/* member interface list */
>  	CK_LIST_HEAD(, bridge_rtnode) *sc_rthash;	/* our forwarding table */
>  	CK_LIST_HEAD(, bridge_rtnode) sc_rtlist;	/* list version of above */
> @@ -251,8 +276,6 @@ struct bridge_softc {
>  	uint32_t		sc_brtexceeded;	/* # of cache drops */
>  	struct ifnet		*sc_ifaddr;	/* member mac copied from */
>  	struct ether_addr	sc_defaddr;	/* Default MAC address */
> -
> -	struct epoch_context	sc_epoch_ctx;
>  };
>  
>  VNET_DEFINE_STATIC(struct mtx, bridge_list_mtx);
> @@ -572,11 +595,6 @@ vnet_bridge_uninit(const void *unused __unused)
>  	if_clone_detach(V_bridge_cloner);
>  	V_bridge_cloner = NULL;
>  	BRIDGE_LIST_LOCK_DESTROY();
> -
> -	/* Before we can destroy the uma zone, because there are callbacks that
> -	 * use it. */
> -	epoch_drain_callbacks(net_epoch_preempt);
> -
>  	uma_zdestroy(V_bridge_rtnode_zone);
>  }
>  VNET_SYSUNINIT(vnet_bridge_uninit, SI_SUB_PSEUDO, SI_ORDER_ANY,
> @@ -739,17 +757,6 @@ bridge_clone_create(struct if_clone *ifc, int unit, ca
>  	return (0);
>  }
>  
> -static void
> -bridge_clone_destroy_cb(struct epoch_context *ctx)
> -{
> -	struct bridge_softc *sc;
> -
> -	sc = __containerof(ctx, struct bridge_softc, sc_epoch_ctx);
> -
> -	BRIDGE_LOCK_DESTROY(sc);
> -	free(sc, M_DEVBUF);
> -}
> -
>  /*
>   * bridge_clone_destroy:
>   *
> @@ -760,9 +767,7 @@ bridge_clone_destroy(struct ifnet *ifp)
>  {
>  	struct bridge_softc *sc = ifp->if_softc;
>  	struct bridge_iflist *bif;
> -	struct epoch_tracker et;
>  
> -	NET_EPOCH_ENTER_ET(et);
>  	BRIDGE_LOCK(sc);
>  
>  	bridge_stop(ifp, 1);
> @@ -787,12 +792,11 @@ bridge_clone_destroy(struct ifnet *ifp)
>  	BRIDGE_LIST_UNLOCK();
>  
>  	bstp_detach(&sc->sc_stp);
> -	NET_EPOCH_EXIT_ET(et);
> -
>  	ether_ifdetach(ifp);
>  	if_free(ifp);
>  
> -	epoch_call(net_epoch_preempt, &sc->sc_epoch_ctx, bridge_clone_destroy_cb);
> +	BRIDGE_LOCK_DESTROY(sc);
> +	free(sc, M_DEVBUF);
>  }
>  
>  /*
> @@ -818,10 +822,7 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t da
>  	struct ifdrv *ifd = (struct ifdrv *) data;
>  	const struct bridge_control *bc;
>  	int error = 0, oldmtu;
> -	struct epoch_tracker et;
>  
> -	NET_EPOCH_ENTER_ET(et);
> -
>  	switch (cmd) {
>  
>  	case SIOCADDMULTI:
> @@ -942,8 +943,6 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t da
>  		break;
>  	}
>  
> -	NET_EPOCH_EXIT_ET(et);
> -
>  	return (error);
>  }
>  
> @@ -958,8 +957,6 @@ bridge_mutecaps(struct bridge_softc *sc)
>  	struct bridge_iflist *bif;
>  	int enabled, mask;
>  
> -	BRIDGE_LOCK_ASSERT(sc);
> -
>  	/* Initial bitmask of capabilities to test */
>  	mask = BRIDGE_IFCAPS_MASK;
>  
> @@ -968,6 +965,7 @@ bridge_mutecaps(struct bridge_softc *sc)
>  		mask &= bif->bif_savedcaps;
>  	}
>  
> +	BRIDGE_XLOCK(sc);
>  	CK_LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
>  		enabled = bif->bif_ifp->if_capenable;
>  		enabled &= ~BRIDGE_IFCAPS_STRIP;
> @@ -978,6 +976,8 @@ bridge_mutecaps(struct bridge_softc *sc)
>  		bridge_set_ifcap(sc, bif, enabled);
>  		BRIDGE_LOCK(sc);
>  	}
> +	BRIDGE_XDROP(sc);
> +
>  }
>  
>  static void
> @@ -1018,7 +1018,7 @@ bridge_lookup_member(struct bridge_softc *sc, const ch
>  	struct bridge_iflist *bif;
>  	struct ifnet *ifp;
>  
> -	MPASS(in_epoch(net_epoch_preempt));
> +	BRIDGE_LOCK_ASSERT(sc);
>  
>  	CK_LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
>  		ifp = bif->bif_ifp;
> @@ -1039,7 +1039,7 @@ bridge_lookup_member_if(struct bridge_softc *sc, struc
>  {
>  	struct bridge_iflist *bif;
>  
> -	MPASS(in_epoch(net_epoch_preempt));
> +	BRIDGE_LOCK_ASSERT(sc);
>  
>  	CK_LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
>  		if (bif->bif_ifp == member_ifp)
> @@ -1049,16 +1049,6 @@ bridge_lookup_member_if(struct bridge_softc *sc, struc
>  	return (NULL);
>  }
>  
> -static void
> -bridge_delete_member_cb(struct epoch_context *ctx)
> -{
> -	struct bridge_iflist *bif;
> -
> -	bif = __containerof(ctx, struct bridge_iflist, bif_epoch_ctx);
> -
> -	free(bif, M_DEVBUF);
> -}
> -
>  /*
>   * bridge_delete_member:
>   *
> @@ -1078,7 +1068,9 @@ bridge_delete_member(struct bridge_softc *sc, struct b
>  		bstp_disable(&bif->bif_stp);
>  
>  	ifs->if_bridge = NULL;
> +	BRIDGE_XLOCK(sc);
>  	CK_LIST_REMOVE(bif, bif_next);
> +	BRIDGE_XDROP(sc);
>  
>  	/*
>  	 * If removing the interface that gave the bridge its mac address, set
> @@ -1137,9 +1129,7 @@ bridge_delete_member(struct bridge_softc *sc, struct b
>  	}
>  	bstp_destroy(&bif->bif_stp);	/* prepare to free */
>  	BRIDGE_LOCK(sc);
> -
> -	epoch_call(net_epoch_preempt, &bif->bif_epoch_ctx,
> -	    bridge_delete_member_cb);
> +	free(bif, M_DEVBUF);
>  }
>  
>  /*
> @@ -1156,9 +1146,7 @@ bridge_delete_span(struct bridge_softc *sc, struct bri
>  	    ("%s: not a span interface", __func__));
>  
>  	CK_LIST_REMOVE(bif, bif_next);
> -
> -	epoch_call(net_epoch_preempt, &bif->bif_epoch_ctx,
> -	    bridge_delete_member_cb);
> +	free(bif, M_DEVBUF);
>  }
>  
>  static int
> @@ -1214,6 +1202,7 @@ bridge_ioctl_add(struct bridge_softc *sc, void *arg)
>  		 * If any, remove all inet6 addresses from the member
>  		 * interfaces.
>  		 */
> +		BRIDGE_XLOCK(sc);
>  		CK_LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
>   			if (in6ifa_llaonifp(bif->bif_ifp)) {
>  				BRIDGE_UNLOCK(sc);
> @@ -1226,6 +1215,7 @@ bridge_ioctl_add(struct bridge_softc *sc, void *arg)
>  				    bif->bif_ifp->if_xname);
>  			}
>  		}
> +		BRIDGE_XDROP(sc);
>  		if (in6ifa_llaonifp(ifs)) {
>  			BRIDGE_UNLOCK(sc);
>  			in6_ifdetach(ifs);
> @@ -1434,9 +1424,9 @@ bridge_ioctl_gifs(struct bridge_softc *sc, void *arg)
>  		bifc->ifbic_len = buflen;
>  		return (0);
>  	}
> -	outbuf = malloc(buflen, M_TEMP, M_NOWAIT | M_ZERO);
> -	if (outbuf == NULL)
> -		return (ENOMEM);
> +	BRIDGE_UNLOCK(sc);
> +	outbuf = malloc(buflen, M_TEMP, M_WAITOK | M_ZERO);
> +	BRIDGE_LOCK(sc);
>  
>  	count = 0;
>  	buf = outbuf;
> @@ -1496,9 +1486,9 @@ bridge_ioctl_rts(struct bridge_softc *sc, void *arg)
>  		count++;
>  	buflen = sizeof(bareq) * count;
>  
> -	outbuf = malloc(buflen, M_TEMP, M_NOWAIT | M_ZERO);
> -	if (outbuf == NULL)
> -		return (ENOMEM);
> +	BRIDGE_UNLOCK(sc);
> +	outbuf = malloc(buflen, M_TEMP, M_WAITOK | M_ZERO);
> +	BRIDGE_LOCK(sc);
>  
>  	count = 0;
>  	buf = outbuf;
> @@ -1539,17 +1529,12 @@ bridge_ioctl_saddr(struct bridge_softc *sc, void *arg)
>  	struct bridge_iflist *bif;
>  	int error;
>  
> -	MPASS(in_epoch(net_epoch_preempt));
> -
>  	bif = bridge_lookup_member(sc, req->ifba_ifsname);
>  	if (bif == NULL)
>  		return (ENOENT);
>  
> -	/* bridge_rtupdate() may acquire the lock. */
> -	BRIDGE_UNLOCK(sc);
>  	error = bridge_rtupdate(sc, req->ifba_dst, req->ifba_vlan, bif, 1,
>  	    req->ifba_flags);
> -	BRIDGE_LOCK(sc);
>  
>  	return (error);
>  }
> @@ -1824,9 +1809,9 @@ bridge_ioctl_gifsstp(struct bridge_softc *sc, void *ar
>  		return (0);
>  	}
>  
> -	outbuf = malloc(buflen, M_TEMP, M_NOWAIT | M_ZERO);
> -	if (outbuf == NULL)
> -		return (ENOMEM);
> +	BRIDGE_UNLOCK(sc);
> +	outbuf = malloc(buflen, M_TEMP, M_WAITOK | M_ZERO);
> +	BRIDGE_LOCK(sc);
>  
>  	count = 0;
>  	buf = outbuf;
> @@ -1888,7 +1873,6 @@ bridge_ifdetach(void *arg __unused, struct ifnet *ifp)
>  {
>  	struct bridge_softc *sc = ifp->if_bridge;
>  	struct bridge_iflist *bif;
> -	struct epoch_tracker et;
>  
>  	if (ifp->if_flags & IFF_RENAMING)
>  		return;
> @@ -1899,7 +1883,6 @@ bridge_ifdetach(void *arg __unused, struct ifnet *ifp)
>  		 */
>  		return;
>  	}
> -	NET_EPOCH_ENTER_ET(et);
>  	/* Check if the interface is a bridge member */
>  	if (sc != NULL) {
>  		BRIDGE_LOCK(sc);
> @@ -1909,7 +1892,6 @@ bridge_ifdetach(void *arg __unused, struct ifnet *ifp)
>  			bridge_delete_member(sc, bif, 1);
>  
>  		BRIDGE_UNLOCK(sc);
> -		NET_EPOCH_EXIT_ET(et);
>  		return;
>  	}
>  
> @@ -1926,7 +1908,6 @@ bridge_ifdetach(void *arg __unused, struct ifnet *ifp)
>  		BRIDGE_UNLOCK(sc);
>  	}
>  	BRIDGE_LIST_UNLOCK();
> -	NET_EPOCH_EXIT_ET(et);
>  }
>  
>  /*
> @@ -2081,26 +2062,23 @@ static int
>  bridge_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *sa,
>      struct rtentry *rt)
>  {
> -	struct epoch_tracker et;
>  	struct ether_header *eh;
>  	struct ifnet *dst_if;
>  	struct bridge_softc *sc;
>  	uint16_t vlan;
>  
> -	NET_EPOCH_ENTER_ET(et);
> -
>  	if (m->m_len < ETHER_HDR_LEN) {
>  		m = m_pullup(m, ETHER_HDR_LEN);
> -		if (m == NULL) {
> -			NET_EPOCH_EXIT_ET(et);
> +		if (m == NULL)
>  			return (0);
> -		}
>  	}
>  
>  	eh = mtod(m, struct ether_header *);
>  	sc = ifp->if_bridge;
>  	vlan = VLANTAGOF(m);
>  
> +	BRIDGE_LOCK(sc);
> +
>  	/*
>  	 * If bridge is down, but the original output interface is up,
>  	 * go ahead and send out that interface.  Otherwise, the packet
> @@ -2122,10 +2100,16 @@ bridge_output(struct ifnet *ifp, struct mbuf *m, struc
>  	if (dst_if == NULL) {
>  		struct bridge_iflist *bif;
>  		struct mbuf *mc;
> -		int used = 0;
> +		int error = 0, used = 0;
>  
>  		bridge_span(sc, m);
>  
> +		BRIDGE_LOCK2REF(sc, error);
> +		if (error) {
> +			m_freem(m);
> +			return (0);
> +		}
> +
>  		CK_LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
>  			dst_if = bif->bif_ifp;
>  
> @@ -2159,7 +2143,7 @@ bridge_output(struct ifnet *ifp, struct mbuf *m, struc
>  		}
>  		if (used == 0)
>  			m_freem(m);
> -		NET_EPOCH_EXIT_ET(et);
> +		BRIDGE_UNREF(sc);
>  		return (0);
>  	}
>  
> @@ -2171,12 +2155,12 @@ sendunicast:
>  	bridge_span(sc, m);
>  	if ((dst_if->if_drv_flags & IFF_DRV_RUNNING) == 0) {
>  		m_freem(m);
> -		NET_EPOCH_EXIT_ET(et);
> +		BRIDGE_UNLOCK(sc);
>  		return (0);
>  	}
>  
> +	BRIDGE_UNLOCK(sc);
>  	bridge_enqueue(sc, dst_if, m);
> -	NET_EPOCH_EXIT_ET(et);
>  	return (0);
>  }
>  
> @@ -2189,28 +2173,25 @@ sendunicast:
>  static int
>  bridge_transmit(struct ifnet *ifp, struct mbuf *m)
>  {
> -	struct epoch_tracker et;
>  	struct bridge_softc *sc;
>  	struct ether_header *eh;
>  	struct ifnet *dst_if;
>  	int error = 0;
>  
> -	NET_EPOCH_ENTER_ET(et);
> -
>  	sc = ifp->if_softc;
>  
>  	ETHER_BPF_MTAP(ifp, m);
>  
>  	eh = mtod(m, struct ether_header *);
>  
> +	BRIDGE_LOCK(sc);
>  	if (((m->m_flags & (M_BCAST|M_MCAST)) == 0) &&
>  	    (dst_if = bridge_rtlookup(sc, eh->ether_dhost, 1)) != NULL) {
> +		BRIDGE_UNLOCK(sc);
>  		error = bridge_enqueue(sc, dst_if, m);
>  	} else
>  		bridge_broadcast(sc, ifp, m, 0);
>  
> -	NET_EPOCH_EXIT_ET(et);
> -
>  	return (error);
>  }
>  
> @@ -2240,8 +2221,6 @@ bridge_forward(struct bridge_softc *sc, struct bridge_
>  	uint8_t *dst;
>  	int error;
>  
> -	MPASS(in_epoch(net_epoch_preempt));
> -
>  	src_if = m->m_pkthdr.rcvif;
>  	ifp = sc->sc_ifp;
>  
> @@ -2320,10 +2299,12 @@ bridge_forward(struct bridge_softc *sc, struct bridge_
>  	    || PFIL_HOOKED(&V_inet6_pfil_hook)
>  #endif
>  	    ) {
> +		BRIDGE_UNLOCK(sc);
>  		if (bridge_pfil(&m, ifp, src_if, PFIL_IN) != 0)
>  			return;
>  		if (m == NULL)
>  			return;
> +		BRIDGE_LOCK(sc);
>  	}
>  
>  	if (dst_if == NULL) {
> @@ -2351,6 +2332,8 @@ bridge_forward(struct bridge_softc *sc, struct bridge_
>  	    dbif->bif_stp.bp_state == BSTP_IFSTATE_DISCARDING)
>  		goto drop;
>  
> +	BRIDGE_UNLOCK(sc);
> +
>  	if (PFIL_HOOKED(&V_inet_pfil_hook)
>  #ifdef INET6
>  	    || PFIL_HOOKED(&V_inet6_pfil_hook)
> @@ -2366,6 +2349,7 @@ bridge_forward(struct bridge_softc *sc, struct bridge_
>  	return;
>  
>  drop:
> +	BRIDGE_UNLOCK(sc);
>  	m_freem(m);
>  }
>  
> @@ -2378,7 +2362,6 @@ drop:
>  static struct mbuf *
>  bridge_input(struct ifnet *ifp, struct mbuf *m)
>  {
> -	struct epoch_tracker et;
>  	struct bridge_softc *sc = ifp->if_bridge;
>  	struct bridge_iflist *bif, *bif2;
>  	struct ifnet *bifp;
> @@ -2387,12 +2370,8 @@ bridge_input(struct ifnet *ifp, struct mbuf *m)
>  	uint16_t vlan;
>  	int error;
>  
> -	NET_EPOCH_ENTER_ET(et);
> -
> -	if ((sc->sc_ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) {
> -		NET_EPOCH_EXIT_ET(et);
> +	if ((sc->sc_ifp->if_drv_flags & IFF_DRV_RUNNING) == 0)
>  		return (m);
> -	}
>  
>  	bifp = sc->sc_ifp;
>  	vlan = VLANTAGOF(m);
> @@ -2409,12 +2388,12 @@ bridge_input(struct ifnet *ifp, struct mbuf *m)
>  		if_inc_counter(bifp, IFCOUNTER_IPACKETS, 1);
>  		if_inc_counter(bifp, IFCOUNTER_IBYTES, m->m_pkthdr.len);
>  		m_freem(m);
> -		NET_EPOCH_EXIT_ET(et);
>  		return (NULL);
>  	}
> +	BRIDGE_LOCK(sc);
>  	bif = bridge_lookup_member_if(sc, ifp);
>  	if (bif == NULL) {
> -		NET_EPOCH_EXIT_ET(et);
> +		BRIDGE_UNLOCK(sc);
>  		return (m);
>  	}
>  
> @@ -2427,13 +2406,13 @@ bridge_input(struct ifnet *ifp, struct mbuf *m)
>  		if (memcmp(eh->ether_dhost, bstp_etheraddr,
>  		    ETHER_ADDR_LEN) == 0) {
>  			bstp_input(&bif->bif_stp, ifp, m); /* consumes mbuf */
> -			NET_EPOCH_EXIT_ET(et);
> +			BRIDGE_UNLOCK(sc);
>  			return (NULL);
>  		}
>  
>  		if ((bif->bif_flags & IFBIF_STP) &&
>  		    bif->bif_stp.bp_state == BSTP_IFSTATE_DISCARDING) {
> -			NET_EPOCH_EXIT_ET(et);
> +			BRIDGE_UNLOCK(sc);
>  			return (m);
>  		}
>  
> @@ -2444,7 +2423,7 @@ bridge_input(struct ifnet *ifp, struct mbuf *m)
>  		 */
>  		mc = m_dup(m, M_NOWAIT);
>  		if (mc == NULL) {
> -			NET_EPOCH_EXIT_ET(et);
> +			BRIDGE_UNLOCK(sc);
>  			return (m);
>  		}
>  
> @@ -2471,13 +2450,12 @@ bridge_input(struct ifnet *ifp, struct mbuf *m)
>  		}
>  
>  		/* Return the original packet for local processing. */
> -		NET_EPOCH_EXIT_ET(et);
>  		return (m);
>  	}
>  
>  	if ((bif->bif_flags & IFBIF_STP) &&
>  	    bif->bif_stp.bp_state == BSTP_IFSTATE_DISCARDING) {
> -		NET_EPOCH_EXIT_ET(et);
> +		BRIDGE_UNLOCK(sc);
>  		return (m);
>  	}
>  
> @@ -2517,6 +2495,7 @@ bridge_input(struct ifnet *ifp, struct mbuf *m)
>  			     OR_PFIL_HOOKED_INET6)) {			\
>  				if (bridge_pfil(&m, NULL, ifp,		\
>  				    PFIL_IN) != 0 || m == NULL) {	\
> +					BRIDGE_UNLOCK(sc);		\
>  					return (NULL);			\
>  				}					\
>  				eh = mtod(m, struct ether_header *);	\
> @@ -2526,13 +2505,13 @@ bridge_input(struct ifnet *ifp, struct mbuf *m)
>  			error = bridge_rtupdate(sc, eh->ether_shost,	\
>  			    vlan, bif, 0, IFBAF_DYNAMIC);		\
>  			if (error && bif->bif_addrmax) {		\
> +				BRIDGE_UNLOCK(sc);			\
>  				m_freem(m);				\
> -				NET_EPOCH_EXIT_ET(et);			\
>  				return (NULL);				\
>  			}						\
>  		}							\
>  		m->m_pkthdr.rcvif = iface;				\
> -		NET_EPOCH_EXIT_ET(et);					\
> +		BRIDGE_UNLOCK(sc);					\
>  		return (m);						\
>  	}								\
>  									\
> @@ -2540,8 +2519,8 @@ bridge_input(struct ifnet *ifp, struct mbuf *m)
>  	if (memcmp(IF_LLADDR((iface)), eh->ether_shost, ETHER_ADDR_LEN) == 0 \
>  	    OR_CARP_CHECK_WE_ARE_SRC((iface))			\
>  	    ) {								\
> +		BRIDGE_UNLOCK(sc);					\
>  		m_freem(m);						\
> -		NET_EPOCH_EXIT_ET(et);					\
>  		return (NULL);						\
>  	}
>  
> @@ -2572,7 +2551,6 @@ bridge_input(struct ifnet *ifp, struct mbuf *m)
>  	/* Perform the bridge forwarding function. */
>  	bridge_forward(sc, bif, m);
>  
> -	NET_EPOCH_EXIT_ET(et);
>  	return (NULL);
>  }
>  
> @@ -2592,12 +2570,16 @@ bridge_broadcast(struct bridge_softc *sc, struct ifnet
>  	struct bridge_iflist *dbif, *sbif;
>  	struct mbuf *mc;
>  	struct ifnet *dst_if;
> -	int used = 0, i;
> +	int error = 0, used = 0, i;
>  
> -	MPASS(in_epoch(net_epoch_preempt));
> -
>  	sbif = bridge_lookup_member_if(sc, src_if);
>  
> +	BRIDGE_LOCK2REF(sc, error);
> +	if (error) {
> +		m_freem(m);
> +		return;
> +	}
> +
>  	/* Filter on the bridge interface before broadcasting */
>  	if (runfilt && (PFIL_HOOKED(&V_inet_pfil_hook)
>  #ifdef INET6
> @@ -2605,9 +2587,9 @@ bridge_broadcast(struct bridge_softc *sc, struct ifnet
>  #endif
>  	    )) {
>  		if (bridge_pfil(&m, sc->sc_ifp, NULL, PFIL_OUT) != 0)
> -			return;
> +			goto out;
>  		if (m == NULL)
> -			return;
> +			goto out;
>  	}
>  
>  	CK_LIST_FOREACH(dbif, &sc->sc_iflist, bif_next) {
> @@ -2670,6 +2652,9 @@ bridge_broadcast(struct bridge_softc *sc, struct ifnet
>  	}
>  	if (used == 0)
>  		m_freem(m);
> +
> +out:
> +	BRIDGE_UNREF(sc);
>  }
>  
>  /*
> @@ -2685,8 +2670,6 @@ bridge_span(struct bridge_softc *sc, struct mbuf *m)
>  	struct ifnet *dst_if;
>  	struct mbuf *mc;
>  
> -	MPASS(in_epoch(net_epoch_preempt));
> -
>  	if (CK_LIST_EMPTY(&sc->sc_spanlist))
>  		return;
>  
> @@ -2718,8 +2701,7 @@ bridge_rtupdate(struct bridge_softc *sc, const uint8_t
>  	struct bridge_rtnode *brt;
>  	int error;
>  
> -	MPASS(in_epoch(net_epoch_preempt));
> -	BRIDGE_UNLOCK_ASSERT(sc);
> +	BRIDGE_LOCK_ASSERT(sc);
>  
>  	/* Check the source address is valid and not multicast. */
>  	if (ETHER_IS_MULTICAST(dst) ||
> @@ -2736,24 +2718,13 @@ bridge_rtupdate(struct bridge_softc *sc, const uint8_t
>  	 * update it, otherwise create a new one.
>  	 */
>  	if ((brt = bridge_rtnode_lookup(sc, dst, vlan)) == NULL) {
> -		BRIDGE_LOCK(sc);
> -
> -		/* Check again, now that we have the lock. There could have
> -		 * been a race and we only want to insert this once. */
> -		if ((brt = bridge_rtnode_lookup(sc, dst, vlan)) != NULL) {
> -			BRIDGE_UNLOCK(sc);
> -			return (0);
> -		}
> -
>  		if (sc->sc_brtcnt >= sc->sc_brtmax) {
>  			sc->sc_brtexceeded++;
> -			BRIDGE_UNLOCK(sc);
>  			return (ENOSPC);
>  		}
>  		/* Check per interface address limits (if enabled) */
>  		if (bif->bif_addrmax && bif->bif_addrcnt >= bif->bif_addrmax) {
>  			bif->bif_addrexceeded++;
> -			BRIDGE_UNLOCK(sc);
>  			return (ENOSPC);
>  		}
>  
> @@ -2763,11 +2734,8 @@ bridge_rtupdate(struct bridge_softc *sc, const uint8_t
>  		 * address.
>  		 */
>  		brt = uma_zalloc(V_bridge_rtnode_zone, M_NOWAIT | M_ZERO);
> -		if (brt == NULL) {
> -			BRIDGE_UNLOCK(sc);
> +		if (brt == NULL)
>  			return (ENOMEM);
> -		}
> -		brt->brt_vnet = curvnet;
>  
>  		if (bif->bif_flags & IFBIF_STICKY)
>  			brt->brt_flags = IFBAF_STICKY;
> @@ -2779,22 +2747,17 @@ bridge_rtupdate(struct bridge_softc *sc, const uint8_t
>  
>  		if ((error = bridge_rtnode_insert(sc, brt)) != 0) {
>  			uma_zfree(V_bridge_rtnode_zone, brt);
> -			BRIDGE_UNLOCK(sc);
>  			return (error);
>  		}
>  		brt->brt_dst = bif;
>  		bif->bif_addrcnt++;
> -
> -		BRIDGE_UNLOCK(sc);
>  	}
>  
>  	if ((brt->brt_flags & IFBAF_TYPEMASK) == IFBAF_DYNAMIC &&
>  	    brt->brt_dst != bif) {
> -		BRIDGE_LOCK(sc);
>  		brt->brt_dst->bif_addrcnt--;
>  		brt->brt_dst = bif;
>  		brt->brt_dst->bif_addrcnt++;
> -		BRIDGE_UNLOCK(sc);
>  	}
>  
>  	if ((flags & IFBAF_TYPEMASK) == IFBAF_DYNAMIC)
> @@ -2815,7 +2778,7 @@ bridge_rtlookup(struct bridge_softc *sc, const uint8_t
>  {
>  	struct bridge_rtnode *brt;
>  
> -	MPASS(in_epoch(net_epoch_preempt));
> +	BRIDGE_LOCK_ASSERT(sc);
>  
>  	if ((brt = bridge_rtnode_lookup(sc, addr, vlan)) == NULL)
>  		return (NULL);
> @@ -3054,7 +3017,7 @@ bridge_rtnode_lookup(struct bridge_softc *sc, const ui
>  	uint32_t hash;
>  	int dir;
>  
> -	MPASS(in_epoch(net_epoch_preempt));
> +	BRIDGE_LOCK_ASSERT(sc);
>  
>  	hash = bridge_rthash(sc, addr);
>  	CK_LIST_FOREACH(brt, &sc->sc_rthash[hash], brt_hash) {
> @@ -3117,18 +3080,6 @@ out:
>  	return (0);
>  }
>  
> -static void
> -bridge_rtnode_destroy_cb(struct epoch_context *ctx)
> -{
> -	struct bridge_rtnode *brt;
> -
> -	brt = __containerof(ctx, struct bridge_rtnode, brt_epoch_ctx);
> -
> -	CURVNET_SET(brt->brt_vnet);
> -	uma_zfree(V_bridge_rtnode_zone, brt);
> -	CURVNET_RESTORE();
> -}
> -
>  /*
>   * bridge_rtnode_destroy:
>   *
> @@ -3144,9 +3095,7 @@ bridge_rtnode_destroy(struct bridge_softc *sc, struct 
>  	CK_LIST_REMOVE(brt, brt_list);
>  	sc->sc_brtcnt--;
>  	brt->brt_dst->bif_addrcnt--;
> -
> -	epoch_call(net_epoch_preempt, &brt->brt_epoch_ctx,
> -	    bridge_rtnode_destroy_cb);
> +	uma_zfree(V_bridge_rtnode_zone, brt);
>  }
>  
>  /*
> @@ -3691,20 +3640,17 @@ bridge_linkstate(struct ifnet *ifp)
>  {
>  	struct bridge_softc *sc = ifp->if_bridge;
>  	struct bridge_iflist *bif;
> -	struct epoch_tracker et;
>  
> -	NET_EPOCH_ENTER_ET(et);
> -
> +	BRIDGE_LOCK(sc);
>  	bif = bridge_lookup_member_if(sc, ifp);
>  	if (bif == NULL) {
> -		NET_EPOCH_EXIT_ET(et);
> +		BRIDGE_UNLOCK(sc);
>  		return;
>  	}
>  	bridge_linkcheck(sc);
> +	BRIDGE_UNLOCK(sc);
>  
>  	bstp_linkstate(&bif->bif_stp);
> -
> -	NET_EPOCH_EXIT_ET(et);
>  }
>  
>  static void
> @@ -3713,8 +3659,7 @@ bridge_linkcheck(struct bridge_softc *sc)
>  	struct bridge_iflist *bif;
>  	int new_link, hasls;
>  
> -	MPASS(in_epoch(net_epoch_preempt));
> -
> +	BRIDGE_LOCK_ASSERT(sc);
>  	new_link = LINK_STATE_DOWN;
>  	hasls = 0;
>  	/* Our link is considered up if at least one of our ports is active */
> _______________________________________________
> svn-src-stable-12@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-stable-12
> To unsubscribe, send any mail to "svn-src-stable-12-unsubscribe@freebsd.org"
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9e260355-3bba-a615-8536-da17c3158141>