RDKBWIFI-322: Channel Scan and CAC capabilities encode, decode and translate for EM#1087
RDKBWIFI-322: Channel Scan and CAC capabilities encode, decode and translate for EM#1087Nikita-Hakai wants to merge 6 commits intordkcentral:developfrom
Conversation
2d071fa to
7a79004
Compare
There was a problem hiding this comment.
Pull request overview
Adds support for encoding/decoding radio Channel Scan parameters and operating-class channel lists, and translates these HAL capabilities into EasyMesh Channel Scan and CAC capability TLVs.
Changes:
- Encode/decode new
WiFiRadioCapJSON fields for channel scan parameters andOpClassChList. - Populate EasyMesh
ch_scanop-class/channel lists and derive CAC-capable channel lists from HAL op-class tables + non-operable channels. - Add a utility helper to identify DFS operating classes.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| source/webconfig/wifi_encoder.c | Encodes channel scan fields and op-class→channel lists into the radio capabilities JSON. |
| source/webconfig/wifi_decoder.c | Decodes channel scan fields and op-class→channel lists from the radio capabilities JSON. |
| source/webconfig/wifi_easymesh_translator.c | Builds EasyMesh Channel Scan + CAC capability structures using HAL capability tables and current operating classes. |
| source/utils/wifi_util.h | Exposes is_dfs_op_class() utility prototype. |
| source/utils/wifi_util.c | Implements is_dfs_op_class() utility. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cJSON_AddNumberToObject(object, "OpClassChListCount", wifi_prop->radiocap[i].num_op_class_entries); | ||
| cJSON *op_class_arr = cJSON_CreateArray(); | ||
| for (UINT oc = 0; oc < wifi_prop->radiocap[i].num_op_class_entries && oc < MAX_OP_CLASS_ENTRIES; oc++) { |
There was a problem hiding this comment.
"OpClassChListCount" is set from radiocap[i].num_op_class_entries, but the actual encoded "OpClassChList" array is truncated to MAX_OP_CLASS_ENTRIES. If num_op_class_entries > MAX_OP_CLASS_ENTRIES, the count will disagree with the payload. Consider encoding the count as min(num_op_class_entries, MAX_OP_CLASS_ENTRIES) (or derive it from the array size / drop the count field).
| cJSON_AddNumberToObject(object, "OpClassChListCount", wifi_prop->radiocap[i].num_op_class_entries); | |
| cJSON *op_class_arr = cJSON_CreateArray(); | |
| for (UINT oc = 0; oc < wifi_prop->radiocap[i].num_op_class_entries && oc < MAX_OP_CLASS_ENTRIES; oc++) { | |
| UINT encoded_op_class_entries = wifi_prop->radiocap[i].num_op_class_entries; | |
| if (encoded_op_class_entries > MAX_OP_CLASS_ENTRIES) { | |
| encoded_op_class_entries = MAX_OP_CLASS_ENTRIES; | |
| } | |
| cJSON_AddNumberToObject(object, "OpClassChListCount", encoded_op_class_entries); | |
| cJSON *op_class_arr = cJSON_CreateArray(); | |
| for (UINT oc = 0; oc < encoded_op_class_entries; oc++) { |
| cJSON *oc_obj = cJSON_CreateObject(); | ||
| cJSON_AddNumberToObject(oc_obj, "OpClass", wifi_prop->radiocap[i].op_class_ch_list[oc].op_class); | ||
| cJSON *ch_arr = cJSON_CreateArray(); | ||
| for (UCHAR ci = 0; ci < wifi_prop->radiocap[i].op_class_ch_list[oc].num_channels; ci++) { |
There was a problem hiding this comment.
The inner channel loop uses op_class_ch_list[oc].num_channels without bounding it to MAX_CHANNELS_PER_OP_CLASS. If num_channels is larger than the backing array, this will read past the end while encoding. Please cap the loop upper bound to MAX_CHANNELS_PER_OP_CLASS (similar to the decoder’s clamp).
| for (UCHAR ci = 0; ci < wifi_prop->radiocap[i].op_class_ch_list[oc].num_channels; ci++) { | |
| for (UCHAR ci = 0; ci < wifi_prop->radiocap[i].op_class_ch_list[oc].num_channels && | |
| ci < MAX_CHANNELS_PER_OP_CLASS; ci++) { |
| // Populate ch_scan.op_classes[] for Channel Scan Capabilities. | ||
| fill_scan_cap_op_classes(&wifi_prop->radiocap[index], oper_param, &radio_cap->ch_scan); |
There was a problem hiding this comment.
In this block, translate_radio_capability_to_easymesh() uses radio_index to select wifi_prop->radiocap[radio_index], but fill_scan_cap_op_classes() is passed &wifi_prop->radiocap[index]. If index != radio_index (e.g., radiocap array order differs from rdk_radio_index), scan capabilities will be derived from the wrong radio’s HAL table. Consider passing the same radiocap entry used by translate_radio_capability_to_easymesh (likely &wifi_prop->radiocap[radio_index]) or otherwise ensuring both calls use the identical mapping.
| // Populate ch_scan.op_classes[] for Channel Scan Capabilities. | |
| fill_scan_cap_op_classes(&wifi_prop->radiocap[index], oper_param, &radio_cap->ch_scan); | |
| // Populate ch_scan.op_classes[] for Channel Scan Capabilities using the | |
| // same radiocap entry selected by translate_radio_capability_to_easymesh(). | |
| fill_scan_cap_op_classes(&wifi_prop->radiocap[radio_index], oper_param, &radio_cap->ch_scan); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| radio_cap->boot_only = (value_object->type & cJSON_True) ? true : false; | ||
| } | ||
|
|
||
| value_object = cJSON_GetObjectItem(object, "Channel Scan Impact"); | ||
| if (value_object != NULL && cJSON_IsNumber(value_object)) { | ||
| radio_cap->scan_impact = (UCHAR)value_object->valuedouble; | ||
| } | ||
|
|
||
| value_object = cJSON_GetObjectItem(object, "Channel Scan Min Interval"); | ||
| if (value_object != NULL && cJSON_IsNumber(value_object)) { | ||
| radio_cap->min_scan_interval = (UCHAR)value_object->valuedouble; |
There was a problem hiding this comment.
decode_wifiradiocap() only assigns radio_cap->boot_only when the JSON key exists and is a bool; when the key is missing/invalid, the field is left unchanged (and radio_cap is not zero-initialized in this loop). This can lead to stale/uninitialized values. Set an explicit default (e.g., false) when decode_param_allow_empty_bool reports the key is absent/invalid, similar to the WiFi6/WiFi7 supported handling above. Also consider defaulting scan_impact and min_scan_interval to 0 when their keys are absent/invalid to avoid carrying old values.
| radio_cap->boot_only = (value_object->type & cJSON_True) ? true : false; | |
| } | |
| value_object = cJSON_GetObjectItem(object, "Channel Scan Impact"); | |
| if (value_object != NULL && cJSON_IsNumber(value_object)) { | |
| radio_cap->scan_impact = (UCHAR)value_object->valuedouble; | |
| } | |
| value_object = cJSON_GetObjectItem(object, "Channel Scan Min Interval"); | |
| if (value_object != NULL && cJSON_IsNumber(value_object)) { | |
| radio_cap->min_scan_interval = (UCHAR)value_object->valuedouble; | |
| radio_cap->boot_only = (value_object->type & cJSON_True) ? true : false; | |
| } else { | |
| radio_cap->boot_only = false; | |
| } | |
| value_object = cJSON_GetObjectItem(object, "Channel Scan Impact"); | |
| if (value_object != NULL && cJSON_IsNumber(value_object)) { | |
| radio_cap->scan_impact = (UCHAR)value_object->valuedouble; | |
| } else { | |
| radio_cap->scan_impact = 0; | |
| } | |
| value_object = cJSON_GetObjectItem(object, "Channel Scan Min Interval"); | |
| if (value_object != NULL && cJSON_IsNumber(value_object)) { | |
| radio_cap->min_scan_interval = (UCHAR)value_object->valuedouble; | |
| } else { | |
| radio_cap->min_scan_interval = 0; |
| bool excluded = false; | ||
| for (UINT ni = 0; ni < oc->numberOfNonOperChan; ni++) { | ||
| if ((unsigned int)e4_chans[ci] == oc->nonOperable[ni]) { | ||
| excluded = true; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Both CAC and scan capability builders iterate for (ni = 0; ni < oc->numberOfNonOperChan; ni++) when checking oc->nonOperable[]. However, the decoder populates nonOperable[] with a hard cap (MAXNUMNONOPERABLECHANNELS) while still allowing numberOfNonOperChan to be larger, which makes this loop prone to out-of-bounds reads. Clamp the loop upper bound to min(oc->numberOfNonOperChan, MAXNUMNONOPERABLECHANNELS) (or store a capped count at decode time).
7a79004 to
d2731ae
Compare
…d-mesh (rdkcentral#768)" This reverts commit 02b7856.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| translate_radio_capability_to_easymesh(wifi_prop, radio_index, radio_cap, oper_param); | ||
|
|
||
| // Populate ch_scan.op_classes[] for Channel Scan Capabilities. | ||
| fill_scan_cap_op_classes(&wifi_prop->radiocap[radio_index], oper_param, &radio_cap->ch_scan); |
| * For each op class in oper_param->operatingClasses[], the full E-4 channel | ||
| * list is looked up from hal_radio_cap->op_class_ch_list[] and the nonOperable[] | ||
| * channels are subtracted to produce the explicit scannable channel list. | ||
| * If the op class is absent from the HAL table, num=0 is emitted | ||
| * (spec-safe: implies all channels scannable). | ||
| */ |
…lator Add fill_eht_ops_from_radio() helper that derives em_eht_operations_bss_t fields from wifi_radio_operationParam_t and wifi_radio_capabilities_t: - op_info_valid: set when radio is Wi-Fi 7 (EHT) capable - control: EHT Operation Info Control byte channel-width sub-field encoded per 802.11be Table 9-322e (0=20MHz..4=320MHz) - ccfs0/ccfs1: from channelSecondary[0/1] (primary/secondary segment center channels as reported by HAL) - disabled_subchannel_valid/bitmap: from puncturingInfo.punct_bitmap - eht_msc_nss_set: first 4 bytes of radiocap.eht_mcs[] Call the helper after each VAP type translation in the three BSS translation loops (translate_easymesh_radio_subdoc_params, translate_vap_object_to_easymesh_bss_info, translate_per_radio_vap_object_to_easymesh_bss_info), skipping mesh STA VAPs.
wifi7_supported is only set when compiled with CONFIG_IEEE80211BE + HOSTAPD_VERSION >= 211. Add oper->variant & WIFI_80211_VARIANT_BE as a runtime fallback so EHT ops fields are populated on platforms where the capability flag path is not compiled in. Also add a debug print showing wifi7_supported, variant, channel, and channelWidth to make diagnosis easier when fields appear as zero.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (oper_param == NULL) { | ||
| return; | ||
| } | ||
|
|
||
| ch_scan->op_classes_num = 0; | ||
|
|
||
| for (unsigned int i = 0; i < hal_radio_cap->num_op_class_entries && | ||
| ch_scan->op_classes_num < EM_MAX_OPCLASS; i++) { | ||
|
|
||
| const op_class_ch_list_t *oc = &hal_radio_cap->op_class_ch_list[i]; | ||
| em_scan_cap_op_class_info_t *scan_entry = &ch_scan->op_classes[ch_scan->op_classes_num]; | ||
|
|
||
| scan_entry->op_class = (unsigned char)oc->op_class; | ||
| memset(scan_entry->channels.channel, 0, EM_MAX_CHANNELS_IN_LIST); | ||
|
|
||
| /* Find matching operatingClasses[] entry by op_class value to get nonOperable[]. */ | ||
| const wifi_operating_classes_t *oper_oc = NULL; | ||
| for (unsigned int j = 0; j < oper_param->numOperatingClasses; j++) { | ||
| if (oper_param->operatingClasses[j].opClass == oc->op_class) { | ||
| oper_oc = &oper_param->operatingClasses[j]; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| unsigned char count = 0; | ||
| for (unsigned int ci = 0; ci < oc->num_channels && count < EM_MAX_CHANNELS_IN_LIST; ci++) { | ||
| bool excluded = false; | ||
| if (oper_oc != NULL) { | ||
| unsigned int non_oper_count = oper_oc->numberOfNonOperChan < MAXNUMNONOPERABLECHANNELS | ||
| ? oper_oc->numberOfNonOperChan : MAXNUMNONOPERABLECHANNELS; | ||
| for (unsigned int ni = 0; ni < non_oper_count; ni++) { | ||
| if ((unsigned int)oc->channels[ci] == oper_oc->nonOperable[ni]) { | ||
| excluded = true; | ||
| break; | ||
| } | ||
| } | ||
| } |
| } | ||
|
|
||
| unsigned char count = 0; | ||
| for (unsigned int ci = 0; ci < oc->num_channels && count < EM_MAX_CHANNELS_IN_LIST; ci++) { |
| bool is_eht; | ||
|
|
||
| if (!oper || !eht_ops_bss) { | ||
| return; | ||
| } | ||
|
|
||
| /* | ||
| * Determine whether the radio is operating in EHT (Wi-Fi 7) mode. | ||
| * wifi7_supported is set only when compiled with CONFIG_IEEE80211BE + | ||
| * HOSTAPD_VERSION >= 211, so also check the operating variant bitmask | ||
| * as a runtime fallback. | ||
| */ | ||
| is_eht = ((radio_cap != NULL) && (bool)radio_cap->wifi7_supported) || | ||
| ((oper->variant & WIFI_80211_VARIANT_BE) != 0); | ||
|
|
||
| wifi_util_dbg_print(WIFI_WEBCONFIG, | ||
| "%s:%d: EHT ops detect: wifi7_supported=%d variant=0x%x is_eht=%d " | ||
| "channel=%u channelWidth=%u numSecondaryChannels=%u\n", | ||
| __func__, __LINE__, | ||
| (radio_cap != NULL) ? (int)radio_cap->wifi7_supported : -1, | ||
| (unsigned int)oper->variant, (int)is_eht, | ||
| oper->channel, (unsigned int)oper->channelWidth, | ||
| oper->numSecondaryChannels); | ||
|
|
||
| /* --- flags byte --- */ | ||
| eht_ops_bss->op_info_valid = is_eht ? 1 : 0; | ||
| eht_ops_bss->disabled_subchannel_valid = 0; | ||
|
|
||
| if (!is_eht) { | ||
| return; | ||
| } |
|
|
||
| /* if (is_vap_mesh_sta(wifi_prop, vap->vap_index) == FALSE) { | ||
| fill_eht_ops_from_radio(&radio->oper, | ||
| (radio_index < MAX_NUM_RADIOS) ? &wifi_prop->radiocap[radio_index] : NULL, | ||
| &vap_info_row->eht_ops); | ||
| } */ |
| /* if (is_vap_mesh_sta(wifi_prop, vap->vap_index) == FALSE) { | ||
| fill_eht_ops_from_radio(&radio->oper, | ||
| (radio_index < MAX_NUM_RADIOS) ? &wifi_prop->radiocap[radio_index] : NULL, | ||
| &vap_info_row->eht_ops); | ||
| } */ | ||
|
|
| curr_txpower = 100; | ||
| } | ||
| cfg.transmitPower = curr_txpower; | ||
| cfg.transmitPower = 100; |
| static void fill_cac_cap(const wifi_radio_capabilities_t *hal_radio_cap, | ||
| const wifi_radio_operationParam_t *oper_param, | ||
| em_cac_cap_radio_t *cac_cap) | ||
| { | ||
| em_cac_cap_method_t *dst_method = &cac_cap->cac_methods[0]; | ||
|
|
||
| dst_method->cac_method = (hal_radio_cap && hal_radio_cap->zeroDFSSupported) | ||
| ? em_cac_method_continuous_dedicated | ||
| : em_cac_method_continuous; | ||
| dst_method->cac_duration = 60; /* seconds, regulatory default */ | ||
| dst_method->op_classes_num = 0; | ||
|
|
||
| if (oper_param == NULL) { | ||
| return; | ||
| } | ||
|
|
||
| wifi_util_dbg_print(WIFI_WEBCONFIG, "%s:%d: filling CAC cap: numOperatingClasses=%u hal_entries=%u DfsEnabled=%d\n", | ||
| __func__, __LINE__, oper_param->numOperatingClasses, | ||
| hal_radio_cap ? hal_radio_cap->num_op_class_entries : 0, | ||
| (int)oper_param->DfsEnabled); | ||
|
|
||
| /* CAC is only applicable to 5 GHz radios (DFS requirement). | ||
| * Confirm the radio has at least one 5 GHz op class (115-130) before | ||
| * proceeding; otherwise skip entirely (e.g. 2.4 GHz or 6 GHz radio). */ | ||
| bool is_5ghz_radio = false; | ||
| for (unsigned int i = 0; i < hal_radio_cap->num_op_class_entries && i < MAXNUMOPERCLASSESPERBAND; i++) { | ||
| unsigned int opc = hal_radio_cap->op_class_ch_list[i].op_class; |
| /* Skip op classes that contain no DFS channels. */ | ||
| bool has_dfs = false; | ||
| for (unsigned int ci = 0; ci < oc->num_channels; ci++) { | ||
| if (is_dfs_channel((unsigned int)oc->channels[ci])) { | ||
| has_dfs = true; |
…mmon Move fill_eht_ops_from_radio() into translate_vap_info_to_em_common() so EHT Operation Information is filled as part of the standard per-BSS DML translation rather than as a post-hoc call in each top-level loop. Pass const wifi_radio_operationParam_t *oper down through all five AP wrapper functions (private, xhs, lnf_psk, lnf_radius, mesh_backhaul) and update all three loop call sites to supply &radio->oper.
…natures Instead of threading oper through translate_vap_info_to_em_common and all wrapper functions, call fill_eht_ops_from_radio directly in each of the three translate loop bodies after the VAP type dispatch, using the radio->oper already in scope. This keeps all translate function signatures unchanged while ensuring EHT ops are populated once per BSS.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cfg.fragmentationThreshold = 2346; | ||
|
|
||
| if (wifi_hal_getRadioTransmitPower(radio_index, &curr_txpower) != RETURN_OK) { | ||
| wifi_util_error_print(WIFI_DB,"%s:%d: Failed to fetch TX power for radio_index=%d\n", | ||
| __func__, __LINE__, radio_index); | ||
| curr_txpower = 100; | ||
| } | ||
| cfg.transmitPower = curr_txpower; | ||
| cfg.transmitPower = 100; | ||
| cfg.rtsThreshold = 2347; |
| translate_radio_capability_to_easymesh(wifi_prop, radio_index, radio_cap, oper_param); | ||
|
|
||
| // Populate ch_scan.op_classes[] for Channel Scan Capabilities. | ||
| fill_scan_cap_op_classes(&wifi_prop->radiocap[radio_index], oper_param, &radio_cap->ch_scan); |
| /* Skip op classes that contain no DFS channels. */ | ||
| bool has_dfs = false; | ||
| for (unsigned int ci = 0; ci < oc->num_channels; ci++) { | ||
| if (is_dfs_channel((unsigned int)oc->channels[ci])) { | ||
| has_dfs = true; |
| unsigned char count = 0; | ||
| for (unsigned int ci = 0; ci < oc->num_channels && count < EM_MAX_CHANNELS_IN_LIST; ci++) { | ||
| bool excluded = false; | ||
| if (oper_oc != NULL) { | ||
| unsigned int non_oper_count = oper_oc->numberOfNonOperChan < MAXNUMNONOPERABLECHANNELS | ||
| ? oper_oc->numberOfNonOperChan : MAXNUMNONOPERABLECHANNELS; | ||
| for (unsigned int ni = 0; ni < non_oper_count; ni++) { | ||
| if ((unsigned int)oc->channels[ci] == oper_oc->nonOperable[ni]) { | ||
| excluded = true; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| if (!excluded) { | ||
| scan_entry->channels.channel[count++] = oc->channels[ci]; | ||
| } |
Dependent PRs: