| Message ID | 1584604406-15452-3-git-send-email-wright.feng@cypress.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | Add AP isolate support and two modules parameters | expand |
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=1 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. > Signed-off-by: Wright Feng <wright.feng@cypress.com> > Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com> > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 9 +++++++++ > drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 5 +++++ > drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h | 2 ++ > 3 files changed, 16 insertions(+)
Arend Van Spriel 於 3/19/2020 4:28 PM 寫道: > 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=1 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. 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. >> Signed-off-by: Wright Feng <wright.feng@cypress.com> >> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com> >> --- >> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 9 >> +++++++++ >> drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 5 +++++ >> drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h | 2 ++ >> 3 files changed, 16 insertions(+)
Wright Feng <wright.feng@cypress.com> writes: > Arend Van Spriel 於 3/19/2020 4:28 PM 寫道: >> 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=1 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.
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index eb49900..07a231c 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -6942,6 +6942,7 @@ static s32 brcmf_config_dongle(struct brcmf_cfg80211_info *cfg) struct wireless_dev *wdev; struct brcmf_if *ifp; s32 power_mode; + s32 eap_restrict; s32 err = 0; if (cfg->dongle_up) @@ -6966,6 +6967,14 @@ static s32 brcmf_config_dongle(struct brcmf_cfg80211_info *cfg) err = brcmf_dongle_roam(ifp); if (err) goto default_conf_out; + + eap_restrict = ifp->drvr->settings->eap_restrict; + if (eap_restrict) { + err = brcmf_fil_iovar_int_set(ifp, "eap_restrict", + eap_restrict); + if (err) + brcmf_info("eap_restrict error (%d)\n", err); + } err = brcmf_cfg80211_change_iface(wdev->wiphy, ndev, wdev->iftype, NULL); if (err) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c index dec25e4..cda6bef 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c @@ -67,6 +67,10 @@ static int brcmf_iapp_enable; module_param_named(iapp, brcmf_iapp_enable, int, 0); MODULE_PARM_DESC(iapp, "Enable partial support for the obsoleted Inter-Access Point Protocol"); +static int brcmf_eap_restrict; +module_param_named(eap_restrict, brcmf_eap_restrict, int, 0400); +MODULE_PARM_DESC(eap_restrict, "Block non-802.1X frames until auth finished"); + #ifdef DEBUG /* always succeed brcmf_bus_started() */ static int brcmf_ignore_probe_fail; @@ -413,6 +417,7 @@ struct brcmf_mp_device *brcmf_get_module_param(struct device *dev, settings->fcmode = brcmf_fcmode; settings->roamoff = !!brcmf_roamoff; settings->iapp = !!brcmf_iapp_enable; + settings->eap_restrict = !!brcmf_eap_restrict; #ifdef DEBUG settings->ignore_probe_fail = !!brcmf_ignore_probe_fail; #endif diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h index 144cf45..059f09c 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h @@ -37,6 +37,7 @@ extern struct brcmf_mp_global_t brcmf_mp_global; * @feature_disable: Feature_disable bitmask. * @fcmode: FWS flow control. * @roamoff: Firmware roaming off? + * @eap_restrict: Not allow data tx/rx until 802.1X auth succeeds * @ignore_probe_fail: Ignore probe failure. * @country_codes: If available, pointer to struct for translating country codes * @bus: Bus specific platform data. Only SDIO at the mmoment. @@ -47,6 +48,7 @@ struct brcmf_mp_device { int fcmode; bool roamoff; bool iapp; + bool eap_restrict; bool ignore_probe_fail; struct brcmfmac_pd_cc *country_codes; const char *board_type;