[PATCH 3/3] wpa_supplicant: Report 40MHz Intolerance to associated AP.

[Jouni Malinen]

On Thu, Jul 21, 2011 at 06:54:38PM +0530, Rajkumar Manoharan wrote:
> This patch adds support for 40MHz intolerance handling in STA
> mode. STA should report of any 40MHz intolerance in case if it
> finds a non-HT AP or a HT AP which prohibits 40MHz transmission
> (i.e 40MHz intolerant bit is set in HT capabilities IE).

Thanks!

While going through the review, I did some cleanup, but could not get to
the point that I would push the changes into hostap.git. Anyway, any
further work on this should be on top of the cleaned up version (see
attached patch set).

> STA shall report this condition using 20/40 BSS coexistence
> action frame.

There are couple of areas that I don't think matches with the standard
on how this the report frame is used. See comments below.

> diff --git a/wpa_supplicant/sme.c b/wpa_supplicant/sme.c
> @@ -589,7 +589,49 @@ void sme_deinit(struct wpa_supplicant *wpa_s)
> +void sme_send_2040_bss_coex(struct wpa_supplicant *wpa_s,
> +			    u8 *chan_list, u8 n_channels)
> +{

> +	ic_report = wpabuf_put(buf, sizeof(*ic_report));
> +	ic_report->element_id = WLAN_EID_20_40_BSS_INTOLERANT;
> +	ic_report->length = n_channels + 1;
> +	freq_to_channel(wpa_s->current_bss->freq, &ic_report->reg_class, NULL);

freq_to_channel converts the frequency to an operating class in the
global table (Table E-4 in 802.11REVmb). However, this coex report frame
needs to use the table that matches with the Country element transmitted
by the AP. This may be different from Table E-4.

> +	if (n_channels) {
> +		pos = wpabuf_put(buf, n_channels);
> +		os_memcpy(pos, chan_list, n_channels);
> +	}

The channels here are collected from potentially multiple operating
classes (there are multiple even within a band). The 20/40 BSS Coex
report frame can be used to advertise channels from only a single
operating class, so multiple frames may need to be transmitted here
depending on which channels are found to be 40 MHz intolerant.

> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
> @@ -2819,3 +2819,66 @@ void wpas_connection_failed(struct wpa_supplicant *wpa_s, const u8 *bssid)
>  }
> +
> +#ifdef CONFIG_SME
> +void wpa_supplicant_proc_40mhz_intolerant(struct wpa_supplicant *wpa_s)
> +{

> +	u8 intol_channels[28], tmp_channel;

Where does this "28" come from?

> +	/* Check whether AP is supporting HT40 */
> +	ie = wpa_bss_get_ie(wpa_s->current_bss, WLAN_EID_HT_OPERATION);
> +	if (!ie || !(ie[3] & HT_INFO_HT_PARAM_REC_TRANS_CHNL_WIDTH))

This needs to verify that IE length is large enough to avoid possible
buffer overflow (included in the cleaned up patches).

> +	assoc_band = freq_to_channel(wpa_s->current_bss->freq, NULL, NULL);
> +	if (assoc_band == WPAS_BAND_INVALID)
> +		return;
> +
> +	dl_list_for_each(bss, &wpa_s->bss, struct wpa_bss, list) {
> +		/* Skip other band bss */
> +		if (freq_to_channel(bss->freq, NULL, &tmp_channel)
> +				!= assoc_band)
> +			continue;

This needs to address different operating classes and the different
definitions of operating classes depending on the Country element.

> +		ie = wpa_bss_get_ie(bss, WLAN_EID_HT_CAP);
> +		if (!ie) {
> +			/* To avoid channel duplication */
> +			if (!n_channels ||
> +			    intol_channels[n_channels - 1] != tmp_channel)
> +				intol_channels[n_channels++] = tmp_channel;
> +			found_intolerant = TRUE;
> +		} else {
> +			ht_cap = WPA_GET_LE16(ie + 2);

This needs to verify that there is enough room in the IE for that field
to avoid potential buffer overflow (included in the cleaned up patches).

> +			if (ht_cap & HT_CAP_INFO_40MHZ_INTOLERANT) {
> +				/* To avoid channel duplication */
> +				if (!n_channels ||
> +				    intol_channels[n_channels - 1]
> +					!= tmp_channel)
> +					intol_channels[n_channels++] =
> +						tmp_channel;
> +				found_intolerant = TRUE;
> +			}
> +		}

It would be nice to combine those duplication removal implementations
into a single one.. Isn't this doing the same thing in both paths? In
addition, please note that the BSS entries are not necessarily ordered
by frequency, so you may get duplicate entries even with this code. In
addition, the operating class difference can make this even stranger, so
more robust implementation would be to remove duplicates in the end
after having figured out channel lists for each operating class.

> +	if (found_intolerant)
> +		sme_send_2040_bss_coex(wpa_s, intol_channels, n_channels);

This is the part that would need to be done separately for each
operating class in which there are 40 MHz intolerant channels.

-- 
Jouni Malinen                                            PGP id EFC895FA
-------------- next part --------------