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
