Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200227000511.256055-1-briannorris@chromium.org> <DB7PR04MB524240B38FF6603D89D694538FE20@DB7PR04MB5242.eurprd04.prod.outlook.com>
In-Reply-To: <DB7PR04MB524240B38FF6603D89D694538FE20@DB7PR04MB5242.eurprd04.prod.outlook.com>
From:   Brian Norris <briannorris@chromium.org>
Date:   Wed, 4 Mar 2020 20:37:36 -0800
Message-ID: <CA+ASDXNvjJLzH+_0g1OUNckmRoLyNJVwiAg9=OPwd7LGk-E2Cg@mail.gmail.com>
Subject: Re: [EXT] [PATCH] mwifiex: set needed_headroom, not hard_header_len
To:     Ganapathi Bhat <ganapathi.bhat@nxp.com>
Cc:     "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Nishant Sarmukadam <nishants@marvell.com>,
        Amitkumar Karwar <amitkarwar@gmail.com>,
        Xinming Hu <huxinming820@gmail.com>,
        Arend Van Spriel <arend@broadcom.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Ganapathi,

On Wed, Mar 4, 2020 at 8:00 PM Ganapathi Bhat <ganapathi.bhat@nxp.com> wrote:
> > hard_header_len provides limitations for things like AF_PACKET, such that
> > we don't allow transmitting packets smaller than this.
>
> OK; However, are we not supposed to mention hard_header_len also?

No, my understanding is that we do not need to bother with
hard_header_len ourselves -- ether_setup() establishes the appropriate
L2 header parameters. I think that's covered a little better below.

> > 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").
>
> OK; I read this commit:
>
> "... According to definition of LL_RESERVED_SPACE() and hard_header_len, we should use hard_header_len to reserve for L2 header, like ethernet header(ETH_HLEN) in our case and use needed_headroom for the additional headroom needed by hardware..."

Yeah, that's probably a little more verbose and accurate description,
which is partly why I referred to that commit :)

> So, does it mean, hard_header_len is already considered by upper layer?

Right, it's set by ether_setup().

Brian