[PATCH] CSA MBSS: CHAN_SWITCH is always failing.

[Baligh GASMI]

>
> 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.

csa_in_progress is per BSS, so if the driver fails on one BSS, it will
not impact other.
right?

>
>
> 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
> >
>