RDKB-66284 : retry DHCPv6 start when link-local DAD is pending - #140
Conversation
Reason for change: On boot, DhcpMgr_checkLinkLocalAddress() returns FALSE when erouter0 link-local address DAD has not completed within the 5 s wait window. The previous code had only a comment with no actual retry logic, leaving dibbler permanently unstarted and IPv6 down for the entire device uptime (observed 3+ days on TCXB7/TCXB8 running 8.6p1s5, RDKB-66284). Fix: re-enqueue a Selfheal_ClientRestart message via DhcpMgr_OpenQueueEnsureThread so the per-interface controller retries dibbler startup once DAD completes. DhcpMgr_checkLinkLocalAddress blocks up to INTF_V6LL_TIMEOUT_IN_MSEC per call so the retry is naturally rate-limited without an explicit counter. Test Procedure: Only for test purpose Risks: Low Priority: P2 Signed-off-by: 64963550+aadhithan01@users.noreply.github.com
There was a problem hiding this comment.
Pull request overview
This PR addresses a boot-time failure mode where the DHCPv6 client (dibbler) can remain permanently stopped if the interface’s IPv6 link-local address is still in DAD/tentative state beyond the current wait window.
Changes:
- Adds retry behavior when
DhcpMgr_checkLinkLocalAddress()times out by re-enqueueing aSelfheal_ClientRestartcontrol message to the per-interface controller. - Improves inline documentation/logging around the link-local readiness gating for DHCPv6 client start.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Update comment to clarify that DhcpMgr_checkLinkLocalAddress() returns FALSE for any link-local timeout (tentative/DAD-pending or no address present), not only when DAD is in progress. Also document that the Linux kernel guarantees link-local resolution so the retry loop terminates. Signed-off-by: 64963550+aadhithan01@users.noreply.github.com
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
source/DHCPMgrUtils/dhcpmgr_controller.c:820
- The Selfheal_ClientRestart enqueue block here duplicates the same message construction used elsewhere (e.g., processKilled() in this file at dhcpmgr_controller.c:1218-1281). Duplicating this logic in multiple places makes future changes error-prone (e.g., if the message shape or ParamName changes). Consider factoring this into a small static helper (e.g., DhcpMgr_EnqueueSelfhealRestart(ifname, DML_DHCPV6)) and reusing it at all call sites.
dhcp_info_t retry_info;
memset(&retry_info, 0, sizeof(retry_info));
strncpy(retry_info.if_name, pDhcp6c->Cfg.Interface, MAX_STR_LEN - 1);
retry_info.if_name[MAX_STR_LEN - 1] = '\0';
retry_info.dhcpType = DML_DHCPV6;
strncpy(retry_info.ParamName, "Selfheal_ClientRestart", sizeof(retry_info.ParamName) - 1);
retry_info.ParamName[sizeof(retry_info.ParamName) - 1] = '\0';
retry_info.value.bValue = FALSE;
if (DhcpMgr_OpenQueueEnsureThread(retry_info) != 0)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (3)
source/DHCPMgrUtils/dhcpmgr_controller.c:805
- Same as above: this line references
checkLinkLocalAddress()but the actual function isDhcpMgr_checkLinkLocalAddress().
* checkLinkLocalAddress() blocks up to INTF_V6LL_TIMEOUT_IN_MSEC
* per call, so this retry is naturally rate-limited.
source/DHCPMgrUtils/dhcpmgr_controller.c:802
- In this comment block, the function name is referred to as
checkLinkLocalAddress(), but the actual function isDhcpMgr_checkLinkLocalAddress(). Keeping the exact name here will avoid confusion when grepping/debugging.
This issue also appears on line 804 of the same file.
* checkLinkLocalAddress() has already force-toggled disable_ipv6
* to regenerate the link-local address, but that completes
* asynchronously. Re-enqueue a restart so the client is started
* once the link-local address is ready, instead of leaving it
* permanently stopped (Info.Status stays Disabled and dibbler is
source/DHCPMgrUtils/dhcpmgr_controller.c:815
- Use
sizeof(retry_info.if_name)for the copy/terminator bounds instead ofMAX_STR_LENto keep this safe even if the struct field size changes independently of the macro.
strncpy(retry_info.if_name, pDhcp6c->Cfg.Interface, MAX_STR_LEN - 1);
retry_info.if_name[MAX_STR_LEN - 1] = '\0';
Reason for change: On boot,
DhcpMgr_checkLinkLocalAddress()returns FALSE when erouter0 link-local address DAD has not completed within the 5 s wait window. The previous code had only a comment//Link local failed. Retrywith no actual retry logic, leaving dibbler permanently unstarted and IPv6 down for the entire device uptime (observed 3+ days on TCXB7/TCXB8 running 8.6p1s5, RDKB-66284).Fix: re-enqueue a
Selfheal_ClientRestartmessage viaDhcpMgr_OpenQueueEnsureThreadso the per-interface controller retries dibbler startup once DAD completes.DhcpMgr_checkLinkLocalAddressblocks up toINTF_V6LL_TIMEOUT_IN_MSECper call, so the retry is naturally rate-limited without an explicit counter.Test Procedure: Only for test purpose
Risks: Low
Priority: P2
Signed-off-by: 64963550+aadhithan01@users.noreply.github.com