Skip to content

RDKB-66065: RFC MLO Enable - #1315

Open
bmilcz-comcast wants to merge 1 commit into
rdkcentral:developfrom
bmilcz-comcast:rdkb-66065
Open

RDKB-66065: RFC MLO Enable#1315
bmilcz-comcast wants to merge 1 commit into
rdkcentral:developfrom
bmilcz-comcast:rdkb-66065

Conversation

@bmilcz-comcast

Copy link
Copy Markdown
Contributor

Reason for change: Send notification to telemetry whenever MLO is completely disabled on device.
Test Procedure: Enable/disable MLO, check if appropriate messages are being sent.
Risks: Low
Priority: P1

Copilot AI review requested due to automatic review settings July 31, 2026 15:59
@bmilcz-comcast
bmilcz-comcast requested a review from a team as a code owner July 31, 2026 15:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a mechanism to notify telemetry (via bus-set TR-181 report parameters) whenever the device’s effective MLO/RFC enable state changes, with retry handling to tolerate transient bus failures.

Changes:

  • Track last-known MLO RFC enable state and notification retry/pending status in wifi_ctrl_t.
  • Compute whether MLO is effectively enabled by scanning VAP MLD configuration and schedule bus updates (with retries) at boot and after relevant webconfig updates.
  • Introduce TR-181 parameter name macros for the two telemetry/report paths being updated.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
source/core/wifi_ctrl.h Adds fields to track last MLO RFC state and notification retry/pending bookkeeping.
source/core/wifi_ctrl.c Implements MLO RFC state evaluation and bus notification scheduling/retry logic; triggers on boot and after MLD group updates.
include/wifi_base.h Adds TR-181 parameter string macros used for telemetry notifications.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread source/core/wifi_ctrl.c Outdated
Comment thread source/core/wifi_ctrl.c Outdated
Comment thread source/core/wifi_ctrl.c Outdated
@bmilcz-comcast
bmilcz-comcast marked this pull request as draft August 3, 2026 14:44
@bmilcz-comcast
bmilcz-comcast force-pushed the rdkb-66065 branch 5 times, most recently from f4dc807 to cc56574 Compare August 5, 2026 10:16
@bmilcz-comcast
bmilcz-comcast marked this pull request as ready for review August 5, 2026 10:19
Copilot AI review requested due to automatic review settings August 5, 2026 10:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

source/core/wifi_ctrl.c:3393

  • update_rfc_mlo_enable() treats any VAP with mld_id < MLD_UNIT_COUNT as “MLO enabled”, but elsewhere (e.g., update_mld_groups) MLO can be disabled by setting mld_enable = false while leaving mld_id in-range. This can cause false positives (reporting MLO enabled when it’s actually disabled). Gate on mld_enable (and typically skip disabled VAPs) and ignore UNDEFINED_MLD_ID.
            if (isVapSTAMesh(vap->vap_index)) {
                continue;
            }

            if (vap->u.bss_info.mld_info.common_info.mld_id < MLD_UNIT_COUNT) {
                mlo_rfc_enable = true;
                break;

Comment thread source/core/wifi_ctrl.c Outdated
Copilot AI review requested due to automatic review settings August 5, 2026 10:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (3)

source/core/wifi_ctrl.c:48

  • set_bus_bool_param() is only used within this translation unit, but it’s declared/defined with external linkage. That increases the chance of duplicate-symbol collisions across the codebase. Consider making it static (and updating both the forward declaration and the definition) or moving the definition above its first use to avoid the prototype entirely.
int set_bus_bool_param(bus_handle_t *handle, const char *paramNames, bool data_value);

source/core/wifi_ctrl.c:3384

  • vap cannot be NULL here because it’s the address of an element in vap_array. This check is dead code and can mask real out-of-bounds issues by implying NULL is possible.
            wifi_vap_info_t *vap = &wifi_mgr->radio_config[i].vaps.vap_map.vap_array[j];
            if (vap == NULL) {
                continue;
            }

source/core/wifi_ctrl.c:3361

  • The doc comment has a grammatical error (“IF there was a RFC status differs…”). Clarifying this improves readability for future maintainers.
 * Update the MLO RFC enable status based on the current VAP configurations.
 * This function checks all radios and their VAPs to determine if any VAP has a
 * valid MLD ID and updates the corresponding MLO RFC enable status IF there was
 * a RFC status differs from previously recorded.

Reason for change: Send notification to telemetry whenever MLO is
completely disabled on device.
Test Procedure: Enable/disable MLO, check if appropriate messages are
being sent.
Risks: Low
Priority: P1

Co-authored-by: Brayan Milczarek <brayan.milczarek@comcast.com>
Signed-off-by: Brayan Milczarek <brayan_milczarek@comcast.com>
Copilot AI review requested due to automatic review settings August 5, 2026 10:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

source/core/wifi_ctrl.c:3390

  • update_rfc_mlo_enable() treats any mld_id < MLD_UNIT_COUNT as “MLO enabled”. Elsewhere the codebase consistently gates MLD-related behavior on mld_enable and mld_id != UNDEFINED_MLD_ID (e.g., source/webconfig/wifi_ovsdb_translator.c:2845 and :3072). Aligning this check avoids incorrectly reporting MLO enabled if mld_id is ever left at a default/inconsistent value while mld_enable is false.
            if (vap->u.bss_info.mld_info.common_info.mld_id < MLD_UNIT_COUNT) {
                mlo_rfc_enable = true;
                break;
            }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants