[V2] HE: add HE support to hostapd_set_freq_params()

The parameters that need to be applied are symmetric to those of VHT,
however the validation code needs to be tweak to check the HE capabilities.

Signed-off-by: Shashidhar Lakkavalli <slakkavalli@datto.com>
Signed-off-by: John Crispin <john@phrozen.org>
---
Changes in V2
* the 3rd switch statement was not properly guarded, thus breaking
  non-VHT/HE modes

 src/common/hw_features_common.c | 69 +++++++++++++++++++++++++++------
 1 file changed, 58 insertions(+), 11 deletions(-)

Hi Jouni,

kind reminder ... while rebasing my local tree, I noticed that this 
patch is still pending.

     John

On 19/06/2019 08:07, John Crispin wrote:
> The parameters that need to be applied are symmetric to those of VHT,
> however the validation code needs to be tweak to check the HE capabilities.
>
> Signed-off-by: Shashidhar Lakkavalli <slakkavalli@datto.com>
> Signed-off-by: John Crispin <john@phrozen.org>
> ---
> Changes in V2
> * the 3rd switch statement was not properly guarded, thus breaking
>    non-VHT/HE modes
>
>   src/common/hw_features_common.c | 69 +++++++++++++++++++++++++++------
>   1 file changed, 58 insertions(+), 11 deletions(-)
>
> diff --git a/src/common/hw_features_common.c b/src/common/hw_features_common.c
> index 3fdbf893d..841c8884f 100644
> --- a/src/common/hw_features_common.c
> +++ b/src/common/hw_features_common.c
> @@ -381,13 +381,44 @@ int hostapd_set_freq_params(struct hostapd_freq_params *data,
>   	data->center_freq2 = 0;
>   	data->bandwidth = sec_channel_offset ? 40 : 20;
>   
> -	if (data->vht_enabled) switch (oper_chwidth) {
> +	if (data->he_enabled) switch (oper_chwidth) {
>   	case CHANWIDTH_USE_HT:
> -		if (center_segment1 ||
> -		    (center_segment0 != 0 &&
> -		     5000 + center_segment0 * 5 != data->center_freq1 &&
> -		     2407 + center_segment0 * 5 != data->center_freq1))
> +		if (mode < HOSTAPD_MODE_IEEE80211A) {
> +			if (!(he_cap->phy_cap[HE_PHYCAP_CHANNEL_WIDTH_SET_IDX] &
> +			      HE_PHYCAP_CHANNEL_WIDTH_SET_40MHZ_IN_2G)) {
> +				wpa_printf(MSG_ERROR,
> +					   "40 channel width is not supported!");
> +				return -1;
> +			}
> +			break;
> +		}
> +		/* fall through */
> +	case CHANWIDTH_80MHZ:
> +		if (!(he_cap->phy_cap[HE_PHYCAP_CHANNEL_WIDTH_SET_IDX] &
> +		      HE_PHYCAP_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G)) {
> +			wpa_printf(MSG_ERROR,
> +				   "40/80 channel width is not supported!");
> +			return -1;
> +		}
> +		break;
> +	case CHANWIDTH_80P80MHZ:
> +		if (!(he_cap->phy_cap[HE_PHYCAP_CHANNEL_WIDTH_SET_IDX] &
> +		      HE_PHYCAP_CHANNEL_WIDTH_SET_80PLUS80MHZ_IN_5G)) {
> +			wpa_printf(MSG_ERROR,
> +				   "80+80 channel width is not supported!");
>   			return -1;
> +		}
> +		break;
> +	case CHANWIDTH_160MHZ:
> +		if (!(he_cap->phy_cap[HE_PHYCAP_CHANNEL_WIDTH_SET_IDX] &
> +		      HE_PHYCAP_CHANNEL_WIDTH_SET_160MHZ_IN_5G)) {
> +			wpa_printf(MSG_ERROR,
> +				   "160 channel width is not supported!");
> +			return -1;
> +		}
> +		break;
> +	} else if (data->vht_enabled) switch (oper_chwidth) {
> +	case CHANWIDTH_USE_HT:
>   		break;
>   	case CHANWIDTH_80P80MHZ:
>   		if (!(vht_caps & VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ)) {
> @@ -395,6 +426,28 @@ int hostapd_set_freq_params(struct hostapd_freq_params *data,
>   				   "80+80 channel width is not supported!");
>   			return -1;
>   		}
> +		/* fall through */
> +	case CHANWIDTH_80MHZ:
> +		break;
> +	case CHANWIDTH_160MHZ:
> +		if (!(vht_caps & (VHT_CAP_SUPP_CHAN_WIDTH_160MHZ |
> +				  VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ))) {
> +			wpa_printf(MSG_ERROR,
> +				   "160MHZ channel width is not supported!");
> +			return -1;
> +		}
> +		break;
> +	}
> +
> +	if (data->he_enabled || data->vht_enabled) switch (oper_chwidth) {
> +	case CHANWIDTH_USE_HT:
> +		if (center_segment1 ||
> +		    (center_segment0 != 0 &&
> +		     5000 + center_segment0 * 5 != data->center_freq1 &&
> +		     2407 + center_segment0 * 5 != data->center_freq1))
> +			return -1;
> +		break;
> +	case CHANWIDTH_80P80MHZ:
>   		if (center_segment1 == center_segment0 + 4 ||
>   		    center_segment1 == center_segment0 - 4)
>   			return -1;
> @@ -439,12 +492,6 @@ int hostapd_set_freq_params(struct hostapd_freq_params *data,
>   		break;
>   	case CHANWIDTH_160MHZ:
>   		data->bandwidth = 160;
> -		if (!(vht_caps & (VHT_CAP_SUPP_CHAN_WIDTH_160MHZ |
> -				  VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ))) {
> -			wpa_printf(MSG_ERROR,
> -				   "160MHZ channel width is not supported!");
> -			return -1;
> -		}
>   		if (center_segment1)
>   			return -1;
>   		if (!sec_channel_offset)

On Wed, Jun 19, 2019 at 08:07:32AM +0200, John Crispin wrote:
> The parameters that need to be applied are symmetric to those of VHT,
> however the validation code needs to be tweak to check the HE capabilities.

These breaks following mac80211_hwsim test cases with "40 channel width
is not supported!":

he80_to_24g_he
he_on_24ghz
he_params
he_open

Is this expected and how can those be fixed?

> diff --git a/src/common/hw_features_common.c b/src/common/hw_features_common.c
> @@ -381,13 +381,44 @@ int hostapd_set_freq_params(struct hostapd_freq_params *data,
> -	if (data->vht_enabled) switch (oper_chwidth) {
> +	if (data->he_enabled) switch (oper_chwidth) {
>  	case CHANWIDTH_USE_HT:
> -		if (center_segment1 ||
> -		    (center_segment0 != 0 &&
> -		     5000 + center_segment0 * 5 != data->center_freq1 &&
> -		     2407 + center_segment0 * 5 != data->center_freq1))
> +		if (mode < HOSTAPD_MODE_IEEE80211A) {
> +			if (!(he_cap->phy_cap[HE_PHYCAP_CHANNEL_WIDTH_SET_IDX] &
> +			      HE_PHYCAP_CHANNEL_WIDTH_SET_40MHZ_IN_2G)) {
> +				wpa_printf(MSG_ERROR,
> +					   "40 channel width is not supported!");
> +				return -1;
> +			}

enum hostapd_hw_mode values should not be compared in that manner, i.e.,
that mode < HOSTAPD_MODE_IEEE80211A needs to explicitly check for 2.4
GHz cases (HOSTAPD_MODE_IEEE80211G in practice since
HOSTAPD_MODE_IEEE80211B does not sound like something that would ever be
used with HE). That error message could be clearer on this missing
support applying specifically to the 2.4 GHz band.

> +	case CHANWIDTH_80MHZ:
> +		if (!(he_cap->phy_cap[HE_PHYCAP_CHANNEL_WIDTH_SET_IDX] &
> +		      HE_PHYCAP_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G)) {
> +			wpa_printf(MSG_ERROR,
> +				   "40/80 channel width is not supported!");
> +			return -1;
> +		}
> +		break;

That _IN_5G part is a bit confusing now that those bits in Supported
Channel Width Set are actually for both 5 and 6 GHz bands, but anyway,
these cases with 80 or 160 MHz mentioned are clearly not for the 2.4 GHz
band, so these are easier to understand in that sense.