Many USB ethernet devices are broken over xhci

Many USB ethernet devices are broken over xhci

[David Laight]

Many of the net/usb ethernet drivers (including common ones like
the smsc95xx) will fail to transmit all packet length properly
when connected to a USB3 port (ie using the xhci driver).

The underlying problem is that they assume the host controller
will honour the URB_ZERO_PACKET flag - which usbnet sets because
they specify FLAG_SEND_ZLP.

However no one has ever added support for URB_ZERO_PACKET to
the xhci driver - so packets that need padding (probably 512
bytes after any header is added) will not be sent correctly
and may have very adverse effects on the usb target.

The ax179_178a driver avoids this by not setting FLAG_SEND_ZLP,
modifying the packet header, and appending an extra zero byte.
(Which has been responsible for its own set of panics.)

I don't think this can be fixed by just clearing (or ignoring)
FLAG_SEND_ZLP as the extra byte will also confuse things.

It needs to be fixed in the xhci code.

I wrote this patch a while ago - worked for me with the ax179_178a
driver. http://www.spinics.net/lists/linux-usb/msg97370.html

The patch is a bit difficult to read, the v1 version contained a copy of
the new function. http://www.spinics.net/lists/linux-usb/msg97183.html

I don't think anything significant has changed (in the main kernel
sources) since I wrote the patch.

	David




--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Many USB ethernet devices are broken over xhci

[Sarah Sharp]

David, you are overwhelming my capacity to handle patches right now.  I
appreciate that you would like to clean up the driver, but the bug fixes
you're sending me are getting lost in the noise of random cleanup
patches.

Can you please limit the xHCI patches you send to bug fixes for now?
Also, can you resend all the patches you consider to be serious bug
fixes in one single patchset?

Thanks,
Sarah Sharp

On Mon, Jan 27, 2014 at 04:06:22PM +0000, David Laight wrote:
> Many of the net/usb ethernet drivers (including common ones like
> the smsc95xx) will fail to transmit all packet length properly
> when connected to a USB3 port (ie using the xhci driver).
> 
> The underlying problem is that they assume the host controller
> will honour the URB_ZERO_PACKET flag - which usbnet sets because
> they specify FLAG_SEND_ZLP.
> 
> However no one has ever added support for URB_ZERO_PACKET to
> the xhci driver - so packets that need padding (probably 512
> bytes after any header is added) will not be sent correctly
> and may have very adverse effects on the usb target.
> 
> The ax179_178a driver avoids this by not setting FLAG_SEND_ZLP,
> modifying the packet header, and appending an extra zero byte.
> (Which has been responsible for its own set of panics.)
> 
> I don't think this can be fixed by just clearing (or ignoring)
> FLAG_SEND_ZLP as the extra byte will also confuse things.
> 
> It needs to be fixed in the xhci code.
> 
> I wrote this patch a while ago - worked for me with the ax179_178a
> driver. http://www.spinics.net/lists/linux-usb/msg97370.html
> 
> The patch is a bit difficult to read, the v1 version contained a copy of
> the new function. http://www.spinics.net/lists/linux-usb/msg97183.html
> 
> I don't think anything significant has changed (in the main kernel
> sources) since I wrote the patch.
> 
> 	David
> 
> 
> 
> 

Re: Many USB ethernet devices are broken over xhci

[Greg KH]

On Mon, Jan 27, 2014 at 04:06:22PM +0000, David Laight wrote:
> Many of the net/usb ethernet drivers (including common ones like
> the smsc95xx) will fail to transmit all packet length properly
> when connected to a USB3 port (ie using the xhci driver).

That's odd, as I've never had a problem with my USB 3.0 ethernet device
(sorry, don't remember what driver it uses), and I stress it all the
time.

I've also run two different USB 2 ethernet devices on xhci with no
issues at all, so perhaps the "many" statement might not be true?

> The underlying problem is that they assume the host controller
> will honour the URB_ZERO_PACKET flag - which usbnet sets because
> they specify FLAG_SEND_ZLP.
> 
> However no one has ever added support for URB_ZERO_PACKET to
> the xhci driver - so packets that need padding (probably 512
> bytes after any header is added) will not be sent correctly
> and may have very adverse effects on the usb target.

Really?  How has things been working so well up to now without this
support?  Doesn't the hardware automatically add this padding with no
explicit need for the xhci driver to do something?

> The ax179_178a driver avoids this by not setting FLAG_SEND_ZLP,
> modifying the packet header, and appending an extra zero byte.
> (Which has been responsible for its own set of panics.)
> 
> I don't think this can be fixed by just clearing (or ignoring)
> FLAG_SEND_ZLP as the extra byte will also confuse things.
> 
> It needs to be fixed in the xhci code.
> 
> I wrote this patch a while ago - worked for me with the ax179_178a
> driver. http://www.spinics.net/lists/linux-usb/msg97370.html

Doing multiple things in a patch is generally considered bad-form, and
this is quite hard to follow.

> The patch is a bit difficult to read, the v1 version contained a copy of
> the new function. http://www.spinics.net/lists/linux-usb/msg97183.html

That's not helpful to the people having to review the v2 patch :)

> I don't think anything significant has changed (in the main kernel
> sources) since I wrote the patch.

Then resubmit it, don't post links to it on the web, there's nothing we
can do with them, sorry.

thanks,

greg k-h

RE: Many USB ethernet devices are broken over xhci

[David Laight]

From: Greg KH 
> On Mon, Jan 27, 2014 at 04:06:22PM +0000, David Laight wrote:
> > Many of the net/usb ethernet drivers (including common ones like
> > the smsc95xx) will fail to transmit all packet length properly
> > when connected to a USB3 port (ie using the xhci driver).
> 
> That's odd, as I've never had a problem with my USB 3.0 ethernet device
> (sorry, don't remember what driver it uses), and I stress it all the
> time.

If it is the ax88179_178a driver then it carefully pads packets
that are multiples of the message size.

> I've also run two different USB 2 ethernet devices on xhci with no
> issues at all, so perhaps the "many" statement might not be true?

Dunno, quite a few will send down requests with URB_SEND_SERO set.
Whether this causes a problem depends on exactly how the ethernet
packets are encapsulated in usb ones.
The ax88179_178a hardware gets very confused if you fail to send
a ZLP - which it why it uses the 'bodge' in usbnet to append a byte.

> > The underlying problem is that they assume the host controller
> > will honour the URB_ZERO_PACKET flag - which usbnet sets because
> > they specify FLAG_SEND_ZLP.
> >
> > However no one has ever added support for URB_ZERO_PACKET to
> > the xhci driver - so packets that need padding (probably 512
> > bytes after any header is added) will not be sent correctly
> > and may have very adverse effects on the usb target.
> 
> Really?  How has things been working so well up to now without this
> support?  Doesn't the hardware automatically add this padding with no
> explicit need for the xhci driver to do something?

Nope - things must work by luck!
Try ping -n $((512-0x32)) on an smsc95xx card - it should need a ZLP.

> > I don't think anything significant has changed (in the main kernel
> > sources) since I wrote the patch.
> 
> Then resubmit it, don't post links to it on the web, there's nothing we
> can do with them, sorry.

I've resubmitted it as a v3 patch (to linux-usb) in small steps so that
the changes are easier to verify.
The final result is slightly different as I spotted a couple of minor
issues when re-reading the final version.
Mostly due to another 2 months of looking at this code.

	David




--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html