NetBSD Problem Report #54070

From gson@gson.org  Sun Mar 24 10:53:44 2019
Return-Path: <gson@gson.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id E0BA87A1B0
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 24 Mar 2019 10:53:44 +0000 (UTC)
Message-Id: <20190324105337.DEC849892F6@guava.gson.org>
Date: Sun, 24 Mar 2019 12:53:37 +0200 (EET)
From: gson@gson.org (Andreas Gustafsson)
Reply-To: gson@gson.org (Andreas Gustafsson)
To: gnats-bugs@NetBSD.org
Subject: urtwn(4) does not work on OHCI
X-Send-Pr-Version: 3.95

>Number:         54070
>Category:       kern
>Synopsis:       urtwn(4) does not work on OHCI
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Mar 24 10:55:00 +0000 2019
>Closed-Date:    Sat Dec 14 17:11:52 +0000 2019
>Last-Modified:  Sat Dec 14 17:11:52 +0000 2019
>Originator:     Andreas Gustafsson
>Release:        NetBSD-current (8.99.36)
>Organization:

>Environment:
System: NetBSD
Architecture: i386
Machine: i386
>Description:

I have an RTL8188EU based USB WiFi adapter that works when attached to
a high-speed USB port driven by an EHCI controller (for some value of
"works").  However, when I attach it to a full-speed USB port on a NEC
Versa DayLite laptop, which uses an OHCI controller, or if I attach it
to a high-speed EHCI/OHCI port on a desktop through a full-speed hub
so that it gets routed to the OHCI companion controller, it can't
connect to the WiFi network.

My current minimal test case involves setting up an open network
(no encryption), plugging the adapter into the USB port or hub,
and simply running

  ifconfig urtwn0 ssid insert-ssid-here up

The driver attaches successfully:

  urtwn0 at uhub14 port 2
  urtwn0: Realtek (0xbda) 802.11n NIC (0x179), rev 2.00/0.00, addr 3
  urtwn0: MAC/BB RTL8188EU, RF 6052 1T1R, address 00:e0:4c:81:88:10
  urtwn0: 1 rx pipe, 2 tx pipes
  urtwn0: 11b rates: 1Mbps 2Mbps 5.5Mbps 11Mbps
  urtwn0: 11g rates: 1Mbps 2Mbps 5.5Mbps 11Mbps 6Mbps 9Mbps 12Mbps 18Mbps 24Mbps 36Mbps 48Mbps 54Mbps

but when I try to inspect the state of the interface by repeatedly
running "ifconfig urtwn0", the output alternates between the following
two states on about a 10-second cycle.  This half-cycle lasts about 3
second:

   urtwn0: flags=0x8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> mtu 1500
           ssid insert-ssid-here
           powersave off
           bssid dc:ee:06:8e:3e:f5 chan 1
           address: 00:e0:4c:81:88:10
           media: IEEE802.11 autoselect (OFDM54 mode 11g)
           status: no network

And this one, about 7 seconds:

   urtwn0: flags=0x8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> mtu 1500
           ssid ""
           powersave off
           address: 00:e0:4c:81:88:10
           media: IEEE802.11 autoselect (OFDM54 mode 11g)
           status: no network

That is, the ssid and bssid come and go, but it always says "no network".

In the access point's display of connected clients, the NetBSD client
is continuously present, but the connection duration field resets to
zero every time it reaches about 30 seconds.

>How-To-Repeat:

>Fix:

>Release-Note:

>Audit-Trail:
From: Andreas Gustafsson <gson@gson.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/54070: urtwn(4) does not work on OHCI
Date: Tue, 26 Nov 2019 16:42:26 +0200

 Investigating this with a USB protocol analyzer showed that the
 first USB bulk IN transfer processed by urtwn_rxeof() was correct,
 but in the second transfer, the first 64 bytes were missing from
 the data received by urtwn_rxeof() even though they were present
 on the wire.

 I believe what is happening is that the first transfer ends with a
 zero-length packet, which is expected.  The code path in the OHCI
 driver that handles short transfers then resets the TD Queue Head
 Pointer in the Endpoint Descriptor by writing the entire 32-bit word
 containing it, which has the desired side effect of clearing the
 Halted bit, but also the undesired side effect of clearing the
 toggleCarry bit.  If the first packet of the next transfer then
 happens to have a data toggle of "1", it is considered to have a
 toggle mismatch and is ignored, causing one packet's worth of data
 (64 bytes) to be lost.  The following packet then has the expected
 toggle and the remainder of the transfer is received normally.

 With the following patch, urtwn at ochi works much better. I intend
 to commit it soon unless someone has a really good objection.

 Index: ohci.c
 ===================================================================
 RCS file: /bracket/repo/src/sys/dev/usb/ohci.c,v
 retrieving revision 1.290
 diff -u -r1.290 ohci.c
 --- ohci.c	11 Aug 2019 22:55:03 -0000	1.290
 +++ ohci.c	26 Nov 2019 14:04:32 -0000
 @@ -1515,8 +1515,9 @@

  			ohci_soft_ed_t *sed = opipe->sed;

 -			/* clear halt and TD chain */
 -			sed->ed.ed_headp = HTOO32(p->physaddr);
 +			/* clear halt and TD chain, preserving toggle carry */
 +			sed->ed.ed_headp = HTOO32(p->physaddr |
 +			    (O32TOH(sed->ed.ed_headp) & OHCI_TOGGLECARRY));
  			usb_syncmem(&sed->dma,
  			    sed->offs + offsetof(ohci_ed_t, ed_headp),
  			    sizeof(sed->ed.ed_headp),

 -- 
 Andreas Gustafsson, gson@gson.org

From: Nick Hudson <nick.hudson@gmx.co.uk>
To: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org,
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
 Andreas Gustafsson <gson@gson.org>
Cc: 
Subject: Re: kern/54070: urtwn(4) does not work on OHCI
Date: Tue, 26 Nov 2019 20:35:27 +0000

 On 26/11/2019 14:45, Andreas Gustafsson wrote:
 > The following reply was made to PR kern/54070; it has been noted by GNAT=
 S.
 >
 > From: Andreas Gustafsson <gson@gson.org>
 > To: gnats-bugs@netbsd.org
 > Cc:
 > Subject: Re: kern/54070: urtwn(4) does not work on OHCI
 > Date: Tue, 26 Nov 2019 16:42:26 +0200
 >
 >   Investigating this with a USB protocol analyzer showed that the
 >   first USB bulk IN transfer processed by urtwn_rxeof() was correct,
 >   but in the second transfer, the first 64 bytes were missing from
 >   the data received by urtwn_rxeof() even though they were present
 >   on the wire.
 >
 >   I believe what is happening is that the first transfer ends with a
 >   zero-length packet, which is expected.  The code path in the OHCI
 >   driver that handles short transfers then resets the TD Queue Head
 >   Pointer in the Endpoint Descriptor by writing the entire 32-bit word
 >   containing it, which has the desired side effect of clearing the
 >   Halted bit, but also the undesired side effect of clearing the
 >   toggleCarry bit.  If the first packet of the next transfer then
 >   happens to have a data toggle of "1", it is considered to have a
 >   toggle mismatch and is ignored, causing one packet's worth of data
 >   (64 bytes) to be lost.  The following packet then has the expected
 >   toggle and the remainder of the transfer is received normally.
 >
 >   With the following patch, urtwn at ochi works much better. I intend
 >   to commit it soon unless someone has a really good objection.

 The patch adjusts the "endpoint is halted" error handling code path.

 http://src.illumos.org/source/xref/netbsd-src/sys/dev/usb/ohci.c#1500

 What is the error code OHCI_CC_DATA_UNDERRUN?

 Maybe OHCI_TD_R should be used more?

 http://src.illumos.org/source/xref/netbsd-src/sys/dev/usb/ohci.c#666

 Nick

From: Andreas Gustafsson <gson@gson.org>
To: Nick Hudson <nick.hudson@gmx.co.uk>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/54070: urtwn(4) does not work on OHCI
Date: Wed, 27 Nov 2019 09:48:45 +0200

 Nick Hudson wrote:
 > What is the error code OHCI_CC_DATA_UNDERRUN?

 Yes.

 > Maybe OHCI_TD_R should be used more?

 I think then we would run into the issue discussed in the following
 quote from http://www.fysnet.net/docs/vol8_future.pdf:

 "If this bit is set, the controller will mark the TD as a successful
 transfer, however, the controller will continue on to the next TD
 pointed to by the NextTD field. Continuing to the next TD may not be
 what you planned on."
 -- 
 Andreas Gustafsson, gson@gson.org

From: Nick Hudson <nick.hudson@gmx.co.uk>
To: Andreas Gustafsson <gson@gson.org>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/54070: urtwn(4) does not work on OHCI
Date: Wed, 27 Nov 2019 20:11:51 +0000

 On 27/11/2019 07:48, Andreas Gustafsson wrote:
 > Nick Hudson wrote:
 >> What is the error code OHCI_CC_DATA_UNDERRUN?
 >
 > Yes.
 >
 >> Maybe OHCI_TD_R should be used more?
 >
 > I think then we would run into the issue discussed in the following
 > quote from http://www.fysnet.net/docs/vol8_future.pdf:
 >
 > "If this bit is set, the controller will mark the TD as a successful
 > transfer, however, the controller will continue on to the next TD
 > pointed to by the NextTD field. Continuing to the next TD may not be
 > what you planned on."

 Ok, I see that you're trying to implement the recommendation in the PDF.

 I think it's best to remove OHCI_TD_R completely for bulk/interrupt
 transfers.

   http://src.illumos.org/source/xref/netbsd-src/sys/dev/usb/ohci.c#666

 I haven't thought about control transfers though.

 Nick

From: Andreas Gustafsson <gson@gson.org>
To: Nick Hudson <nick.hudson@gmx.co.uk>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/54070: urtwn(4) does not work on OHCI
Date: Wed, 27 Nov 2019 22:53:30 +0200

 Nick Hudson wrote:
 >  Ok, I see that you're trying to implement the recommendation in the PDF.

 That's not quite how I would put it; I actually devised the patch
 before I found the PDF.  I am just trying to address the specific bug
 that is preventing urtwn from working on ohci.

 >  I think it's best to remove OHCI_TD_R completely for bulk/interrupt
 >  transfers.
 >  
 >    http://src.illumos.org/source/xref/netbsd-src/sys/dev/usb/ohci.c#666

 Line of the beast :) That line is setting OHCI_TD_R on the last TD
 only, which is probably safe since there is no next TD to continue to,
 but leaving it cleared should also work, and I have no opinion on
 which option is better.  Leaving it cleared should make the need for
 my patch more obvious as the bug would then also hit drivers that
 request transfers of 8k or less, not just those that request more
 than 8k so that multiple TDs are required.

 >  I haven't thought about control transfers though.

 Neither have I.
 -- 
 Andreas Gustafsson, gson@gson.org

From: Nick Hudson <nick.hudson@gmx.co.uk>
To: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org,
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
 Andreas Gustafsson <gson@gson.org>
Cc: 
Subject: Re: kern/54070: urtwn(4) does not work on OHCI
Date: Thu, 28 Nov 2019 07:36:30 +0000

 This is a multi-part message in MIME format.
 --------------C60D37FCA30B99AC048D2B86
 Content-Type: text/plain; charset=utf-8; format=flowed
 Content-Transfer-Encoding: 7bit

 How about this untested patch?

 I think it might help 50278 as well.

 Nick

 --------------C60D37FCA30B99AC048D2B86
 Content-Type: text/plain; charset=UTF-8;
  name="ohci.c.diff"
 Content-Transfer-Encoding: base64
 Content-Disposition: attachment;
  filename="ohci.c.diff"

 SW5kZXg6IHN5cy9kZXYvdXNiL29oY2kuYwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09
 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09ClJDUyBmaWxlOiAvY3Zz
 cm9vdC9zcmMvc3lzL2Rldi91c2Ivb2hjaS5jLHYKcmV0cmlldmluZyByZXZpc2lvbiAxLjI5
 MApkaWZmIC11IC1wIC1yMS4yOTAgb2hjaS5jCi0tLSBzeXMvZGV2L3VzYi9vaGNpLmMJMTEg
 QXVnIDIwMTkgMjI6NTU6MDMgLTAwMDAJMS4yOTAKKysrIHN5cy9kZXYvdXNiL29oY2kuYwky
 OCBOb3YgMjAxOSAwNzozNToxMyAtMDAwMApAQCAtMTUxNSw4ICsxNTE1LDkgQEAgb2hjaV9z
 b2Z0aW50cih2b2lkICp2KQogCiAJCQlvaGNpX3NvZnRfZWRfdCAqc2VkID0gb3BpcGUtPnNl
 ZDsKIAotCQkJLyogY2xlYXIgaGFsdCBhbmQgVEQgY2hhaW4gKi8KLQkJCXNlZC0+ZWQuZWRf
 aGVhZHAgPSBIVE9PMzIocC0+cGh5c2FkZHIpOworCQkJLyogY2xlYXIgaGFsdCBhbmQgVEQg
 Y2hhaW4sIHByZXNlcnZpbmcgdG9nZ2xlIGNhcnJ5ICovCisJCQlzZWQtPmVkLmVkX2hlYWRw
 ID0gSFRPTzMyKHAtPnBoeXNhZGRyKSB8CisJCQkgICAoTzMyVE9IKHNlZC0+ZWQuZWRfaGVh
 ZHApICYgT0hDSV9UT0dHTEVDQVJSWSkpOwogCQkJdXNiX3N5bmNtZW0oJnNlZC0+ZG1hLAog
 CQkJICAgIHNlZC0+b2ZmcyArIG9mZnNldG9mKG9oY2lfZWRfdCwgZWRfaGVhZHApLAogCQkJ
 ICAgIHNpemVvZihzZWQtPmVkLmVkX2hlYWRwKSwKQEAgLTE1MjQsNiArMTUyNSw3IEBAIG9o
 Y2lfc29mdGludHIodm9pZCAqdikKIAogCQkJT1dSSVRFNChzYywgT0hDSV9DT01NQU5EX1NU
 QVRVUywgT0hDSV9DTEYpOwogCisJCQkvKiBYWFggVVNCRF9TSE9SVF9YRkVSX09LID8/ICov
 CiAJCQlpZiAoY2MgPT0gT0hDSV9DQ19EQVRBX1VOREVSUlVOKQogCQkJCXhmZXItPnV4X3N0
 YXR1cyA9IFVTQkRfTk9STUFMX0NPTVBMRVRJT047CiAJCQllbHNlIGlmIChjYyA9PSBPSENJ
 X0NDX1NUQUxMKQpAQCAtMzA1MSw4ICszMDUzLDExIEBAIG9oY2lfZGV2aWNlX2J1bGtfc3Rh
 cnQoc3RydWN0IHVzYmRfeGZlciAKIAkgICAgKHVpbnRwdHJfdClveC0+b3hfc3Rkc1swXSwg
 KHVpbnRwdHJfdCl0YWlsLCAwKTsKIAlLQVNTRVJUKG9waXBlLT50YWlsLnRkID09IHRhaWwp
 OwogCi0JLyogV2Ugd2FudCBpbnRlcnJ1cHQgYXQgdGhlIGVuZCBvZiB0aGUgdHJhbnNmZXIu
 ICovCi0JbGFzdC0+dGQudGRfZmxhZ3MgJj0gSFRPTzMyKH5PSENJX1REX0lOVFJfTUFTSyk7
 CisJLyoKKwkgKiBXZSB3YW50IGludGVycnVwdCBhdCB0aGUgZW5kIG9mIHRoZSB0cmFuc2Zl
 ciBhbmQgZm9yIHJvdW5kIHRvIGJlIGNsZWFyLgorCSAqIEFsbCBzaG9ydCB0cmFuc2ZlcnMg
 YXJlIGhhbmRsZWQgdmlhIE9IQ0lfQ0NfREFUQV9VTkRFUlJVTgorCSAqLworCWxhc3QtPnRk
 LnRkX2ZsYWdzICY9IH5IVE9PMzIoT0hDSV9URF9JTlRSX01BU0sgfCBPSENJX1REX1IpOwog
 CWxhc3QtPnRkLnRkX2ZsYWdzIHw9IEhUT08zMihPSENJX1REX1NFVF9ESSgxKSk7CiAJbGFz
 dC0+dGQudGRfbmV4dHRkID0gSFRPTzMyKHRhaWwtPnBoeXNhZGRyKTsKIAlsYXN0LT5uZXh0
 dGQgPSB0YWlsOwpAQCAtMzI1NCw4ICszMjU5LDExIEBAIG9oY2lfZGV2aWNlX2ludHJfc3Rh
 cnQoc3RydWN0IHVzYmRfeGZlciAKIAkgICAgKHVpbnRwdHJfdCl0YWlsLCAwLCAwKTsKIAlL
 QVNTRVJUKG9waXBlLT50YWlsLnRkID09IHRhaWwpOwogCi0JLyogV2Ugd2FudCBpbnRlcnJ1
 cHQgYXQgdGhlIGVuZCBvZiB0aGUgdHJhbnNmZXIuICovCi0JbGFzdC0+dGQudGRfZmxhZ3Mg
 Jj0gSFRPTzMyKH5PSENJX1REX0lOVFJfTUFTSyk7CisJLyoKKwkgKiBXZSB3YW50IGludGVy
 cnVwdCBhdCB0aGUgZW5kIG9mIHRoZSB0cmFuc2ZlciBhbmQgZm9yIHJvdW5kIHRvIGJlIGNs
 ZWFyLgorCSAqIEFsbCBzaG9ydCB0cmFuc2ZlcnMgYXJlIGhhbmRsZWQgdmlhIE9IQ0lfQ0Nf
 REFUQV9VTkRFUlJVTgorCSAqLworCWxhc3QtPnRkLnRkX2ZsYWdzICY9IH5IVE9PMzIoT0hD
 SV9URF9JTlRSX01BU0sgfCBPSENJX1REX1IpOwogCWxhc3QtPnRkLnRkX2ZsYWdzIHw9IEhU
 T08zMihPSENJX1REX1NFVF9ESSgxKSk7CiAKIAlsYXN0LT50ZC50ZF9uZXh0dGQgPSBIVE9P
 MzIodGFpbC0+cGh5c2FkZHIpOwo=
 --------------C60D37FCA30B99AC048D2B86--

From: Andreas Gustafsson <gson@gson.org>
To: Nick Hudson <nick.hudson@gmx.co.uk>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/54070: urtwn(4) does not work on OHCI
Date: Thu, 28 Nov 2019 12:18:32 +0200

 Nick Hudson wrote:
 > How about this untested patch?

 It has an extra close parenthesis after OHCI_TOGGLECARRY.  After
 fixing that, based on some rather limited testing with urtwn and
 umidi, it behaves the same as my patch.

 > I think it might help 50278 as well.

 It does not.
 -- 
 Andreas Gustafsson, gson@gson.org

From: "Andreas Gustafsson" <gson@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54070 CVS commit: src/sys/dev/usb
Date: Fri, 29 Nov 2019 14:13:04 +0000

 Module Name:	src
 Committed By:	gson
 Date:		Fri Nov 29 14:13:04 UTC 2019

 Modified Files:
 	src/sys/dev/usb: ohci.c

 Log Message:
 Preserve the toggleCarry bit in the Endpoint Descriptor when handling
 a DataUnderrun condition.  Fixes PR kern/54070.


 To generate a diff of this commit:
 cvs rdiff -u -r1.290 -r1.291 src/sys/dev/usb/ohci.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Andreas Gustafsson <gson@gson.org>
To: Nick Hudson <nick.hudson@gmx.co.uk>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/54070: urtwn(4) does not work on OHCI
Date: Fri, 29 Nov 2019 16:24:51 +0200

 Nick,

 I have now committed my patch.  As I see it, the question of whether
 the last TD should be handled via OHCI_T_R or OHCI_CC_DATA_UNDERRUN is
 a separate one, and may be just a matter of taste, so I'll leave it to
 you to argue for and/or make that change.
 -- 
 Andreas Gustafsson, gson@gson.org

State-Changed-From-To: open->needs-pullups
State-Changed-By: gson@NetBSD.org
State-Changed-When: Fri, 29 Nov 2019 14:37:01 +0000
State-Changed-Why:
Fix committed.


State-Changed-From-To: needs-pullups->pending-pullups
State-Changed-By: gson@NetBSD.org
State-Changed-When: Sat, 14 Dec 2019 11:05:28 +0000
State-Changed-Why:
Pullup requested.


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54070 CVS commit: [netbsd-9] src/sys/dev/usb
Date: Sat, 14 Dec 2019 12:30:58 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Sat Dec 14 12:30:58 UTC 2019

 Modified Files:
 	src/sys/dev/usb [netbsd-9]: ohci.c

 Log Message:
 Pull up following revision(s) (requested by gson in ticket #553):

 	sys/dev/usb/ohci.c: revision 1.291

 Preserve the toggleCarry bit in the Endpoint Descriptor when handling
 a DataUnderrun condition.  Fixes PR kern/54070.


 To generate a diff of this commit:
 cvs rdiff -u -r1.289.4.1 -r1.289.4.2 src/sys/dev/usb/ohci.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: gson@NetBSD.org
State-Changed-When: Sat, 14 Dec 2019 17:11:52 +0000
State-Changed-Why:
Pulled up.


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.45 2018/12/21 14:23:33 maya Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2017 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.