[PATCH] CSA MBSS: CHAN_SWITCH is always failing.

[Benjamin Berg]

On Fri, 2023-10-13 at 12:07 +0200, Baligh GASMI wrote:
> > 
> > Hi,
> > 
> > On Thu, 2023-10-12 at 23:12 +0200, Baligh Gasmi wrote:
> > > When asking hostapd to switch channel in the multi-BSS
> > > configuration,
> > > only the first (associated to the first BSS) request is executed
> > > correctly by some driver, other BSS's request are failing.
> > > This is because multi-BSS configuration is using same Radio and
> > > one
> > > request seems to be suffisant to driver to switch on.
> > > 
> > > Solution in here is to return SUCCESS if there was at least one
> > > BSS
> > > switched OK.
> > 
> > I think this solution may confuse the internal state a bit as
> > hostapd_switch_channel resets some state on failure and also sets
> > csa_in_progress on success.
> > 
> > I am thinking right now that two things might make sense:
> >    1. Either splitting channel switch into three steps, or always
> >       switching the BSSs together.
> >    2. Only doing a single call to the driver interface with the
> >       information for all BSSs. After all, the switch will need to
> >       happen at the same point in time.
> > 
> > But, I don't really know right now what underlying drivers are
> > expecting, so maybe the suggestion is just wrong. Another solution
> > could be that the driver detects that the switch is already in
> > progress
> > and ensures a success return if appropriate.
> > 
> > Benjamin
> > 
> 
> Hello,
> 
> Sorry, but I do not get it!
> I'm not saying that the patch is correct or to be used as it, trying
> just to understand.
> 
> With the patch, when hostapd_switch_channel() fails, that means that
> no switch on any BSS has succussed, so may the reset of the state is
> coherent.
> Other way, if it successed with one BSS (some or all), that means that
> the CSA is ongoing, and having csa_in_progress is coherent too.

I was talking about the internal hostapd state. i.e.:

int hostapd_switch_channel(struct hostapd_data *hapd,
			   struct csa_settings *settings)
{
	int ret;
...
	ret = hostapd_drv_switch_channel(hapd, settings);
	free_beacon_data(&settings->beacon_csa);
	free_beacon_data(&settings->beacon_after);

	if (ret) {
		/* if we failed, clean cs parameters */
		hostapd_cleanup_cs_params(hapd);
		return ret;
	}

	hapd->csa_in_progress = 1;
	return 0;
}

If hostapd_drv_switch_channel fails (which you expect to happen), then
csa_in_progress is not set to 1 and hostapd_cleanup_cs_params is
called.

Both of this seem like an undesired side-effect.

Benjamin

> 
> I can think about adding CHAN_SWITCH argument, according to which we
> use all BSSes or only the main BSS, but still not global solution I
> think, since we will have to detect driver behaviour to add/remove
> new
> arg.
> 
> Baligh
>