Skip to content

RDKB-62636 : Stop running DHCP client before starting a new one via DHCP Manager #191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
S-Parthiban-Selvaraj wants to merge 1 commit into
base: main
Choose a base branch
from
Open

RDKB-62636 : Stop running DHCP client before starting a new one via DHCP Manager #191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions source/WanManager/wanmgr_net_utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,56 @@ int WanManager_RestartDhcpv6Client(DML_VIRTUAL_IFACE* pVirtIf, IFACE_TYPE IfaceT
return 0;
}

#if defined( FEATURE_RDKB_DHCP_MANAGER )
/***************************************************************************
* @brief Check if a DHCP client (v4 or v6) is currently running via DHCP Manager
* and stop it before a new start. This avoids duplicate client instances.
* @param pVirtIf Pointer to the virtual interface structure
* @param isIPv4 TRUE to check DHCPv4 client, FALSE to check DHCPv6 client
* @return ANSC_STATUS_SUCCESS if the client was not running or was stopped
* successfully, ANSC_STATUS_FAILURE on error.
****************************************************************************/
ANSC_STATUS WanManager_StopDhcpClientIfRunning(DML_VIRTUAL_IFACE* pVirtIf, BOOL isIPv4)
{
if (pVirtIf == NULL)
{
CcspTraceError(("%s %d: Invalid args\n", __FUNCTION__, __LINE__));
return ANSC_STATUS_FAILURE;
}

char dmlStatus[256] = {0};
char statusValue[64] = {0};
const char *dhcpIface = isIPv4 ? pVirtIf->IP.DHCPv4Iface : pVirtIf->IP.DHCPv6Iface;

/* Build the Status DML path, e.g. Device.DHCPv4.Client.1.Status */
snprintf(dmlStatus, sizeof(dmlStatus), "%s.Status", dhcpIface);

if (ANSC_STATUS_SUCCESS == WanMgr_RdkBus_GetParamValues(DHCPMGR_COMPONENT_NAME, DHCPMGR_DBUS_PATH, dmlStatus, statusValue))
{
CcspTraceInfo(("%s %d: %s current status = %s\n", __FUNCTION__, __LINE__, dmlStatus, statusValue));
if (strncasecmp(statusValue, "Enabled", 7) == 0)
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

Using strncasecmp(..., \"Enabled\", 7) will treat values like \"EnabledXYZ\" as enabled. If DHCP Manager’s Status is expected to be an exact token (e.g., Enabled / Disabled), prefer an exact comparison (case-sensitive or case-insensitive as appropriate) or validate a full-token match (e.g., ensure the next character is string terminator/whitespace) to avoid false positives.

Copilot uses AI. Check for mistakes.
{
CcspTraceInfo(("%s %d: %s client is already running on %s, stopping it first\n",
__FUNCTION__, __LINE__, isIPv4 ? "DHCPv4" : "DHCPv6", pVirtIf->Name));
if (isIPv4)
{
WanManager_StopDhcpv4Client(pVirtIf, STOP_DHCP_WITHOUT_RELEASE);
}
else
{
WanManager_StopDhcpv6Client(pVirtIf, STOP_DHCP_WITHOUT_RELEASE);
}
}
}
else
{
CcspTraceWarning(("%s %d: Failed to get status for %s, proceeding with start\n", __FUNCTION__, __LINE__, dmlStatus));
}

return ANSC_STATUS_SUCCESS;
}
Comment on lines +494 to +532
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

This helper returns ANSC_STATUS_SUCCESS in all non-NULL cases and ignores the return values from WanManager_StopDhcpv4Client() / WanManager_StopDhcpv6Client(). That makes it impossible for callers to detect failures and also contradicts the function’s stated contract (failure on error). Consider (a) capturing and propagating the stop call’s return value when a stop is attempted, and (b) deciding whether a DHCP Manager status query failure should return ANSC_STATUS_FAILURE or updating the contract to explicitly describe the “best effort, always proceed” behavior.

Copilot uses AI. Check for mistakes.
#endif /* FEATURE_RDKB_DHCP_MANAGER */

int WanManager_StartDhcpv6Client(DML_VIRTUAL_IFACE* pVirtIf, IFACE_TYPE IfaceType)
{
if (pVirtIf == NULL)
Expand Down Expand Up @@ -514,6 +564,9 @@ int WanManager_StartDhcpv6Client(DML_VIRTUAL_IFACE* pVirtIf, IFACE_TYPE IfaceTyp
}

#if defined( FEATURE_RDKB_DHCP_MANAGER )
/* Stop DHCPv6 client if it is already running */
WanManager_StopDhcpClientIfRunning(pVirtIf, FALSE);

char dmlName[256] = {0};
WanMgr_SubscribeDhcpClientEvents(pVirtIf->IP.DHCPv6Iface);
snprintf( dmlName, sizeof(dmlName), "%s.Interface", pVirtIf->IP.DHCPv6Iface );
Expand Down Expand Up @@ -623,6 +676,9 @@ int WanManager_StartDhcpv4Client(DML_VIRTUAL_IFACE* pVirtIf, char* baseInterface
return 0;
}
#if defined( FEATURE_RDKB_DHCP_MANAGER )
/* Stop DHCPv4 client if it is already running */
WanManager_StopDhcpClientIfRunning(pVirtIf, TRUE);

char dmlName[256] = {0};
WanMgr_SubscribeDhcpClientEvents(pVirtIf->IP.DHCPv4Iface);
snprintf( dmlName, sizeof(dmlName), "%s.Interface", pVirtIf->IP.DHCPv4Iface );
Expand Down
12 changes: 12 additions & 0 deletions source/WanManager/wanmgr_net_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,18 @@ int WanManager_StartDhcpv4Client(DML_VIRTUAL_IFACE* pVirtIf, char* baseInterface
***************************************************************************/
ANSC_STATUS WanManager_StopDhcpv4Client(DML_VIRTUAL_IFACE* pVirtIf, DHCP_RELEASE_BEHAVIOUR IsReleaseNeeded);

#if defined( FEATURE_RDKB_DHCP_MANAGER )
/***************************************************************************
* @brief Check if a DHCP client (v4 or v6) is currently running via DHCP Manager
* and stop it before a new start. This avoids duplicate client instances.
* @param pVirtIf Pointer to the virtual interface structure
* @param isIPv4 TRUE to check DHCPv4 client, FALSE to check DHCPv6 client
* @return ANSC_STATUS_SUCCESS if the client was not running or was stopped
* successfully, ANSC_STATUS_FAILURE on error.
Comment on lines +145 to +146
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The header comment states the function returns ANSC_STATUS_FAILURE on error, but the implementation logs a warning when the status query fails and still returns success (and also doesn’t surface stop failures). Either align the implementation to return failure for those error conditions, or update this docstring to reflect the intended best-effort semantics (e.g., always returns success unless arguments are invalid).

Suggested change
* @return ANSC_STATUS_SUCCESS if the client was not running or was stopped
* successfully, ANSC_STATUS_FAILURE on error.
* @return ANSC_STATUS_SUCCESS on best-effort completion, including when no
* client was running or when stopping the client fails internally.
* ANSC_STATUS_FAILURE is only returned if the input arguments are
* invalid.

Copilot uses AI. Check for mistakes.
****************************************************************************/
ANSC_STATUS WanManager_StopDhcpClientIfRunning(DML_VIRTUAL_IFACE* pVirtIf, BOOL isIPv4);
#endif /* FEATURE_RDKB_DHCP_MANAGER */

/***************************************************************************
* @brief API used to restart Dhcpv6 client application.
* @param ifName_info interface name
Expand Down