Re: [PATCH 6/9] 6G: Do not add VHT IEs in 6 GHz band

On 2020-05-17 07:05, Jouni Malinen wrote:
> On Fri, Apr 24, 2020 at 03:45:39PM -0700, Rajkumar Manoharan wrote:
>> Do not account VHT IEs for total beacon length and also add 6 GHz 
>> checks
>> in Wide band VHT channel switch, VHT Tx power and Vendor VHT IE
>> construction.
> 
> It would be correct to remove the IEs that are specific to VHT, but 
> that
> list of elements does not look correct.
> 
> "Wide band VHT channel switch" is called "Wide Bandwidth Channel Switch
> subelement" and it is included in the "Channel Switch Wrapper element".
> It is not VHT specific in the current standard baseline. Can you please
> clarify why it would not be allowed on the 6 GHz band?
> 
> "VHT Tx power IE" is called "Transmit Power Envelope element" in the
> current standard baseline, i.e., it is not specific to VHT. Can you
> please clarify why it would not be allowed on the 6 GHz band?
> 
My bad. I wrongly interpret them as VHT specific. Will remove checks 
from wide band
chan switch and TPC IE.

> Furthermore, it does not look like these changes are really needed 
> since
> the cases covered here are all for invalid configuration cases:
> 
>> diff --git a/src/ap/beacon.c b/src/ap/beacon.c
>> @@ -452,7 +452,8 @@ static u8 * hostapd_gen_probe_resp(struct 
>> hostapd_data *hapd,
>> -	if (hapd->conf->vendor_vht) {
>> +	if (hapd->conf->vendor_vht &&
>> +	    !is_6ghz_op_class(hapd->iconf->op_class)) {
>>  		buflen += 5 + 2 + sizeof(struct ieee80211_vht_capabilities) +
>>  			2 + sizeof(struct ieee80211_vht_operation);
> 
> Why would this be needed? vendor_vht=1 is used only on the 2.4 GHz band
> and it would not be valid to set it when operating on 6 GHz.
> 
Yes. Just to avoid accidentally enabling vht IEs in case of wrong user 
config in 6 GHz band.
Perhaps It would be better to check mode->mode != 
HOSTAPD_MODE_IEEE80211G instead of 6 GHz. no?

>> @@ -558,8 +559,7 @@ static u8 * hostapd_gen_probe_resp(struct 
>> hostapd_data *hapd,
>>  #ifdef CONFIG_IEEE80211AC
>> -	if (hapd->iconf->ieee80211ac && !hapd->conf->disable_11ac &&
>> -	    !is_6ghz_op_class(hapd->iconf->op_class)) {
>> +	if (hapd->iconf->ieee80211ac && !hapd->conf->disable_11ac) {
>>  		pos = hostapd_eid_vht_capabilities(hapd, pos, 0);
>>  		pos = hostapd_eid_vht_operation(hapd, pos);
>>  		pos = hostapd_eid_txpower_envelope(hapd, pos);
> 
> Why would this be needed? ieee80211ac=1 is used only on the 5 GHz band
> (not including 6 GHz) and it would not be valid to set it when 
> operating
> on 6 GHz.
> 
Don't understand why is_6ghz_op_class check is remove from above hunk.
As I explain in another patch, both ieee80211n, ieee80211ac are set to 1 
in order to verify
user config of ht_capab and vht_capab against phy cap. Otherwise those 
configs are skipped.
Herewith I attached my hostapd config for your reference.

>> @@ -1156,7 +1156,8 @@ int ieee802_11_build_ap_params(struct 
>> hostapd_data *hapd,
>>  #ifdef CONFIG_IEEE80211AC
>> -	if (hapd->conf->vendor_vht) {
>> +	if (hapd->conf->vendor_vht &&
>> +	    !is_6ghz_op_class(hapd->iconf->op_class)) {
>>  		tail_len += 5 + 2 + sizeof(struct ieee80211_vht_capabilities) +
>>  			2 + sizeof(struct ieee80211_vht_operation);
> 
> Why would this be needed? vendor_vht=1 is used only on the 2.4 GHz band
> and it would not be valid to set it when operating on 6 GHz.
> 
As clarified above, better to add 2 GHz band check instead of 6 GHz. 
Will send patch for this
if you are fine.

>> diff --git a/src/ap/ieee802_11_vht.c b/src/ap/ieee802_11_vht.c
>> @@ -176,6 +176,9 @@ u8 * hostapd_eid_wb_chsw_wrapper(struct 
>> hostapd_data *hapd, u8 *eid)
>> +	if (is_6ghz_op_class(hapd->iconf->op_class))
>> +		return eid;
> 
> This function is called only if ieee80211ac=1 configuration is used. 
> Why
> would that be used if operating on 6 GHz?
> 
>> @@ -234,6 +237,9 @@ u8 * hostapd_eid_txpower_envelope(struct 
>> hostapd_data *hapd, u8 *eid)
>> +	if (is_6ghz_op_class(hapd->iconf->op_class))
>> +		return eid;
> 
> This function is called only if ieee80211ac=1 configuration is used. 
> Why
> would that be used if operating on 6 GHz?
> 
ieee80211ac is set to 1 to verify vht user config that is used to build 
6 GHz config. Hence
added this check. Anyway I agree that txpower and wide_chsw are needed 
for 6 GHz. Will remove these checks.

>> @@ -428,6 +434,9 @@ u8 * hostapd_eid_vendor_vht(struct hostapd_data 
>> *hapd, u8 *eid)
>> +	if (is_6ghz_op_class(hapd->iconf->op_class))
>> +		return eid;
> 
> This function is called only if vendor_vht=1 configuration is used. Why
> would that be used if operating on 6 GHz?
Same as above.

-Rajkumar

_______________________________________________
Hostap mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/hostap

driver=nl80211
logger_syslog=127
logger_syslog_level=2
logger_stdout=127
logger_stdout_level=2
hw_mode=a
channel=1
op_class=131

ieee80211n=1
ht_capab=[MAX-AMSDU-7935]
ieee80211ac=1
vht_capab=[RX-ANTENNA-PATTERN][TX-ANTENNA-PATTERN][MAX-MPDU-11454][MAX-A-MPDU-LEN-EXP7]
ieee80211ax=1
he_su_beamformer=1
he_mu_beamformer=1
he_bss_color=1
he_default_pe_duration=4
he_mu_edca_qos_info_param_count=0
he_mu_edca_qos_info_q_ack=0
he_mu_edca_qos_info_queue_request=0
he_mu_edca_qos_info_txop_request=0
he_mu_edca_ac_be_aifsn=8
he_mu_edca_ac_be_aci=0
he_mu_edca_ac_be_ecwmin=9
he_mu_edca_ac_be_ecwmax=10
he_mu_edca_ac_be_timer=255
he_mu_edca_ac_bk_aifsn=15
he_mu_edca_ac_bk_aci=1
he_mu_edca_ac_bk_ecwmin=9
he_mu_edca_ac_bk_ecwmax=10
he_mu_edca_ac_bk_timer=255
he_mu_edca_ac_vi_ecwmin=5
he_mu_edca_ac_vi_ecwmax=7
he_mu_edca_ac_vi_aifsn=5
he_mu_edca_ac_vi_aci=2
he_mu_edca_ac_vi_timer=255
he_mu_edca_ac_vo_aifsn=5
he_mu_edca_ac_vo_aci=3
he_mu_edca_ac_vo_ecwmin=5
he_mu_edca_ac_vo_ecwmax=7
he_mu_edca_ac_vo_timer=255

interface=wlan0
ctrl_interface=/var/run/hostapd
max_num_sta=128
disassoc_low_ack=0
preamble=1
wmm_enabled=1
ignore_broadcast_ssid=0
uapsd_advertisement_enabled=1
auth_algs=1
wpa=0
ssid=vmraj-6g
bridge=br-lan
#bssid=00:03:7f:12:a5:cb