Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 428C8C433CB
From:   Kalle Valo <kvalo@codeaurora.org>
To:     Wright Feng <wright.feng@cypress.com>
Cc:     Arend Van Spriel <arend.vanspriel@broadcom.com>,
        franky.lin@broadcom.com, hante.meuleman@broadcom.com,
        chi-hsien.lin@cypress.com, linux-wireless@vger.kernel.org,
        brcm80211-dev-list.pdl@broadcom.com
Subject: Re: [PATCH 2/3] brcmfmac: make firmware eap_restrict a module parameter
References: <1584604406-15452-1-git-send-email-wright.feng@cypress.com>
        <1584604406-15452-3-git-send-email-wright.feng@cypress.com>
        <aa29c77c-cfe4-3d71-1860-f9bcb9e0282b@broadcom.com>
        <9683e5c3-5f68-5632-e1cf-c31e51379673@cypress.com>
Date:   Sun, 22 Mar 2020 16:29:31 +0200
In-Reply-To: <9683e5c3-5f68-5632-e1cf-c31e51379673@cypress.com> (Wright Feng's
        message of "Fri, 20 Mar 2020 16:06:11 +0800")
Message-ID: <871rpkcvjo.fsf@kamboji.qca.qualcomm.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Wright Feng <wright.feng@cypress.com> writes:

> Arend Van Spriel =E6=96=BC 3/19/2020 4:28 PM =E5=AF=AB=E9=81=93:
>> On 3/19/2020 8:53 AM, Wright Feng wrote:
>>> When eap_restrict is enabled, firmware will toss non-802.1x frames from
>>> tx/rx data path if station not yet authorized.
>>> Internal firmware eap_restrict is disabled by default. This patch makes
>>> it possible to enable firmware eap_restrict by specifying
>>> eap_restrict=3D1 as module parameter.
>>
>> What is the reason for not having this toss behavior as default?
>> Don't see much reason for having the module parameter.
>
> The eap_restrict feature in most of firmwares are disabled as default,
> and refer to our experience, using eap_restrict increases roam time
> for associations in some cases.

What are these these cases exactly?

> So what we do in this patch is not changing the default firmware
> behavior but still give users a way to enable eap_resrtict feature.

You should have mentioned this (ie. answer the "Why?" part) in the
commit log in the first place.

But I don't like adding module parameters unless with really good
reasons. And in this case there's no proper documentation when and how a
user should use the module parameter so this is nowhere near a proper
justifiction.

--=20
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatc=
hes