From: Martin Fuzzey <hidden> Date: 2010-08-11 09:41:41
Hi all,
I have a DLINK DUB-E100 USB / Ethernet adapter with using the Asix
AX88772 chipset.
This device works fine on linux x86 using the usbnet based asix driver
however on ARM (iMX21 SoC, kernel 2.6.35) transmit works but receive
fails with:
"asix_rx_fixup() Bad Header Length"
Digging a bit showed that the driver was submitting a non aligned data
pointer in the URB.
This is due to:
skb_reserve (skb, NET_IP_ALIGN);
in net/usb/usbnet.c rx_submit()
NET_IP_ALIGN is defined as 2 in skbuff.h (default value as not defined
by ARM) hence the data buffer address was offset by 2 whereas the hcd
requires 4 byte alignment to work at all and cache line (32 byte here)
alignment to work reliably.
Note that usbnet uses skb->data as the buffer submitted to USB in the URB.
Removing this call to skb_reserve() fixes the problem for me.
However the comments in skbuff.h make it clear that this is done
deliberately knowing that it will cause non aligned DMA but seem to
suggest that this should work anyway (even if less efficiently).
My understanding, based on previous discussions on -usb a year back
while I was working on the usb hcd is that the hcd does _not_ have to
support unaligned DMA.
So what is the correct fix for this problem?
The options seem to be:
1) Remove skb_reserve() from usbnet
But this will cause the ip header to be non aligned on platforms that
don't have the DMA alignment requirement.
2) Define NET_IP_ALIGN to 0 for ARM
Seems a bit intrusive to me.
3) Change the HCD to copy if not aligned
I'd like to avoid that.
4) Change usbnet to use seperate buffers for the usb side and skb.
Copy between them so both remain aligned
Seems unlikely that the gain further up the stack from ensuring the ip
header is aligned will offset the cost of the copy.
Any ideas?
On a related note, this is the second time I've run into this type of
problem.It would be much easier to debug if the HCD warned (or
refused) misaligned URBs. If HCDs provided information on their
alignment requirements then the USB core could do this in a uniform
way. For example adding an alignment field to struct hc_driver or
struct usb_hcd?
Cheers,
Martin
--
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
From: Russell King - ARM Linux <hidden> Date: 2010-08-11 09:54:54
On Wed, Aug 11, 2010 at 11:41:41AM +0200, Martin Fuzzey wrote:
Digging a bit showed that the driver was submitting a non aligned data
pointer in the URB.
This is due to:
skb_reserve (skb, NET_IP_ALIGN);
in net/usb/usbnet.c rx_submit()
NET_IP_ALIGN is defined as 2 in skbuff.h (default value as not defined
by ARM) hence the data buffer address was offset by 2 whereas the hcd
requires 4 byte alignment to work at all and cache line (32 byte here)
alignment to work reliably.
x86 also has NET_IP_ALIGN as 2, so it will also be IP-header aligned there
too, so the restriction won't be from the USB stack.
Removing this call to skb_reserve() fixes the problem for me.
However, that makes parsing and creating IP headers really inefficient,
especially as we take a trap each time a misaligned access is performed.
3) Change the HCD to copy if not aligned
I'd like to avoid that.
You haven't said what HCD you're using.
On a related note, this is the second time I've run into this type of
problem.It would be much easier to debug if the HCD warned (or
refused) misaligned URBs.
If HCD can't cope with misaligned URBs, it should fail across the board -
or be made to warn across the board - but I suspect it's a specific HCD
that you're using which is only applicable to ARM.
In that case, I'd say the HCD must handle misaligned URBs itself so
behaviour is consistent between all implementations on different
platforms.
--
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
So what is the correct fix for this problem?
The options seem to be:
1) Remove skb_reserve() from usbnet
But this will cause the ip header to be non aligned on platforms that
don't have the DMA alignment requirement.
And cause lot's of unaligned access for the IP stack. This will slow
down arm platform...
2) Define NET_IP_ALIGN to 0 for ARM
Seems a bit intrusive to me.
Again this will generate lot's of unaligned access.
3) Change the HCD to copy if not aligned
I'd like to avoid that.
4) Change usbnet to use seperate buffers for the usb side and skb.
Copy between them so both remain aligned
Seems unlikely that the gain further up the stack from ensuring the ip
header is aligned will offset the cost of the copy.
Any ideas?
I got the same problem on the gadget side :
http://article.gmane.org/gmane.linux.usb.general/28700
A solution could be the hcd driver to tell it doesn't support unaligned
transfert and in this case the usb stack/usb driver align it.
It is best to solve in usb driver, because for example in Asix case the
driver already do copy (see asix_rx_fixup/asix_tx_fixup).
Matthieu
PS : what hcd driver do you use ?
--
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
From: Martin Fuzzey <hidden> Date: 2010-08-11 11:38:37
On Wed, Aug 11, 2010 at 11:59 AM, Matthieu CASTET
[off-list ref] wrote:
It is best to solve in usb driver, because for example in Asix case the
driver already do copy (see asix_rx_fixup/asix_tx_fixup).
Yes, however those functions are only called for devices which register them.
Looking at the driver_info and product id tables shows that it's only
a subset of the devices that asix supports (88772 and 88178 chips)
In fact my hardware _is_ included
// DLink DUB-E100 H/W Ver B1 Alternate
USB_DEVICE (0x2001, 0x3c05),
.driver_info = (unsigned long) &ax88772_info,
but fixing this in asix_rx_fixup wouldn't solve it for the other
supported devices.
It seems to me these fixup functions are not intended to solve
alignment issues but rather implement device specific framing (such as
when the hardware packs multiple ethernet frames into a single urb)
PS : what hcd driver do you use ?
imx21_hcd
Martin
--
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
No, that thread is about stack vs. heap allocations, not about alignment
issues.
Well although the issue discussed in that thread was caused by a stack
allocation isn't the issue here the same?
My understanding is that a heap allocation as returned by kmalloc() will be:
1) correctly aligned for DMA
and
2) in a memory zone accessible to DMA
whereas a stack allocation is not guaranteed to have either of these properties.
The problem I described in that thread was due to case 1
(misalignment) rather than the stack memory zone not being accessible
at all to DMA.
To which was the reply was basically "use a heap allocation".
So the question is are hcds expected to accept arbitarilly aligned but
heap allocated pointers (such as the result of kmalloc() + 1)?
regards,
Martin
No, that thread is about stack vs. heap allocations, not about alignment
issues.
Well although the issue discussed in that thread was caused by a stack
allocation isn't the issue here the same?
My understanding is that a heap allocation as returned by kmalloc() will be:
1) correctly aligned for DMA
and
2) in a memory zone accessible to DMA
whereas a stack allocation is not guaranteed to have either of these properties.
The problem I described in that thread was due to case 1
(misalignment) rather than the stack memory zone not being accessible
at all to DMA.
To which was the reply was basically "use a heap allocation".
So the question is are hcds expected to accept arbitarilly aligned but
heap allocated pointers (such as the result of kmalloc() + 1)?
It sounds like your HCD doesn't like this, so perhaps we should make
that rule :)
If you allocate the urb with a kmalloc() call with no offset, does it
all work properly? The driver should be calling usb_alloc_urb() which
does this automatically for them, right? Or is it trying to allocate
things on its own somehow?
thanks,
greg k-h
--
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
From: Martin Fuzzey <hidden> Date: 2010-08-11 19:07:27
Greg KH wrote:
quoted
So the question is are hcds expected to accept arbitarilly aligned but
heap allocated pointers (such as the result of kmalloc() + 1)?
It sounds like your HCD doesn't like this, so perhaps we should make
that rule :)
If you allocate the urb with a kmalloc() call with no offset, does it
all work properly?
Yes
The driver should be calling usb_alloc_urb() which
does this automatically for them, right? Or is it trying to allocate
things on its own somehow?
It's not the URB itself (which is allocated by usb_alloc_urb) but rather
the buffer pointer within the URB that causes the problem.
It's the asix driver (or more exactly the usbnet core used by that driver).
It does (rx_submit() in drivers/net/usb/usbnet.c):
urb = usb_alloc_urb();
skb = alloc_skb (...);
skb_reserve (skb, NET_IP_ALIGN);
usb_fill_bulk_urb (urb,... skb->data);
usb_submit_urb(urb)
skb->data as returned by alloc_skb() is aligned
but skb_reserve adds 2.
Thus removing the skb_reserve() call makes it work.
BUT if I do that the IP header is no longer aligned so accesses further
up the network stack have to be fixed up by exception handlers which is
expensive (even with hcds which don't require this)
cheers,
Martin
--
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
Am Mittwoch 11 August 2010 19:42:38 schrieb Greg KH:
quoted
So the question is are hcds expected to accept arbitarilly aligned but
heap allocated pointers (such as the result of kmalloc() + 1)?
It sounds like your HCD doesn't like this, so perhaps we should make
that rule :)
If you allocate the urb with a kmalloc() call with no offset, does it
all work properly? The driver should be calling usb_alloc_urb() which
does this automatically for them, right? Or is it trying to allocate
things on its own somehow?
The buffer is the problem not the URB. And up to now the alignment
was not specified, but drivers are generally written assuming byte
granularity.
Regards
Oliver
--
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
On Wed, Aug 11, 2010 at 09:07:27PM +0200, Martin Fuzzey wrote:
Greg KH wrote:
quoted
quoted
So the question is are hcds expected to accept arbitarilly aligned but
heap allocated pointers (such as the result of kmalloc() + 1)?
It sounds like your HCD doesn't like this, so perhaps we should make
that rule :)
If you allocate the urb with a kmalloc() call with no offset, does it
all work properly?
Yes
quoted
The driver should be calling usb_alloc_urb() which
does this automatically for them, right? Or is it trying to allocate
things on its own somehow?
It's not the URB itself (which is allocated by usb_alloc_urb) but rather
the buffer pointer within the URB that causes the problem.
Doh, you are right, sorry about that.
It's the asix driver (or more exactly the usbnet core used by that driver).
It does (rx_submit() in drivers/net/usb/usbnet.c):
urb = usb_alloc_urb();
skb = alloc_skb (...);
skb_reserve (skb, NET_IP_ALIGN);
usb_fill_bulk_urb (urb,... skb->data);
usb_submit_urb(urb)
skb->data as returned by alloc_skb() is aligned
but skb_reserve adds 2.
Thus removing the skb_reserve() call makes it work.
BUT if I do that the IP header is no longer aligned so accesses further
up the network stack have to be fixed up by exception handlers which is
expensive (even with hcds which don't require this)
Can you fix this in the host controller driver?
thanks,
greg k-h
From: Martin Fuzzey <hidden> Date: 2010-08-11 22:31:14
Greg KH wrote:
Can you fix this in the host controller driver?
Technically yes (by copying the unaligned buffers).
But if we do decide that USB must support unaligned buffers wouldn't it
be better to have the HCD indicate it's alignment requirement to the
core and have the core do the copying?
Gary King sent a message to this thread but only to the arm list saying
he has the same problem with the tegra hcd (which doesn't seem to be in
the tree yet.) I don't know if any of the other in tree HCDs have this
problem.
Also if this is a new requirement for HCDs usbtest probably needs a new
test...
Martin
--
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
Technically yes (by copying the unaligned buffers).
But if we do decide that USB must support unaligned buffers wouldn't it
be better to have the HCD indicate it's alignment requirement to the
core and have the core do the copying?
Gary King sent a message to this thread but only to the arm list saying
he has the same problem with the tegra hcd (which doesn't seem to be in
the tree yet.) I don't know if any of the other in tree HCDs have this
problem.
dwc otg also got this limitation. I don't know if the version submitted
for inclusion handle unaligned buffers.
Matthieu