diff mbox

[1/3] hostapd: Fix VHT unsolicitated channel switching

Message ID 1395146256-18815-2-git-send-email-michal.kazior@tieto.com
State Changes Requested
Headers show

Commit Message

Michal Kazior March 18, 2014, 12:37 p.m. UTC
The ieee80211ac config wasn't updated upon channel switch notification.
This led to inconsistent beacons in some cases.

It's not possible to deduce VHT status for an unsolicitated channel
switch now in all cases.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 src/ap/drv_callbacks.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Peer, Ilan March 18, 2014, 2:58 p.m. UTC | #1
Hi Michal,

> -----Original Message-----
> From: hostap-bounces@lists.shmoo.com [mailto:hostap-
> bounces@lists.shmoo.com] On Behalf Of Michal Kazior
> Sent: Tuesday, March 18, 2014 14:38
> To: j@w1.fi
> Cc: hostap@lists.shmoo.com
> Subject: [PATCH 1/3] hostapd: Fix VHT unsolicitated channel switching
> 
> The ieee80211ac config wasn't updated upon channel switch notification.
> This led to inconsistent beacons in some cases.
> 
> It's not possible to deduce VHT status for an unsolicitated channel switch
> now in all cases.
> 

[...]

> +	/* FIXME: It's impossible to tell how exactly an unsolicitated channel
> +	 * switch downgraded channel definition in all cases. */
> +	if (width == CHAN_WIDTH_80 ||
> +	    width == CHAN_WIDTH_80P80 ||
> +	    width == CHAN_WIDTH_160)
> +		hapd->iconf->ieee80211ac = 1;
> +	if (!ht)
> +		hapd->iconf->ieee80211ac = 0;
> +

As far as I understand the ieee80211ac variable states weather 80211ac is enabled or not, and thus I do not think that it should be changed based on the actual operating mode/channel width.

Regardless, how is it possible that the CS ended with being on VHT if hostap did not ask to use VHT in the first place?

Thanks in advance,

Ilan.
Michal Kazior March 18, 2014, 3:27 p.m. UTC | #2
On 18 March 2014 15:58, Peer, Ilan <ilan.peer@intel.com> wrote:
> Hi Michal,
>
>> -----Original Message-----
>> From: hostap-bounces@lists.shmoo.com [mailto:hostap-
>> bounces@lists.shmoo.com] On Behalf Of Michal Kazior
>> Sent: Tuesday, March 18, 2014 14:38
>> To: j@w1.fi
>> Cc: hostap@lists.shmoo.com
>> Subject: [PATCH 1/3] hostapd: Fix VHT unsolicitated channel switching
>>
>> The ieee80211ac config wasn't updated upon channel switch notification.
>> This led to inconsistent beacons in some cases.
>>
>> It's not possible to deduce VHT status for an unsolicitated channel switch
>> now in all cases.
>>
>
> [...]
>
>> +     /* FIXME: It's impossible to tell how exactly an unsolicitated channel
>> +      * switch downgraded channel definition in all cases. */
>> +     if (width == CHAN_WIDTH_80 ||
>> +         width == CHAN_WIDTH_80P80 ||
>> +         width == CHAN_WIDTH_160)
>> +             hapd->iconf->ieee80211ac = 1;
>> +     if (!ht)
>> +             hapd->iconf->ieee80211ac = 0;
>> +
>
> As far as I understand the ieee80211ac variable states weather 80211ac is enabled or not, and thus I do not think that it should be changed based on the actual operating mode/channel width.

You could apply this for ieee80211n, yet, it is possible for channel
switch to change it.


> Regardless, how is it possible that the CS ended with being on VHT if hostap did not ask to use VHT in the first place?
>
> Thanks in advance,

Technically a driver can call cfg80211_ch_switch_notify() with any
chandef. It's not a necessity for an actual CSA to be requested first.


Michał
Peer, Ilan March 18, 2014, 3:58 p.m. UTC | #3
> >> -----Original Message-----
> >> From: hostap-bounces@lists.shmoo.com [mailto:hostap-
> >> bounces@lists.shmoo.com] On Behalf Of Michal Kazior
> >> Sent: Tuesday, March 18, 2014 14:38
> >> To: j@w1.fi
> >> Cc: hostap@lists.shmoo.com
> >> Subject: [PATCH 1/3] hostapd: Fix VHT unsolicitated channel switching
> >>
> >> The ieee80211ac config wasn't updated upon channel switch notification.
> >> This led to inconsistent beacons in some cases.
> >>
> >> It's not possible to deduce VHT status for an unsolicitated channel
> >> switch now in all cases.
> >>
> >
> > [...]
> >
> >> +     /* FIXME: It's impossible to tell how exactly an unsolicitated channel
> >> +      * switch downgraded channel definition in all cases. */
> >> +     if (width == CHAN_WIDTH_80 ||
> >> +         width == CHAN_WIDTH_80P80 ||
> >> +         width == CHAN_WIDTH_160)
> >> +             hapd->iconf->ieee80211ac = 1;
> >> +     if (!ht)
> >> +             hapd->iconf->ieee80211ac = 0;
> >> +
> >
> > As far as I understand the ieee80211ac variable states weather 80211ac is
> enabled or not, and thus I do not think that it should be changed based on
> the actual operating mode/channel width.
> 
> You could apply this for ieee80211n, yet, it is possible for channel switch to
> change it.

I would consider this a wrong behavior. For example if 80211ac was enabled at first, and disabled due to this change, then a subsequent request to CS (for example due to DFS), would not allow to switch to VHT.

> 
> 
> > Regardless, how is it possible that the CS ended with being on VHT if hostap
> did not ask to use VHT in the first place?
> >
> 
> Technically a driver can call cfg80211_ch_switch_notify() with any chandef.
> It's not a necessity for an actual CSA to be requested first.
> 

But what's the use case? Is this for actual CS or only for bandwidth change? Might be my misunderstanding, but I did not expect a CS to be triggered without the knowledge of wpa_supplicant/hostapd as this might break things if wpa_supplicant/hostapd also try to trigger CS without the knowledge. In addition such change should also involve changing some of the IEs etc.

Regards,

Ilan.
Michal Kazior March 19, 2014, 7:04 a.m. UTC | #4
On 18 March 2014 16:58, Peer, Ilan <ilan.peer@intel.com> wrote:
>> >> -----Original Message-----
>> >> From: hostap-bounces@lists.shmoo.com [mailto:hostap-
>> >> bounces@lists.shmoo.com] On Behalf Of Michal Kazior
>> >> Sent: Tuesday, March 18, 2014 14:38
>> >> To: j@w1.fi
>> >> Cc: hostap@lists.shmoo.com
>> >> Subject: [PATCH 1/3] hostapd: Fix VHT unsolicitated channel switching
>> >>
>> >> The ieee80211ac config wasn't updated upon channel switch notification.
>> >> This led to inconsistent beacons in some cases.
>> >>
>> >> It's not possible to deduce VHT status for an unsolicitated channel
>> >> switch now in all cases.
>> >>
>> >
>> > [...]
>> >
>> >> +     /* FIXME: It's impossible to tell how exactly an unsolicitated channel
>> >> +      * switch downgraded channel definition in all cases. */
>> >> +     if (width == CHAN_WIDTH_80 ||
>> >> +         width == CHAN_WIDTH_80P80 ||
>> >> +         width == CHAN_WIDTH_160)
>> >> +             hapd->iconf->ieee80211ac = 1;
>> >> +     if (!ht)
>> >> +             hapd->iconf->ieee80211ac = 0;
>> >> +
>> >
>> > As far as I understand the ieee80211ac variable states weather 80211ac is
>> enabled or not, and thus I do not think that it should be changed based on
>> the actual operating mode/channel width.
>>
>> You could apply this for ieee80211n, yet, it is possible for channel switch to
>> change it.
>
> I would consider this a wrong behavior. For example if 80211ac was enabled at first, and disabled due to this change, then a subsequent request to CS (for example due to DFS), would not allow to switch to VHT.

I get your point. For this to be handled perfectly you probably need
to have hostapd to have an additional variable to describe whether
11ac is actually enabled, or not (while the other says if it's ever
okay to use 11ac). The same would be necessary for ht_capab with
[HT40+/-] flag. This can prove to be tricky.


>> > Regardless, how is it possible that the CS ended with being on VHT if hostap
>> did not ask to use VHT in the first place?
>> >
>>
>> Technically a driver can call cfg80211_ch_switch_notify() with any chandef.
>> It's not a necessity for an actual CSA to be requested first.
>>
>
> But what's the use case? Is this for actual CS or only for bandwidth change? Might be my misunderstanding, but I did not expect a CS to be triggered without the knowledge of wpa_supplicant/hostapd as this might break things if wpa_supplicant/hostapd also try to trigger CS without the knowledge.

This very patch is probably least important of the three. I don't know
if there's any driver that can do such unsolicited CS. For explicit
CSA bandwidth change (i.e, chan_switch) there are patches [2/3] and
[3/3].


> In addition such change should also involve changing some of the IEs etc.

Good point. Unsolicited CS doesn't seem to update beacons.
ieee802_11_set_beacons() is called only if a CSA was requested
earlier. Perhaps it should be called regardless?


Michał
Peer, Ilan March 19, 2014, 8:17 a.m. UTC | #5
> >> >> +     /* FIXME: It's impossible to tell how exactly an unsolicitated
> channel
> >> >> +      * switch downgraded channel definition in all cases. */
> >> >> +     if (width == CHAN_WIDTH_80 ||
> >> >> +         width == CHAN_WIDTH_80P80 ||
> >> >> +         width == CHAN_WIDTH_160)
> >> >> +             hapd->iconf->ieee80211ac = 1;
> >> >> +     if (!ht)
> >> >> +             hapd->iconf->ieee80211ac = 0;
> >> >> +
> >> >
> >> > As far as I understand the ieee80211ac variable states weather
> >> > 80211ac is
> >> enabled or not, and thus I do not think that it should be changed
> >> based on the actual operating mode/channel width.
> >>
> >> You could apply this for ieee80211n, yet, it is possible for channel
> >> switch to change it.
> >
> > I would consider this a wrong behavior. For example if 80211ac was enabled
> at first, and disabled due to this change, then a subsequent request to CS
> (for example due to DFS), would not allow to switch to VHT.
> 
> I get your point. For this to be handled perfectly you probably need to have
> hostapd to have an additional variable to describe whether 11ac is actually
> enabled, or not (while the other says if it's ever okay to use 11ac). The same
> would be necessary for ht_capab with [HT40+/-] flag. This can prove to be
> tricky.
> 

Yep ... hope it is not that tricky :)

> 
> >> > Regardless, how is it possible that the CS ended with being on VHT
> >> > if hostap
> >> did not ask to use VHT in the first place?
> >> >
> >>
> >> Technically a driver can call cfg80211_ch_switch_notify() with any
> chandef.
> >> It's not a necessity for an actual CSA to be requested first.
> >>
> >
> > But what's the use case? Is this for actual CS or only for bandwidth change?
> Might be my misunderstanding, but I did not expect a CS to be triggered
> without the knowledge of wpa_supplicant/hostapd as this might break
> things if wpa_supplicant/hostapd also try to trigger CS without the
> knowledge.
> 
> This very patch is probably least important of the three. I don't know if
> there's any driver that can do such unsolicited CS. For explicit CSA bandwidth
> change (i.e, chan_switch) there are patches [2/3] and [3/3].
> 

Will have look at the others :) ... other than that, even a bandwidth change requires that CSA/eCSA would include the WB channel switch IEs etc.

> 
> > In addition such change should also involve changing some of the IEs etc.
> 
> Good point. Unsolicited CS doesn't seem to update beacons.
> ieee802_11_set_beacons() is called only if a CSA was requested earlier.
> Perhaps it should be called regardless?

I think that this will only cause a mess without proper support in the stack and would avoid it ...

Regards,

Ilan.
diff mbox

Patch

diff --git a/src/ap/drv_callbacks.c b/src/ap/drv_callbacks.c
index a8c24eb..439bf66 100644
--- a/src/ap/drv_callbacks.c
+++ b/src/ap/drv_callbacks.c
@@ -462,6 +462,15 @@  void hostapd_event_ch_switch(struct hostapd_data *hapd, int freq, int ht,
 		break;
 	}
 
+	/* FIXME: It's impossible to tell how exactly an unsolicitated channel
+	 * switch downgraded channel definition in all cases. */
+	if (width == CHAN_WIDTH_80 ||
+	    width == CHAN_WIDTH_80P80 ||
+	    width == CHAN_WIDTH_160)
+		hapd->iconf->ieee80211ac = 1;
+	if (!ht)
+		hapd->iconf->ieee80211ac = 0;
+
 	hapd->iconf->channel = channel;
 	hapd->iconf->ieee80211n = ht;
 	hapd->iconf->secondary_channel = offset;