Re: [PATCH -next 1/2] brcmfmac: Support multiple AP interfaces and fix STA disconnection issue

[Franky Lin]

On Fri, Jul 22, 2022 at 5:30 AM Alvin Šipraga <alvin@xxxxxxx> wrote:
>
> From: Soontak Lee <soontak.lee@xxxxxxxxxxx>
>
> Support multiple AP interfaces for STA + AP + AP usecase.

AFAIK, Broadcom's fullmac firmware doesn't support such 2AP + 1STA use case.

> And fix STA disconnection when deactivating AP interface.
>
> Signed-off-by: Soontak Lee <soontak.lee@xxxxxxxxxxx>
> Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@xxxxxxxxxxx>
> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
> Signed-off-by: Alvin Šipraga <alsi@xxxxxxxxxxxxxxx>
> ---
>  .../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.

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.

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature