[PATCH] hostapd: add VHT channel switch IEs

[Rajkumar Manoharan]

On Fri, May 08, 2015 at 05:11:33PM +0300, Jouni Malinen wrote:
> On Tue, May 05, 2015 at 01:25:50PM +0530, Rajkumar Manoharan wrote:
> > Add wide band channel switch and VHT tx power envelope IEs for
> > VHT bandwidth channel switch.
> 
> > @@ -316,7 +316,7 @@ static u8 * hostapd_eid_csa(struct hostapd_data *hapd, u8 *eid)
> > -static u8 * hostapd_eid_secondary_channel(struct hostapd_data *hapd, u8 *eid)
> > +static u8 *hostapd_eid_secondary_channel(struct hostapd_data *hapd, u8 *eid)
> 
> Please do not include unrelated whitespace changes in the patch
> (especially when they are breaking the expected style).
>
Understood. Since checkpatch is complaining style, I did that. Will try
to avoid such unnecessary changes

> > @@ -337,6 +337,72 @@ static u8 * hostapd_eid_secondary_channel(struct hostapd_data *hapd, u8 *eid)
> > +static u8 *hostapd_eid_wide_bw_chansw(struct hostapd_data *hapd, u8 *eid)
> 
> > +	switch (params->bandwidth) {
> > +	case 40:
> > +		*eid++ = VHT_CHANWIDTH_USE_HT;
> > +		break;
> > +	case 80:
> > +		*eid++ = VHT_CHANWIDTH_80MHZ;
> > +		break;
> > +	case 160:
> > +		*eid++ = VHT_CHANWIDTH_160MHZ;
> > +		break;
> > +	}
> 
> This should likely handle case of something else, e.g., by returning
> without adding the IE.
> 
> > +	*eid++ = params->freq + params->sec_channel_offset * 10;
> > +	*eid++ = params->center_freq2;
> 
> This cannot be correct.. params->freq and center_freq2 (I'd assume) are
> in MHz while the *eid is a one octet field (channel center frequency
> index, not frequency).

Just got to know that similar change was already posted ("ap: Add WIDE_BANDWIDTH
channel switch IE") by Ilan Peer.

http://lists.shmoo.com/pipermail/hostap/2014-May/030243.html

Why it is not taken? Shall I rework my change on top of above commit?
> 
> > +	*eid++ = WLAN_EID_VHT_TRANSMIT_POWER_ENVELOPE;
> > +	*eid++ = 5;
> > +	*eid++ = 2;
> > +	*eid++ = chan->max_tx_power;
> > +	*eid++ = chan->max_tx_power;
> > +	*eid++ = chan->max_tx_power;
> > +	*eid++ = chan->max_tx_power;
> 
> Could you please clarify why this is adding the same max TX power for
> all four different cases and especially why that is done regardless of
> the channel bandwidth? In particular, why would 160/80+80 MHz value be
> included if the target channel is only 40 or 80 MHz wide?
>
40/80MHz alone are handled now. I will added sanity checks for
160/80+80 MHz. In that case I assumed that tx power would be same for secondary
channels. Correct me If I am wrong.

Should the tx power be in n 0.5dB steps 2's complement representation?

> That value 2 as the Transmit Power Information does not match rest of
> the element. It would indicate LocalMaximumTransmitPower Count 2 which
> means TX power limits for 20, 40, and 80 MHz, i.e., only three values
> while four values are being filled int.
> 
Yeah. I should not fill last (4th) element. Will correct that in next
patch.

-Rajkumar