RDKB-63242 : [epon-manager development ] hal integration #5

S-Parthiban-Selvaraj · 2026-01-26T14:57:57Z

Pull request overview

This PR integrates the EPON HAL library as an external dependency and updates function signatures to use the proper epon_hal_return_t return type throughout the codebase.

Changes:

Updated HAL function return types from int to epon_hal_return_t for type safety
Added new epon_hal_deinit() function implementation in the mock HAL
Removed epon_hal.h from local headers and added external include path configuration
Added proper HAL deinitialization in controller cleanup sequence

…files

…t include paths in Makefiles

Updated all HAL API calls in eponMgr_data.c: epon_hal_init() epon_hal_get_link_stats() epon_hal_get_transceiver_stats() epon_hal_get_llid_info() epon_hal_get_interface_list() epon_hal_get_olt_info() epon_hal_get_manufacturer_info() epon_hal_get_link_info() epon_hal_set_oam_log_mask() dpoe_hal_get_cpe_mac_table() Updated function declarations in eponMgr_data.h: eponMgr_data_hal_init() - return type changed to epon_hal_return_t eponMgr_data_set_oam_log_level() - return type changed to epon_hal_return_t 2. HAL Callback Signature Updates Fixed alarm callback in eponMgr_controller.c line 110: Changed from epon_alarm_info_t *alarm_info to const epon_alarm_info_t *alarm_info 3. Added HAL Deinitialization Added epon_hal_deinit() call in eponMgr_controller.c:776 in the eponMgr_controller_destroy() function This ensures proper HAL cleanup when the controller exits 4. Mock HAL Updates (epon_hal_mock.c) Updated all mock function return types from int to epon_hal_return_t Added new epon_hal_deinit() mock implementation for testing

Copilot

Pull request overview

This PR integrates the EPON HAL library as an external dependency and updates function signatures to use the proper epon_hal_return_t return type throughout the codebase.

Changes:

Updated HAL function return types from int to epon_hal_return_t for type safety
Added new epon_hal_deinit() function implementation in the mock HAL
Removed epon_hal.h from local headers and added external include path configuration
Added proper HAL deinitialization in controller cleanup sequence

Reviewed changes

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

Show a summary per file

File	Description
tests/hal_mock/epon_hal_mock.c	Updated return types to `epon_hal_return_t` and added `epon_hal_deinit()` implementation
tests/hal_mock/Makefile.am	Added external EPON HAL include path
src/rbus/Makefile.am	Added external EPON HAL include path
src/core/stats_poller/Makefile.am	Added external EPON HAL include path
src/core/data_structures/eponMgr_data.h	Updated function signatures to return `epon_hal_return_t`
src/core/data_structures/eponMgr_data.c	Updated variable types and function signatures to use `epon_hal_return_t`
src/core/data_structures/Makefile.am	Added external EPON HAL include path
src/core/controller/eponMgr_controller.c	Updated callback signature and added HAL deinitialization call
src/core/controller/Makefile.am	Added external EPON HAL include path
src/core/Makefile.am	Added external EPON HAL include path
include/Makefile.am	Removed local `epon_hal.h` header file
design_docs/06_Configuration.md	Removed `ccsp-common-library` from dependencies list

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

tests/hal_mock/epon_hal_mock.c

src/core/controller/eponMgr_controller.c

…ndex and updating parameter paths

…eventExitStatus for Rogers partner

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

src/rbus/wanmanager_update/eponMgr_wanmanager_update.c:231

Casting uint32_t to int could lead to incorrect behavior if the number of entries exceeds INT_MAX. Consider using uint32_t for num_entries and updating the loop variable accordingly, or add validation to ensure the value is within acceptable range.

    int num_entries = (int)rbusValue_GetUInt32(count_value);

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

src/core/controller/eponMgr_controller.c

Copilot · 2026-01-27T11:33:35Z

src/rbus/wanmanager_update/eponMgr_wanmanager_update.c

+        return -1;
+    }
+
+    int num_entries = (int)rbusValue_GetUInt32(count_value);


Casting uint32_t to int could lead to incorrect behavior if the number of entries exceeds INT_MAX. Consider using uint32_t for num_entries and updating the loop variable accordingly, or add validation to ensure the value is within acceptable range.

…estartPreventExitStatus for Rogers partner" This reverts commit d6728c4.

…duction

Copilot

Pull request overview

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

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

…itialization

Copilot

Pull request overview

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

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

Copilot · 2026-02-09T11:38:09Z

src/core/controller/eponMgr_controller.c

+    if (sscanf(mac_str, "%02x:%02x:%02x:%02x:%02x:%02x",
+               (unsigned int*)&device_mac[0], (unsigned int*)&device_mac[1],
+               (unsigned int*)&device_mac[2], (unsigned int*)&device_mac[3],
+               (unsigned int*)&device_mac[4], (unsigned int*)&device_mac[5]) != 6) {
+        EPONMGR_LOG_WARN("Failed to parse MAC address from: %s\n", mac_str);
+        return -1;
+    }


sscanf is writing unsigned int values into uint8_t storage via casted pointers, which is undefined behavior and can corrupt the stack/locals (writes 4 bytes into 1). Parse into temporary unsigned int variables (or a unsigned int tmp[6]), validate range 0..255, then assign into device_mac[].

Suggested change

if (sscanf(mac_str, "%02x:%02x:%02x:%02x:%02x:%02x",

(unsigned int*)&device_mac[0], (unsigned int*)&device_mac[1],

(unsigned int*)&device_mac[2], (unsigned int*)&device_mac[3],

(unsigned int*)&device_mac[4], (unsigned int*)&device_mac[5]) != 6) {

EPONMGR_LOG_WARN("Failed to parse MAC address from: %s\n", mac_str);

return -1;

}

unsigned int mac_tmp[6];

int i;

if (sscanf(mac_str, "%02x:%02x:%02x:%02x:%02x:%02x",

&mac_tmp[0], &mac_tmp[1],

&mac_tmp[2], &mac_tmp[3],

&mac_tmp[4], &mac_tmp[5]) != 6) {

EPONMGR_LOG_WARN("Failed to parse MAC address from: %s\n", mac_str);

return -1;

}

for (i = 0; i < 6; ++i) {

if (mac_tmp[i] > 0xFFU) {

EPONMGR_LOG_WARN("Parsed MAC octet out of range in: %s\n", mac_str);

return -1;

}

device_mac[i] = (uint8_t)mac_tmp[i];

}

Copilot · 2026-02-09T11:38:09Z

src/core/controller/eponMgr_controller.c

+    int ret = pclose(fp_create);
+    if (ret != 0) {
+        if (strlen(output) > 0) {
+            EPONMGR_LOG_ERROR("Failed to create MACVLAN interface %s (exit=%d): %s\n", 
+                            macvlan_name, ret, output);
+        } else {
+            EPONMGR_LOG_ERROR("Failed to create MACVLAN interface %s (exit=%d)\n", 
+                            macvlan_name, ret);
+        }
+        return -1;
+    }


pclose() returns a wait status (like waitpid), not a direct exit code. Logging ret as exit=%d is inaccurate, and ret != 0 doesn’t distinguish signal termination vs non-zero exit. Use WIFEXITED(ret)/WEXITSTATUS(ret) (and WIFSIGNALED/WTERMSIG) to compute a real exit status for both logic and logs. Same issue also applies to the later pclose(fp_up) handling.

Copilot · 2026-02-09T11:38:10Z

src/core/controller/eponMgr_controller.c

+    char cmd[256];
+    snprintf(cmd, sizeof(cmd), 
+             "ip link add link %s name %s address %02x:%02x:%02x:%02x:%02x:%02x type macvlan mode bridge 2>&1",
+             ifname, macvlan_name,
+             new_mac[0], new_mac[1], new_mac[2],
+             new_mac[3], new_mac[4], new_mac[5]);


Building a shell command with ifname/macvlan_name and executing it via popen() is command-injection prone if interface names are ever influenced by external inputs (or contain unexpected characters). Prefer a non-shell approach (netlink/libnl or ioctl-based configuration), or at minimum strictly validate ifname and macvlan_name against an allowlist character set (e.g., [A-Za-z0-9_.-]+) and execute via fork/execv with argv (no shell parsing).

Copilot · 2026-02-09T11:38:10Z

src/core/controller/eponMgr_controller.c

+
+    EPONMGR_LOG_DEBUG("Executing: %s\n", cmd);
+
+    FILE *fp_create = popen(cmd, "r");


Building a shell command with ifname/macvlan_name and executing it via popen() is command-injection prone if interface names are ever influenced by external inputs (or contain unexpected characters). Prefer a non-shell approach (netlink/libnl or ioctl-based configuration), or at minimum strictly validate ifname and macvlan_name against an allowlist character set (e.g., [A-Za-z0-9_.-]+) and execute via fork/execv with argv (no shell parsing).

Copilot · 2026-02-09T11:38:10Z

src/core/controller/eponMgr_controller.c

+    // Calculate MAC offset based on VEIP number
+    uint8_t mac_offset = veip_num + 2;
+
+    // Calculate new MAC address
+    uint8_t new_mac[6];
+    memcpy(new_mac, device_mac, 6);
+
+    // Subtract offset from last byte of MAC address
+    new_mac[5] -= mac_offset;


new_mac[5] -= mac_offset; can underflow and silently wrap (and it ignores borrow across bytes). If the intention is “device MAC minus offset”, implement proper 48-bit subtraction with borrow propagation (or convert MAC to a uint64_t, subtract, then convert back), and decide what to do when the subtraction would go below 00:00:00:00:00:00.

Copilot · 2026-02-09T11:38:10Z

src/rbus/wanmanager_update/eponMgr_wanmanager_update.c

+/* Cached WanManager interface index (0 = not yet discovered) */
+static int g_wanmanager_if_index = 0;


g_wanmanager_if_index is a shared global cache with unsynchronized reads/writes. If eponMgr_rbus_notify_wanmanager_phy_status() and eponMgr_rbus_update_virtual_interface() can run concurrently (e.g., different threads/events), this introduces a data race. Protect the discovery + cache assignment with a mutex (or use an atomic + “initialize once” pattern).

Copilot · 2026-02-09T11:38:11Z

src/rbus/wanmanager_update/eponMgr_wanmanager_update.c

+static int discover_wanmanager_interface_index(void) {
+    /* Return cached value if already discovered */
+    if (g_wanmanager_if_index > 0) {
+        return g_wanmanager_if_index;
+    }


g_wanmanager_if_index is a shared global cache with unsynchronized reads/writes. If eponMgr_rbus_notify_wanmanager_phy_status() and eponMgr_rbus_update_virtual_interface() can run concurrently (e.g., different threads/events), this introduces a data race. Protect the discovery + cache assignment with a mutex (or use an atomic + “initialize once” pattern).

Copilot · 2026-02-09T11:38:11Z

src/core/data_structures/eponMgr_data.c

+    EPONMGR_LOG_INFO("HAL configuration: status_cb=%p, interface_cb=%p, alarm_cb=%p, dpoe_supported=%d, config_size=%zu\n",
+                         eponData->hal_config.status_callback,
+                         eponData->hal_config.interface_status_callback,
+                         eponData->hal_config.alarm_callback,
+                         eponData->hal_config.dpoe_supported,
+                         sizeof(epon_hal_config_t));


%p is for void*, but these are function pointers. Passing function pointers to %p is not portable and can be undefined behavior on some platforms/ABIs. If you need to log callback addresses, log them via an integer type like uintptr_t with the appropriate format macro, or omit the addresses entirely.

Copilot · 2026-02-09T11:38:11Z

src/core/controller/eponMgr_controller.c

+ * @return 0 on success, -1 on failure
+ */
+static int create_veip_macvlan(eponMgr_controller_t *ctrl, const char *ifname, 
+                               char *macvlan_name, size_t macvlan_name_size) {


ctrl is not used in this function body; with -Wunused-parameter enabled (seen in src/core/Makefile.am), this will generate warnings and can break builds that treat warnings as errors. Either remove the parameter, or explicitly mark it unused (e.g., (void)ctrl;) if it’s intentionally reserved for future use.

Suggested change

char *macvlan_name, size_t macvlan_name_size) {

char *macvlan_name, size_t macvlan_name_size) {

(void)ctrl;

…rfaces" This reverts commit bd97d8c.

S-Parthiban-Selvaraj added 4 commits January 26, 2026 13:24

RDKB-63242 : [epon-manager development ] hal integration

9aa74ea

Add EPON HAL integration support by including EPON_HAL_CFLAGS in Make…

8d9b660

…files

Refactor EPON HAL integration by replacing EPON_HAL_CFLAGS with direc…

b002d1f

…t include paths in Makefiles

S-Parthiban-Selvaraj marked this pull request as ready for review January 26, 2026 14:58

Copilot AI review requested due to automatic review settings January 26, 2026 14:58

Remove deprecated epon_hal_old.h header file

b95b538

Copilot AI reviewed Jan 26, 2026

View reviewed changes

tests/hal_mock/epon_hal_mock.c Show resolved Hide resolved

src/core/controller/eponMgr_controller.c Show resolved Hide resolved

S-Parthiban-Selvaraj added 2 commits January 26, 2026 16:28

Enhance WanManager integration by dynamically discovering interface i…

4224c6f

…ndex and updating parameter paths

Enhance RDK EPON Manager service by adding ExecStartPre and RestartPr…

d6728c4

…eventExitStatus for Rogers partner

Copilot AI review requested due to automatic review settings January 27, 2026 11:31

Copilot AI reviewed Jan 27, 2026

View reviewed changes

S-Parthiban-Selvaraj added 2 commits January 27, 2026 16:05

Revert "Enhance RDK EPON Manager service by adding ExecStartPre and R…

97cb270

…estartPreventExitStatus for Rogers partner" This reverts commit d6728c4.

Update Makefile to conditionally link HAL library for testing and pro…

af87cc9

…duction

Copilot AI review requested due to automatic review settings January 28, 2026 15:07

Copilot AI reviewed Jan 28, 2026

View reviewed changes

S-Parthiban-Selvaraj added 2 commits January 30, 2026 13:46

Fix HAL library linkage in Makefile and enhance logging during HAL in…

498b6ef

…itialization

Add MAC address management and MACVLAN creation for VEIP interfaces

bd97d8c

Copilot AI review requested due to automatic review settings February 9, 2026 11:35

Copilot AI reviewed Feb 9, 2026

View reviewed changes

Revert "Add MAC address management and MACVLAN creation for VEIP inte…

2639fff

…rfaces" This reverts commit bd97d8c.


		EPONMGR_LOG_DEBUG("Executing: %s\n", cmd);

		FILE *fp_create = popen(cmd, "r");

		/* Cached WanManager interface index (0 = not yet discovered) */
		static int g_wanmanager_if_index = 0;

	char *macvlan_name, size_t macvlan_name_size) {
	char *macvlan_name, size_t macvlan_name_size) {
	(void)ctrl;

RDKB-63242 : [epon-manager development ] hal integration #5

Are you sure you want to change the base?

RDKB-63242 : [epon-manager development ] hal integration #5

Uh oh!

Conversation

S-Parthiban-Selvaraj commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

S-Parthiban-Selvaraj commented Jan 26, 2026 •

edited

Loading