Revert RDKBDEV-2506: restore original AddressSource detection logic by veeraputhiran-thangavel · Pull Request #88 · rdkcentral/lan-manager-lite

veeraputhiran-thangavel · 2026-07-23T02:45:53Z

Summary

Reverts the RDKBDEV-2506 fix (RDKCOM-4714) which introduced static vs DHCP client differentiation via ARP neighbour table and dnsmasq.leases lookups. This is equivalent to the Gerrit stable2 revert patches (#871260, #876998).

Reason

The fix caused regression RDKB-66031: stale ARP entries for DHCP clients were misclassified as Static, causing excessive stale static host table entries and eventually hitting the MAX host limit. Multiple revert patches were raised on stable2 and 7.12s6_hub6 branches for the same reason.

Changes Reverted

`source/lm/lm_wrapper.c`

getAddressSource():

Removed char ipAddress[50] local variable
Removed getIPAddress(physAddress, ipAddress) call
Restored MAC-based comparison: !strcasecmp(physAddress, dhcpHost.phyAddr) (was IP-based: !strcasecmp(ipAddress, dhcpHost.ipAddr))
Removed if(rc == -1) → "Static" fallback block

getIPAddress():

Restored original single-strategy implementation (grep ip nei → /tmp/LMgetIP.txt)
Removed char buf[200] and #if 0 wrapper around the original code
Removed FIX START/END block with CASE1/CASE2/CASE3 neighbour table + dnsmasq logic

`source/lm/cosa_hosts_dml.c`

Removed AddressSource early-exit intercept in Host_GetParamStringValue (was calling getAddressSource() live and bypassing cache)
Removed IPAddress early-exit intercept (was calling getIPAddress() live)

Related Issues

RDKCOM-4714 — original fix
RDKB-66031 — regression caused by the fix
Gerrit reverts: #871260, #876998 (stable2), #877152 (7.12s6_hub6)

Reverts the RDKBDEV-2506 fix (RDKCOM-4714) which introduced static vs DHCP client differentiation via ARP neighbour table and dnsmasq.leases lookups. The fix caused a regression (RDKB-66031): stale ARP entries for DHCP clients were misclassified as Static, leading to excessive stale static entries in the host table and eventually hitting the MAX host limit. Multiple revert patches were raised on stable2 and 7.12s6_hub6 branches (Gerrit changes #871260, #876998, #877152) for the same reason. Changes reverted: - lm_wrapper.c: getAddressSource() restored to MAC-based comparison against dnsmasq leases (phyAddr). Removed ipAddress local variable, getIPAddress() call, and if(rc==-1) Static fallback block. getIPAddress() restored to original single-strategy implementation; removed FIX START/END block with CASE1/CASE2/CASE3 neighbour logic. - cosa_hosts_dml.c: removed AddressSource and IPAddress early-exit intercepts in Host_GetParamStringValue. Relates-to: RDKCOM-4714 Relates-to: RDKB-66031

github-actions · 2026-07-23T02:46:04Z

📋 PR Format Reminder

Title: Revert RDKBDEV-2506: restore original AddressSource detection logic — expected TICKET-123 : description
(Multiple tickets OK: RDKCOM-5492 RDKBDEV-3336 : ... | Include US ticket + subtask for user-stories)
Description missing:
- Reason for change
- Test Procedure
- Risks (Low / Medium / High)
- Priority (P0 / P1 / P2)

Expected:

TICKET-123 : brief description

Reason for change: why
Test Procedure: how to verify
Risks: Low / Medium / High
Priority: P0 / P1 / P2

Copilot

Pull request overview

Reverts the RDKCOM-4714/RDKBDEV-2506 changes to address-source/IP detection to restore the pre-fix behavior, addressing the RDKB-66031 regression where stale ARP entries caused DHCP clients to be misclassified and host table entries to accumulate.

Changes:

Restore MAC-based AddressSource detection in getAddressSource() and remove the IP-based lookup / fallback logic.
Restore the original getIPAddress() implementation (neighbor-table grep strategy) and remove the multi-case neighbour/dnsmasq logic added by the reverted fix.
Remove the live AddressSource / IPAddress intercept paths in Host_GetParamStringValue, returning to cached host values.

Reviewed changes

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

File	Description
source/lm/lm_wrapper.c	Reverts address source and IP detection logic back to the original behavior.
source/lm/cosa_hosts_dml.c	Removes live recomputation intercepts for `AddressSource` and `IPAddress` in host parameter reads.

Comments suppressed due to low confidence (1)

source/lm/lm_wrapper.c:1650

getIPAddress() copies the raw fgets() line (including trailing newline) into IPAddress. That newline propagates into host tables (e.g., Host_AddIPAddress() stores it verbatim), leading to incorrect comparisons/persistence. Also, writing to a predictable path in /tmp via shell redirection is unsafe (symlink/TOCTOU), and the unquoted %s in the shell pipeline risks command injection unless the MAC is strictly validated.

Consider validating physAddress (expected format xx:xx:xx:xx:xx:xx), reading the command output via popen() (no temp file), and stripping \r/\n before copying into IPAddress.

    v_secure_system("ip -4 nei show | grep brlan0 | grep -v 192.168.10 | grep -i %s | awk '{print $1}' | tail -1 > /tmp/LMgetIP.txt ", physAddress);

    fp = fopen ("/tmp/LMgetIP.txt", "r");

    if (fp != NULL)

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

The original RDKBDEV-2506 fix also modified lm_wrapper_get_dhcpv4_client() to use getIPAddress() for IP resolution instead of using dhcpHost.ipAddr directly. Revert this as part of the complete RDKBDEV-2506 revert. Restores: - Remove char ipAddress[50] local variable - Remove getIPAddress(dhcpHost.phyAddr, ipAddress) call - Restore Host_AddIPv4Address call with (char *)dhcpHost.ipAddr Relates-to: RDKCOM-4714 Relates-to: RDKB-66031

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

source/lm/lm_wrapper.c:1650

getIPAddress() uses a predictable /tmp/LMgetIP.txt temporary file via shell redirection. Because /tmp is typically world-writable, this pattern is vulnerable to symlink/file-clobber attacks and is unnecessary here. Prefer v_secure_popen() (already used in this file) to capture the command output directly without touching the filesystem.

    v_secure_system("ip -4 nei show | grep brlan0 | grep -v 192.168.10 | grep -i %s | awk '{print $1}' | tail -1 > /tmp/LMgetIP.txt ", physAddress);

    fp = fopen ("/tmp/LMgetIP.txt", "r");

    if (fp != NULL)

    int ret;
    LM_host_entry_t dhcpHost;
    errno_t rc = -1;
-    char ipAddress[50] = {0};
-    getIPAddress(physAddress , ipAddress);

    if ( (fp=fopen(DNSMASQ_LEASES_FILE, "r")) == NULL )
    {


These were added by RDKB-60569 to support the scoring logic inside the getIPAddress() FIX block introduced by RDKBDEV-2506. With the FIX block removed by the RDKBDEV-2506 revert, these symbols have no callers and would cause unused-variable/unused-function warnings (or errors under -Werror on RDKB platform builds). Removed: - NEIGH_SCORE_REACHABLE, NEIGH_SCORE_DELAY, NEIGH_SCORE_STALE defines - ARRAY_SIZE macro - struct neigh_state_score_map definition - static const neigh_score_map[] array - static int neigh_state_score() function Relates-to: RDKCOM-4714

Copilot

Pull request overview

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

@@ -1659,207 +1623,12 @@ int getIPAddress(char *physAddress,char *IPAddress)
    {
        while(fgets(output, sizeof(output), fp)!=NULL);
        fclose (fp);
-        fp=NULL
+        fp=NULL;
    }

    rc = STRCPY_S_NOCLOBBER(IPAddress, 50,output);
    ERR_CHK(rc);


veeraputhiran-thangavel requested review from a team as code owners July 23, 2026 02:45

Copilot AI review requested due to automatic review settings July 23, 2026 02:45

Copilot started reviewing on behalf of veeraputhiran-thangavel July 23, 2026 02:46 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 23, 2026 07:04

Copilot started reviewing on behalf of veeraputhiran-thangavel July 23, 2026 07:04 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Comment thread source/lm/lm_wrapper.c

Comment on lines 1501 to 1506

int ret;

LM_host_entry_t dhcpHost;

errno_t rc = -1;

char ipAddress[50] = {0};

getIPAddress(physAddress , ipAddress);

if ( (fp=fopen(DNSMASQ_LEASES_FILE, "r")) == NULL )

{

Copilot AI review requested due to automatic review settings July 23, 2026 07:36

Copilot started reviewing on behalf of veeraputhiran-thangavel July 23, 2026 07:36 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Comment thread source/lm/lm_wrapper.c

Comment on lines 1618 to 1630

@@ -1659,207 +1623,12 @@ int getIPAddress(char *physAddress,char *IPAddress)

{

while(fgets(output, sizeof(output), fp)!=NULL);

fclose (fp);

fp=NULL

fp=NULL;

}

rc = STRCPY_S_NOCLOBBER(IPAddress, 50,output);

ERR_CHK(rc);

Navigation Menu

Uh oh!

Revert RDKBDEV-2506: restore original AddressSource detection logic - #88

Revert RDKBDEV-2506: restore original AddressSource detection logic#88
veeraputhiran-thangavel wants to merge 3 commits into
developfrom
revert/RDKBDEV-2506-address-source-fix

veeraputhiran-thangavel commented Jul 23, 2026

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

veeraputhiran-thangavel commented Jul 23, 2026

Summary

Reason

Changes Reverted

source/lm/lm_wrapper.c

source/lm/cosa_hosts_dml.c

Related Issues

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

`source/lm/lm_wrapper.c`

`source/lm/cosa_hosts_dml.c`