OS-6040 DLS stat delete and aggr kstat can deadlock

Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Approved by: Jerry Jelinek <jerry.jelinek@joyent.com>

3 files changed, 33 insertions, 6 deletions

diff --git a/usr/src/uts/common/io/aggr/aggr_grp.c b/usr/src/uts/common/io/aggr/aggr_grp.c
index ad66e342bd..d25e7c21ba 100644
--- a/usr/src/uts/common/io/aggr/aggr_grp.c
+++ b/usr/src/uts/common/io/aggr/aggr_grp.c
@@ -324,6 +324,7 @@ aggr_grp_attach_port(aggr_grp_t *grp, aggr_port_t *port)
 		return (B_FALSE);
 	}
 
+	mutex_enter(&grp->lg_stat_lock);
 	if (grp->lg_ifspeed == 0) {
 		/*
 		 * The group inherits the speed of the first link being
@@ -337,8 +338,10 @@ aggr_grp_attach_port(aggr_grp_t *grp, aggr_port_t *port)
 		 * the group link speed, as per 802.3ad. Since it is
 		 * not, the attach is cancelled.
 		 */
+		mutex_exit(&grp->lg_stat_lock);
 		return (B_FALSE);
 	}
+	mutex_exit(&grp->lg_stat_lock);
 
 	grp->lg_nattached_ports++;
 
@@ -347,7 +350,9 @@ aggr_grp_attach_port(aggr_grp_t *grp, aggr_port_t *port)
 	 */
 	if (grp->lg_link_state != LINK_STATE_UP) {
 		grp->lg_link_state = LINK_STATE_UP;
+		mutex_enter(&grp->lg_stat_lock);
 		grp->lg_link_duplex = LINK_DUPLEX_FULL;
+		mutex_exit(&grp->lg_stat_lock);
 		link_state_changed = B_TRUE;
 	}
 
@@ -405,9 +410,11 @@ aggr_grp_detach_port(aggr_grp_t *grp, aggr_port_t *port)
 	grp->lg_nattached_ports--;
 	if (grp->lg_nattached_ports == 0) {
 		/* the last attached MAC port of the group is being detached */
-		grp->lg_ifspeed = 0;
 		grp->lg_link_state = LINK_STATE_DOWN;
+		mutex_enter(&grp->lg_stat_lock);
+		grp->lg_ifspeed = 0;
 		grp->lg_link_duplex = LINK_DUPLEX_UNKNOWN;
+		mutex_exit(&grp->lg_stat_lock);
 		link_state_changed = B_TRUE;
 	}
 
@@ -1545,7 +1552,9 @@ aggr_grp_rem_port(aggr_grp_t *grp, aggr_port_t *port,
 			continue;
 		val = aggr_port_stat(port, stat);
 		val -= port->lp_stat[i];
+		mutex_enter(&grp->lg_stat_lock);
 		grp->lg_stat[i] += val;
+		mutex_exit(&grp->lg_stat_lock);
 	}
 	for (i = 0; i < ETHER_NSTAT; i++) {
 		stat = i + MACTYPE_STAT_MIN;
@@ -1553,7 +1562,9 @@ aggr_grp_rem_port(aggr_grp_t *grp, aggr_port_t *port,
 			continue;
 		val = aggr_port_stat(port, stat);
 		val -= port->lp_ether_stat[i];
+		mutex_enter(&grp->lg_stat_lock);
 		grp->lg_ether_stat[i] += val;
+		mutex_exit(&grp->lg_stat_lock);
 	}
 
 	grp->lg_nports--;
@@ -1884,6 +1895,8 @@ aggr_grp_stat(aggr_grp_t *grp, uint_t stat, uint64_t *val)
 	aggr_port_t	*port;
 	uint_t		stat_index;
 
+	ASSERT(MUTEX_HELD(&grp->lg_stat_lock));
+
 	/* We only aggregate counter statistics. */
 	if (IS_MAC_STAT(stat) && !MAC_STAT_ISACOUNTER(stat) ||
 	    IS_MACTYPE_STAT(stat) && !ETHER_STAT_ISACOUNTER(stat)) {
@@ -1952,10 +1965,9 @@ static int
 aggr_m_stat(void *arg, uint_t stat, uint64_t *val)
 {
 	aggr_grp_t		*grp = arg;
-	mac_perim_handle_t	mph;
 	int			rval = 0;
 
-	mac_perim_enter_by_mh(grp->lg_mh, &mph);
+	mutex_enter(&grp->lg_stat_lock);
 
 	switch (stat) {
 	case MAC_STAT_IFSPEED:
@@ -1975,7 +1987,7 @@ aggr_m_stat(void *arg, uint_t stat, uint64_t *val)
 		rval = aggr_grp_stat(grp, stat, val);
 	}
 
-	mac_perim_exit(mph);
+	mutex_exit(&grp->lg_stat_lock);
 	return (rval);
 }
 
diff --git a/usr/src/uts/common/io/aggr/aggr_port.c b/usr/src/uts/common/io/aggr/aggr_port.c
index a39110255a..b4320bec80 100644
--- a/usr/src/uts/common/io/aggr/aggr_port.c
+++ b/usr/src/uts/common/io/aggr/aggr_port.c
@@ -22,6 +22,7 @@
  * Copyright 2010 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  * Copyright 2012 OmniTI Computer Consulting, Inc  All rights reserved.
+ * Copyright (c) 2017 Joyent, Inc.
  */
 
 /*
@@ -374,10 +375,14 @@ aggr_port_notify_link(aggr_grp_t *grp, aggr_port_t *port)
 	/* link speed changes? */
 	ifspeed = aggr_port_stat(port, MAC_STAT_IFSPEED);
 	if (port->lp_ifspeed != ifspeed) {
+		mutex_enter(&grp->lg_stat_lock);
+
 		if (port->lp_state == AGGR_PORT_STATE_ATTACHED)
 			do_detach |= (ifspeed != grp->lg_ifspeed);
 		else
 			do_attach |= (ifspeed == grp->lg_ifspeed);
+
+		mutex_exit(&grp->lg_stat_lock);
 	}
 	port->lp_ifspeed = ifspeed;
 
@@ -529,7 +534,9 @@ aggr_port_promisc(aggr_port_t *port, boolean_t on)
 
 	if (on) {
 		mac_rx_clear(port->lp_mch);
-		/* We use the promisc callback because without hardware
+
+		/*
+		 * We use the promisc callback because without hardware
 		 * rings, we deliver through flows that will cause duplicate
 		 * delivery of packets when we've flipped into this mode
 		 * to compensate for the lack of hardware MAC matching
diff --git a/usr/src/uts/common/sys/aggr_impl.h b/usr/src/uts/common/sys/aggr_impl.h
index a4c0409304..0964628f71 100644
--- a/usr/src/uts/common/sys/aggr_impl.h
+++ b/usr/src/uts/common/sys/aggr_impl.h
@@ -22,6 +22,7 @@
  * Copyright 2010 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  * Copyright 2012 OmniTI Computer Consulting, Inc  All rights reserved.
+ * Copyright (c) 2017 Joyent, Inc.
  */
 
 #ifndef	_SYS_AGGR_IMPL_H
@@ -187,11 +188,18 @@ typedef struct aggr_grp_s {
 	uint_t		lg_tx_ports_size;	/* size of lg_tx_ports */
 	uint32_t	lg_tx_policy;		/* outbound policy */
 	uint8_t		lg_mac_tx_policy;
-	uint64_t	lg_ifspeed;
 	link_state_t	lg_link_state;
+
+
+	/*
+	 * The lg_stat_lock must be held when accessing these fields.
+	 */
+	kmutex_t	lg_stat_lock;
+	uint64_t	lg_ifspeed;
 	link_duplex_t	lg_link_duplex;
 	uint64_t	lg_stat[MAC_NSTAT];
 	uint64_t	lg_ether_stat[ETHER_NSTAT];
+
 	aggr_lacp_mode_t lg_lacp_mode;		/* off, active, or passive */
 	Agg_t		aggr;			/* 802.3ad data */
 	uint32_t	lg_hcksum_txflags;