Hello,
On Mon, Jul 27, 2020 at 12:41 PM Xie He <xie.he.0...@gmail.com> wrote:
>
> Hi Cong Wang,
>
> I'm wishing to change a driver from using "hard_header_len" to using
> "needed_headroom" to declare its needed headroom. I submitted a patch
> and it is decided it needs to be reviewed. I see you participated in
> "hard_header_len vs needed_headroom" discussions in the past. Can you
> help me review this patch? Thanks!
>
> The patch is at:
> http://patchwork.ozlabs.org/project/netdev/patch/20200726110524.151957-1-xie.he.0...@gmail.com/
>
> In my understanding, hard_header_len should be the length of the header
> created by dev_hard_header. Any additional headroom needed should be
> declared in needed_headroom instead of hard_header_len. I came to this
> conclusion by examining the logic of net/packet/af_packet.c:packet_snd.
I am not familiar with this WAN driver, but I suggest you to look at
the following commit, which provides a lot of useful information:
commit 9454f7a895b822dd8fb4588fc55fda7c96728869
Author: Brian Norris <briannor...@chromium.org>
Date: Wed Feb 26 16:05:11 2020 -0800
mwifiex: set needed_headroom, not hard_header_len
hard_header_len provides limitations for things like AF_PACKET, such
that we don't allow transmitting packets smaller than this.
needed_headroom provides a suggested minimum headroom for SKBs, so that
we can trivally add our headers to the front.
The latter is the correct field to use in this case, while the former
mostly just prevents sending small AF_PACKET frames.
In any case, mwifiex already does its own bounce buffering [1] if we
don't have enough headroom, so hints (not hard limits) are all that are
needed.
This is the essentially the same bug (and fix) that brcmfmac had, fixed
in commit cb39288fd6bb ("brcmfmac: use ndev->needed_headroom to reserve
additional header space").
[1] mwifiex_hard_start_xmit():
if (skb_headroom(skb) < MWIFIEX_MIN_DATA_HEADER_LEN) {
[...]
/* Insufficient skb headroom - allocate a new skb */
Hope this helps.
Thanks.
