changeset 10654:ff1ef49164ba

6883381 GLDv3 incorrectly assumes mac instance == ddi_get_instance()
author Garrett D'Amore <Garrett.Damore@Sun.COM>
date Fri, 25 Sep 2009 19:43:05 -0700
parents e0ceed15cf0a
children 1fc5061b760c
files usr/src/uts/common/io/dld/dld_str.c usr/src/uts/common/io/dls/dls_link.c usr/src/uts/common/io/dls/dls_mgmt.c usr/src/uts/common/io/mac/mac_provider.c usr/src/uts/common/io/mac/mac_stat.c usr/src/uts/common/io/softmac/softmac_main.c usr/src/uts/common/sys/dld.h usr/src/uts/common/sys/mac.h usr/src/uts/common/sys/mac_impl.h usr/src/uts/common/sys/mac_provider.h usr/src/uts/intel/ia32/ml/modstubs.s usr/src/uts/sparc/ml/modstubs.s
diffstat 12 files changed, 199 insertions(+), 77 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/uts/common/io/dld/dld_str.c	Fri Sep 25 18:57:37 2009 -0700
+++ b/usr/src/uts/common/io/dld/dld_str.c	Fri Sep 25 19:43:05 2009 -0700
@@ -129,6 +129,7 @@
 typedef struct i_dld_str_state_s {
 	major_t		ds_major;
 	minor_t		ds_minor;
+	int		ds_instance;
 	dev_info_t	*ds_dip;
 } i_dld_str_state_t;
 
@@ -152,8 +153,10 @@
 		 * walk if we find a matching stream -- even if we fail
 		 * to obtain the devinfo.
 		 */
-		if (mh != NULL)
+		if (mh != NULL) {
 			statep->ds_dip = mac_devinfo_get(mh);
+			statep->ds_instance = mac_minor(mh) - 1;
+		}
 		return (MH_WALK_TERMINATE);
 	}
 	return (MH_WALK_CONTINUE);
@@ -177,13 +180,58 @@
 	state.ds_minor = getminor(dev);
 	state.ds_major = getmajor(dev);
 	state.ds_dip = NULL;
+	state.ds_instance = -1;
 
 	mod_hash_walk(str_hashp, i_dld_str_walker, &state);
 	return (state.ds_dip);
 }
 
+int
+dld_devt_to_instance(dev_t dev)
+{
+	minor_t			minor;
+	i_dld_str_state_t	state;
+
+	/*
+	 * GLDv3 numbers DLPI style 1 node as the instance number + 1.
+	 * Minor number 0 is reserved for the DLPI style 2 unattached
+	 * node.
+	 */
+
+	if ((minor = getminor(dev)) == 0)
+		return (-1);
+
+	/*
+	 * Check for style 2 unassociated node, this is quick and
+	 * easy.  Note that this doesn't *necessarily* work for legacy
+	 * devices, but this code is only called within the
+	 * getinfo(9e) implementation for true GLDv3 devices, so it
+	 * doesn't matter.
+	 */
+	if (minor > 0 && minor <= DLS_MAX_MINOR) {
+		return (DLS_MINOR2INST(minor));
+	}
+
+	state.ds_minor = getminor(dev);
+	state.ds_major = getmajor(dev);
+	state.ds_dip = NULL;
+	state.ds_instance = -1;
+
+	mod_hash_walk(str_hashp, i_dld_str_walker, &state);
+	return (state.ds_instance);
+}
+
 /*
  * devo_getinfo: getinfo(9e)
+ *
+ * NB: This may be called for a provider before the provider's
+ * instances are attached.  Hence, if a particular provider needs a
+ * special mapping (the mac instance != ddi_get_instance()), then it
+ * may need to provide its own implmentation using the
+ * MAC_MINOR_TO_INSTANCE() function, and translating the returned mac
+ * instance to a devinfo instance.  For dev_t's where the minor number
+ * is too large (i.e. > MAC_MAX_MINOR), the provider can call this
+ * function indirectly via the mac_getinfo() function.
  */
 /*ARGSUSED*/
 int
--- a/usr/src/uts/common/io/dls/dls_link.c	Fri Sep 25 18:57:37 2009 -0700
+++ b/usr/src/uts/common/io/dls/dls_link.c	Fri Sep 25 19:43:05 2009 -0700
@@ -768,7 +768,8 @@
 
 	if ((drv = ddi_major_to_name(getmajor(dev))) == NULL)
 		return (NULL);
-	(void) snprintf(macname, MAXNAMELEN, "%s%d", drv, getminor(dev) - 1);
+	(void) snprintf(macname, MAXNAMELEN, "%s%d", drv,
+	    DLS_MINOR2INST(getminor(dev)));
 
 	/*
 	 * The code below assumes that the name constructed above is the
--- a/usr/src/uts/common/io/dls/dls_mgmt.c	Fri Sep 25 18:57:37 2009 -0700
+++ b/usr/src/uts/common/io/dls/dls_mgmt.c	Fri Sep 25 19:43:05 2009 -0700
@@ -1068,13 +1068,14 @@
 	if ((drv = ddi_major_to_name(getmajor(dev))) == NULL)
 		return (EINVAL);
 
-	(void) snprintf(name, sizeof (name), "%s%d", drv, getminor(dev) - 1);
+	(void) snprintf(name, sizeof (name), "%s%d", drv,
+	    DLS_MINOR2INST(getminor(dev)));
 
 	/*
 	 * Hold this link to prevent it being detached in case of a
 	 * GLDv3 physical link.
 	 */
-	if (getminor(dev) - 1 < MAC_MAX_MINOR)
+	if (DLS_MINOR2INST(getminor(dev)) < DLS_MAX_MINOR)
 		(void) softmac_hold_device(dev, &ddh);
 
 	rw_enter(&i_dls_devnet_lock, RW_WRITER);
@@ -1179,7 +1180,7 @@
 	if ((major = ddi_name_to_major(drv)) == (major_t)-1)
 		return (ENOENT);
 
-	phy_dev = makedevice(major, (minor_t)ppa + 1);
+	phy_dev = makedevice(major, DLS_PPA2MINOR(ppa));
 	if (softmac_hold_device(phy_dev, &ddh) != 0)
 		return (ENOENT);
 
@@ -1232,7 +1233,7 @@
 		return (EINVAL);
 
 	(void) snprintf(macname, sizeof (macname), "%s%d", drv,
-	    getminor(dev) - 1);
+	    DLS_MINOR2INST(getminor(dev)));
 	return (dls_devnet_macname2linkid(macname, linkidp));
 }
 
--- a/usr/src/uts/common/io/mac/mac_provider.c	Fri Sep 25 18:57:37 2009 -0700
+++ b/usr/src/uts/common/io/mac/mac_provider.c	Fri Sep 25 19:43:05 2009 -0700
@@ -40,6 +40,7 @@
 #include <sys/mac_client_impl.h>
 #include <sys/mac_client_priv.h>
 #include <sys/mac_soft_ring.h>
+#include <sys/dld.h>
 #include <sys/modctl.h>
 #include <sys/fs/dv_node.h>
 #include <sys/thread.h>
@@ -315,7 +316,7 @@
 		mip->mi_phy_dev = mip->mi_capab_legacy.ml_dev;
 	} else {
 		mip->mi_phy_dev = makedevice(ddi_driver_major(mip->mi_dip),
-		    ddi_get_instance(mip->mi_dip) + 1);
+		    mip->mi_minor);
 	}
 
 	/*
@@ -825,6 +826,33 @@
 	i_mac_notify(mip, MAC_NOTE_LINK);
 }
 
+/* MINOR NODE HANDLING */
+
+/*
+ * Given a dev_t, return the instance number (PPA) associated with it.
+ * Drivers can use this in their getinfo(9e) implementation to lookup
+ * the instance number (i.e. PPA) of the device, to use as an index to
+ * their own array of soft state structures.
+ *
+ * Returns -1 on error.
+ */
+int
+mac_devt_to_instance(dev_t devt)
+{
+	return (dld_devt_to_instance(devt));
+}
+
+/*
+ * This function returns the first minor number that is available for
+ * driver private use.  All minor numbers smaller than this are
+ * reserved for GLDv3 use.
+ */
+minor_t
+mac_private_minor(void)
+{
+	return (MAC_PRIVATE_MINOR);
+}
+
 /* OTHER CONTROL INFORMATION */
 
 /*
--- a/usr/src/uts/common/io/mac/mac_stat.c	Fri Sep 25 18:57:37 2009 -0700
+++ b/usr/src/uts/common/io/mac/mac_stat.c	Fri Sep 25 19:43:05 2009 -0700
@@ -19,12 +19,10 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
 /*
  * MAC Services Module
  */
@@ -157,16 +155,9 @@
 	major_t		major = getmajor(mip->mi_phy_dev);
 
 	count = MAC_MOD_NKSTAT + MAC_NKSTAT + mip->mi_type->mt_statcount;
-	if (!GLDV3_DRV(major)) {
-		ksp = kstat_create((const char *)ddi_major_to_name(major),
-		    getminor(mip->mi_phy_dev) - 1, MAC_KSTAT_NAME,
-		    MAC_KSTAT_CLASS, KSTAT_TYPE_NAMED, count, 0);
-	} else {
-		major = ddi_driver_major(mip->mi_dip);
-		ksp = kstat_create((const char *)ddi_major_to_name(major),
-		    mip->mi_minor - 1, MAC_KSTAT_NAME,
-		    MAC_KSTAT_CLASS, KSTAT_TYPE_NAMED, count, 0);
-	}
+	ksp = kstat_create((const char *)ddi_major_to_name(major),
+	    getminor(mip->mi_phy_dev) - 1, MAC_KSTAT_NAME,
+	    MAC_KSTAT_CLASS, KSTAT_TYPE_NAMED, count, 0);
 	if (ksp == NULL)
 		return;
 
--- a/usr/src/uts/common/io/softmac/softmac_main.c	Fri Sep 25 18:57:37 2009 -0700
+++ b/usr/src/uts/common/io/softmac/softmac_main.c	Fri Sep 25 19:43:05 2009 -0700
@@ -326,20 +326,50 @@
 		return (ENXIO);
 	}
 
-	ppa = ddi_get_instance(dip);
-	(void) snprintf(devname, MAXNAMELEN, "%s%d", ddi_driver_name(dip), ppa);
+	if (GLDV3_DRV(ddi_driver_major(dip))) {
+		minor_t minor = getminor(dev);
+		/*
+		 * For GLDv3, we don't care about the DLPI style 2
+		 * compatibility node.  (We know that all such devices
+		 * have style 1 nodes.)
+		 */
+		if ((strcmp(ddi_driver_name(dip), "clone") == 0) ||
+		    (getmajor(dev) == ddi_name_to_major("clone")) ||
+		    (minor == 0)) {
+			return (0);
+		}
 
-	/*
-	 * We expect legacy devices have at most two minor nodes - one style-1
-	 * and one style-2.
-	 */
-	if (!GLDV3_DRV(ddi_driver_major(dip)) &&
-	    i_ddi_minor_node_count(dip, DDI_NT_NET) > 2) {
-		cmn_err(CE_WARN, "%s has more than 2 minor nodes; unsupported",
-		    devname);
-		return (ENOTSUP);
+		/*
+		 * Likewise, we know that the minor number for DLPI style 1
+		 * nodes is constrained to a maximum value.
+		 */
+		if (minor >= DLS_MAX_MINOR) {
+			return (ENOTSUP);
+		}
+		/*
+		 * Otherwise we can decode the instance from the minor number,
+		 * which allows for situations with multiple mac instances
+		 * for a single dev_info_t.
+		 */
+		ppa = DLS_MINOR2INST(minor);
+	} else {
+		/*
+		 * For legacy drivers, we just have to limit them to
+		 * two minor nodes, one style 1 and one style 2, and
+		 * we assume the ddi_get_instance() is the PPA.
+		 * Drivers that need more flexibility should be ported
+		 * to GLDv3.
+		 */
+		ppa = ddi_get_instance(dip);
+		if (i_ddi_minor_node_count(dip, DDI_NT_NET) > 2) {
+			cmn_err(CE_WARN, "%s has more than 2 minor nodes; "
+			    "unsupported", devname);
+			return (ENOTSUP);
+		}
 	}
 
+	(void) snprintf(devname, MAXNAMELEN, "%s%d", ddi_driver_name(dip), ppa);
+
 	/*
 	 * Check whether the softmac for the specified device already exists
 	 */
@@ -373,24 +403,13 @@
 		softmac->smac_uppa = ppa;
 
 		/*
-		 * Note that for GLDv3 devices, we create devfs minor nodes
-		 * for VLANs as well. Assume a GLDv3 driver on which only
-		 * a VLAN is created. During the detachment of this device
-		 * instance, the following would happen:
-		 * a. the pre-detach callback softmac_destroy() succeeds.
-		 *    Because the physical link itself is not in use,
-		 *    softmac_destroy() succeeds and destroys softmac_t;
-		 * b. the device detach fails in mac_unregister() because
-		 *    this MAC is still used by a VLAN.
-		 * c. the post-attach callback is then called which leads
-		 *    us here. Note that ddi_minor_node_count() returns 3
-		 *    (including the minior node of the VLAN). In that case,
-		 *    we must correct the minor node count to 2 as that is
-		 *    the count of minor nodes that go through post-attach.
+		 * For GLDv3, we ignore the style 2 node (see the logic
+		 * above on that), and we should have exactly one attach
+		 * per MAC instance (possibly more than one per dev_info_t).
 		 */
 		if (GLDV3_DRV(ddi_driver_major(dip))) {
 			softmac->smac_flags |= SOFTMAC_GLDV3;
-			softmac->smac_cnt = 2;
+			softmac->smac_cnt = 1;
 		} else {
 			softmac->smac_cnt =
 			    i_ddi_minor_node_count(dip, DDI_NT_NET);
@@ -926,7 +945,25 @@
 	mac_handle_t		smac_mh;
 	uint32_t		smac_flags;
 
-	ppa = ddi_get_instance(dip);
+	if (GLDV3_DRV(ddi_driver_major(dip))) {
+		minor_t minor = getminor(dev);
+		/*
+		 * For an explanation of this logic, see the
+		 * equivalent code in softmac_create.
+		 */
+		if ((strcmp(ddi_driver_name(dip), "clone") == 0) ||
+		    (getmajor(dev) == ddi_name_to_major("clone")) ||
+		    (minor == 0)) {
+			return (0);
+		}
+		if (minor >= DLS_MAX_MINOR) {
+			return (ENOTSUP);
+		}
+		ppa = DLS_MINOR2INST(minor);
+	} else {
+		ppa = ddi_get_instance(dip);
+	}
+
 	(void) snprintf(devname, MAXNAMELEN, "%s%d", ddi_driver_name(dip), ppa);
 
 	/*
@@ -1467,7 +1504,24 @@
 	const char	*drvname;
 	char		devname[MAXNAMELEN];
 	softmac_t	*softmac;
-	int		ppa, err;
+	int		ppa, err, inst;
+
+	drvname = ddi_major_to_name(getmajor(dev));
+
+	/*
+	 * Exclude non-physical network device instances, for example, aggr0.
+	 */
+	if (!NETWORK_DRV(getmajor(dev)) || (strcmp(drvname, "aggr") == 0) ||
+	    (strcmp(drvname, "vnic") == 0)) {
+		return (ENOENT);
+	}
+
+	/*
+	 * We have to lookup the device instance using getinfo(9e).
+	 */
+	inst = dev_to_instance(dev);
+	if (inst < 0)
+		return (ENOENT);
 
 	if ((ppa = getminor(dev) - 1) > 1000)
 		return (ENOENT);
@@ -1476,21 +1530,9 @@
 	 * First try to hold this device instance to force the MAC
 	 * to be registered.
 	 */
-	if ((dip = ddi_hold_devi_by_instance(getmajor(dev), ppa, 0)) == NULL)
+	if ((dip = ddi_hold_devi_by_instance(getmajor(dev), inst, 0)) == NULL)
 		return (ENOENT);
 
-	drvname = ddi_driver_name(dip);
-
-	/*
-	 * Exclude non-physical network device instances, for example, aggr0.
-	 */
-	if ((ddi_driver_major(dip) != getmajor(dev)) ||
-	    !NETWORK_DRV(getmajor(dev)) || (strcmp(drvname, "aggr") == 0) ||
-	    (strcmp(drvname, "vnic") == 0)) {
-		ddi_release_devi(dip);
-		return (ENOENT);
-	}
-
 	/*
 	 * This is a network device; wait for its softmac to be registered.
 	 */
--- a/usr/src/uts/common/sys/dld.h	Fri Sep 25 18:57:37 2009 -0700
+++ b/usr/src/uts/common/sys/dld.h	Fri Sep 25 19:43:05 2009 -0700
@@ -403,6 +403,7 @@
 } dld_capab_lso_t;
 
 int	dld_getinfo(dev_info_t *, ddi_info_cmd_t, void *, void **);
+int	dld_devt_to_instance(dev_t);
 int	dld_open(queue_t *, dev_t *, int, int, cred_t *);
 int	dld_close(queue_t *);
 void	dld_wput(queue_t *, mblk_t *);
--- a/usr/src/uts/common/sys/mac.h	Fri Sep 25 18:57:37 2009 -0700
+++ b/usr/src/uts/common/sys/mac.h	Fri Sep 25 19:43:05 2009 -0700
@@ -43,7 +43,7 @@
 /*
  * MAC Information (text emitted by modinfo(1m))
  */
-#define	MAC_INFO	"MAC Services v1.20"
+#define	MAC_INFO	"MAC Services"
 
 /*
  * MAC-Type version identifier.  This is used by mactype_alloc() and
@@ -67,8 +67,6 @@
 #define	DATALINK_ALL_LINKID	0
 #define	DATALINK_MAX_LINKID	0xffffffff
 
-#define	MAC_MAX_MINOR	1000
-
 typedef enum {
 	LINK_STATE_UNKNOWN = -1,
 	LINK_STATE_DOWN,
--- a/usr/src/uts/common/sys/mac_impl.h	Fri Sep 25 18:57:37 2009 -0700
+++ b/usr/src/uts/common/sys/mac_impl.h	Fri Sep 25 19:43:05 2009 -0700
@@ -38,6 +38,27 @@
 extern "C" {
 #endif
 
+/*
+ * This is the first minor number available for MAC provider private
+ * use.  This makes it possible to deliver a driver that is both a MAC
+ * provider and a regular character/block device.  See PSARC 2009/380
+ * for more detail about the construction of such devices.  The value
+ * chosen leaves half of the 32-bit minor numbers (which are really
+ * only 18 bits wide) available for driver private use.  Drivers can
+ * easily identify their private number by the presence of this value
+ * in the bits that make up the minor number, since its just the
+ * highest bit available for such minor numbers.
+ */
+#define	MAC_PRIVATE_MINOR		((MAXMIN32 + 1) / 2)
+
+/*
+ * The maximum minor number that corresponds to a real instance.  This
+ * limits the number of physical ports that a mac provider can offer.
+ * Note that this macro must be synchronized with DLS_MAX_MINOR in
+ * <sys/dls.h>
+ */
+#define	MAC_MAX_MINOR			1000
+
 typedef struct mac_margin_req_s	mac_margin_req_t;
 
 struct mac_margin_req_s {
--- a/usr/src/uts/common/sys/mac_provider.h	Fri Sep 25 18:57:37 2009 -0700
+++ b/usr/src/uts/common/sys/mac_provider.h	Fri Sep 25 19:43:05 2009 -0700
@@ -50,19 +50,6 @@
 #define	MAC_VERSION	0x2
 
 /*
- * This is the first minor number available for MAC provider private
- * use.  This makes it possible to deliver a driver that is both a MAC
- * provider and a regular character/block device.  See PSARC 2009/380
- * for more detail about the construction of such devices.  The value
- * chosen leaves half of the 32-bit minor numbers (which are really
- * only 18 bits wide) available for driver private use.  Drivers can
- * easily identify their private number by the presence of this value
- * in the bits that make up the minor number, since its just the
- * highest bit available for such minor numbers.
- */
-#define	MAC_PRIVATE_MINOR	((MAXMIN32 + 1) / 2)
-
-/*
  * Opaque handle types
  */
 typedef struct __mac_rule_handle	*mac_rule_handle_t;
@@ -481,6 +468,8 @@
 				    boolean_t);
 extern void			mac_init_ops(struct dev_ops *, const char *);
 extern void			mac_fini_ops(struct dev_ops *);
+extern int			mac_devt_to_instance(dev_t);
+extern minor_t			mac_private_minor(void);
 
 extern mactype_register_t	*mactype_alloc(uint_t);
 extern void			mactype_free(mactype_register_t *);
--- a/usr/src/uts/intel/ia32/ml/modstubs.s	Fri Sep 25 18:57:37 2009 -0700
+++ b/usr/src/uts/intel/ia32/ml/modstubs.s	Fri Sep 25 19:43:05 2009 -0700
@@ -1256,6 +1256,7 @@
 	MODULE(dld,drv);
 	STUB(dld, dld_init_ops, nomod_void);
 	STUB(dld, dld_fini_ops, nomod_void);
+	STUB(dld, dld_devt_to_instance, nomod_minus_one);
 	STUB(dld, dld_autopush, nomod_minus_one);
 	STUB(dld, dld_ioc_register, nomod_einval);
 	STUB(dld, dld_ioc_unregister, nomod_void);
--- a/usr/src/uts/sparc/ml/modstubs.s	Fri Sep 25 18:57:37 2009 -0700
+++ b/usr/src/uts/sparc/ml/modstubs.s	Fri Sep 25 19:43:05 2009 -0700
@@ -1201,6 +1201,7 @@
 	STUB(dld, dld_init_ops, nomod_void);
 	STUB(dld, dld_fini_ops, nomod_void);
 	STUB(dld, dld_autopush, nomod_minus_one);
+	STUB(dld, dld_devt_to_instance, nomod_minus_one);
 	STUB(dld, dld_ioc_register, nomod_einval);
 	STUB(dld, dld_ioc_unregister, nomod_void);
 	END_MODULE(dld);