| Message ID | 20250604085539.2803896-5-arend.vanspriel@broadcom.com |
|---|---|
| State | New |
| Headers | show |
| Series | alternative changes for NL80211_CMD_SET_BSS support | expand |
On June 4, 2025 4:37:02 PM Jeff Johnson <jeff.johnson@oss.qualcomm.com> wrote: > On 6/4/2025 1:55 AM, Arend van Spriel wrote: >> From: Wright Feng <wright.feng@cypress.com> >> >> 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@cypress.com> >> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com> >> Signed-off-by: Gokul Sivakumar <gokulkumar.sivakumar@infineon.com> >> [arend: indicate ap_isolate support in struct wiphy::bss_param_support] >> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com> >> --- >> .../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. The first patch that initiated this was a brcmfmac patch which probably followed the same approach as the oot driver you know so well. Here is the patchwork link with the discussion that followed: https://patchwork.kernel.org/project/linux-wireless/patch/20250423175125.7233-1-gokulkumar.sivakumar@infineon.com/ Regards, Arend
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, }; 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 |