MFC: r285909, r285913 (partial)

- Probe UICLASS_CDC/UISUBCLASS_ABSTRACT_CONTROL_MODEL/0xff again. This
  variant of Microsoft RNDIS, i. e. their unofficial version of CDC ACM,
  has been disabled in r261544 (r262363 in stable/10) for resolving a
  conflict with umodem(4). Eventually, in r275790 (r276243 in stable/10)
  that problem was dealt with in the right way. However, r275790 failed
  to put probing of RNDIS devices in question back.
- Initialize the device prior to querying it, as required by the RNDIS
  specification. Otherwise already determining the MAC address may fail
  rightfully.
- On detach, halt the device again.
- Use UCDC_SEND_ENCAPSULATED_{COMMAND,RESPONSE}. While these macros are
  resolving to the same values as UR_{CLEAR_FEATURE,GET_STATUS}, the
  former set is way more appropriate in this context.
- Report unknown - rather: unimplemented - events unconditionally and
  not just in debug mode. This ensures that we'll get some hint of what
  is going wrong instead of the driver silently failing.
- Deal with the Microsoft ActiveSync requirement of using an input buffer
  the size of the expected reply or larger - except for variably sized
  replies - when querying a device.
- Fix some pointless NULL checks, style bugs etc.

This changes allow urndis(4) to communicate with a Microsoft-certified
USB RNDIS test token.

1 files changed, 107 insertions, 71 deletions

diff --git a/sys/dev/usb/net/if_urndis.c b/sys/dev/usb/net/if_urndis.c
index 6509e78..5d2a637 100644
--- a/sys/dev/usb/net/if_urndis.c
+++ b/sys/dev/usb/net/if_urndis.c
@@ -78,12 +78,20 @@ static uether_fn_t urndis_start;
 static uether_fn_t urndis_setmulti;
 static uether_fn_t urndis_setpromisc;
 
-static uint32_t urndis_ctrl_query(struct urndis_softc *, uint32_t, const void **, uint16_t *);
-static uint32_t urndis_ctrl_set(struct urndis_softc *, uint32_t, struct urndis_set_req *, uint16_t);
-static uint32_t urndis_ctrl_handle_init(struct urndis_softc *, const struct urndis_comp_hdr *);
-static uint32_t urndis_ctrl_handle_query(struct urndis_softc *, const struct urndis_comp_hdr *, const void **, uint16_t *);
-static uint32_t urndis_ctrl_handle_reset(struct urndis_softc *, const struct urndis_comp_hdr *);
-static uint32_t urndis_ctrl_init(struct urndis_softc *);
+static uint32_t	urndis_ctrl_query(struct urndis_softc *sc, uint32_t oid,
+		    struct urndis_query_req *msg, uint16_t len,
+		    const void **rbuf, uint16_t *rbufsz);
+static uint32_t	urndis_ctrl_set(struct urndis_softc *sc, uint32_t oid,
+		    struct urndis_set_req *msg, uint16_t len);
+static uint32_t	urndis_ctrl_handle_init(struct urndis_softc *sc,
+		    const struct urndis_comp_hdr *hdr);
+static uint32_t	urndis_ctrl_handle_query(struct urndis_softc *sc,
+		    const struct urndis_comp_hdr *hdr, const void **buf,
+		    uint16_t *bufsz);
+static uint32_t	urndis_ctrl_handle_reset(struct urndis_softc *sc,
+		    const struct urndis_comp_hdr *hdr);
+static uint32_t	urndis_ctrl_init(struct urndis_softc *sc);
+static uint32_t	urndis_ctrl_halt(struct urndis_softc *sc);
 
 #ifdef USB_DEBUG
 static int urndis_debug = 0;
@@ -93,7 +101,6 @@ SYSCTL_INT(_hw_usb_urndis, OID_AUTO, debug, CTLFLAG_RW, &urndis_debug, 0,
 #endif
 
 static const struct usb_config urndis_config[URNDIS_N_TRANSFER] = {
-
 	[URNDIS_BULK_RX] = {
 		.type = UE_BULK,
 		.endpoint = UE_ADDR_ANY,
@@ -154,7 +161,7 @@ static driver_t urndis_driver = {
 
 static devclass_t urndis_devclass;
 
-DRIVER_MODULE(urndis, uhub, urndis_driver, urndis_devclass, NULL, 0);
+DRIVER_MODULE(urndis, uhub, urndis_driver, urndis_devclass, NULL, NULL);
 MODULE_VERSION(urndis, 1);
 MODULE_DEPEND(urndis, uether, 1, 1, 1);
 MODULE_DEPEND(urndis, usb, 1, 1, 1);
@@ -171,6 +178,9 @@ static const struct usb_ether_methods urndis_ue_methods = {
 
 static const STRUCT_USB_HOST_ID urndis_host_devs[] = {
 	/* Generic RNDIS class match */
+	{USB_IFACE_CLASS(UICLASS_CDC),
+		USB_IFACE_SUBCLASS(UISUBCLASS_ABSTRACT_CONTROL_MODEL),
+		USB_IFACE_PROTOCOL(0xff)},
 	{USB_IFACE_CLASS(UICLASS_WIRELESS), USB_IFACE_SUBCLASS(UISUBCLASS_RF),
 		USB_IFACE_PROTOCOL(UIPROTO_RNDIS)},
 	{USB_IFACE_CLASS(UICLASS_IAD), USB_IFACE_SUBCLASS(UISUBCLASS_SYNC),
@@ -192,21 +202,27 @@ urndis_probe(device_t dev)
 static void
 urndis_attach_post(struct usb_ether *ue)
 {
+
 	/* no-op */
-	return;
 }
 
 static int
 urndis_attach(device_t dev)
 {
+	static struct {
+		union {
+			struct urndis_query_req query;
+			struct urndis_set_req set;
+		} hdr;
+		union {
+			uint8_t eaddr[ETHER_ADDR_LEN];
+			uint32_t filter;
+		} ibuf;
+	} msg;
 	struct urndis_softc *sc = device_get_softc(dev);
 	struct usb_ether *ue = &sc->sc_ue;
 	struct usb_attach_arg *uaa = device_get_ivars(dev);
 	struct usb_cdc_cm_descriptor *cmd;
-	struct {
-		struct urndis_set_req hdr;
-		uint32_t filter;
-	} msg_filter;
 	const void *buf;
 	uint16_t bufsz;
 	uint8_t iface_index[2] = { uaa->info.bIfaceIndex + 1, uaa->info.bIfaceIndex };
@@ -228,9 +244,7 @@ urndis_attach(device_t dev)
 	mtx_init(&sc->sc_mtx, device_get_nameunit(dev), NULL, MTX_DEF);
 
 	/* scan the alternate settings looking for a valid one */
-
 	for (i = 0; i != 32; i++) {
-
 		error = usbd_set_alt_interface_index(uaa->device,
 		    iface_index[0], i);
 
@@ -244,16 +258,27 @@ urndis_attach(device_t dev)
 		if (error == 0)
 			break;
 	}
-
 	if ((error != 0) || (i == 32)) {
-		device_printf(dev, "No valid alternate "
-		    "setting found\n");
+		device_printf(dev, "No valid alternate setting found\n");
 		goto detach;
 	}
+
+	/* Initialize device - must be done before even querying it */
 	URNDIS_LOCK(sc);
-	error = urndis_ctrl_query(sc, OID_802_3_PERMANENT_ADDRESS, &buf, &bufsz);
+	error = urndis_ctrl_init(sc);
 	URNDIS_UNLOCK(sc);
+	if (error != (int)RNDIS_STATUS_SUCCESS) {
+		device_printf(dev, "Unable to initialize hardware\n");
+		goto detach;
+	}
 
+	/* Determine MAC address */
+	memset(msg.ibuf.eaddr, 0, sizeof(msg.ibuf.eaddr));
+	URNDIS_LOCK(sc);
+	error = urndis_ctrl_query(sc, OID_802_3_PERMANENT_ADDRESS,
+	    &msg.hdr.query, sizeof(msg.hdr.query) + sizeof(msg.ibuf.eaddr),
+	    &buf, &bufsz);
+	URNDIS_UNLOCK(sc);
 	if (error != (int)RNDIS_STATUS_SUCCESS) {
 		device_printf(dev, "Unable to get hardware address\n");
 		goto detach;
@@ -267,17 +292,16 @@ urndis_attach(device_t dev)
 	/* Initialize packet filter */
 	sc->sc_filter = RNDIS_PACKET_TYPE_BROADCAST |
 	    RNDIS_PACKET_TYPE_ALL_MULTICAST;
-	msg_filter.filter = htole32(sc->sc_filter);
-
+	msg.ibuf.filter = htole32(sc->sc_filter);
 	URNDIS_LOCK(sc);
 	error = urndis_ctrl_set(sc, OID_GEN_CURRENT_PACKET_FILTER,
-	    &msg_filter.hdr, sizeof(msg_filter));
+	    &msg.hdr.set, sizeof(msg.hdr.set) + sizeof(msg.ibuf.filter));
 	URNDIS_UNLOCK(sc);
-
 	if (error != (int)RNDIS_STATUS_SUCCESS) {
 		device_printf(dev, "Unable to set data filters\n");
 		goto detach;
 	}
+
 	ue->ue_sc = sc;
 	ue->ue_dev = dev;
 	ue->ue_udev = uaa->device;
@@ -298,7 +322,7 @@ urndis_attach(device_t dev)
 	return (0);			/* success */
 
 detach:
-	urndis_detach(dev);
+	(void)urndis_detach(dev);
 	return (ENXIO);			/* failure */
 }
 
@@ -313,6 +337,10 @@ urndis_detach(device_t dev)
 
 	uether_ifdetach(ue);
 
+	URNDIS_LOCK(sc);
+	(void)urndis_ctrl_halt(sc);
+	URNDIS_UNLOCK(sc);
+
 	mtx_destroy(&sc->sc_mtx);
 
 	return (0);
@@ -340,8 +368,6 @@ urndis_init(struct usb_ether *ue)
 
 	ifp->if_drv_flags |= IFF_DRV_RUNNING;
 
-	urndis_ctrl_init(sc);
-
 	/* stall data write direction, which depends on USB mode */
 	usbd_xfer_set_stall(sc->sc_xfer[URNDIS_BULK_TX]);
 
@@ -369,20 +395,21 @@ urndis_stop(struct usb_ether *ue)
 static void
 urndis_setmulti(struct usb_ether *ue)
 {
+
 	/* no-op */
-	return;
 }
 
 static void
 urndis_setpromisc(struct usb_ether *ue)
 {
+
 	/* no-op */
-	return;
 }
 
 static int
 urndis_suspend(device_t dev)
 {
+
 	device_printf(dev, "Suspending\n");
 	return (0);
 }
@@ -390,6 +417,7 @@ urndis_suspend(device_t dev)
 static int
 urndis_resume(device_t dev)
 {
+
 	device_printf(dev, "Resuming\n");
 	return (0);
 }
@@ -416,8 +444,8 @@ urndis_ctrl_send(struct urndis_softc *sc, void *buf, uint16_t len)
 {
 	usb_error_t err;
 
-	err = urndis_ctrl_msg(sc, UT_WRITE_CLASS_INTERFACE, UR_GET_STATUS,
-	    sc->sc_ifaceno_ctl, 0, buf, len);
+	err = urndis_ctrl_msg(sc, UT_WRITE_CLASS_INTERFACE,
+	    UCDC_SEND_ENCAPSULATED_COMMAND, sc->sc_ifaceno_ctl, 0, buf, len);
 
 	DPRINTF("%s\n", usbd_errstr(err));
 
@@ -430,8 +458,9 @@ urndis_ctrl_recv(struct urndis_softc *sc)
 	struct urndis_comp_hdr *hdr;
 	usb_error_t err;
 
-	err = urndis_ctrl_msg(sc, UT_READ_CLASS_INTERFACE, UR_CLEAR_FEATURE,
-	    sc->sc_ifaceno_ctl, 0, sc->sc_response_buf, RNDIS_RESPONSE_LEN);
+	err = urndis_ctrl_msg(sc, UT_READ_CLASS_INTERFACE,
+	    UCDC_GET_ENCAPSULATED_RESPONSE, sc->sc_ifaceno_ctl, 0,
+	    sc->sc_response_buf, RNDIS_RESPONSE_LEN);
 
 	if (err != USB_ERR_NORMAL_COMPLETION)
 		return (NULL);
@@ -480,7 +509,8 @@ urndis_ctrl_handle(struct urndis_softc *sc, struct urndis_comp_hdr *hdr,
 		break;
 
 	default:
-		DPRINTF("ctrl message error: unknown event 0x%x\n",
+		device_printf(sc->sc_ue.ue_dev,
+		    "ctrl message error: unknown event 0x%x\n",
 		    le32toh(hdr->rm_type));
 		rval = RNDIS_STATUS_FAILURE;
 		break;
@@ -548,10 +578,8 @@ urndis_ctrl_handle_query(struct urndis_softc *sc,
 	    le32toh(msg->rm_infobuflen),
 	    le32toh(msg->rm_infobufoffset));
 
-	if (buf != NULL && bufsz != NULL) {
-		*buf = NULL;
-		*bufsz = 0;
-	}
+	*buf = NULL;
+	*bufsz = 0;
 	if (le32toh(msg->rm_status) != RNDIS_STATUS_SUCCESS) {
 		DPRINTF("query failed 0x%x\n", le32toh(msg->rm_status));
 		return (le32toh(msg->rm_status));
@@ -571,10 +599,10 @@ urndis_ctrl_handle_query(struct urndis_softc *sc,
 		    le32toh(msg->rm_len));
 		return (RNDIS_STATUS_FAILURE);
 	}
-	if (buf != NULL && bufsz != NULL) {
-		*buf = ((const uint8_t *)msg) + RNDIS_HEADER_OFFSET + le32toh(msg->rm_infobufoffset);
-		*bufsz = le32toh(msg->rm_infobuflen);
-	}
+	*buf = ((const uint8_t *)msg) + RNDIS_HEADER_OFFSET +
+	    le32toh(msg->rm_infobufoffset);
+	*bufsz = le32toh(msg->rm_infobuflen);
+
 	return (le32toh(msg->rm_status));
 }
 
@@ -627,7 +655,7 @@ urndis_ctrl_init(struct urndis_softc *sc)
 
 	msg.rm_type = htole32(REMOTE_NDIS_INITIALIZE_MSG);
 	msg.rm_len = htole32(sizeof(msg));
-	msg.rm_rid = htole32(0);
+	msg.rm_rid = 0;
 	msg.rm_ver_major = htole32(1);
 	msg.rm_ver_minor = htole32(1);
 	msg.rm_max_xfersz = htole32(RNDIS_RX_MAXLEN);
@@ -656,7 +684,6 @@ urndis_ctrl_init(struct urndis_softc *sc)
 	return (rval);
 }
 
-#if 0
 static uint32_t
 urndis_ctrl_halt(struct urndis_softc *sc)
 {
@@ -675,39 +702,48 @@ urndis_ctrl_halt(struct urndis_softc *sc)
 	rval = urndis_ctrl_send(sc, &msg, sizeof(msg));
 
 	if (rval != RNDIS_STATUS_SUCCESS)
-		printf("halt failed\n");
+		DPRINTF("halt failed\n");
 
 	return (rval);
 }
 
-#endif
-
+/*
+ * NB: Querying a device has the requirment of using an input buffer the size
+ *     of the expected reply or larger, except for variably sized replies.
+ */
 static uint32_t
-urndis_ctrl_query(struct urndis_softc *sc, uint32_t oid, const void **rbuf, uint16_t *rbufsz)
+urndis_ctrl_query(struct urndis_softc *sc, uint32_t oid,
+    struct urndis_query_req *msg, uint16_t len, const void **rbuf,
+    uint16_t *rbufsz)
 {
-	struct urndis_query_req msg;
-	uint32_t rval;
 	struct urndis_comp_hdr *hdr;
+	uint32_t datalen, rval;
 
-	msg.rm_type = htole32(REMOTE_NDIS_QUERY_MSG);
-	msg.rm_len = htole32(sizeof(msg));
-	msg.rm_rid = 0;			/* XXX */
-	msg.rm_oid = htole32(oid);
-	msg.rm_infobuflen = htole32(0);
-	msg.rm_infobufoffset = 0;
-	msg.rm_devicevchdl = 0;
+	msg->rm_type = htole32(REMOTE_NDIS_QUERY_MSG);
+	msg->rm_len = htole32(len);
+	msg->rm_rid = 0;		/* XXX */
+	msg->rm_oid = htole32(oid);
+	datalen = len - sizeof(*msg);
+	msg->rm_infobuflen = htole32(datalen);
+	if (datalen != 0) {
+		msg->rm_infobufoffset = htole32(sizeof(*msg) -
+		    RNDIS_HEADER_OFFSET);
+	} else {
+		msg->rm_infobufoffset = 0;
+	}
+	msg->rm_devicevchdl = 0;
 
 	DPRINTF("type %u len %u rid %u oid 0x%x "
 	    "infobuflen %u infobufoffset %u devicevchdl %u\n",
-	    le32toh(msg.rm_type),
-	    le32toh(msg.rm_len),
-	    le32toh(msg.rm_rid),
-	    le32toh(msg.rm_oid),
-	    le32toh(msg.rm_infobuflen),
-	    le32toh(msg.rm_infobufoffset),
-	    le32toh(msg.rm_devicevchdl));
+	    le32toh(msg->rm_type),
+	    le32toh(msg->rm_len),
+	    le32toh(msg->rm_rid),
+	    le32toh(msg->rm_oid),
+	    le32toh(msg->rm_infobuflen),
+	    le32toh(msg->rm_infobufoffset),
+	    le32toh(msg->rm_devicevchdl));
 
-	rval = urndis_ctrl_send(sc, &msg, sizeof(msg));
+	rval = urndis_ctrl_send(sc, msg, len);
 
 	if (rval != RNDIS_STATUS_SUCCESS) {
 		DPRINTF("query failed\n");
@@ -723,19 +759,21 @@ urndis_ctrl_query(struct urndis_softc *sc, uint32_t oid, const void **rbuf, uint
 }
 
 static uint32_t
-urndis_ctrl_set(struct urndis_softc *sc, uint32_t oid, struct urndis_set_req *msg, uint16_t len)
+urndis_ctrl_set(struct urndis_softc *sc, uint32_t oid,
+    struct urndis_set_req *msg, uint16_t len)
 {
 	struct urndis_comp_hdr *hdr;
-	uint32_t rval;
-	uint32_t datalen = len - sizeof(*msg);
+	uint32_t datalen, rval;
 
 	msg->rm_type = htole32(REMOTE_NDIS_SET_MSG);
 	msg->rm_len = htole32(len);
 	msg->rm_rid = 0;		/* XXX */
 	msg->rm_oid = htole32(oid);
+	datalen = len - sizeof(*msg);
 	msg->rm_infobuflen = htole32(datalen);
 	if (datalen != 0) {
-		msg->rm_infobufoffset = htole32(sizeof(*msg) - RNDIS_HEADER_OFFSET);
+		msg->rm_infobufoffset = htole32(sizeof(*msg) -
+		    RNDIS_HEADER_OFFSET);
 	} else {
 		msg->rm_infobufoffset = 0;
 	}
@@ -782,13 +820,11 @@ urndis_bulk_read_callback(struct usb_xfer *xfer, usb_error_t error)
 
 	switch (USB_GET_STATE(xfer)) {
 	case USB_ST_TRANSFERRED:
-
 		usbd_xfer_status(xfer, &actlen, NULL, &aframes, NULL);
 
 		DPRINTFN(1, "received %u bytes in %u frames\n", actlen, aframes);
 
 		for (offset = 0; actlen >= (uint32_t)sizeof(msg);) {
-
 			/* copy out header */
 			usbd_copy_out(pc, offset, &msg, sizeof(msg));
 
@@ -930,7 +966,7 @@ tr_setup:
 
 			usbd_xfer_set_frame_offset(xfer, x * RNDIS_TX_MAXLEN, x);
 
-	next_pkt:
+next_pkt:
 			IFQ_DRV_DEQUEUE(&ifp->if_snd, m);
 
 			if (m == NULL)