[PATCH v2 1/2] can: mcba_usb: fix typo

[PATCH v2 1/2] can: mcba_usb: fix typo

[Martin Kelly]

Fix typo "analizer" --> "Analyzer".

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
 drivers/net/can/usb/mcba_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index 7f0272558bef..c4355f0a20d5 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -862,7 +862,7 @@ static int mcba_usb_probe(struct usb_interface *intf,
 		goto cleanup_unregister_candev;
 	}
 
-	dev_info(&intf->dev, "Microchip CAN BUS analizer connected\n");
+	dev_info(&intf->dev, "Microchip CAN BUS Analyzer connected\n");
 
 	return 0;
 
-- 
2.11.0

[PATCH v2 2/2] can: mcba_usb: fix device disconnect bug

[Martin Kelly]

Currently, when you disconnect the device, the driver infinitely
resubmits all URBs, so you see:

Rx URB aborted (-32)

in an infinite loop.

Fix this by catching -EPIPE (what we get in urb->status when the device
disconnects) and not resubmitting.

With this patch, I can plug and unplug many times and the driver
recovers correctly.

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
 drivers/net/can/usb/mcba_usb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index c4355f0a20d5..ef417dcddbf7 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -592,6 +592,7 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)
 		break;
 
 	case -ENOENT:
+	case -EPIPE:
 	case -ESHUTDOWN:
 		return;
 
-- 
2.11.0

Re: [PATCH v2 1/2] can: mcba_usb: fix typo

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 452 bytes --]

On 11/28/2017 12:49 AM, Martin Kelly wrote:
> Fix typo "analizer" --> "Analyzer".
> 
> Signed-off-by: Martin Kelly <mkelly@xevo.com>

Both applied to can.

Tnx,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] can: mcba_usb: fix typo

[Martin Kelly]

On 11/28/2017 12:46 AM, Marc Kleine-Budde wrote:
> On 11/28/2017 12:49 AM, Martin Kelly wrote:
>> Fix typo "analizer" --> "Analyzer".
>>
>> Signed-off-by: Martin Kelly <mkelly@xevo.com>
> 
> Both applied to can.
> 

Thanks! By the way as far as I can tell from code inspection, it appears 
that most of the other drivers in net/can/usb should have the same 
disconnect bug. gs_usb appears to be clear, as it returns in its default 
case. Unfortunately mcba_usb is the only device I have to test with, but 
those with other devices may want to check for this.

CAN-USB adapter unplug (was: Re: [PATCH v2 1/2] can: mcba_usb: fix typo)

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 889 bytes --]

On 11/28/2017 10:09 PM, Martin Kelly wrote:
>> Both applied to can.
> 
> Thanks! By the way as far as I can tell from code inspection, it appears 
> that most of the other drivers in net/can/usb should have the same 
> disconnect bug. gs_usb appears to be clear, as it returns in its default 
> case. Unfortunately mcba_usb is the only device I have to test with, but 
> those with other devices may want to check for this.

Can you create patches for the affected drivers and send them to the
list and the maintainers of the driver on Cc?

I don't have access to every USB adapter neither.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: CAN-USB adapter unplug

[Stefan Mätje]

Am 29.11.2017 um 13:20 schrieb Marc Kleine-Budde:
> On 11/28/2017 10:09 PM, Martin Kelly wrote:
>>> Both applied to can.
>>
>> Thanks! By the way as far as I can tell from code inspection, it appears 
>> that most of the other drivers in net/can/usb should have the same 
>> disconnect bug. gs_usb appears to be clear, as it returns in its default 
>> case. Unfortunately mcba_usb is the only device I have to test with, but 
>> those with other devices may want to check for this.
> 
> Can you create patches for the affected drivers and send them to the
> list and the maintainers of the driver on Cc?
> 
> I don't have access to every USB adapter neither.
> 
> Marc
> 

Hi,

I have seen Martin's emails these days and tried to reproduce the error here
with an esd CAN-USB/2 device (handled by esd_usb2.c). The only thing I get 
in the log are some messages like this:

kernel: [14776.152459] usb 2-1: Rx URB aborted (-71)

There is no endless loop. How could I reproduce the bad behavior? For the
quick test I used an Ubuntu 4.4.0-101-generic kernel.

In any case I will add the patch handling EPIPE on a test system and have a 
look what it might change.

Regards,
Stefan Mätje

------------------------------------------------------------------------
Dipl.-Ing. Stefan Mätje
System-Design

esd electronics gmbh                                                    
Vahrenwalder Str. 207 - 30165 Hannover - GERMANY                        
Telefon: 0511-37298-146 - Fax: 0511-37298-68                            
E-Mail: Stefan.Maetje@esd.eu                                            
Bitte besuchen Sie uns im Internet unter http://www.esd.eu              
Quality Products - Made in Germany                                      
------------------------------------------------------------------------
Geschäftsführer: Klaus Detering, Norbert Gemmeke                        
Amtsgericht Hannover HRB 51373 - VAT-ID DE 115672832                    
------------------------------------------------------------------------

Re: CAN-USB adapter unplug

[Martin Kelly]

On 11/29/2017 04:20 AM, Marc Kleine-Budde wrote:
> On 11/28/2017 10:09 PM, Martin Kelly wrote:
>>> Both applied to can.
>>
>> Thanks! By the way as far as I can tell from code inspection, it appears
>> that most of the other drivers in net/can/usb should have the same
>> disconnect bug. gs_usb appears to be clear, as it returns in its default
>> case. Unfortunately mcba_usb is the only device I have to test with, but
>> those with other devices may want to check for this.
> 
> Can you create patches for the affected drivers and send them to the
> list and the maintainers of the driver on Cc?
> 

Yes, I can do this, although they will be untested.

> I don't have access to every USB adapter neither.
> 
> Marc
> 

Re: CAN-USB adapter unplug

[Martin Kelly]

On 11/29/2017 07:21 AM, Stefan Mätje wrote:
> Am 29.11.2017 um 13:20 schrieb Marc Kleine-Budde:
>> On 11/28/2017 10:09 PM, Martin Kelly wrote:
>>>> Both applied to can.
>>>
>>> Thanks! By the way as far as I can tell from code inspection, it appears
>>> that most of the other drivers in net/can/usb should have the same
>>> disconnect bug. gs_usb appears to be clear, as it returns in its default
>>> case. Unfortunately mcba_usb is the only device I have to test with, but
>>> those with other devices may want to check for this.
>>
>> Can you create patches for the affected drivers and send them to the
>> list and the maintainers of the driver on Cc?
>>
>> I don't have access to every USB adapter neither.
>>
>> Marc
>>
> 
> Hi,
> 
> I have seen Martin's emails these days and tried to reproduce the error here
> with an esd CAN-USB/2 device (handled by esd_usb2.c). The only thing I get
> in the log are some messages like this:
> 
> kernel: [14776.152459] usb 2-1: Rx URB aborted (-71)
> 
> There is no endless loop. How could I reproduce the bad behavior? For the
> quick test I used an Ubuntu 4.4.0-101-generic kernel.

Interesting. Do you see just a few -71 RX URB aborted messages (one per 
outstanding URB) rather than an endless loop? If so, then I think 
everything is OK on that device, as the URBs are not being resubmitted.

In case it helps, my test case for the mcba_usb is on a Raspberry Pi 3. 
I don't know whether or not that could influence the USB error code we 
see, since you are seeing EPROTO instead of EPIPE when the device gets 
unplugged.

> 
> In any case I will add the patch handling EPIPE on a test system and have a
> look what it might change.
> 
> Regards,
> Stefan Mätje
>

Re: CAN-USB adapter unplug

[Jimmy Assarsson]

On 2017-11-29 18:07, Martin Kelly wrote:
> On 11/29/2017 07:21 AM, Stefan Mätje wrote:
>> Am 29.11.2017 um 13:20 schrieb Marc Kleine-Budde:
>>> On 11/28/2017 10:09 PM, Martin Kelly wrote:
>>>>> Both applied to can.
>>>>
>>>> Thanks! By the way as far as I can tell from code inspection, it 
>>>> appears
>>>> that most of the other drivers in net/can/usb should have the same
>>>> disconnect bug. gs_usb appears to be clear, as it returns in its 
>>>> default
>>>> case. Unfortunately mcba_usb is the only device I have to test with, 
>>>> but
>>>> those with other devices may want to check for this.
>>>
>>> Can you create patches for the affected drivers and send them to the
>>> list and the maintainers of the driver on Cc?
>>>
>>> I don't have access to every USB adapter neither.
>>>
>>> Marc
>>>
>>
>> Hi,
>>
>> I have seen Martin's emails these days and tried to reproduce the 
>> error here
>> with an esd CAN-USB/2 device (handled by esd_usb2.c). The only thing I 
>> get
>> in the log are some messages like this:
>>
>> kernel: [14776.152459] usb 2-1: Rx URB aborted (-71)
>>
>> There is no endless loop. How could I reproduce the bad behavior? For the
>> quick test I used an Ubuntu 4.4.0-101-generic kernel.

Hi,

I got same result for kvaser_usb, a single "Rx URB aborted (-71)". When 
tested on Ubuntu with 4.4.0-93-generic.

> Interesting. Do you see just a few -71 RX URB aborted messages (one per 
> outstanding URB) rather than an endless loop? If so, then I think 
> everything is OK on that device, as the URBs are not being resubmitted.
> 
> In case it helps, my test case for the mcba_usb is on a Raspberry Pi 3. 
> I don't know whether or not that could influence the USB error code we 
> see, since you are seeing EPROTO instead of EPIPE when the device gets 
> unplugged.

However, I do get lots of (500+) "Rx URB aborted (-71)" printouts, on 
Raspberry Pi 3, running Raspbian with kernel 4.9.35-v7. But no EPIPE seen.

Regards,
Jimmy

>>
>> In any case I will add the patch handling EPIPE on a test system and 
>> have a
>> look what it might change.
>>
>> Regards,
>> Stefan Mätje

Re: CAN-USB adapter unplug

[Martin Kelly]

On 11/30/2017 02:18 AM, Jimmy Assarsson wrote:
> On 2017-11-29 18:07, Martin Kelly wrote:
>> On 11/29/2017 07:21 AM, Stefan Mätje wrote:
>>> Am 29.11.2017 um 13:20 schrieb Marc Kleine-Budde:
>>>> On 11/28/2017 10:09 PM, Martin Kelly wrote:
>>>>>> Both applied to can.
>>>>>
>>>>> Thanks! By the way as far as I can tell from code inspection, it 
>>>>> appears
>>>>> that most of the other drivers in net/can/usb should have the same
>>>>> disconnect bug. gs_usb appears to be clear, as it returns in its 
>>>>> default
>>>>> case. Unfortunately mcba_usb is the only device I have to test 
>>>>> with, but
>>>>> those with other devices may want to check for this.
>>>>
>>>> Can you create patches for the affected drivers and send them to the
>>>> list and the maintainers of the driver on Cc?
>>>>
>>>> I don't have access to every USB adapter neither.
>>>>
>>>> Marc
>>>>
>>>
>>> Hi,
>>>
>>> I have seen Martin's emails these days and tried to reproduce the 
>>> error here
>>> with an esd CAN-USB/2 device (handled by esd_usb2.c). The only thing 
>>> I get
>>> in the log are some messages like this:
>>>
>>> kernel: [14776.152459] usb 2-1: Rx URB aborted (-71)
>>>
>>> There is no endless loop. How could I reproduce the bad behavior? For 
>>> the
>>> quick test I used an Ubuntu 4.4.0-101-generic kernel.
> 
> Hi,
> 
> I got same result for kvaser_usb, a single "Rx URB aborted (-71)". When 
> tested on Ubuntu with 4.4.0-93-generic.
> 
>> Interesting. Do you see just a few -71 RX URB aborted messages (one 
>> per outstanding URB) rather than an endless loop? If so, then I think 
>> everything is OK on that device, as the URBs are not being resubmitted.
>>
>> In case it helps, my test case for the mcba_usb is on a Raspberry Pi 
>> 3. I don't know whether or not that could influence the USB error code 
>> we see, since you are seeing EPROTO instead of EPIPE when the device 
>> gets unplugged.
> 
> However, I do get lots of (500+) "Rx URB aborted (-71)" printouts, on 
> Raspberry Pi 3, running Raspbian with kernel 4.9.35-v7. But no EPIPE seen.
> 

Very interesting; this means that there are multiple possible error 
codes from a USB disconnect. If that's the case, is it possible that the 
best thing to do is what gs_usb does? In gs_usb's receive callback, we have:

     switch (urb->status) {
     case 0: /* success */
         break;
     case -ENOENT:
     case -ESHUTDOWN:
         return;
     default:
         /* do not resubmit aborted urbs. eg: when device goes down */
         return;
     }

The default case is to *not* resubmit URBs, rather than to resubmit is a 
default and try to catch all possible error codes in the non-default 
case, as the other drivers are doing.

If we don't do something like this, then we need to understand all the 
possible error codes that could occur and catch them all.

> Regards,
> Jimmy
> 
>>>
>>> In any case I will add the patch handling EPIPE on a test system and 
>>> have a
>>> look what it might change.
>>>
>>> Regards,
>>> Stefan Mätje

Re: CAN-USB adapter unplug

[Martin Kelly]

On 11/30/2017 02:18 AM, Jimmy Assarsson wrote:
> On 2017-11-29 18:07, Martin Kelly wrote:
>> On 11/29/2017 07:21 AM, Stefan Mätje wrote:
>>> Am 29.11.2017 um 13:20 schrieb Marc Kleine-Budde:
>>>> On 11/28/2017 10:09 PM, Martin Kelly wrote:
>>>>>> Both applied to can.
>>>>>
>>>>> Thanks! By the way as far as I can tell from code inspection, it 
>>>>> appears
>>>>> that most of the other drivers in net/can/usb should have the same
>>>>> disconnect bug. gs_usb appears to be clear, as it returns in its 
>>>>> default
>>>>> case. Unfortunately mcba_usb is the only device I have to test 
>>>>> with, but
>>>>> those with other devices may want to check for this.
>>>>
>>>> Can you create patches for the affected drivers and send them to the
>>>> list and the maintainers of the driver on Cc?
>>>>
>>>> I don't have access to every USB adapter neither.
>>>>
>>>> Marc
>>>>
>>>
>>> Hi,
>>>
>>> I have seen Martin's emails these days and tried to reproduce the 
>>> error here
>>> with an esd CAN-USB/2 device (handled by esd_usb2.c). The only thing 
>>> I get
>>> in the log are some messages like this:
>>>
>>> kernel: [14776.152459] usb 2-1: Rx URB aborted (-71)
>>>
>>> There is no endless loop. How could I reproduce the bad behavior? For 
>>> the
>>> quick test I used an Ubuntu 4.4.0-101-generic kernel.
> 
> Hi,
> 
> I got same result for kvaser_usb, a single "Rx URB aborted (-71)". When 
> tested on Ubuntu with 4.4.0-93-generic.
> 

I tried this same test with Debian 4.9.51-1 and get a stream of "Rx URB 
aborted (-71)" messages, much as you saw on the Raspberry Pi 3. The 
stream overloads the /dev/kmsg buffer, but eventually we do get "USB 
disconnect", "device disconnected", and the messages stop.

I think this is basically the same issue with EPROTO instead of EPIPE. 
It appears that the exact error code you get and the timing between 
disconnect and URB cancellations varies per system.

>> Interesting. Do you see just a few -71 RX URB aborted messages (one 
>> per outstanding URB) rather than an endless loop? If so, then I think 
>> everything is OK on that device, as the URBs are not being resubmitted.
>>
>> In case it helps, my test case for the mcba_usb is on a Raspberry Pi 
>> 3. I don't know whether or not that could influence the USB error code 
>> we see, since you are seeing EPROTO instead of EPIPE when the device 
>> gets unplugged.
> 
> However, I do get lots of (500+) "Rx URB aborted (-71)" printouts, on 
> Raspberry Pi 3, running Raspbian with kernel 4.9.35-v7. But no EPIPE seen.
> 
> Regards,
> Jimmy
> 
>>>
>>> In any case I will add the patch handling EPIPE on a test system and 
>>> have a
>>> look what it might change.
>>>
>>> Regards,
>>> Stefan Mätje

Re: CAN-USB adapter unplug

[Jimmy Assarsson]

On 2017-11-30 19:19, Martin Kelly wrote:
> On 11/30/2017 02:18 AM, Jimmy Assarsson wrote:
>> On 2017-11-29 18:07, Martin Kelly wrote:
>>> On 11/29/2017 07:21 AM, Stefan Mätje wrote:
>>>> Am 29.11.2017 um 13:20 schrieb Marc Kleine-Budde:
>>>>> On 11/28/2017 10:09 PM, Martin Kelly wrote:
>>>>>>> Both applied to can.
>>>>>>
>>>>>> Thanks! By the way as far as I can tell from code inspection, it 
>>>>>> appears
>>>>>> that most of the other drivers in net/can/usb should have the same
>>>>>> disconnect bug. gs_usb appears to be clear, as it returns in its 
>>>>>> default
>>>>>> case. Unfortunately mcba_usb is the only device I have to test 
>>>>>> with, but
>>>>>> those with other devices may want to check for this.
>>>>>
>>>>> Can you create patches for the affected drivers and send them to the
>>>>> list and the maintainers of the driver on Cc?
>>>>>
>>>>> I don't have access to every USB adapter neither.
>>>>>
>>>>> Marc
>>>>>
>>>>
>>>> Hi,
>>>>
>>>> I have seen Martin's emails these days and tried to reproduce the 
>>>> error here
>>>> with an esd CAN-USB/2 device (handled by esd_usb2.c). The only thing 
>>>> I get
>>>> in the log are some messages like this:
>>>>
>>>> kernel: [14776.152459] usb 2-1: Rx URB aborted (-71)
>>>>
>>>> There is no endless loop. How could I reproduce the bad behavior? 
>>>> For the
>>>> quick test I used an Ubuntu 4.4.0-101-generic kernel.
>>
>> Hi,
>>
>> I got same result for kvaser_usb, a single "Rx URB aborted (-71)". 
>> When tested on Ubuntu with 4.4.0-93-generic.
>>
>>> Interesting. Do you see just a few -71 RX URB aborted messages (one 
>>> per outstanding URB) rather than an endless loop? If so, then I think 
>>> everything is OK on that device, as the URBs are not being resubmitted.
>>>
>>> In case it helps, my test case for the mcba_usb is on a Raspberry Pi 
>>> 3. I don't know whether or not that could influence the USB error 
>>> code we see, since you are seeing EPROTO instead of EPIPE when the 
>>> device gets unplugged.
>>
>> However, I do get lots of (500+) "Rx URB aborted (-71)" printouts, on 
>> Raspberry Pi 3, running Raspbian with kernel 4.9.35-v7. But no EPIPE 
>> seen.
>>
> 
> Very interesting; this means that there are multiple possible error 
> codes from a USB disconnect. If that's the case, is it possible that the 
> best thing to do is what gs_usb does? In gs_usb's receive callback, we 
> have:
> 
>      switch (urb->status) {
>      case 0: /* success */
>          break;
>      case -ENOENT:
>      case -ESHUTDOWN:
>          return;
>      default:
>          /* do not resubmit aborted urbs. eg: when device goes down */
>          return;
>      }
> 
> The default case is to *not* resubmit URBs, rather than to resubmit is a 
> default and try to catch all possible error codes in the non-default 
> case, as the other drivers are doing.
> 
> If we don't do something like this, then we need to understand all the 
> possible error codes that could occur and catch them all.

Well, gs_usb never resubmit any URBs if urb->status is non-zero. As long 
as any error (urb->status != 0) is the result of a disconnect, it should 
be safe to never resubmit? I doubt this is the case, but I really don't 
know.

>> Regards,
>> Jimmy
>>
>>>>
>>>> In any case I will add the patch handling EPIPE on a test system and 
>>>> have a
>>>> look what it might change.
>>>>
>>>> Regards,
>>>> Stefan Mätje

Re: CAN-USB adapter unplug

[Martin Kelly]

On 12/04/2017 12:39 AM, Jimmy Assarsson wrote:
> On 2017-11-30 19:19, Martin Kelly wrote:
>> On 11/30/2017 02:18 AM, Jimmy Assarsson wrote:
>>> On 2017-11-29 18:07, Martin Kelly wrote:
>>>> On 11/29/2017 07:21 AM, Stefan Mätje wrote:
>>>>> Am 29.11.2017 um 13:20 schrieb Marc Kleine-Budde:
>>>>>> On 11/28/2017 10:09 PM, Martin Kelly wrote:
>>>>>>>> Both applied to can.
>>>>>>>
>>>>>>> Thanks! By the way as far as I can tell from code inspection, it 
>>>>>>> appears
>>>>>>> that most of the other drivers in net/can/usb should have the same
>>>>>>> disconnect bug. gs_usb appears to be clear, as it returns in its 
>>>>>>> default
>>>>>>> case. Unfortunately mcba_usb is the only device I have to test 
>>>>>>> with, but
>>>>>>> those with other devices may want to check for this.
>>>>>>
>>>>>> Can you create patches for the affected drivers and send them to the
>>>>>> list and the maintainers of the driver on Cc?
>>>>>>
>>>>>> I don't have access to every USB adapter neither.
>>>>>>
>>>>>> Marc
>>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> I have seen Martin's emails these days and tried to reproduce the 
>>>>> error here
>>>>> with an esd CAN-USB/2 device (handled by esd_usb2.c). The only 
>>>>> thing I get
>>>>> in the log are some messages like this:
>>>>>
>>>>> kernel: [14776.152459] usb 2-1: Rx URB aborted (-71)
>>>>>
>>>>> There is no endless loop. How could I reproduce the bad behavior? 
>>>>> For the
>>>>> quick test I used an Ubuntu 4.4.0-101-generic kernel.
>>>
>>> Hi,
>>>
>>> I got same result for kvaser_usb, a single "Rx URB aborted (-71)". 
>>> When tested on Ubuntu with 4.4.0-93-generic.
>>>
>>>> Interesting. Do you see just a few -71 RX URB aborted messages (one 
>>>> per outstanding URB) rather than an endless loop? If so, then I 
>>>> think everything is OK on that device, as the URBs are not being 
>>>> resubmitted.
>>>>
>>>> In case it helps, my test case for the mcba_usb is on a Raspberry Pi 
>>>> 3. I don't know whether or not that could influence the USB error 
>>>> code we see, since you are seeing EPROTO instead of EPIPE when the 
>>>> device gets unplugged.
>>>
>>> However, I do get lots of (500+) "Rx URB aborted (-71)" printouts, on 
>>> Raspberry Pi 3, running Raspbian with kernel 4.9.35-v7. But no EPIPE 
>>> seen.
>>>
>>
>> Very interesting; this means that there are multiple possible error 
>> codes from a USB disconnect. If that's the case, is it possible that 
>> the best thing to do is what gs_usb does? In gs_usb's receive 
>> callback, we have:
>>
>>      switch (urb->status) {
>>      case 0: /* success */
>>          break;
>>      case -ENOENT:
>>      case -ESHUTDOWN:
>>          return;
>>      default:
>>          /* do not resubmit aborted urbs. eg: when device goes down */
>>          return;
>>      }
>>
>> The default case is to *not* resubmit URBs, rather than to resubmit is 
>> a default and try to catch all possible error codes in the non-default 
>> case, as the other drivers are doing.
>>
>> If we don't do something like this, then we need to understand all the 
>> possible error codes that could occur and catch them all.
> 
> Well, gs_usb never resubmit any URBs if urb->status is non-zero. As long 
> as any error (urb->status != 0) is the result of a disconnect, it should 
> be safe to never resubmit? I doubt this is the case, but I really don't 
> know.
> 

Yeah, it's unclear to me whether what gs_usb does is safe. I have kept 
my patches just using -EPIPE/-EPROTO rather than a catch-all default, 
just to be safe.

>>> Regards,
>>> Jimmy
>>>
>>>>>
>>>>> In any case I will add the patch handling EPIPE on a test system 
>>>>> and have a
>>>>> look what it might change.
>>>>>
>>>>> Regards,
>>>>> Stefan Mätje

Re: CAN-USB adapter unplug

[Jimmy Assarsson]

On 2017-12-06 00:42, Martin Kelly wrote:
> On 12/04/2017 12:39 AM, Jimmy Assarsson wrote:
>> On 2017-11-30 19:19, Martin Kelly wrote:
>>> On 11/30/2017 02:18 AM, Jimmy Assarsson wrote:
>>>> On 2017-11-29 18:07, Martin Kelly wrote:
>>>>> On 11/29/2017 07:21 AM, Stefan Mätje wrote:
>>>>>> Am 29.11.2017 um 13:20 schrieb Marc Kleine-Budde:
>>>>>>> On 11/28/2017 10:09 PM, Martin Kelly wrote:
>>>>>>>>> Both applied to can.
>>>>>>>>
>>>>>>>> Thanks! By the way as far as I can tell from code inspection, it 
>>>>>>>> appears
>>>>>>>> that most of the other drivers in net/can/usb should have the same
>>>>>>>> disconnect bug. gs_usb appears to be clear, as it returns in its 
>>>>>>>> default
>>>>>>>> case. Unfortunately mcba_usb is the only device I have to test 
>>>>>>>> with, but
>>>>>>>> those with other devices may want to check for this.
>>>>>>>
>>>>>>> Can you create patches for the affected drivers and send them to the
>>>>>>> list and the maintainers of the driver on Cc?
>>>>>>>
>>>>>>> I don't have access to every USB adapter neither.
>>>>>>>
>>>>>>> Marc
>>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I have seen Martin's emails these days and tried to reproduce the 
>>>>>> error here
>>>>>> with an esd CAN-USB/2 device (handled by esd_usb2.c). The only 
>>>>>> thing I get
>>>>>> in the log are some messages like this:
>>>>>>
>>>>>> kernel: [14776.152459] usb 2-1: Rx URB aborted (-71)
>>>>>>
>>>>>> There is no endless loop. How could I reproduce the bad behavior? 
>>>>>> For the
>>>>>> quick test I used an Ubuntu 4.4.0-101-generic kernel.
>>>>
>>>> Hi,
>>>>
>>>> I got same result for kvaser_usb, a single "Rx URB aborted (-71)". 
>>>> When tested on Ubuntu with 4.4.0-93-generic.
>>>>
>>>>> Interesting. Do you see just a few -71 RX URB aborted messages (one 
>>>>> per outstanding URB) rather than an endless loop? If so, then I 
>>>>> think everything is OK on that device, as the URBs are not being 
>>>>> resubmitted.
>>>>>
>>>>> In case it helps, my test case for the mcba_usb is on a Raspberry 
>>>>> Pi 3. I don't know whether or not that could influence the USB 
>>>>> error code we see, since you are seeing EPROTO instead of EPIPE 
>>>>> when the device gets unplugged.
>>>>
>>>> However, I do get lots of (500+) "Rx URB aborted (-71)" printouts, 
>>>> on Raspberry Pi 3, running Raspbian with kernel 4.9.35-v7. But no 
>>>> EPIPE seen.
>>>>
>>>
>>> Very interesting; this means that there are multiple possible error 
>>> codes from a USB disconnect. If that's the case, is it possible that 
>>> the best thing to do is what gs_usb does? In gs_usb's receive 
>>> callback, we have:
>>>
>>>      switch (urb->status) {
>>>      case 0: /* success */
>>>          break;
>>>      case -ENOENT:
>>>      case -ESHUTDOWN:
>>>          return;
>>>      default:
>>>          /* do not resubmit aborted urbs. eg: when device goes down */
>>>          return;
>>>      }
>>>
>>> The default case is to *not* resubmit URBs, rather than to resubmit 
>>> is a default and try to catch all possible error codes in the 
>>> non-default case, as the other drivers are doing.
>>>
>>> If we don't do something like this, then we need to understand all 
>>> the possible error codes that could occur and catch them all.
>>
>> Well, gs_usb never resubmit any URBs if urb->status is non-zero. As 
>> long as any error (urb->status != 0) is the result of a disconnect, it 
>> should be safe to never resubmit? I doubt this is the case, but I 
>> really don't know.
>>
> 
> Yeah, it's unclear to me whether what gs_usb does is safe. I have kept 
> my patches just using -EPIPE/-EPROTO rather than a catch-all default, 
> just to be safe.

Sounds good :)

>>>> Regards,
>>>> Jimmy
>>>>
>>>>>>
>>>>>> In any case I will add the patch handling EPIPE on a test system 
>>>>>> and have a
>>>>>> look what it might change.
>>>>>>
>>>>>> Regards,
>>>>>> Stefan Mätje