Messages in this thread | | | Subject | Re: [PATCH] ipheth.c: Enable IP header alignment | From | Ben Hutchings <> | Date | Mon, 02 May 2011 22:04:34 +0100 |
| |
On Mon, 2011-05-02 at 21:35 +0200, L. Alberto Giménez wrote: > Since commit ea812ca1b06113597adcd8e70c0f84a413d97544 (x86: Align skb w/ start > of cacheline on newer core 2/Xeon Arch), NET_IP_ALIGN changed from 2 to 0, and > the constant is used to reserve more room for the socket buffer. > > Some people have reported that tethering stopped working and David Hill > submited a patch that redefined NET_IP_ALIGN. Pointed by Ben Hutchings, the > patch has been reworked to use a private constant. > > I have no more an iPhone device to test it, so it is only compile-tested. > > Signed-off-by: L. Alberto Giménez <agimenez@sysvalve.es> > --- > drivers/net/usb/ipheth.c | 8 +++++--- > 1 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/usb/ipheth.c b/drivers/net/usb/ipheth.c > index 7d42f9a..8f1ffc7 100644 > --- a/drivers/net/usb/ipheth.c > +++ b/drivers/net/usb/ipheth.c > @@ -80,6 +80,8 @@ > #define IPHETH_CARRIER_CHECK_TIMEOUT round_jiffies_relative(1 * HZ) > #define IPHETH_CARRIER_ON 0x04 > > +#define IPHETH_IP_ALIGN 2 > + > static struct usb_device_id ipheth_table[] = { > { USB_DEVICE_AND_INTERFACE_INFO( > USB_VENDOR_APPLE, USB_PRODUCT_IPHONE, > @@ -205,15 +207,15 @@ static void ipheth_rcvbulk_callback(struct urb *urb) > len = urb->actual_length; > buf = urb->transfer_buffer; > > - skb = dev_alloc_skb(NET_IP_ALIGN + len); > + skb = dev_alloc_skb(IPHETH_IP_ALIGN + len); > if (!skb) { > err("%s: dev_alloc_skb: -ENOMEM", __func__); > dev->net->stats.rx_dropped++; > return; > } > > - skb_reserve(skb, NET_IP_ALIGN); > - memcpy(skb_put(skb, len), buf + NET_IP_ALIGN, len - NET_IP_ALIGN); > + skb_reserve(skb, IPHETH_IP_ALIGN); > + memcpy(skb_put(skb, len), buf + IPHETH_IP_ALIGN, len - IPHETH_IP_ALIGN); [...]
So this was using NET_IP_ALIGN as an offset into the URB. Which was totally bogus, as its value has long been architecture-dependent. The code is also claiming to put len bytes but only copying len - delta.
The correct code would be something like:
if (urb->actual_length <= IPHETH_IP_ALIGN) { dev->net->stats.rx_length_errors++; return; } len = urb->actual_length - IPHETH_IP_ALIGN; buf = urb->transfer_buffer + IPHETH_IP_ALIGN; dev_alloc_skb(len); ... memcpy(skb_put(skb, len), buf, len);
Ben.
> skb->dev = dev->net; > skb->protocol = eth_type_trans(skb, dev->net); >
-- Ben Hutchings, Senior Software Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |