Re: [PATCH v3 net-next 09/21] net: usb: aqc111: Implement RX data path

[David Miller]

From: Igor Russkikh <igor.russk...@aquantia.com>
Date: Wed, 21 Nov 2018 10:13:39 +0000

> +     desc_hdr = *(u64 *)skb_tail_pointer(skb);
> +     le64_to_cpus(&desc_hdr);

This is:

        desc_hdr = le64_to_cpup(skb_tail_pointer(skb));

> +     /* Get the first RX packet descriptor */
> +     pkt_desc = (u64 *)(skb->data + desc_offset);
> +
> +     while (pkt_count--) {
> +             u32 pkt_len = 0;
> +             u32 pkt_len_with_padd = 0;
> +
> +             le64_to_cpus(pkt_desc);

Probably better to load the translated value into a local variable
once:

                u64 cpu_desc = le64_to_cpup(pkt_desc);

and then use 'cpu_desc' instead of dereferencing "*pkt_desc" over and
over again.

> +             /* Clone SKB */
> +             new_skb = skb_clone(skb, GFP_ATOMIC);
> +
> +             if (!new_skb)
> +                     goto err;
> +
> +             new_skb->len = pkt_len;
> +             skb_pull(new_skb, AQ_RX_HW_PAD);
> +             skb_set_tail_pointer(new_skb, new_skb->len);
> +
> +             new_skb->truesize = new_skb->len + sizeof(struct sk_buff);

I see lots of USB drivers doing this, but it's not correct.

We have a SKB_TRUESIZE() macro, please use it.