| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20220817085015.z3ubo4vhi5jeiopo@bang-olufsen.dk>
Date: Wed, 17 Aug 2022 08:50:15 +0000
From: Alvin Šipraga <ALSI@...g-olufsen.dk>
To: Franky Lin <franky.lin@...adcom.com>
CC: Alvin Šipraga <alvin@...s.dk>,
Arend van Spriel <aspriel@...il.com>,
Hante Meuleman <hante.meuleman@...adcom.com>,
Kalle Valo <kvalo@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Soon Tak Lee <soontak.lee@...ress.com>,
Chi-Hsien Lin <chi-hsien.lin@...ress.com>,
Ahmad Fatoum <a.fatoum@...gutronix.de>,
"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
"brcm80211-dev-list.pdl@...adcom.com"
<brcm80211-dev-list.pdl@...adcom.com>,
"SHA-cyfmac-dev-list@...ineon.com" <SHA-cyfmac-dev-list@...ineon.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -next 1/2] brcmfmac: Support multiple AP interfaces and
fix STA disconnection issue
Hi Franky,
On Wed, Aug 10, 2022 at 02:32:06PM -0700, Franky Lin wrote:
> On Fri, Jul 22, 2022 at 5:30 AM Alvin Šipraga <alvin@...s.dk> wrote:
> >
> > From: Soontak Lee <soontak.lee@...ress.com>
> >
> > Support multiple AP interfaces for STA + AP + AP usecase.
>
> AFAIK, Broadcom's fullmac firmware doesn't support such 2AP + 1STA use case.
Thanks for the clarification. The series should be ignored by Kalle
then.
>
> > And fix STA disconnection when deactivating AP interface.
> >
> > Signed-off-by: Soontak Lee <soontak.lee@...ress.com>
> > Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@...ress.com>
> > Signed-off-by: Ahmad Fatoum <a.fatoum@...gutronix.de>
> > Signed-off-by: Alvin Šipraga <alsi@...g-olufsen.dk>
> > ---
> > .../broadcom/brcm80211/brcmfmac/cfg80211.c | 48 +++++++++++++++----
> > .../broadcom/brcm80211/brcmfmac/cfg80211.h | 1 +
> > .../broadcom/brcm80211/brcmfmac/common.c | 5 ++
> > 3 files changed, 44 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > index 3ae6779fe153..856fd5516ddf 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > @@ -4747,6 +4747,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
> > settings->inactivity_timeout);
> > dev_role = ifp->vif->wdev.iftype;
> > mbss = ifp->vif->mbss;
> > + brcmf_dbg(TRACE, "mbss %s\n", mbss ? "enabled" : "disabled");
> >
> > /* store current 11d setting */
> > if (brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_REGULATORY,
> > @@ -4961,6 +4962,9 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
> > if ((err) && (!mbss)) {
> > brcmf_set_mpc(ifp, 1);
> > brcmf_configure_arp_nd_offload(ifp, true);
> > + } else {
> > + cfg->num_softap++;
> > + brcmf_dbg(TRACE, "Num of SoftAP %u\n", cfg->num_softap);
> > }
> > return err;
> > }
> > @@ -4975,6 +4979,7 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
> > s32 err;
> > struct brcmf_fil_bss_enable_le bss_enable;
> > struct brcmf_join_params join_params;
> > + s32 apsta = 0;
> >
> > brcmf_dbg(TRACE, "Enter\n");
> >
> > @@ -4983,6 +4988,27 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
> > /* first to make sure they get processed by fw. */
> > msleep(400);
> >
> > + cfg->num_softap--;
> > +
> > + /* Clear bss configuration and SSID */
> > + bss_enable.bsscfgidx = cpu_to_le32(ifp->bsscfgidx);
> > + bss_enable.enable = cpu_to_le32(0);
> > + err = brcmf_fil_iovar_data_set(ifp, "bss", &bss_enable,
> > + sizeof(bss_enable));
> > + if (err < 0)
> > + brcmf_err("bss_enable config failed %d\n", err);
> > +
> > + memset(&join_params, 0, sizeof(join_params));
> > + err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SET_SSID,
> > + &join_params, sizeof(join_params));
> > + if (err < 0)
> > + bphy_err(drvr, "SET SSID error (%d)\n", err);
> > +
> > + if (cfg->num_softap) {
> > + brcmf_dbg(TRACE, "Num of SoftAP %u\n", cfg->num_softap);
> > + return 0;
> > + }
> > +
> > if (profile->use_fwauth != BIT(BRCMF_PROFILE_FWAUTH_NONE)) {
> > if (profile->use_fwauth & BIT(BRCMF_PROFILE_FWAUTH_PSK))
> > brcmf_set_pmk(ifp, NULL, 0);
> > @@ -5000,17 +5026,18 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
> > if (ifp->bsscfgidx == 0)
> > brcmf_fil_iovar_int_set(ifp, "closednet", 0);
> >
> > - memset(&join_params, 0, sizeof(join_params));
> > - err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SET_SSID,
> > - &join_params, sizeof(join_params));
> > - if (err < 0)
> > - bphy_err(drvr, "SET SSID error (%d)\n", err);
> > - err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_DOWN, 1);
> > - if (err < 0)
> > - bphy_err(drvr, "BRCMF_C_DOWN error %d\n", err);
> > - err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
> > + err = brcmf_fil_iovar_int_get(ifp, "apsta", &apsta);
> > if (err < 0)
> > - bphy_err(drvr, "setting AP mode failed %d\n", err);
> > + brcmf_err("wl apsta failed (%d)\n", err);
> > +
> > + if (!apsta) {
> > + err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_DOWN, 1);
> > + if (err < 0)
> > + bphy_err(drvr, "BRCMF_C_DOWN error %d\n", err);
> > + err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
> > + if (err < 0)
> > + bphy_err(drvr, "Set AP mode error %d\n", err);
> > + }
> > if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MBSS))
> > brcmf_fil_iovar_int_set(ifp, "mbss", 0);
> > brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_REGULATORY,
> > @@ -7641,6 +7668,7 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
> >
> > cfg->wiphy = wiphy;
> > cfg->pub = drvr;
> > + cfg->num_softap = 0;
> > init_vif_event(&cfg->vif_event);
> > INIT_LIST_HEAD(&cfg->vif_list);
> >
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> > index e90a30808c22..e4ebc2fa6ebb 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> > @@ -371,6 +371,7 @@ struct brcmf_cfg80211_info {
> > struct brcmf_cfg80211_wowl wowl;
> > struct brcmf_pno_info *pno;
> > u8 ac_priority[MAX_8021D_PRIO];
> > + u8 num_softap;
> > };
> >
> > /**
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > index fe01da9e620d..83e023a22f9b 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > @@ -303,6 +303,11 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
> > brcmf_dbg(INFO, "CLM version = %s\n", clmver);
> > }
> >
> > + /* set apsta */
> > + err = brcmf_fil_iovar_int_set(ifp, "apsta", 1);
> > + if (err)
> > + brcmf_info("failed setting apsta, %d\n", err);
> > +
>
> I do not understand why entering apsta mode by default. The mode is
> supposed to be enabled only when an AP interface is created in
> brcmf_cfg80211_start_ap. I think one of the side effects of apsta mode
> is that memory footprint significantly increases. It should remain
> disabled for STA only mode (which is the major use case) for better
> performance.
By better performance, do you just mean "lower chance of memory
exhaustion"? If so, surely the firmware would be designed such that it
doesn't run out of memory under the advertised use-cases (STA, AP+STA
etc.), regardless of the current apsta setting?
I would hope that somebody from Infineon could chime in here, as I am
not privy to the FW design.
Kind regards,
Alvin
>
> Regards,
> - Franky
>
> > /* set mpc */
> > err = brcmf_fil_iovar_int_set(ifp, "mpc", 1);
> > if (err) {
> > --
> > 2.37.0
> >
>
> --
> This electronic communication and the information and any files transmitted
> with it, or attached to it, are confidential and are intended solely for
> the use of the individual or entity to whom it is addressed and may contain
> information that is confidential, legally privileged, protected by privacy
> laws, or otherwise restricted from disclosure to anyone else. If you are
> not the intended recipient or the person responsible for delivering the
> e-mail to the intended recipient, you are hereby notified that any use,
> copying, distributing, dissemination, forwarding, printing, or copying of
> this e-mail is strictly prohibited. If you received this e-mail in error,
> please return the e-mail to the sender, delete it from your computer, and
> destroy any printed copy of it.
Powered by blists - more mailing lists