Re: Bug 103411 - Ethernet frames get broken for g_ether / g_cdc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2015-12-01 at 10:41 +0100, Alexander Krause wrote:

> Messages on the host are:
> [44685.879438] usb 3-1.2: new high-speed USB device number 83 using
> xhci_hcd
> [44685.979721] usb 3-1.2: config 1 interface 0 altsetting 0 endpoint
> 0x83 has an invalid bInterval 32, changing to 9
> [44685.979726] usb 3-1.2: config 1 interface 1 altsetting 1 bulk
> endpoint 0x81 has invalid maxpacket 64
> [44685.979729] usb 3-1.2: config 1 interface 1 altsetting 1 bulk
> endpoint 0x2 has invalid maxpacket 64
> [44685.979731] usb 3-1.2: config 1 interface 2 altsetting 0 endpoint
> 0x86 has an invalid bInterval 32, changing to 9
> [44685.979734] usb 3-1.2: config 1 interface 3 altsetting 0 bulk
> endpoint 0x84 has invalid maxpacket 64
> [44685.979736] usb 3-1.2: config 1 interface 3 altsetting 0 bulk
> endpoint 0x5 has invalid maxpacket 64
> [44685.980032] usb 3-1.2: New USB device found, idVendor=0525,
> idProduct=a4aa
> [44685.980035] usb 3-1.2: New USB device strings: Mfr=1, Product=2,
> SerialNumber=0
> [44685.980036] usb 3-1.2: Product: CDC Composite Gadget
> [44685.980038] usb 3-1.2: Manufacturer: Linux 4.2.6-arietta with
> atmel_usba_udc
> [44685.981609] cdc_ether 3-1.2:1.0 usb0: register 'cdc_ether' at usb
> -0000:00:14.0-1.2, CDC Ethernet Device, fa:b1:ab:00:00:01
> [44685.982199] cdc_acm 3-1.2:1.2: ttyACM0: USB ACM device
> [44686.036753] cdc_ether 3-1.2:1.0 enp0s20u1u2: renamed from usb0
> [44686.125452] IPv6: ADDRCONF(NETDEV_UP): enp0s20u1u2: link is not
> ready

So far so good.

> [44686.125652] cdc_ether 3-1.2:1.0 enp0s20u1u2: kevent 12 may have been
> dropped

EVENT_SET_RX_MODE

> [44686.125657] cdc_ether 3-1.2:1.0 enp0s20u1u2: kevent 12 may have been
> dropped
> [44686.153902] cdc_ether 3-1.2:1.0 enp0s20u1u2: CDC: unexpected
> notification fd!

What is this?

I am attaching a patch that improves debugging. Could you test with
that?

	Regards
		Oliver

From 4e707ca6676e2bb36352b6282e563ce34bdfdc3d Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@xxxxxxxx>
Date: Wed, 25 Nov 2015 12:26:59 +0100
Subject: [PATCH] usbnet: fix debugging output for work items

usbnet uses a common work handler for all events. Thus multiple
events can be scheduled at a single time. An inability to schedule
a work item is not an error provided the corresponding flag is set
before an attempt to schedule the work is made.

Signed-off-by: Oliver Neukum <oneukum@xxxxxxxx>
---
 drivers/net/usb/usbnet.c | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 0744bf2..5229369 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -444,19 +444,25 @@ static enum skb_state defer_bh(struct usbnet *dev, struct sk_buff *skb,
 	return old_state;
 }
 
-/* some work can't be done in tasklets, so we use keventd
+/* some work can't be done in tasklets, so we use work items
  *
- * NOTE:  annoying asymmetry:  if it's active, schedule_work() fails,
- * but tasklet_schedule() doesn't.  hope the failure is rare.
+ * Order is important to prevent from events being lost
+ * a work item can be scheduled while it is being processed
+ * so we need to make sure the flag is always set before
+ * a work item is scheduled. A failure to schedule is not
+ * an error, as the work handler will do all jobs.
+ * For that reason a work handler with nothing to do is not
+ * a bug. It just has lost the race.
  */
-void usbnet_defer_kevent (struct usbnet *dev, int work)
+void usbnet_defer_kevent(struct usbnet *dev, int event)
 {
-	set_bit (work, &dev->flags);
+	set_bit (event, &dev->flags);
 	if (!schedule_work (&dev->kevent)) {
-		if (net_ratelimit())
-			netdev_err(dev->net, "kevent %d may have been dropped\n", work);
+		netdev_dbg(dev->net,
+			"kevent %d, but work  already scheduled\n",
+			event);
 	} else {
-		netdev_dbg(dev->net, "kevent %d scheduled\n", work);
+		netdev_dbg(dev->net, "kevent %d scheduled\n", event);
 	}
 }
 EXPORT_SYMBOL_GPL(usbnet_defer_kevent);
@@ -1099,6 +1105,7 @@ usbnet_deferred_kevent (struct work_struct *work)
 
 	/* usb_clear_halt() needs a thread context */
 	if (test_bit (EVENT_TX_HALT, &dev->flags)) {
+		netdev_dbg(dev->net, "event EVENT_TX_HALT processing\n");
 		unlink_urbs (dev, &dev->txq);
 		status = usb_autopm_get_interface(dev->intf);
 		if (status < 0)
@@ -1119,6 +1126,7 @@ fail_pipe:
 		}
 	}
 	if (test_bit (EVENT_RX_HALT, &dev->flags)) {
+		netdev_dbg(dev->net, "event EVENT_RX_HALT processing\n");
 		unlink_urbs (dev, &dev->rxq);
 		status = usb_autopm_get_interface(dev->intf);
 		if (status < 0)
@@ -1143,6 +1151,7 @@ fail_halt:
 		struct urb	*urb = NULL;
 		int resched = 1;
 
+		netdev_dbg(dev->net, "event EVENT_RX_MEMORY processing\n");
 		if (netif_running (dev->net))
 			urb = usb_alloc_urb (0, GFP_KERNEL);
 		else
@@ -1168,6 +1177,7 @@ fail_lowmem:
 		int			retval = 0;
 
 		clear_bit (EVENT_LINK_RESET, &dev->flags);
+		netdev_dbg(dev->net, "event EVENT_LINK_RESET processing\n");
 		status = usb_autopm_get_interface(dev->intf);
 		if (status < 0)
 			goto skip_reset;
@@ -1187,11 +1197,15 @@ skip_reset:
 		__handle_link_change(dev);
 	}
 
-	if (test_bit (EVENT_LINK_CHANGE, &dev->flags))
+	if (test_bit (EVENT_LINK_CHANGE, &dev->flags)) {
+		netdev_dbg(dev->net, "event EVENT_LINK_CHANGE processing\n");
 		__handle_link_change(dev);
+	}
 
-	if (test_bit (EVENT_SET_RX_MODE, &dev->flags))
+	if (test_bit (EVENT_SET_RX_MODE, &dev->flags)) {
+		netdev_dbg(dev->net, "event EVENT_SET_RX_MODE processing\n");
 		__handle_set_rx_mode(dev);
+	}
 
 
 	if (dev->flags)
-- 
2.1.4


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux