On 6/4/2025 1:55 AM, Arend van Spriel wrote:
> From: Wright Feng <wright.feng@xxxxxxxxxxx>
>
> hostapd & wpa_supplicant userspace daemons exposes an AP mode specific
> config file parameter "ap_isolate" to the user, which is used to control
> low-level bridging of frames between the stations associated in the BSS.
>
> In driver, handle this user setting in the newly defined cfg80211_ops
> function brcmf_cfg80211_change_bss() by enabling "ap_isolate" IOVAR in
> the firmware.
>
> In AP mode, the "ap_isolate" value from the cfg80211 layer represents,
> 0 = allow low-level bridging of frames between associated stations
> 1 = restrict low-level bridging of frames to isolate associated stations
> -1 = do not change existing setting
>
> Signed-off-by: Wright Feng <wright.feng@xxxxxxxxxxx>
> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@xxxxxxxxxxx>
> Signed-off-by: Gokul Sivakumar <gokulkumar.sivakumar@xxxxxxxxxxxx>
> [arend: indicate ap_isolate support in struct wiphy::bss_param_support]
> Signed-off-by: Arend van Spriel <arend.vanspriel@xxxxxxxxxxxx>
> ---
> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 24 +++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index dc2383faddd1..d6c8ad7ebced 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -5933,6 +5933,26 @@ static int brcmf_cfg80211_del_pmk(struct wiphy *wiphy, struct net_device *dev,
> return brcmf_set_pmk(ifp, NULL, 0);
> }
>
> +static int brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device *dev,
> + struct bss_parameters *params)
> +{
> + struct brcmf_if *ifp = netdev_priv(dev);
> + int ret = 0;
> +
> + /* In AP mode, the "ap_isolate" value represents
> + * 0 = allow low-level bridging of frames between associated stations
> + * 1 = restrict low-level bridging of frames to isolate associated stations
> + * -1 = do not change existing setting
> + */
> + if (params->ap_isolate >= 0) {
> + ret = brcmf_fil_iovar_int_set(ifp, "ap_isolate", params->ap_isolate);
> + if (ret < 0)
> + brcmf_err("ap_isolate iovar failed: ret=%d\n", ret);
> + }
> +
> + return ret;
> +}
> +
> static struct cfg80211_ops brcmf_cfg80211_ops = {
> .add_virtual_intf = brcmf_cfg80211_add_iface,
> .del_virtual_intf = brcmf_cfg80211_del_iface,
> @@ -5980,6 +6000,7 @@ static struct cfg80211_ops brcmf_cfg80211_ops = {
> .update_connect_params = brcmf_cfg80211_update_conn_params,
> .set_pmk = brcmf_cfg80211_set_pmk,
> .del_pmk = brcmf_cfg80211_del_pmk,
> + .change_bss = brcmf_cfg80211_change_bss,
> };
So the real question is do we really *need* all of the other patches, or is it
OK for the driver to just process the one attribute it wants without
indicating that is the only attribute it handles to userspace? I know of one
out-of-tree cfg80211 driver that has only handled AP isolate for a long time,
and AFAIK there have not been any issues.
In other words, why isn't just the two diffs above enough to satisfy the need?
>
> struct cfg80211_ops *brcmf_cfg80211_get_ops(struct brcmf_mp_device *settings)
> @@ -7634,6 +7655,9 @@ static int brcmf_setup_wiphy(struct wiphy *wiphy, struct brcmf_if *ifp)
> BIT(NL80211_BSS_SELECT_ATTR_BAND_PREF) |
> BIT(NL80211_BSS_SELECT_ATTR_RSSI_ADJUST);
>
> +
> + wiphy->bss_param_support = WIPHY_BSS_PARAM_AP_ISOLATE;
> +
> wiphy->flags |= WIPHY_FLAG_NETNS_OK |
> WIPHY_FLAG_PS_ON_BY_DEFAULT |
> WIPHY_FLAG_HAVE_AP_SME |
Ultimately that out-of-tree driver I know so well would adopt this scheme if
it lands, but it currently support AP isolate without all of the other changes.
/jeff
