Skip to contentSkip to conversation

RNDIS filter issue

RFC In review Oliver Neukum 2 participants 2 messages May 18, 2017

Subsystems (4)
Networking Network drivers USB networking drivers USB cdc Ethernet driver
lore
Share & tools
lore

Oliver Neukum

author
#0 lore raw

RFC 1/1 RNDIS filter issue

Hi,

now, as this patch is conceptually not my thing, here for review.

Regards
Oliver

From f8f775817b6f3bd636e3b45cb34db4af279492ff Mon Sep 17 00:00:00 2001
From: Oliver Neukum <[email protected]>
Date: Wed, 17 May 2017 11:40:12 +0200
Subject: [PATCH] cdc-ether: divorce initialisation with a filter reset and a
generic method

Some devices need their multicast filter reset but others are crashed by that. So the methods need to be separated.

Signed-off-by Oliver Neukum <[email protected]>
Reported-by "Ridgway, Keith" <[email protected]>
---
 drivers/net/usb/cdc_ether.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)
drivers/net/usb/cdc_ether.c +24 -8
index f3ae88fdf332..311f6df57316 100644
@@ -310,6 +310,26 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 		return -ENODEV;
 	}
 
+	return 0;
+
+bad_desc:
+	dev_info(&dev->udev->dev, "bad CDC descriptors\n");
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(usbnet_generic_cdc_bind);
+
+
+/* like usbnet_generic_cdc_bind() but handles filter initialization
+ * correctly
+ */
+int usbnet_ether_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+	int rv;
+
+	rv = usbnet_generic_cdc_bind(dev, intf);
+	if (rv < 0)
+		goto bail_out;
+
 	/* Some devices don't initialise properly. In particular
 	 * the packet filter is not reset. There are devices that
 	 * don't do reset all the way. So the packet filter should

@@ -317,13 +337,10 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 	 */
 	usbnet_cdc_update_filter(dev);
 
-	return 0;
-
-bad_desc:
-	dev_info(&dev->udev->dev, "bad CDC descriptors\n");
-	return -ENODEV;
+bail_out:
+	return rv;
 }
-EXPORT_SYMBOL_GPL(usbnet_generic_cdc_bind);
+EXPORT_SYMBOL_GPL(usbnet_ether_cdc_bind);
 
 void usbnet_cdc_unbind(struct usbnet *dev, struct usb_interface *intf)
 {

@@ -417,7 +434,7 @@ int usbnet_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 	BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data)
 			< sizeof(struct cdc_state)));
 
-	status = usbnet_generic_cdc_bind(dev, intf);
+	status = usbnet_ether_cdc_bind(dev, intf);
 	if (status < 0)
 		return status;
 

@@ -472,7 +489,6 @@ static void usbnet_cdc_zte_status(struct usbnet *dev, struct urb *urb)
 
 	if (urb->actual_length < sizeof(*event))
 		return;
-
 	event = urb->transfer_buffer;
 
 	if (event->bNotificationType != USB_CDC_NOTIFY_NETWORK_CONNECTION) {
Signature · 1 line
2.12.0
#1 lore raw

drivers/net/usb/cdc_ether.c static void usbnet_cdc_zte_status(struct usbnet *dev, struct urb *urb) · line 494

Quoted 8 lines · Oliver Neukum
drivers/net/usb/cdc_ether.c +0 -1
@@ -472,7 +489,6 @@ static void usbnet_cdc_zte_status(struct usbnet *dev, struct urb *urb)
 
 	if (urb->actual_length < sizeof(*event))
 		return;
-
 	event = urb->transfer_buffer;
 
 	if (event->bNotificationType != USB_CDC_NOTIFY_NETWORK_CONNECTION) {

Spurious whitespace.

And building gives:

/usr/local/src/git/linux/drivers/net/usb/cdc_ether.c:325:5: warning: symbol 'usbnet_ether_cdc_bind' was not declared. Should it be static?

and maybe it should? There is no use for it in any other module, yet. Or you could declare it in usbnet.h, I guess. Your choice.

Looks good to me if those two minor issues are fixed. But do note that I currently don't have any way to test this on real hardware...

Bjørn