RDKB-65825: RFC Enabled By Default - #1239
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates OneWiFi’s default configuration so certain fully deployed RFC features are enabled by default (notably WiFi Interworking, Passpoint, and 2G 802.11ax), and aligns related database defaults/schema versioning to persist those defaults.
Changes:
- Enable default RFC values for WiFi Interworking and Passpoint when PSM values are absent.
- Change DB-side RFC defaults to enable Interworking/Passpoint and make 2G 802.11ax enabled by default (including an upgrade/override path).
- Update WHIX log interval default and bump the OVSDB schema version.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| source/dml/tr_181/ml/cosa_wifi_internal.c | Changes default Interworking/Passpoint RFC values from disabled to enabled when PSM keys are missing. |
| source/db/wifi_db_apis.c | Updates RFC defaults, WHIX log interval default, and adds RFC override/upgrade helper logic for 2G 11ax enablement. |
| config/rdkb-wifi.ovsschema | Bumps OVSDB schema version to 1.00.052. |
Comments suppressed due to low confidence (1)
source/db/wifi_db_apis.c:4914
- The WhixLoginterval upgrade logic is no longer gated by db_version, so every call to wifidb_global_config_upgrade() will re-read from PSM (or reset to DEFAULT_WHIX_LOGINTERVAL) and can overwrite the DB value repeatedly. This breaks the one-time "upgrade" contract used by the other upgrade blocks below.
memset(strValue, 0, sizeof(strValue));
str = (char *)p_ccsp_desc->psm_get_value_fn(WhixLoginterval, strValue, sizeof(strValue));
if (str != NULL) {
g_wifidb->global_config.global_parameters.whix_log_interval = atoi(str);
wifi_util_dbg_print(WIFI_DB, "whix_log_interval is %d and str is %s \n",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reason for change: RFC Enabled By Default on fully deployed RFCs Test Procedure: Below parameters should be set to respective default value Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.WifiClient.ActiveMeasurements.Enable Device.DeviceInfo.X_RDKCENTRAL-COM_WIFI_TELEMETRY.LogInterval Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.WiFi-Interworking.Enable Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.WiFi-Passpoint.Enable Device.WiFi.2G80211axEnable Risks: None Signed-off-by: [email protected]
e0e4309 to
8ba58fd
Compare
|
@navyasher please address the copilot comments. Thanks. |
mateuszCieslak-GL
left a comment
There was a problem hiding this comment.
please address copilot comments;
🔎 clang-tidy (advisory)🔎 clang-tidy: clean on changed files |
🔨 Build summaryCommit Banana Pi R4 - MLO — build success
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (3)
source/db/wifi_db_apis.c:6429
wifidb_overide_rfc_config()always setsmodified = truefordb_version < ONEWIFI_DB_VERSION_2G11AXENABLE_RFC_FLAG, even whentwoG80211axEnable_rfcis alreadytrue. With current schema db-version (100052) this results in an unnecessary RFC table write (and log spam) on every initialization.
Only mark modified (and print) when the value actually changes, and avoid running the override when db_version is 0 (unknown).
if (g_wifidb->db_version < ONEWIFI_DB_VERSION_2G11AXENABLE_RFC_FLAG) {
wifi_util_info_print(WIFI_DB, "%s:%d Overriding twoG80211axEnable_rfc=true\n", __func__, __LINE__);
rfc_param->twoG80211axEnable_rfc = true;
modified = true;
}
source/db/wifi_db_apis.c:4915
- In
wifidb_global_config_upgrade(), the WhixLoginterval sync block has leftover indentation and a misaligned closing brace after removing the surrounding DB-version gate. This makes the control flow hard to read and easy to mis-edit (it currently looks like it’s still nested under a missingif).
Re-indent this block so the if/else is clearly scoped.
memset(strValue, 0, sizeof(strValue));
str = (char *)p_ccsp_desc->psm_get_value_fn(WhixLoginterval, strValue, sizeof(strValue));
if (str != NULL) {
g_wifidb->global_config.global_parameters.whix_log_interval = atoi(str);
wifi_util_dbg_print(WIFI_DB, "whix_log_interval is %d and str is %s \n",
source/db/wifi_db_apis.c:99
ONEWIFI_DB_VERSION_2G11AXENABLE_RFC_FLAGis set to 100053, but the current OVSDB schema version is still 1.00.052 (db-version 100052). Sincedb_versioncomes fromovsdb-tool db-version, this makes the new override logic treat all existing DBs as "older than" the flag indefinitely, so the migration never becomes a one-time upgrade.
Either bump the schema version to 1.00.053 (and keep checksum in sync) so db-version can reach 100053, or implement a different persistent migration marker so the override stops running after it has been applied once.
#define ONEWIFI_DB_VERSION_HOSTAP_MGMT_FRAME_CTRL_NEW_FLAG 100051
#define ONEWIFI_DB_VERSION_2G11AXENABLE_RFC_FLAG 100053
#define ONEWIFI_DB_VERSION_MLD_LINK_ID_FLAG 100052
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
source/db/wifi_db_apis.c:6432
wifidb_overide_rfc_config()applies the override wheng_wifidb->db_versionis0(unknown/unread), because0 < ONEWIFI_DB_VERSION_2G11AXENABLE_RFC_FLAG. Other upgrade paths in this file treatdb_version == 0as a signal to skip upgrade/migration logic; applying this override when the version is unknown can unintentionally flip a user-configuredtwoG80211axEnable_rfcvalue.
static bool wifidb_overide_rfc_config(wifi_rfc_dml_parameters_t *rfc_param)
{
wifi_mgr_t *g_wifidb = get_wifimgr_obj();
bool modified = false;
if (g_wifidb->db_version < ONEWIFI_DB_VERSION_2G11AXENABLE_RFC_FLAG) {
wifi_util_info_print(WIFI_DB, "%s:%d Overriding twoG80211axEnable_rfc=true\n", __func__, __LINE__);
rfc_param->twoG80211axEnable_rfc = true;
modified = true;
}
Review comments already addressed
Reason for change: RFC Enabled By Default on fully deployed RFCs Test Procedure: Below parameters should be set to respective default value Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.WifiClient.ActiveMeasurements.Enable Device.DeviceInfo.X_RDKCENTRAL-COM_WIFI_TELEMETRY.LogInterval Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.WiFi-Interworking.Enable Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.WiFi-Passpoint.Enable Device.WiFi.2G80211axEnable Risks: None Signed-off-by: [email protected]
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
source/db/wifi_db_apis.c:6389
wifidb_overide_rfc_config()writes to the shared RFC parameter cache without takingdata_cache_lock. Other writers in this file (e.g.,callback_Wifi_Rfc_Config) mutate the same struct under the mutex, so this introduces a potential data race during init (especially sincedbwrittenis set true before this call).
cfg.tcm_secure_2g_rfc = rfc_param->tcm_secure_2g_rfc;
cfg.tcm_secure_5g_rfc = rfc_param->tcm_secure_5g_rfc;
cfg.tcm_secure_6g_rfc = rfc_param->tcm_secure_6g_rfc;
cfg.wpa3_compatibility_enable = rfc_param->wpa3_compatibility_enable;
cfg.csi_analytics_enabled_rfc = rfc_param->csi_analytics_enabled_rfc;
source/db/wifi_db_apis.c:4920
- In
wifidb_global_config_upgrade(), thewhix_log_intervalblock is now over-indented and its closing brace is misaligned, making brace matching and future edits error-prone (this looks like leftover indentation after removing the outer version gate). Re-indent this block consistently within the function.
g_wifidb->global_config.global_parameters.whix_log_interval, str);
} else {
g_wifidb->global_config.global_parameters.whix_log_interval = DEFAULT_WHIX_LOGINTERVAL;
wifi_util_error_print(WIFI_DB, ":%s:%d str value for whix_log_interval is null \r\n",
__func__, __LINE__);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
source/db/wifi_db_apis.c:4915
- In
wifidb_global_config_upgrade(), thewhix_log_intervalPSM sync block is indented as if it’s still inside a removed outerifblock, and the closing brace is also mis-indented. This makes the control flow harder to read and easy to misinterpret during future edits.
memset(strValue, 0, sizeof(strValue));
str = (char *)p_ccsp_desc->psm_get_value_fn(WhixLoginterval, strValue, sizeof(strValue));
if (str != NULL) {
g_wifidb->global_config.global_parameters.whix_log_interval = atoi(str);
wifi_util_dbg_print(WIFI_DB, "whix_log_interval is %d and str is %s \n",
Reason for change: RFC Enabled By Default on fully deployed RFCs
Test Procedure: Below parameters should be set to respective default value Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.WifiClient.ActiveMeasurements.Enable Device.DeviceInfo.X_RDKCENTRAL-COM_WIFI_TELEMETRY.LogInterval Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.WiFi-Interworking.Enable Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.WiFi-Passpoint.Enable Device.WiFi.2G80211axEnable
Risks: None
Signed-off-by: [email protected]