Patchwork [net-next] asix: asix_rx_fixup surgery to reduce skb truesizes

login
register
mail settings
Submitter Eric Dumazet
Date March 15, 2012, 6:18 a.m.
Message ID <1331792312.2543.16.camel@edumazet-laptop>
Download mbox | patch
Permalink /patch/146840/
State Accepted
Delegated to: David Miller
Headers show

Comments

Eric Dumazet - March 15, 2012, 6:18 a.m.
asix_rx_fixup() is complex, and does some unnecessary memory copies (at
least on x86 where NET_IP_ALIGN is 0)

Also, it tends to provide skbs with a big truesize (4096+256 with
MTU=1500) to upper stack, so incoming trafic consume a lot of memory and
I noticed early packet drops because we hit socket rcvbuf too fast.

Switch to a different strategy, using copybreak so that we provide nice
skbs to upper stack (including the NET_SKB_PAD to avoid future head
reallocations in some paths)

With this patch, I no longer see packets drops or tcp collapses on
various tcp workload with a AX88772 adapter.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Aurelien Jacobs <aurel@gnuage.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Trond Wuellner <trond@chromium.org>
Cc: Grant Grundler <grundler@chromium.org>
Cc: Paul Stewart <pstew@chromium.org>
---
 drivers/net/usb/asix.c |   90 +++++++++------------------------------
 1 file changed, 21 insertions(+), 69 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grant Grundler - March 15, 2012, 6:42 a.m.
Adding ASIX (Allan Chou)

On Wed, Mar 14, 2012 at 11:18 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> asix_rx_fixup() is complex, and does some unnecessary memory copies (at
> least on x86 where NET_IP_ALIGN is 0)
>
> Also, it tends to provide skbs with a big truesize (4096+256 with
> MTU=1500) to upper stack, so incoming trafic consume a lot of memory and
> I noticed early packet drops because we hit socket rcvbuf too fast.
>
> Switch to a different strategy, using copybreak so that we provide nice
> skbs to upper stack (including the NET_SKB_PAD to avoid future head
> reallocations in some paths)

Your rewrite is definitely cleaner/simpler. If it works, ship it! :)

I'm traveling right now and don't have access to the HW to test it.
I'm hoping that ASIX could run a few simple tests as well.

> With this patch, I no longer see packets drops or tcp collapses on
> various tcp workload with a AX88772 adapter.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Aurelien Jacobs <aurel@gnuage.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Trond Wuellner <trond@chromium.org>
> Cc: Grant Grundler <grundler@chromium.org>

Reviewed-by: Grant Grundler <grundler@chromium.org>

thanks!
grant

> Cc: Paul Stewart <pstew@chromium.org>
> ---
>  drivers/net/usb/asix.c |   90 +++++++++------------------------------
>  1 file changed, 21 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
> index 8e84f5b..27d5440 100644
> --- a/drivers/net/usb/asix.c
> +++ b/drivers/net/usb/asix.c
> @@ -305,88 +305,40 @@ asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
>
>  static int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
>  {
> -       u8  *head;
> -       u32  header;
> -       char *packet;
> -       struct sk_buff *ax_skb;
> -       u16 size;
> +       int offset = 0;
>
> -       head = (u8 *) skb->data;
> -       memcpy(&header, head, sizeof(header));
> -       le32_to_cpus(&header);
> -       packet = head + sizeof(header);
> -
> -       skb_pull(skb, 4);
> -
> -       while (skb->len > 0) {
> -               if ((header & 0x07ff) != ((~header >> 16) & 0x07ff))
> -                       netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
> +       while (offset + sizeof(u32) < skb->len) {
> +               struct sk_buff *ax_skb;
> +               u16 size;
> +               u32 header = get_unaligned_le32(skb->data + offset);
>
> +               offset += sizeof(u32);
> +
>                /* get the packet length */
> -               size = (u16) (header & 0x000007ff);
> -
> -               if ((skb->len) - ((size + 1) & 0xfffe) == 0) {
> -                       u8 alignment = (unsigned long)skb->data & 0x3;
> -                       if (alignment != 0x2) {
> -                               /*
> -                                * not 16bit aligned so use the room provided by
> -                                * the 32 bit header to align the data
> -                                *
> -                                * note we want 16bit alignment as MAC header is
> -                                * 14bytes thus ip header will be aligned on
> -                                * 32bit boundary so accessing ipheader elements
> -                                * using a cast to struct ip header wont cause
> -                                * an unaligned accesses.
> -                                */
> -                               u8 realignment = (alignment + 2) & 0x3;
> -                               memmove(skb->data - realignment,
> -                                       skb->data,
> -                                       size);
> -                               skb->data -= realignment;
> -                               skb_set_tail_pointer(skb, size);
> -                       }
> -                       return 2;
> +               size = (u16) (header & 0x7ff);
> +               if (size != ((~header >> 16) & 0x07ff)) {
> +                       netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
> +                       return 0;
>                }
>
> -               if (size > dev->net->mtu + ETH_HLEN) {
> +               if ((size > dev->net->mtu + ETH_HLEN) ||
> +                   (size + offset > skb->len)) {
>                        netdev_err(dev->net, "asix_rx_fixup() Bad RX Length %d\n",
>                                   size);
>                        return 0;
>                }
> -               ax_skb = skb_clone(skb, GFP_ATOMIC);
> -               if (ax_skb) {
> -                       u8 alignment = (unsigned long)packet & 0x3;
> -                       ax_skb->len = size;
> -
> -                       if (alignment != 0x2) {
> -                               /*
> -                                * not 16bit aligned use the room provided by
> -                                * the 32 bit header to align the data
> -                                */
> -                               u8 realignment = (alignment + 2) & 0x3;
> -                               memmove(packet - realignment, packet, size);
> -                               packet -= realignment;
> -                       }
> -                       ax_skb->data = packet;
> -                       skb_set_tail_pointer(ax_skb, size);
> -                       usbnet_skb_return(dev, ax_skb);
> -               } else {
> +               ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
> +               if (!ax_skb)
>                        return 0;
> -               }
> -
> -               skb_pull(skb, (size + 1) & 0xfffe);
>
> -               if (skb->len < sizeof(header))
> -                       break;
> +               skb_put(ax_skb, size);
> +               memcpy(ax_skb->data, skb->data + offset, size);
> +               usbnet_skb_return(dev, ax_skb);
>
> -               head = (u8 *) skb->data;
> -               memcpy(&header, head, sizeof(header));
> -               le32_to_cpus(&header);
> -               packet = head + sizeof(header);
> -               skb_pull(skb, 4);
> +               offset += (size + 1) & 0xfffe;
>        }
>
> -       if (skb->len < 0) {
> +       if (skb->len != offset) {
>                netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d\n",
>                           skb->len);
>                return 0;
> @@ -1541,7 +1493,7 @@ static const struct driver_info ax88772_info = {
>        .status = asix_status,
>        .link_reset = ax88772_link_reset,
>        .reset = ax88772_reset,
> -       .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR,
> +       .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
>        .rx_fixup = asix_rx_fixup,
>        .tx_fixup = asix_tx_fixup,
>  };
>
>
allan - March 15, 2012, 11:34 a.m.
Dear Grant and All,

We are going to do some testing on the revised asix.c driver with ASIX's different USB to LAN solutions (AX88178/AX88772B/AX88772A/AX88772). Please advise where can I download the latest revised asix.c driver source with this driver patch for further testing? (Sorry I am newbie on Linux kernel driver patch threads. =_="") Thanks a lot.

---
Best regards,
Allan Chou

-----Original Message-----
From: grundler@google.com [mailto:grundler@google.com] On Behalf Of Grant Grundler
Sent: Thursday, March 15, 2012 2:42 PM
To: Eric Dumazet
Cc: Greg Kroah-Hartman; David Miller; linux-usb@vger.kernel.org; netdev; Aurelien Jacobs; Trond Wuellner; Paul Stewart; Allan Chou
Subject: Re: [PATCH net-next] asix: asix_rx_fixup surgery to reduce skb truesizes

Adding ASIX (Allan Chou)

On Wed, Mar 14, 2012 at 11:18 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> asix_rx_fixup() is complex, and does some unnecessary memory copies (at
> least on x86 where NET_IP_ALIGN is 0)
>
> Also, it tends to provide skbs with a big truesize (4096+256 with
> MTU=1500) to upper stack, so incoming trafic consume a lot of memory and
> I noticed early packet drops because we hit socket rcvbuf too fast.
>
> Switch to a different strategy, using copybreak so that we provide nice
> skbs to upper stack (including the NET_SKB_PAD to avoid future head
> reallocations in some paths)

Your rewrite is definitely cleaner/simpler. If it works, ship it! :)

I'm traveling right now and don't have access to the HW to test it.
I'm hoping that ASIX could run a few simple tests as well.

> With this patch, I no longer see packets drops or tcp collapses on
> various tcp workload with a AX88772 adapter.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Aurelien Jacobs <aurel@gnuage.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Trond Wuellner <trond@chromium.org>
> Cc: Grant Grundler <grundler@chromium.org>

Reviewed-by: Grant Grundler <grundler@chromium.org>

thanks!
grant

> Cc: Paul Stewart <pstew@chromium.org>
> ---
>  drivers/net/usb/asix.c |   90 +++++++++------------------------------
>  1 file changed, 21 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
> index 8e84f5b..27d5440 100644
> --- a/drivers/net/usb/asix.c
> +++ b/drivers/net/usb/asix.c
> @@ -305,88 +305,40 @@ asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
>
>  static int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
>  {
> -       u8  *head;
> -       u32  header;
> -       char *packet;
> -       struct sk_buff *ax_skb;
> -       u16 size;
> +       int offset = 0;
>
> -       head = (u8 *) skb->data;
> -       memcpy(&header, head, sizeof(header));
> -       le32_to_cpus(&header);
> -       packet = head + sizeof(header);
> -
> -       skb_pull(skb, 4);
> -
> -       while (skb->len > 0) {
> -               if ((header & 0x07ff) != ((~header >> 16) & 0x07ff))
> -                       netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
> +       while (offset + sizeof(u32) < skb->len) {
> +               struct sk_buff *ax_skb;
> +               u16 size;
> +               u32 header = get_unaligned_le32(skb->data + offset);
>
> +               offset += sizeof(u32);
> +
>                /* get the packet length */
> -               size = (u16) (header & 0x000007ff);
> -
> -               if ((skb->len) - ((size + 1) & 0xfffe) == 0) {
> -                       u8 alignment = (unsigned long)skb->data & 0x3;
> -                       if (alignment != 0x2) {
> -                               /*
> -                                * not 16bit aligned so use the room provided by
> -                                * the 32 bit header to align the data
> -                                *
> -                                * note we want 16bit alignment as MAC header is
> -                                * 14bytes thus ip header will be aligned on
> -                                * 32bit boundary so accessing ipheader elements
> -                                * using a cast to struct ip header wont cause
> -                                * an unaligned accesses.
> -                                */
> -                               u8 realignment = (alignment + 2) & 0x3;
> -                               memmove(skb->data - realignment,
> -                                       skb->data,
> -                                       size);
> -                               skb->data -= realignment;
> -                               skb_set_tail_pointer(skb, size);
> -                       }
> -                       return 2;
> +               size = (u16) (header & 0x7ff);
> +               if (size != ((~header >> 16) & 0x07ff)) {
> +                       netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
> +                       return 0;
>                }
>
> -               if (size > dev->net->mtu + ETH_HLEN) {
> +               if ((size > dev->net->mtu + ETH_HLEN) ||
> +                   (size + offset > skb->len)) {
>                        netdev_err(dev->net, "asix_rx_fixup() Bad RX Length %d\n",
>                                   size);
>                        return 0;
>                }
> -               ax_skb = skb_clone(skb, GFP_ATOMIC);
> -               if (ax_skb) {
> -                       u8 alignment = (unsigned long)packet & 0x3;
> -                       ax_skb->len = size;
> -
> -                       if (alignment != 0x2) {
> -                               /*
> -                                * not 16bit aligned use the room provided by
> -                                * the 32 bit header to align the data
> -                                */
> -                               u8 realignment = (alignment + 2) & 0x3;
> -                               memmove(packet - realignment, packet, size);
> -                               packet -= realignment;
> -                       }
> -                       ax_skb->data = packet;
> -                       skb_set_tail_pointer(ax_skb, size);
> -                       usbnet_skb_return(dev, ax_skb);
> -               } else {
> +               ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
> +               if (!ax_skb)
>                        return 0;
> -               }
> -
> -               skb_pull(skb, (size + 1) & 0xfffe);
>
> -               if (skb->len < sizeof(header))
> -                       break;
> +               skb_put(ax_skb, size);
> +               memcpy(ax_skb->data, skb->data + offset, size);
> +               usbnet_skb_return(dev, ax_skb);
>
> -               head = (u8 *) skb->data;
> -               memcpy(&header, head, sizeof(header));
> -               le32_to_cpus(&header);
> -               packet = head + sizeof(header);
> -               skb_pull(skb, 4);
> +               offset += (size + 1) & 0xfffe;
>        }
>
> -       if (skb->len < 0) {
> +       if (skb->len != offset) {
>                netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d\n",
>                           skb->len);
>                return 0;
> @@ -1541,7 +1493,7 @@ static const struct driver_info ax88772_info = {
>        .status = asix_status,
>        .link_reset = ax88772_link_reset,
>        .reset = ax88772_reset,
> -       .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR,
> +       .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
>        .rx_fixup = asix_rx_fixup,
>        .tx_fixup = asix_tx_fixup,
>  };
>
>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet - March 15, 2012, 11:49 a.m.
Le jeudi 15 mars 2012 à 19:34 +0800, ASIX Allan Email [office] a écrit :
> Dear Grant and All,
> 
> We are going to do some testing on the revised asix.c driver with
> ASIX's different USB to LAN solutions
> (AX88178/AX88772B/AX88772A/AX88772). Please advise where can I
> download the latest revised asix.c driver source with this driver
> patch for further testing? (Sorry I am newbie on Linux kernel driver
> patch threads. =_="") Thanks a lot.
> 
Hi Allan

I based this patch on net-next tree, the official network 'next' tree
maintained by David Miller, available here :

http://git2.kernel.org/?p=linux/kernel/git/davem/net-next.git;a=summary

Does it answer to your question ?


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - March 16, 2012, 8:52 a.m.
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 14 Mar 2012 23:18:32 -0700

> asix_rx_fixup() is complex, and does some unnecessary memory copies (at
> least on x86 where NET_IP_ALIGN is 0)
> 
> Also, it tends to provide skbs with a big truesize (4096+256 with
> MTU=1500) to upper stack, so incoming trafic consume a lot of memory and
> I noticed early packet drops because we hit socket rcvbuf too fast.
> 
> Switch to a different strategy, using copybreak so that we provide nice
> skbs to upper stack (including the NET_SKB_PAD to avoid future head
> reallocations in some paths)
> 
> With this patch, I no longer see packets drops or tcp collapses on
> various tcp workload with a AX88772 adapter.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
allan - March 20, 2012, 2:14 p.m.
Dear All,

Resend this email due to illegal attachment in my previous email. Please let me know if you need to get the attachment. Thanks a lot.

---
Best regards,
Allan Chou
Technical Support Division
ASIX Electronics Corporation
TEL: 886-3-5799500 ext.228
FAX: 886-3-5799558
E-mail: allan@asix.com.tw 
http://www.asix.com.tw/ 

-----Original Message-----
From: ASIX Allan Email [office] [mailto:allan@asix.com.tw] 
Sent: Tuesday, March 20, 2012 8:42 PM
To: 'Grant Grundler'; 'Eric Dumazet'
Cc: 'Greg Kroah-Hartman'; 'David Miller'; 'linux-usb@vger.kernel.org'; 'netdev'; 'Aurelien Jacobs'; 'Trond Wuellner'; 'Paul Stewart'
Subject: RE: [PATCH net-next] asix: asix_rx_fixup surgery to reduce skb truesizes
Importance: High

Dear All,

I did some basic network functionality of this driver patch on Linux kernel 3.3.0-rc3 x86 platform with below ASIX USB to LAN dongles and it seems this driver patch works fine in our site. You can refer to the driver log files in attached driver package for more details if necessary. Thanks a lot.

====================
AX88772B USB dongle
AX88772A USB dongle
AX88772 USB dongle
AX88178 + RTL8251CN GigaPHY USB dongle
AX88178 + RTL8211CL GigaPHY USB dongle
AX88178 + M88E1111 GigaPHY USB dongle
Belkin F5D5055 AX88178 USB dongle


---
Best regards,
Allan Chou
Technical Support Division
ASIX Electronics Corporation
TEL: 886-3-5799500 ext.228
FAX: 886-3-5799558
E-mail: allan@asix.com.tw 
http://www.asix.com.tw/ 


-----Original Message-----
From: ASIX Allan Email [office] [mailto:allan@asix.com.tw] 
Sent: Thursday, March 15, 2012 7:35 PM
To: 'Grant Grundler'; 'Eric Dumazet'
Cc: 'Greg Kroah-Hartman'; 'David Miller'; 'linux-usb@vger.kernel.org'; 'netdev'; 'Aurelien Jacobs'; 'Trond Wuellner'; 'Paul Stewart'
Subject: RE: [PATCH net-next] asix: asix_rx_fixup surgery to reduce skb truesizes

Dear Grant and All,

We are going to do some testing on the revised asix.c driver with ASIX's different USB to LAN solutions (AX88178/AX88772B/AX88772A/AX88772). Please advise where can I download the latest revised asix.c driver source with this driver patch for further testing? (Sorry I am newbie on Linux kernel driver patch threads. =_="") Thanks a lot.

---
Best regards,
Allan Chou

-----Original Message-----
From: grundler@google.com [mailto:grundler@google.com] On Behalf Of Grant Grundler
Sent: Thursday, March 15, 2012 2:42 PM
To: Eric Dumazet
Cc: Greg Kroah-Hartman; David Miller; linux-usb@vger.kernel.org; netdev; Aurelien Jacobs; Trond Wuellner; Paul Stewart; Allan Chou
Subject: Re: [PATCH net-next] asix: asix_rx_fixup surgery to reduce skb truesizes

Adding ASIX (Allan Chou)

On Wed, Mar 14, 2012 at 11:18 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> asix_rx_fixup() is complex, and does some unnecessary memory copies (at
> least on x86 where NET_IP_ALIGN is 0)
>
> Also, it tends to provide skbs with a big truesize (4096+256 with
> MTU=1500) to upper stack, so incoming trafic consume a lot of memory and
> I noticed early packet drops because we hit socket rcvbuf too fast.
>
> Switch to a different strategy, using copybreak so that we provide nice
> skbs to upper stack (including the NET_SKB_PAD to avoid future head
> reallocations in some paths)

Your rewrite is definitely cleaner/simpler. If it works, ship it! :)

I'm traveling right now and don't have access to the HW to test it.
I'm hoping that ASIX could run a few simple tests as well.

> With this patch, I no longer see packets drops or tcp collapses on
> various tcp workload with a AX88772 adapter.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Aurelien Jacobs <aurel@gnuage.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Trond Wuellner <trond@chromium.org>
> Cc: Grant Grundler <grundler@chromium.org>

Reviewed-by: Grant Grundler <grundler@chromium.org>

thanks!
grant

> Cc: Paul Stewart <pstew@chromium.org>
> ---
>  drivers/net/usb/asix.c |   90 +++++++++------------------------------
>  1 file changed, 21 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
> index 8e84f5b..27d5440 100644
> --- a/drivers/net/usb/asix.c
> +++ b/drivers/net/usb/asix.c
> @@ -305,88 +305,40 @@ asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
>
>  static int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
>  {
> -       u8  *head;
> -       u32  header;
> -       char *packet;
> -       struct sk_buff *ax_skb;
> -       u16 size;
> +       int offset = 0;
>
> -       head = (u8 *) skb->data;
> -       memcpy(&header, head, sizeof(header));
> -       le32_to_cpus(&header);
> -       packet = head + sizeof(header);
> -
> -       skb_pull(skb, 4);
> -
> -       while (skb->len > 0) {
> -               if ((header & 0x07ff) != ((~header >> 16) & 0x07ff))
> -                       netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
> +       while (offset + sizeof(u32) < skb->len) {
> +               struct sk_buff *ax_skb;
> +               u16 size;
> +               u32 header = get_unaligned_le32(skb->data + offset);
>
> +               offset += sizeof(u32);
> +
>                /* get the packet length */
> -               size = (u16) (header & 0x000007ff);
> -
> -               if ((skb->len) - ((size + 1) & 0xfffe) == 0) {
> -                       u8 alignment = (unsigned long)skb->data & 0x3;
> -                       if (alignment != 0x2) {
> -                               /*
> -                                * not 16bit aligned so use the room provided by
> -                                * the 32 bit header to align the data
> -                                *
> -                                * note we want 16bit alignment as MAC header is
> -                                * 14bytes thus ip header will be aligned on
> -                                * 32bit boundary so accessing ipheader elements
> -                                * using a cast to struct ip header wont cause
> -                                * an unaligned accesses.
> -                                */
> -                               u8 realignment = (alignment + 2) & 0x3;
> -                               memmove(skb->data - realignment,
> -                                       skb->data,
> -                                       size);
> -                               skb->data -= realignment;
> -                               skb_set_tail_pointer(skb, size);
> -                       }
> -                       return 2;
> +               size = (u16) (header & 0x7ff);
> +               if (size != ((~header >> 16) & 0x07ff)) {
> +                       netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
> +                       return 0;
>                }
>
> -               if (size > dev->net->mtu + ETH_HLEN) {
> +               if ((size > dev->net->mtu + ETH_HLEN) ||
> +                   (size + offset > skb->len)) {
>                        netdev_err(dev->net, "asix_rx_fixup() Bad RX Length %d\n",
>                                   size);
>                        return 0;
>                }
> -               ax_skb = skb_clone(skb, GFP_ATOMIC);
> -               if (ax_skb) {
> -                       u8 alignment = (unsigned long)packet & 0x3;
> -                       ax_skb->len = size;
> -
> -                       if (alignment != 0x2) {
> -                               /*
> -                                * not 16bit aligned use the room provided by
> -                                * the 32 bit header to align the data
> -                                */
> -                               u8 realignment = (alignment + 2) & 0x3;
> -                               memmove(packet - realignment, packet, size);
> -                               packet -= realignment;
> -                       }
> -                       ax_skb->data = packet;
> -                       skb_set_tail_pointer(ax_skb, size);
> -                       usbnet_skb_return(dev, ax_skb);
> -               } else {
> +               ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
> +               if (!ax_skb)
>                        return 0;
> -               }
> -
> -               skb_pull(skb, (size + 1) & 0xfffe);
>
> -               if (skb->len < sizeof(header))
> -                       break;
> +               skb_put(ax_skb, size);
> +               memcpy(ax_skb->data, skb->data + offset, size);
> +               usbnet_skb_return(dev, ax_skb);
>
> -               head = (u8 *) skb->data;
> -               memcpy(&header, head, sizeof(header));
> -               le32_to_cpus(&header);
> -               packet = head + sizeof(header);
> -               skb_pull(skb, 4);
> +               offset += (size + 1) & 0xfffe;
>        }
>
> -       if (skb->len < 0) {
> +       if (skb->len != offset) {
>                netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d\n",
>                           skb->len);
>                return 0;
> @@ -1541,7 +1493,7 @@ static const struct driver_info ax88772_info = {
>        .status = asix_status,
>        .link_reset = ax88772_link_reset,
>        .reset = ax88772_reset,
> -       .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR,
> +       .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
>        .rx_fixup = asix_rx_fixup,
>        .tx_fixup = asix_tx_fixup,
>  };
>
>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet - March 20, 2012, 2:28 p.m.
On Tue, 2012-03-20 at 22:14 +0800, allan wrote:
> Dear All,
> 
> Resend this email due to illegal attachment in my previous email. Please let me know if you need to get the attachment. Thanks a lot.

Thanks a lot for testing Allan


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

Patch

diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
index 8e84f5b..27d5440 100644
--- a/drivers/net/usb/asix.c
+++ b/drivers/net/usb/asix.c
@@ -305,88 +305,40 @@  asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
 
 static int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
-	u8  *head;
-	u32  header;
-	char *packet;
-	struct sk_buff *ax_skb;
-	u16 size;
+	int offset = 0;
 
-	head = (u8 *) skb->data;
-	memcpy(&header, head, sizeof(header));
-	le32_to_cpus(&header);
-	packet = head + sizeof(header);
-
-	skb_pull(skb, 4);
-
-	while (skb->len > 0) {
-		if ((header & 0x07ff) != ((~header >> 16) & 0x07ff))
-			netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
+	while (offset + sizeof(u32) < skb->len) {
+		struct sk_buff *ax_skb;
+		u16 size;
+		u32 header = get_unaligned_le32(skb->data + offset);
 
+		offset += sizeof(u32);
+	
 		/* get the packet length */
-		size = (u16) (header & 0x000007ff);
-
-		if ((skb->len) - ((size + 1) & 0xfffe) == 0) {
-			u8 alignment = (unsigned long)skb->data & 0x3;
-			if (alignment != 0x2) {
-				/*
-				 * not 16bit aligned so use the room provided by
-				 * the 32 bit header to align the data
-				 *
-				 * note we want 16bit alignment as MAC header is
-				 * 14bytes thus ip header will be aligned on
-				 * 32bit boundary so accessing ipheader elements
-				 * using a cast to struct ip header wont cause
-				 * an unaligned accesses.
-				 */
-				u8 realignment = (alignment + 2) & 0x3;
-				memmove(skb->data - realignment,
-					skb->data,
-					size);
-				skb->data -= realignment;
-				skb_set_tail_pointer(skb, size);
-			}
-			return 2;
+		size = (u16) (header & 0x7ff);
+		if (size != ((~header >> 16) & 0x07ff)) {
+			netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
+			return 0;
 		}
 
-		if (size > dev->net->mtu + ETH_HLEN) {
+		if ((size > dev->net->mtu + ETH_HLEN) ||
+		    (size + offset > skb->len)) {
 			netdev_err(dev->net, "asix_rx_fixup() Bad RX Length %d\n",
 				   size);
 			return 0;
 		}
-		ax_skb = skb_clone(skb, GFP_ATOMIC);
-		if (ax_skb) {
-			u8 alignment = (unsigned long)packet & 0x3;
-			ax_skb->len = size;
-
-			if (alignment != 0x2) {
-				/*
-				 * not 16bit aligned use the room provided by
-				 * the 32 bit header to align the data
-				 */
-				u8 realignment = (alignment + 2) & 0x3;
-				memmove(packet - realignment, packet, size);
-				packet -= realignment;
-			}
-			ax_skb->data = packet;
-			skb_set_tail_pointer(ax_skb, size);
-			usbnet_skb_return(dev, ax_skb);
-		} else {
+		ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
+		if (!ax_skb)
 			return 0;
-		}
-
-		skb_pull(skb, (size + 1) & 0xfffe);
 
-		if (skb->len < sizeof(header))
-			break;
+		skb_put(ax_skb, size);
+		memcpy(ax_skb->data, skb->data + offset, size);
+		usbnet_skb_return(dev, ax_skb);
 
-		head = (u8 *) skb->data;
-		memcpy(&header, head, sizeof(header));
-		le32_to_cpus(&header);
-		packet = head + sizeof(header);
-		skb_pull(skb, 4);
+		offset += (size + 1) & 0xfffe;
 	}
 
-	if (skb->len < 0) {
+	if (skb->len != offset) {
 		netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d\n",
 			   skb->len);
 		return 0;
@@ -1541,7 +1493,7 @@  static const struct driver_info ax88772_info = {
 	.status = asix_status,
 	.link_reset = ax88772_link_reset,
 	.reset = ax88772_reset,
-	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR,
+	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
 	.rx_fixup = asix_rx_fixup,
 	.tx_fixup = asix_tx_fixup,
 };