Re: [PATCH] brcmfmac: support AP isolation to restrict reachability between stations

[Johannes Berg <johannes@sipsolutions.net>]

On Thu, 2025-04-24 at 13:36 +0200, Arend van Spriel wrote:
> On 4/24/2025 12:22 PM, Johannes Berg wrote:
> > On Thu, 2025-04-24 at 11:50 +0200, Arend van Spriel wrote:
> > > 
> > > Looked at other drivers implementing this callback and here are the results:
> > > 
> > > [wil6210] wil_cfg80211_change_bss(): does exactly the same thing.
> > > [wilc1000] change_bss(): worse! it accepts everything and does nothing.
> > > [rtl8723bs] cfg80211_rtw_change_bss(): same. just an empty callback.
> > 
> > OK, though I guess other drivers being bad doesn't mean this one should
> > be :)
> 
> Sure. I am on your team in this. Can you recommend a plan of attack 
> here? Should we add a mechanism to expose what BSS parameter changes the 
> driver can handle similar to what is used for struct 
> station_info::bss_params?

I don't even know off the top of my head what's there and what isn't,
just thought the code looked odd. I guess mac80211 just mostly supports
all of them anyway.

Today the change is simply rejected by cfg80211 with -EOPNOTSUPP:

static int nl80211_set_bss(struct sk_buff *skb, struct genl_info *info)
{
...
        if (!rdev->ops->change_bss)
                return -EOPNOTSUPP;


why not keep doing that for everything but ap_isolate?

Oh, I see what you mean, there's no way to keep that updated since you'd
have to check each value for being changed and reject that ...

Hmm, yeah, that's not great. I guess ideally then we'd have a bitmap of
what changed, and what the driver can support?

enum {
	CFG80211_BSS_PARAM_CHANGE_CTS_PROT	= BIT(0),
	CFG80211_BSS_PARAM_CHANGE_SHORT_PRE	= BIT(1),
	CFG80211_BSS_PARAM_CHANGE_SHORT_SLOT	= BIT(2),
...
	CFG80211_BSS_PARAM_CHANGE_AP_ISOLATE	= BIT(N),
...
};

and

struct bss_parameters {
	int link_id;
	u32 changed;
	...
}

and then this driver can just do

	if (params->changed & ~CFG80211_BSS_PARAM_CHANGE_AP_ISOLATE)
		return -EOPNOTSUPP;

or so?


Not sure how that'd be related to station_info::bss_param, that's just
output from the driver.

johannes