RDKB-63098: Handle IPv6 delegation for business vs residential Partner ID as part of single build#68
RDKB-63098: Handle IPv6 delegation for business vs residential Partner ID as part of single build#68
Conversation
Reason for change: Added One Stack MACRO handling Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <[email protected]>
Reason for change: Added One Stack MACRO handling Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <[email protected]>
There was a problem hiding this comment.
Pull request overview
Adds OneStack-specific conditional behavior intended to support IPv6 prefix delegation enablement/disablement within a single build (business vs residential differentiation).
Changes:
- Introduces a OneStack “feature support” helper and uses it to gate prefix-delegation-specific behavior.
- Extends several
CISCO_CONFIG_DHCPV6_PREFIX_DELEGATIONcompile-time branches to also compile under_ONESTACK_PRODUCT_REQ_. - Updates DHCPv6 pool/interface handling across TR-181 and DHCPv6 server utilities to switch between
IAInterfacevsInterfaceand PD vs non-PD flows.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| source/TR-181/middle_layer_src/cosa_dhcpv6_dml.c | Adds OneStack feature helper and uses it to choose PD vs non-PD behavior in pool string getters. |
| source/TR-181/include/cosa_dhcpv6_apis.h | Expands compile-time guards to include _ONESTACK_PRODUCT_REQ_ for sysevent naming and PD API exposure. |
| source/TR-181/board_sbapi/cosa_dhcpv6_apis.c | Adds OneStack runtime gating across utopia/syscfg handling and DHCPv6 server/client control paths. |
| source/DHCPServerUtils/DHCPv6Server/service_ipv6.c | Adds OneStack runtime gating when reading interface configuration and selecting active LAN interfaces. |
| source/DHCPServerUtils/DHCPv6Server/dhcpv6server_function.c | Expands build-time conditions for OneStack and adds runtime gating in PD-specific config generation paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | ||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| #endif |
There was a problem hiding this comment.
OneStack-specific code calls isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION), but this file doesn’t declare/define that symbol or the feature macro. Unless provided by an included header, _ONESTACK_PRODUCT_REQ_ builds will fail to compile; consider adding a shared feature header and including it here.
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | ||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| #endif | ||
| { | ||
| DHCPV6S_SYSCFG_GETS(DHCPV6S_NAME, "pool", cfg->index, "", 0, "IAInterface", iface_name); | ||
| #else | ||
| } | ||
| #endif | ||
| #if !defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | ||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| if (!isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| #endif | ||
| { | ||
| DHCPV6S_SYSCFG_GETS(DHCPV6S_NAME, "pool", cfg->index, "", 0, "Interface", iface_name); | ||
| } |
There was a problem hiding this comment.
The new OneStack runtime gating calls isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION), but neither symbol is defined in this file. _ONESTACK_PRODUCT_REQ_ builds will fail to compile unless a shared header is added and included here (the helper is currently only file-local in cosa_dhcpv6_dml.c).
| #if !defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | ||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| if (!isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| #endif | ||
| { |
There was a problem hiding this comment.
In get_active_lanif, the #if !defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) block opens a { (line 660) but the corresponding logic and closing brace are placed in the #else branch. This produces unmatched braces / incorrect compilation depending on macros. Please restructure so the runtime if/else (OneStack) wraps both PD and non-PD implementations within the same compiled branch.
| /* ===== OneStack Feature Support Patch ===== */ | ||
|
|
||
| #ifdef _ONESTACK_PRODUCT_REQ_ | ||
|
|
||
| #ifndef FEATURE_IPV6_DELEGATION | ||
| #define FEATURE_IPV6_DELEGATION 1 | ||
| #endif | ||
|
|
||
| /* Dummy runtime feature check — always enabled */ | ||
| static inline bool isFeatureSupportedInCurrentMode(int feature) | ||
| { | ||
| (void)feature; | ||
| return true; | ||
| } |
There was a problem hiding this comment.
The OneStack feature helper is implemented as a file-local static inline and hardcoded to always return true, so OneStack builds can never disable IPv6 delegation (and other translation units can’t reuse the helper). Consider moving the feature flag + helper to a shared header/source with a real runtime check (e.g., partner ID / mode) so business vs residential can be selected at runtime.
| /* Dummy runtime feature check — always enabled */ | ||
| static inline bool isFeatureSupportedInCurrentMode(int feature) | ||
| { |
There was a problem hiding this comment.
isFeatureSupportedInCurrentMode returns bool, but this file does not include <stdbool.h> and doesn’t otherwise define bool locally. If ansc_platform.h doesn’t provide bool, OneStack builds will fail to compile; consider including <stdbool.h> here or using the project’s BOOL type consistently.
| #include "cosa_common_util.h" | ||
|
|
||
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && defined(_COSA_BCM_MIPS_) | ||
| #if defined((CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && defined(_COSA_BCM_MIPS_)) || defined(_ONESTACK_PRODUCT_REQ_) |
There was a problem hiding this comment.
The preprocessor expression uses defined((... && defined(...))), which is invalid: defined only accepts a single macro identifier. This will not compile. Use (defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && defined(_COSA_BCM_MIPS_)) || defined(_ONESTACK_PRODUCT_REQ_) instead.
| #if defined((CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && defined(_COSA_BCM_MIPS_)) || defined(_ONESTACK_PRODUCT_REQ_) | |
| #if (defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && defined(_COSA_BCM_MIPS_)) || defined(_ONESTACK_PRODUCT_REQ_) |
| return CosaDmlDhcpv6sGetIAPDPrefixes2(&pPool->Cfg, pValue, pUlSize); | ||
| #endif |
There was a problem hiding this comment.
prefixdelegation_enabled is only set inside the ParamName == "Interface" branch, but it is later used in the "IAPDPrefixes" branch. When the getter is called for IAPDPrefixes, the flag will still be 0 and the PD path will never execute. Recompute the flag in the IAPDPrefixes branch (or factor into a helper) before using it.
| return CosaDmlDhcpv6sGetIAPDPrefixes2(&pPool->Cfg, pValue, pUlSize); | |
| #endif | |
| BOOL prefixdelegation_enabled = FALSE; | |
| #ifdef CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION | |
| prefixdelegation_enabled = TRUE; | |
| #endif | |
| #ifdef _ONESTACK_PRODUCT_REQ_ | |
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | |
| { | |
| prefixdelegation_enabled = TRUE; | |
| } | |
| #endif | |
| if (prefixdelegation_enabled) | |
| { | |
| return CosaDmlDhcpv6sGetIAPDPrefixes2(&pPool->Cfg, pValue, pUlSize); | |
| } | |
| #endif |
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | ||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| #endif | ||
| { |
There was a problem hiding this comment.
These new OneStack runtime checks call isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION), but neither the function nor the FEATURE_IPV6_DELEGATION macro are declared/defined in this translation unit (they currently only exist as a file-local helper in cosa_dhcpv6_dml.c). This will fail to compile for _ONESTACK_PRODUCT_REQ_; move the helper/macro to a shared header/source and include it here.
| #ifdef _ONESTACK_PRODUCT_REQ_ | ||
| if (!isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| { | ||
| return 0; /* behave as non-PD build */ |
There was a problem hiding this comment.
In the OneStack-disabled path this function returns 0 without writing to pValue or updating *pSize, which can leave callers with stale/garbage output. If the intent is “non-PD build behavior”, either populate an empty string (and size=1) or return the same “buffer too small / success” semantics as the non-PD path so callers can handle it deterministically.
| return 0; /* behave as non-PD build */ | |
| /* behave as non-PD build: no prefixes, return empty string with | |
| * the same buffer-size semantics as the non-PD path | |
| */ | |
| if (pSize == NULL || pValue == NULL) | |
| { | |
| /* Conservatively indicate failure if outputs are invalid */ | |
| return 1; | |
| } | |
| if (*pSize < 1) | |
| { | |
| *pSize = 1; | |
| return 1; /* buffer too small */ | |
| } | |
| pValue[0] = '\0'; | |
| return 0; |
| #if defined ((CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && ! defined(DHCPV6_PREFIX_FIX)) || defined(_ONESTACK_PRODUCT_REQ_) | ||
| UNREFERENCED_PARAMETER(OnlyTrigger); | ||
| ifl_set_event("dhcpv6_server-restart", ""); | ||
| #else |
There was a problem hiding this comment.
Same issue here: defined ((CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && ! defined(DHCPV6_PREFIX_FIX)) is not valid preprocessor syntax and will fail compilation. Wrap the expression outside of defined, e.g. (defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && !defined(DHCPV6_PREFIX_FIX)) || defined(_ONESTACK_PRODUCT_REQ_).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if defined ((CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && ! defined(DHCPV6_PREFIX_FIX)) || defined(_ONESTACK_PRODUCT_REQ_) | ||
| UNREFERENCED_PARAMETER(OnlyTrigger); | ||
| ifl_set_event("dhcpv6_server-restart", ""); | ||
| #else |
There was a problem hiding this comment.
This #if is not valid: defined ((CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && ! defined(DHCPV6_PREFIX_FIX)) passes an expression to defined, which will not compile. Use normal boolean composition instead, e.g. #if (defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && !defined(DHCPV6_PREFIX_FIX)) || defined(_ONESTACK_PRODUCT_REQ_).
| #if !defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | ||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| if (!isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| #endif | ||
| { | ||
| #else | ||
| /* Get active bridge count from PSM */ |
There was a problem hiding this comment.
The #if ... #else structure around the non-PD path is inverted/broken: the PSM-based logic is currently under the #else of #if !defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_), which means it will be compiled only when CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION is defined and ONESTACK is not. For non-PD builds (and ONESTACK builds), this instead compiles an empty block, leaving *num unset and skipping the intended PSM path. Restructure this to match the earlier pattern in get_dhcpv6s_pool_cfg: #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) (with an ONESTACK runtime if) for the PD syscfg path, and a single #else for the PSM path.
| libCcspDhcpMgr_board_sbapi_la_LDFLAGS = -lccsp_common -lhal_moca -lulog -lsyscfg -lsysevent -ltime_conversion -llmapi -lwebconfig_framework -lmsgpackc -lcm_mgnt -lnanomsg | ||
| if ONESTACK_PRODUCT_REQ | ||
| libCcspDhcpMgr_board_sbapi_la_LDFLAGS += -ldevicemode -lonestack_syscfg -lonestack_log -lrdkb_feature_mode_gate | ||
| endif |
There was a problem hiding this comment.
if ONESTACK_PRODUCT_REQ introduces an Automake conditional that is not defined in this repository’s configure.ac/m4 macros. This will cause automake to fail. Define AM_CONDITIONAL([ONESTACK_PRODUCT_REQ], ...) (and the option/define that controls it) or switch to an existing conditional.
| libCcspDhcpMgr_board_sbapi_la_LDFLAGS = -lccsp_common -lhal_moca -lulog -lsyscfg -lsysevent -ltime_conversion -llmapi -lwebconfig_framework -lmsgpackc -lcm_mgnt -lnanomsg | |
| if ONESTACK_PRODUCT_REQ | |
| libCcspDhcpMgr_board_sbapi_la_LDFLAGS += -ldevicemode -lonestack_syscfg -lonestack_log -lrdkb_feature_mode_gate | |
| endif | |
| libCcspDhcpMgr_board_sbapi_la_LDFLAGS = -lccsp_common -lhal_moca -lulog -lsyscfg -lsysevent -ltime_conversion -llmapi -lwebconfig_framework -lmsgpackc -lcm_mgnt -lnanomsg -ldevicemode -lonestack_syscfg -lonestack_log -lrdkb_feature_mode_gate |
| if (prefixdelegation_enabled) | ||
| { | ||
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | ||
| return CosaDmlDhcpv6sGetIAPDPrefixes2(&pPool->Cfg, pValue, pUlSize); | ||
| #endif | ||
| } | ||
|
|
There was a problem hiding this comment.
prefixdelegation_enabled is only set inside the ParamName == "Interface" branch, but it is later used in the "IAPDPrefixes" branch as well. As written, requests for IAPDPrefixes will always see prefixdelegation_enabled == 0 and will never call CosaDmlDhcpv6sGetIAPDPrefixes2, even when IPv6 delegation is enabled. Compute the delegation-enabled flag once before the per-parameter strcmp checks (or recompute it in the IAPDPrefixes block) so both parameters use the correct mode.
| if (prefixdelegation_enabled) | |
| { | |
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | |
| return CosaDmlDhcpv6sGetIAPDPrefixes2(&pPool->Cfg, pValue, pUlSize); | |
| #endif | |
| } | |
| int prefixdelegation_enabled_local = 0; | |
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | |
| /* | |
| * When prefix delegation support is compiled in, use the | |
| * delegation-aware API to retrieve IAPD prefixes. | |
| */ | |
| prefixdelegation_enabled_local = 1; | |
| #endif | |
| if (prefixdelegation_enabled_local) | |
| { | |
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | |
| return CosaDmlDhcpv6sGetIAPDPrefixes2(&pPool->Cfg, pValue, pUlSize); | |
| #endif | |
| } |
| #include "cosa_common_util.h" | ||
|
|
||
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && defined(_COSA_BCM_MIPS_) | ||
| #if defined((CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && defined(_COSA_BCM_MIPS_)) || defined(_ONESTACK_PRODUCT_REQ_) |
There was a problem hiding this comment.
This preprocessor condition is invalid C: defined cannot take an expression like (CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && defined(_COSA_BCM_MIPS_). This will fail compilation. Rewrite as #if (defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && defined(_COSA_BCM_MIPS_)) || defined(_ONESTACK_PRODUCT_REQ_).
| #if defined((CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && defined(_COSA_BCM_MIPS_)) || defined(_ONESTACK_PRODUCT_REQ_) | |
| #if (defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && defined(_COSA_BCM_MIPS_)) || defined(_ONESTACK_PRODUCT_REQ_) |
No description provided.