Hi Louis, Please help to take care of this problem. Thanks a lot. --- Best regards, Allan Chou ASIX Electronics Corporation TEL: 886-3-5799500 ext.228 FAX: 886-3-5799558 E-mail: allan@xxxxxxxxxxx https://www.asix.com.tw/ -----Original Message----- From: Jeremy Kerr <jk@xxxxxxxxxx> Sent: Tuesday, June 2, 2020 11:18 AM To: Freddy Xin <freddy@xxxxxxxxxxx>; Allan Chou <allan@xxxxxxxxxxx> Cc: Peter Fink <pfink@xxxxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx Subject: Re: [RFC PATCH] net: usb: ax88179_178a: fix packet alignment padding Hi Freddy and Allan, Just following up on the RFC patch below: Can you confirm whether the packet len (in the hardware-provided packet RX metadata) includes the two-byte padding field? Is this the same for all ax88179 devices? Cheers, Jeremy > Using a AX88179 device (0b95:1790), I see two bytes of appended data > on every RX packet. For example, this 48-byte ping, using 0xff as a > payload byte: > > 04:20:22.528472 IP 192.168.1.1 > 192.168.1.2: ICMP echo request, id 2447, seq 1, length 64 > 0x0000: 000a cd35 ea50 000a cd35 ea4f 0800 4500 > 0x0010: 0054 c116 4000 4001 f63e c0a8 0101 c0a8 > 0x0020: 0102 0800 b633 098f 0001 87ea cd5e 0000 > 0x0030: 0000 dcf2 0600 0000 0000 ffff ffff ffff > 0x0040: ffff ffff ffff ffff ffff ffff ffff ffff > 0x0050: ffff ffff ffff ffff ffff ffff ffff ffff > 0x0060: ffff 961f > > Those last two bytes - 96 1f - aren't part of the original packet. > > In the ax88179 RX path, the usbnet rx_fixup function trims a 2-byte > 'alignment pseudo header' from the start of the packet, and sets the > length from a per-packet field populated by hardware. It looks like > that length field *includes* the 2-byte header; the current driver > assumes that it's excluded. > > This change trims the 2-byte alignment header after we've set the > packet length, so the resulting packet length is correct. While we're > moving the comment around, this also fixes the spelling of 'pseudo'. > > Signed-off-by: Jeremy Kerr <jk@xxxxxxxxxx> > > --- > RFC: I don't have access to docs for this hardware, so this is all > based on observed behaviour of the reported packet length. > --- > drivers/net/usb/ax88179_178a.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/usb/ax88179_178a.c > b/drivers/net/usb/ax88179_178a.c index 93044cf1417a..1fe4cc28d154 > 100644 > --- a/drivers/net/usb/ax88179_178a.c > +++ b/drivers/net/usb/ax88179_178a.c > @@ -1414,10 +1414,10 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb) > } > > if (pkt_cnt == 0) { > - /* Skip IP alignment psudo header */ > - skb_pull(skb, 2); > skb->len = pkt_len; > - skb_set_tail_pointer(skb, pkt_len); > + /* Skip IP alignment pseudo header */ > + skb_pull(skb, 2); > + skb_set_tail_pointer(skb, skb->len); > skb->truesize = pkt_len + sizeof(struct sk_buff); > ax88179_rx_checksum(skb, pkt_hdr); > return 1; > @@ -1426,8 +1426,9 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb) > ax_skb = skb_clone(skb, GFP_ATOMIC); > if (ax_skb) { > ax_skb->len = pkt_len; > - ax_skb->data = skb->data + 2; > - skb_set_tail_pointer(ax_skb, pkt_len); > + /* Skip IP alignment pseudo header */ > + skb_pull(ax_skb, 2); > + skb_set_tail_pointer(ax_skb, ax_skb->len); > ax_skb->truesize = pkt_len + sizeof(struct sk_buff); > ax88179_rx_checksum(ax_skb, pkt_hdr); > usbnet_skb_return(dev, ax_skb); >
