Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

hostap: fix VHT channel center segment0 #82113

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+127 −1
Open

hostap: fix VHT channel center segment0 #82113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5294215
hostap: fix VHT channel center segment0
gangli02 Nov 27, 2024
ba10173
net: wifi: add "bandwidth" parameter to "wifi ap enable" command
gangli02 Nov 27, 2024
a323598
drivers: wifi: nxp: set bandwidth to driver
gangli02 Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions drivers/wifi/nxp/nxp_wifi_drv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,17 @@ static int nxp_wifi_start_ap(const struct device *dev, struct wifi_connect_req_p
return -EAGAIN;
}

switch (params->bandwidth) {
case WIFI_FREQ_BANDWIDTH_20MHZ:
case WIFI_FREQ_BANDWIDTH_40MHZ:
case WIFI_FREQ_BANDWIDTH_80MHZ:
wlan_uap_set_bandwidth(params->bandwidth);
break;
default:
LOG_ERR("Invalid bandwidth");
return -EAGAIN;
}

ret = wlan_add_network(&nxp_wlan_network);
if (ret != WM_SUCCESS) {
status = NXP_WIFI_RET_FAIL;
Expand Down
2 changes: 2 additions & 0 deletions include/zephyr/net/wifi_mgmt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,8 @@ struct wifi_connect_req_params {
const uint8_t *identities[WIFI_ENT_IDENTITY_MAX_USERS];
/** User Passwords */
const uint8_t *passwords[WIFI_ENT_IDENTITY_MAX_USERS];
/** Parameter used for frequency band */
enum wifi_frequency_bandwidths bandwidth;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this commit should be the first one as VHT seg0 frequency relies on the this param

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

};

/** @brief Wi-Fi connect result codes. To be overlaid on top of \ref wifi_status
Expand Down
98 changes: 98 additions & 0 deletions modules/hostap/src/supp_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2082,6 +2082,78 @@
return ret;
}

static int hapd_config_chan_center_seg0(struct wifi_connect_req_params *params)
{
int ret = 0;
uint8_t center_freq_seg0_idx = 0;
uint8_t oper_chwidth = CHANWIDTH_USE_HT;
uint8_t *center_freq = NULL;
uint8_t center_freq_40MHz[] = {38, 46, 54, 62, 102, 110, 118, 126, 134, 142, 151, 159};
uint8_t center_freq_80MHz[] = {42, 58, 106, 122, 138, 155};
uint8_t index, index_max, chan_idx, ch_offset = 0;

/* Unless ACS is being used, both "channel" and "vht_oper_centr_freq_seg0_idx"
* parameters must be set. */

Check warning on line 2096 in modules/hostap/src/supp_api.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

BLOCK_COMMENT_STYLE 

modules/hostap/src/supp_api.c:2096  Block comments use a trailing */ on a separate line
switch (params->bandwidth) {
case WIFI_FREQ_BANDWIDTH_20MHZ:
oper_chwidth = CHANWIDTH_USE_HT;
center_freq_seg0_idx = params->channel;
break;
case WIFI_FREQ_BANDWIDTH_40MHZ:
if (!hostapd_cli_cmd_v("set ht_capab [HT40+][SHORT-GI-40]")) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHORT-GI should be set as per the underlying driver/chipset capablities

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the choice of HT40+/- should still be user configurable, no? 36-40MHz-HT40+ === 40-40MHz-HT40- == 38.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When starting soft AP, it cannot get the driver/chipset capabilities.
What do you think about putting these configurations in the "wifi ap config" command?
Before starting the AP, users can freely set it, similar to the hostapd.conf file.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that should do.

goto out;
}
oper_chwidth = CHANWIDTH_USE_HT;
center_freq = center_freq_40MHz;
index_max = ARRAY_SIZE(center_freq_40MHz);
ch_offset = 2;
break;
case WIFI_FREQ_BANDWIDTH_80MHZ:
if (!hostapd_cli_cmd_v("set ht_capab [HT40+][SHORT-GI-40]")) {
goto out;
}
oper_chwidth = CHANWIDTH_80MHZ;
center_freq = center_freq_80MHz;
index_max = ARRAY_SIZE(center_freq_80MHz);
ch_offset = 6;
break;
default:
return -EINVAL;
}

if (params->bandwidth != WIFI_FREQ_BANDWIDTH_20MHZ)

Check failure on line 2124 in modules/hostap/src/supp_api.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

OPEN_BRACE 

modules/hostap/src/supp_api.c:2124  that open brace { should be on the previous line
{
chan_idx = params->channel;
for (index = 0; index < index_max; index++)

Check failure on line 2127 in modules/hostap/src/supp_api.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

OPEN_BRACE 

modules/hostap/src/supp_api.c:2127  that open brace { should be on the previous line
{
if ((chan_idx >= (center_freq[index] - ch_offset)) && (chan_idx <= (center_freq[index] + ch_offset)))

Check warning on line 2129 in modules/hostap/src/supp_api.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LONG_LINE 

modules/hostap/src/supp_api.c:2129  line length of 125 exceeds 100 columns

Check failure on line 2129 in modules/hostap/src/supp_api.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

OPEN_BRACE 

modules/hostap/src/supp_api.c:2129  that open brace { should be on the previous line

Check warning on line 2129 in modules/hostap/src/supp_api.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

SUSPECT_CODE_INDENT 

modules/hostap/src/supp_api.c:2129  suspect code indent for conditional statements (24, 40)
{
center_freq_seg0_idx = center_freq[index];
break;
}
}
}
Comment on lines +2124 to +2135
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we just do below?

	if (params->bandwidth != WIFI_FREQ_BANDWIDTH_20MHZ)
	{
		center_freq_seg0_idx = params->channel + ch_offset;
	}

We don't even need the static array of frequencies? Of course +/- depends on the HT40+/- user input.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will check it.


if (!hostapd_cli_cmd_v("set vht_oper_chwidth %d", oper_chwidth)) {
goto out;
}
if (!hostapd_cli_cmd_v("set vht_oper_centr_freq_seg0_idx %d", center_freq_seg0_idx)) {
goto out;
}
#ifdef CONFIG_WIFI_NM_WPA_SUPPLICANT_11AX
if (!hostapd_cli_cmd_v("set he_oper_chwidth %d", oper_chwidth)) {
goto out;
}
if (!hostapd_cli_cmd_v("set he_oper_centr_freq_seg0_idx %d", center_freq_seg0_idx)) {
goto out;
}
#endif

return ret;
out:
return -EINVAL;
}

int hapd_config_network(struct hostapd_iface *iface,
struct wifi_connect_req_params *params)
{
Expand Down Expand Up @@ -2118,6 +2190,11 @@
goto out;
}

ret = hapd_config_chan_center_seg0(params);
if (ret) {
goto out;
}

if (params->security != WIFI_SECURITY_TYPE_NONE) {
if (params->security == WIFI_SECURITY_TYPE_WPA_PSK) {
if (!hostapd_cli_cmd_v("set wpa 1")) {
Expand Down Expand Up @@ -2470,6 +2547,21 @@
}
#endif

static int set_ap_bandwidth(const struct device *dev, enum wifi_frequency_bandwidths bandwidth)
{
const struct wifi_mgmt_ops *const wifi_mgmt_api = get_wifi_mgmt_api(dev);
struct wifi_ap_config_params params = { 0 };

if (!wifi_mgmt_api || !wifi_mgmt_api->ap_bandwidth) {
wpa_printf(MSG_ERROR, "ap_bandwidth not supported");
return -ENOTSUP;
}

params.bandwidth = bandwidth;
params.oper = WIFI_MGMT_SET;
return wifi_mgmt_api->ap_bandwidth(dev, &params);
}

int supplicant_ap_enable(const struct device *dev,
struct wifi_connect_req_params *params)
{
Expand All @@ -2489,6 +2581,12 @@
return -1;
}

ret = set_ap_bandwidth(dev, params->bandwidth);
if (ret) {
wpa_printf(MSG_ERROR, "Failed to set ap bandwidth");
return -EINVAL;
}

k_mutex_lock(&wpa_supplicant_mutex, K_FOREVER);

#ifdef CONFIG_WIFI_NM_HOSTAPD_AP
Expand Down
17 changes: 16 additions & 1 deletion subsys/net/l2/wifi/wifi_shell.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,7 @@ static int __wifi_args_to_params(const struct shell *sh, size_t argc, char *argv
{"channel", required_argument, 0, 'c'},
{"timeout", required_argument, 0, 't'},
{"anon-id", required_argument, 0, 'a'},
{"bandwidth", required_argument, 0, 'B'},
{"key1-pwd", required_argument, 0, 'K'},
{"key2-pwd", required_argument, 0, 'K'},
{"suiteb-type", required_argument, 0, 'S'},
Expand Down Expand Up @@ -620,8 +621,9 @@ static int __wifi_args_to_params(const struct shell *sh, size_t argc, char *argv
params->security = WIFI_SECURITY_TYPE_NONE;
params->mfp = WIFI_MFP_OPTIONAL;
params->eap_ver = 1;
params->bandwidth = WIFI_FREQ_BANDWIDTH_20MHZ;

while ((opt = getopt_long(argc, argv, "s:p:k:e:w:b:c:m:t:a:K:S:V:I:P:Rh",
while ((opt = getopt_long(argc, argv, "s:p:k:e:w:b:c:m:t:a:B:K:S:V:I:P:Rh",
long_options, &opt_index)) != -1) {
state = getopt_state_get();
switch (opt) {
Expand Down Expand Up @@ -734,6 +736,18 @@ static int __wifi_args_to_params(const struct shell *sh, size_t argc, char *argv
return -EINVAL;
}
break;
case 'B':
switch (atoi(state->optarg)) {
case 1:
case 2:
case 3:
params->bandwidth = atoi(state->optarg);
break;
default:
PR_ERROR("Invalid bandwidth: %d\n", atoi(state->optarg));
return -EINVAL;
}
break;
case 'K':
if (key_passwd_cnt >= 2) {
PR_WARNING("too many key_passwd (max 2 key_passwd)\n");
Expand Down Expand Up @@ -3138,6 +3152,7 @@ SHELL_STATIC_SUBCMD_SET_CREATE(
"0:Disable, 1:Optional, 2:Required\n"
"-b --band=<band> (2 -2.6GHz, 5 - 5Ghz, 6 - 6GHz)\n"
"-m --bssid=<BSSID>\n"
"[-B, --bandwidth=<bandwidth>]: 1:20MHz, 2:40MHz, 3:80MHz\n"
"[-K, --key1-pwd for eap phase1 or --key2-pwd for eap phase2]:\n"
"Private key passwd for enterprise mode. Default no password for private key.\n"
"[-S, --suiteb-type]: 1:suiteb, 2:suiteb-192. Default 0: not suiteb mode.\n"
Expand Down
Loading