RDKB-63098: [ONESTACK]-Handle IPv6 delegation for business vs residential Partner ID as part of single build#238
RDKB-63098: [ONESTACK]-Handle IPv6 delegation for business vs residential Partner ID as part of single build#238
Conversation
vs residential Partner ID as part of single build
Reason for change: Prefix delegation handling
Test Procedure:
- Build OneStack Image
- In Business-mode, Check dibbler server is started and server.conf
has prefix-delegation class
- In Residential-mode, check whether device behaves as a non-CBR device
Risks: None
Priority: P1
Signed-off-by: rirfha948 <rasina_irfhan@comcast.com>
eeb4f20 to
c2e4e85
Compare
There was a problem hiding this comment.
Pull request overview
Adds OneStack-aware IPv6 prefix delegation handling, toggling behavior between “business” (PD enabled) and “residential” (PD disabled) modes in a single build.
Changes:
- Gate PD-related routed/IPv6/firewall behavior at runtime via
rdkb_feature_mode_gate(FEATURE_IPV6_DELEGATION). - Register different sysevent custom events for business vs residential mode (including new
ipv6_prefix_delegationevent). - Update build/link integration to include OneStack libraries when
ONESTACK_PRODUCT_REQis enabled.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| source/util/utils/Makefile.am | Links OneStack feature/mode-gate libs when enabled |
| source/service_routed/service_routed.c | Runtime-gates PD vs non-PD code paths |
| source/service_routed/Makefile.am | Adds include/link flags and OneStack libs |
| source/service_ipv6/service_ipv6.c | Runtime-gates PD behavior; updates helper and start/stop logic |
| source/service_ipv6/Makefile.am | Links OneStack feature/mode-gate libs when enabled |
| source/scripts/init/service.d/service_routed_bci.sh | Adds ipv6_prefix_delegation event handling; expands model list |
| source/scripts/init/service.d/service_registration_functions.sh | Adds debug prints during event registration |
| source/scripts/init/service.d/service_dhcpv6_server_bci.sh | Adds ipv6_prefix_delegation event handling; expands model list |
| source/scripts/init/service.d/service_crond.sh | Expands model list for cron entry |
| source/scripts/init/c_registration/Makefile.am | Links OneStack feature/mode-gate libs when enabled |
| source/scripts/init/c_registration/20_routing.c | Chooses custom events list based on PD feature mode |
| source/scripts/init/c_registration/15_dhcpv6_server.c | Chooses custom events list based on PD feature mode |
| source/scripts/init/c_registration/15_dhcpv6_client.c | Enables client custom events under OneStack |
| source/firewall/firewall_ipv6.c | Runtime-gates multinet firewall rules under OneStack |
| source/firewall/firewall.h | Exposes multinet firewall prototype for OneStack builds |
| source/firewall/Makefile.am | Links OneStack feature/mode-gate libs when enabled |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "dhcpv6_server-stop|/etc/utopia/service.d/service_dhcpv6_server_bci.sh", | ||
| "dhcpv6_server-restart|/etc/utopia/service.d/service_dhcpv6_server_bci.sh", | ||
| "ipv6_prefix_delegation|/etc/utopia/service.d/service_dhcpv6_server_bci.sh", | ||
| "dhcpv6_option_changed|/etc/utopia/service.d/service_dhcpv6_server_bci.sh|NULL|"TUPLE_FLAG_EVENT,NULL |
There was a problem hiding this comment.
This array initializer is syntactically invalid: the final element combines a string literal and NULL without a separating comma, which will fail compilation. Split the last entry into a string element and a separate NULL terminator (and ensure the TUPLE_FLAG_EVENT concatenation remains part of the string literal).
| "dhcpv6_option_changed|/etc/utopia/service.d/service_dhcpv6_server_bci.sh|NULL|"TUPLE_FLAG_EVENT,NULL | |
| "dhcpv6_option_changed|/etc/utopia/service.d/service_dhcpv6_server_bci.sh|NULL|"TUPLE_FLAG_EVENT, | |
| NULL |
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| const char** SERVICE_CUSTOM_EVENTS = NULL; |
There was a problem hiding this comment.
Under configurations where _ONESTACK_PRODUCT_REQ_ and CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION are both defined, this will produce a redefinition of SERVICE_CUSTOM_EVENTS with incompatible types (const char** vs const char* []). Refactor to define SERVICE_CUSTOM_EVENTS exactly once via a single #if / #elif / #else chain, and consider using const char* const *SERVICE_CUSTOM_EVENTS for the pointer form to preserve const-correctness.
| #if defined(_ONESTACK_PRODUCT_REQ_) | |
| const char** SERVICE_CUSTOM_EVENTS = NULL; | |
| #if defined(_ONESTACK_PRODUCT_REQ_) && !defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) | |
| const char* const *SERVICE_CUSTOM_EVENTS = NULL; |
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| const char** SERVICE_CUSTOM_EVENTS = NULL; | ||
| #endif | ||
|
|
||
| #if defined(_CBR_PRODUCT_REQ_) && !defined(_CBR2_PRODUCT_REQ_) | ||
| const char* SERVICE_CUSTOM_EVENTS[] = { |
There was a problem hiding this comment.
This introduces the same potential symbol clash as in 20_routing.c: if _ONESTACK_PRODUCT_REQ_ is defined alongside another branch that defines SERVICE_CUSTOM_EVENTS[], compilation will fail due to redefinition. Prefer a single compile-time selection that declares SERVICE_CUSTOM_EVENTS once, and use a const-correct pointer type (const char* const *) when you need to re-point it at different static arrays.
| "routeset-ula|/usr/bin/service_routed|NULL|"TUPLE_FLAG_EVENT, | ||
| "routeunset-ula|/usr/bin//service_routed|NULL|"TUPLE_FLAG_EVENT, |
There was a problem hiding this comment.
The routeunset-ula handler path contains a double slash (/usr/bin//service_routed). While it may resolve on many systems, it’s an easy source of inconsistency and can break path comparisons/tools. Use /usr/bin/service_routed.
| #if defined( _CBR_PRODUCT_REQ_) || defined (_ONESTACK_PRODUCT_REQ_) | ||
| STATIC uint64_t helper_ntoh64(const long long unsigned int *inputval) |
There was a problem hiding this comment.
Changing the parameter type from const uint64_t * to const long long unsigned int * is not portable: uint64_t is not guaranteed to be unsigned long long on all toolchains, and this can introduce incompatible-pointer warnings/errors at call sites. Keep the parameter as const uint64_t * (or change to a byte-oriented type like const uint8_t */const void * since the implementation treats it as raw bytes).
|
|
||
| STATIC int serv_ipv6_stop(struct serv_ipv6 *si6) | ||
| { | ||
| fprintf(stderr, "Entered serv_ipv6_stop \n"); |
There was a problem hiding this comment.
These unconditional fprintf(stderr, ...) debug prints will spam logs in production and are inconsistent with the rest of the service logging patterns. Please gate them behind an existing debug macro/compile flag (or remove them) and use the project’s standard logging facility if available.
| fprintf(stderr, "Entered serv_ipv6_stop \n"); |
| return 0 | ||
| fi | ||
|
|
||
| echo "[DEBUG] sm_rm_event ${SMR_SERVICE} ${SMR_EVENT_NAME}" |
There was a problem hiding this comment.
These debug echo statements are unconditional and will add noise to boot/service manager logs on all devices. Consider guarding them behind a debug flag/environment variable (or using an existing structured logger like ulog if that’s the convention for this repo).
| echo "[DEBUG] sm_rm_event ${SMR_SERVICE} ${SMR_EVENT_NAME}" | |
| ulog srvmgr status "[DEBUG] sm_rm_event ${SMR_SERVICE} ${SMR_EVENT_NAME}" |
| IFS=';' | ||
| for custom in $SM_CUSTOM_EVENTS ; do | ||
| if [ -n "$custom" ] && [ " " != "$custom" ] ; then | ||
| echo "[DHCPV6][DEBUG] Registering custom event: '$custom' for service '$SM_SERVICE'" |
There was a problem hiding this comment.
These debug echo statements are unconditional and will add noise to boot/service manager logs on all devices. Consider guarding them behind a debug flag/environment variable (or using an existing structured logger like ulog if that’s the convention for this repo).
| echo "[DHCPV6][DEBUG] Registering custom event: '$custom' for service '$SM_SERVICE'" | |
| ulog dhcpv6 debug "Registering custom event: '$custom' for service '$SM_SERVICE'" |
| -I$(top_srcdir)/source/util/utils \ | ||
| $(DBUS_CFLAGS) | ||
|
|
||
| AM_LDFLAGS = -lsecure_wrapper -lccsp_common $(DBUS_LIBS) |
There was a problem hiding this comment.
These Makefile changes put libraries (-l...) into AM_LDFLAGS / service_routed_LDFLAGS. In Automake, *_LDFLAGS is intended for linker flags (e.g., -L, -Wl, options), while libraries should generally go into *_LDADD (or LIBS). Moving -lccsp_common, $(DBUS_LIBS), -lnet, and the OneStack libraries to service_routed_LDADD will avoid link-order issues and align with standard Automake conventions.
| service_routed_LDFLAGS = -L${PKG_CONFIG_SYSROOT_DIR}${libdir} | ||
| if ONESTACK_PRODUCT_REQ | ||
| service_routed_LDFLAGS += -ldevicemode -lonestack_syscfg -lonestack_log -lrdkb_feature_mode_gate | ||
| endif | ||
| if CORE_NET_LIB_FEATURE_SUPPORT | ||
| service_routed_LDFLAGS = -lnet | ||
| service_routed_LDFLAGS += -lnet |
There was a problem hiding this comment.
These Makefile changes put libraries (-l...) into AM_LDFLAGS / service_routed_LDFLAGS. In Automake, *_LDFLAGS is intended for linker flags (e.g., -L, -Wl, options), while libraries should generally go into *_LDADD (or LIBS). Moving -lccsp_common, $(DBUS_LIBS), -lnet, and the OneStack libraries to service_routed_LDADD will avoid link-order issues and align with standard Automake conventions.
vs residential Partner ID as part of single build
Reason for change: Prefix delegation handling
Test Procedure:
- Build OneStack Image
- In Business-mode, Check dibbler server is started and server.conf
has prefix-delegation class
- In Residential-mode, check whether device behaves as a non-CBR device
Risks: None
Priority: P1
Signed-off-by: rirfha948 <rasina_irfhan@comcast.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (1)
source/service_routed/service_routed.c:1795
- Inconsistent indentation and formatting. The 'if' statement on line 1786 is not indented properly, and the variable declarations starting at line 1789 should follow standard C89/C90 convention of being declared at the beginning of the block (before any executable statements), or the code should be compiled with C99 or later standards. Consider adding proper indentation and following the code style of the surrounding context.
if (!isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION))
#endif
{
char cmd[100];
char out[100];
char interface_name[32] = {0};
char *token = NULL;
char *pt;
char pref_rx[16];
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "ipv6_ntp_server|/etc/utopia/service.d/service_dhcpv6_server.sh", | ||
| "dhcp_domain|/etc/utopia/service.d/service_dhcpv6_server.sh", | ||
| "current_lan_ipv6address|/etc/utopia/service.d/service_dhcpv6_server.sh", | ||
| NULL |
There was a problem hiding this comment.
Inconsistent indentation in array. Line 70 uses a tab character while surrounding lines use spaces. This creates visual inconsistency and should be standardized to match the indentation style of the rest of the array.
| NULL | |
| NULL |
| DHCPV6S_SYSCFG_GETS(DHCPV6S_NAME, "pool", cfg->index, "", 0, "IAInterface", iface_name); | ||
| #else | ||
| DHCPV6S_SYSCFG_GETS(DHCPV6S_NAME, "pool", cfg->index, "", 0, "Interface", iface_name); | ||
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) |
There was a problem hiding this comment.
Missing space in the preprocessor directive condition. The code has '#if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(ONESTACK_PRODUCT_REQ)' with two spaces after '||'. While this doesn't cause a compilation error, it's inconsistent with the single space used elsewhere in the file.
| DHCPV6S_SYSCFG_GETS(DHCPV6S_NAME, "pool", cfg->index, "", 0, "IAInterface", iface_name); | ||
| } | ||
| #endif | ||
| #if !defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) |
There was a problem hiding this comment.
Inconsistent spacing in preprocessor directive. This line has two spaces after '||' while other similar conditions use a single space. Consider using consistent spacing throughout.
| #if !defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | |
| #if !defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) |
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION) == false) | ||
| #endif | ||
| { | ||
| DHCPV6S_SYSCFG_GETS(DHCPV6S_NAME, "pool", cfg->index, "", 0, "Interface", iface_name); |
There was a problem hiding this comment.
Inconsistent indentation. This line uses a tab character for indentation while the surrounding code uses spaces. This creates visual inconsistency and can cause issues with different editor tab settings.
| DHCPV6S_SYSCFG_GETS(DHCPV6S_NAME, "pool", cfg->index, "", 0, "Interface", iface_name); | |
| DHCPV6S_SYSCFG_GETS(DHCPV6S_NAME, "pool", cfg->index, "", 0, "Interface", iface_name); |
| @@ -2098,10 +2125,17 @@ STATIC int serv_ipv6_start(struct serv_ipv6 *si6) | |||
| sysevent_set(si6->sefd, si6->setok, "service_ipv6-status", "starting", 0); | |||
|
|
|||
| /* Fix for IPv6 prefix not getting updated in dibbler server conf file on WAN mode change */ | |||
| #if defined(_CBR2_PRODUCT_REQ_) | |||
| #if defined(_CBR2_PRODUCT_REQ_) | |||
| sysevent_get(si6->sefd, si6->setok, "ipv6_prefix", si6->mso_prefix, sizeof(si6->mso_prefix)); | |||
| sysevent_set(si6->sefd, si6->setok, "ipv6_prefix-divided", "", 0); | |||
| #endif | |||
| #if defined(_ONESTACK_PRODUCT_REQ_) | |||
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | |||
| { | |||
| sysevent_get(si6->sefd, si6->setok, "ipv6_prefix_delegation", si6->mso_prefix, sizeof(si6->mso_prefix)); | |||
| sysevent_set(si6->sefd, si6->setok, "ipv6_prefix-divided", "", 0); | |||
| } | |||
| #endif | |||
|
|
|||
| /* | |||
| * Update MTU of all the enabled IPv6 instances | |||
| @@ -2118,13 +2152,22 @@ STATIC int serv_ipv6_start(struct serv_ipv6 *si6) | |||
| * 5) Send RA, start DHCPv6 server | |||
| */ | |||
| /* For CBR product the lan(brlan0) v6 address set is done as part of PandM process*/ | |||
| #if !defined(_CBR_PRODUCT_REQ_) && !defined(_BWG_PRODUCT_REQ_) | |||
| #if !defined(_CBR_PRODUCT_REQ_) && !defined(_BWG_PRODUCT_REQ_) && !defined(_ONESTACK_PRODUCT_REQ_) | |||
| if (lan_addr6_set(si6) !=0) { | |||
| fprintf(stderr, "assign IPv6 address for lan interfaces error!\n"); | |||
| sysevent_set(si6->sefd, si6->setok, "service_ipv6-status", "error", 0); | |||
| return -1; | |||
| } | |||
| #endif | |||
| #if defined (_ONESTACK_PRODUCT_REQ_) | |||
| if (!isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)){ | |||
| if (lan_addr6_set(si6) !=0) { | |||
| fprintf(stderr, "assign IPv6 address for lan interfaces error!\n"); | |||
| sysevent_set(si6->sefd, si6->setok, "service_ipv6-status", "error", 0); | |||
| return -1; | |||
| } | |||
| } | |||
| #endif | |||
|
|
|||
| //Intel Proposed RDKB Generic Bug Fix from XB6 SDK | |||
| /*start zebra*/ | |||
| @@ -2152,6 +2195,7 @@ STATIC int serv_ipv6_start(struct serv_ipv6 *si6) | |||
|
|
|||
| STATIC int serv_ipv6_stop(struct serv_ipv6 *si6) | |||
| { | |||
| fprintf(stderr, "Entered serv_ipv6_stop \n"); | |||
| if (!serv_can_stop(si6->sefd, si6->setok, "service_ipv6")) | |||
| return -1; | |||
|
|
|||
| @@ -2162,7 +2206,7 @@ STATIC int serv_ipv6_stop(struct serv_ipv6 *si6) | |||
| sysevent_set(si6->sefd, si6->setok, "service_ipv6-status", "error", 0); | |||
| return -1; | |||
| } | |||
| #if !defined(_CBR_PRODUCT_REQ_) && !defined(_BWG_PRODUCT_REQ_) | |||
| #if !defined(_CBR_PRODUCT_REQ_) && !defined(_BWG_PRODUCT_REQ_) && !defined(_ONESTACK_PRODUCT_REQ_) | |||
| del_addr6_flg = false; | |||
| if (lan_addr6_unset(si6) !=0) { | |||
| fprintf(stderr, "unset IPv6 address for lan interfaces error!\n"); | |||
| @@ -2171,6 +2215,18 @@ STATIC int serv_ipv6_stop(struct serv_ipv6 *si6) | |||
| return -1; | |||
| } | |||
| del_addr6_flg = true; | |||
| #endif | |||
| #if defined (_ONESTACK_PRODUCT_REQ_) | |||
| if (!isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)){ | |||
| del_addr6_flg = false; | |||
| if (lan_addr6_unset(si6) !=0) { | |||
| fprintf(stderr, "unset IPv6 address for lan interfaces error!\n"); | |||
| sysevent_set(si6->sefd, si6->setok, "service_ipv6-status", "error", 0); | |||
| del_addr6_flg = true; | |||
| return -1; | |||
| } | |||
| del_addr6_flg = true; | |||
| } | |||
| #endif | |||
| sysevent_set(si6->sefd, si6->setok, "service_ipv6-status", "stopped", 0); | |||
| return 0; | |||
| @@ -2219,12 +2275,28 @@ STATIC int serv_ipv6_init(struct serv_ipv6 *si6) | |||
| return -1; | |||
| #endif | |||
| } | |||
|
|
|||
| #if defined (_ONESTACK_PRODUCT_REQ_) | |||
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | |||
| { | |||
| sysevent_get(si6->sefd, si6->setok, "ipv6_prefix_delegation", si6->mso_prefix, sizeof(si6->mso_prefix)); | |||
| } | |||
| else | |||
| { | |||
| sysevent_get(si6->sefd, si6->setok, "ipv6_prefix", si6->mso_prefix, sizeof(si6->mso_prefix)); | |||
| } | |||
| #else | |||
| sysevent_get(si6->sefd, si6->setok, "ipv6_prefix", si6->mso_prefix, sizeof(si6->mso_prefix)); | |||
| #endif | |||
| fprintf(stderr, "IPv6 Prefix :%s\n", si6->mso_prefix); | |||
| if (strlen(si6->mso_prefix)) | |||
| { | |||
| si6->wan_ready = true; | |||
| } | |||
| else | |||
| { | |||
| fprintf(stderr, "[%s] -- Init ipv6_prefix retrieval failed\n", PROG_NAME); | |||
| return -1; | |||
| } | |||
|
|
|||
| sysevent_get(si6->sefd, si6->setok, "erouter_topology-mode", buf, sizeof(buf)); | |||
| switch(atoi(buf)) { | |||
| @@ -2307,8 +2379,10 @@ int service_ipv6_main(int argc, char *argv[]) | |||
| } | |||
|
|
|||
| if (serv_ipv6_init(&si6) != 0) | |||
| { | |||
| fprintf(stderr, "[%s] -- Service IPV6 Initialization failed\n", PROG_NAME); | |||
| exit(1); | |||
|
|
|||
| } | |||
| for (i = 0; i < NELEMS(cmd_ops); i++) { | |||
| if (strcmp(argv[1], cmd_ops[i].cmd) != 0 || !cmd_ops[i].exec) | |||
| continue; | |||
| @@ -2323,6 +2397,7 @@ int service_ipv6_main(int argc, char *argv[]) | |||
| if (i == NELEMS(cmd_ops)) | |||
| fprintf(stderr, "[%s] unknown command: %s\n", PROG_NAME, argv[1]); | |||
|
|
|||
| fprintf(stderr, "[%s] -- DHCPv6 start in progress\n", PROG_NAME); | |||
There was a problem hiding this comment.
The debug fprintf statements added throughout this function should be removed or converted to proper logging. Debug print statements left in production code can impact performance and clutter logs. Consider using a conditional debug macro or removing these before merging.
| IFS=';' | ||
| for custom in $SM_CUSTOM_EVENTS ; do | ||
| if [ -n "$custom" ] && [ " " != "$custom" ] ; then | ||
| echo "[DHCPV6][DEBUG] Registering custom event: '$custom' for service '$SM_SERVICE'" |
There was a problem hiding this comment.
Debug echo statements are being added to shell scripts. These should be removed or made conditional before production deployment to avoid cluttering logs and potentially exposing sensitive information.
| echo "[DHCPV6][DEBUG] Registering custom event: '$custom' for service '$SM_SERVICE'" |
| #if defined (_COSA_BCM_MIPS_) | ||
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) | ||
| // For CBR platform, the captive portal redirection feature was removed | ||
| // inWifiCp = 1; | ||
| #else | ||
| inWifiCp = 1; | ||
| inWifiCp = 1; | ||
| #endif | ||
| #else | ||
| inWifiCp = 1; | ||
| inWifiCp = 1; | ||
| #endif | ||
| } | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation: this code block uses tab characters while the surrounding code may use spaces. The lines 1491-1501 should use consistent indentation matching the rest of the file to maintain code style uniformity.
| #ifdef WAN_FAILOVER_SUPPORTED | ||
| "routeset-ula|/usr/bin/service_routed|NULL|"TUPLE_FLAG_EVENT, | ||
| "routeunset-ula|/usr/bin//service_routed|NULL|"TUPLE_FLAG_EVENT, | ||
| #endif | ||
| NULL |
There was a problem hiding this comment.
Using preprocessor directives inside array initialization is non-standard and could cause compilation issues or unexpected behavior depending on the compiler. If WAN_FAILOVER_SUPPORTED is not defined, this will result in a trailing comma before NULL, which while typically handled by most C compilers, is technically a C99 feature and may not be portable. Consider defining two complete separate arrays or restructuring to avoid conditionals within the array.
88076ef to
0443b75
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(MULTILAN_FEATURE) || defined(_ONESTACK_PRODUCT_REQ_) | ||
| #ifdef _ONESTACK_PRODUCT_REQ_ | ||
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| #endif | ||
| { |
There was a problem hiding this comment.
In route_unset(), adding _ONESTACK_PRODUCT_REQ_ to the #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(MULTILAN_FEATURE) ... condition makes the preprocessor always select this branch for OneStack builds, so the #elif !defined(WAN_MANAGER_UNIFICATION_ENABLED) route/rule cleanup path is not compiled at all. In residential mode (when FEATURE_IPV6_DELEGATION is not supported), the runtime if (isFeatureSupportedInCurrentMode(...)) will skip the body and route_unset() effectively becomes a no-op, potentially leaving stale IPv6 rules/routes. Restructure this conditional so OneStack can execute the non-PD cleanup path when the feature gate is false (e.g., compile both paths for OneStack and select at runtime).
| #if defined( _CBR_PRODUCT_REQ_) || defined (_ONESTACK_PRODUCT_REQ_) | ||
| STATIC uint64_t helper_ntoh64(const long long unsigned int *inputval) | ||
| { |
There was a problem hiding this comment.
helper_ntoh64()/helper_hton64() now take const long long unsigned int *, which is non-portable (on some platforms uint64_t is unsigned long). Prefer using const uint64_t * and changing tmp_prefix/sub_prefix to uint64_t (or casting at the callsites) to avoid ABI and pointer-type mismatches.
| fprintf(stderr, "[%s] -- Init ipv6_prefix retrieval failed\n", PROG_NAME); | ||
| return -1; |
There was a problem hiding this comment.
serv_ipv6_init() now returns -1 when the prefix is missing. Since service_ipv6_main() exits on init failure, this can prevent stop/restart flows from running during boot or transient WAN states. Consider not failing init solely due to an empty prefix; set wan_ready = false and let serv_ipv6_start() handle readiness.
| fprintf(stderr, "[%s] -- Init ipv6_prefix retrieval failed\n", PROG_NAME); | |
| return -1; | |
| fprintf(stderr, "[%s] -- Init ipv6_prefix retrieval failed\n", PROG_NAME); | |
| si6->mso_prefix[0] = '\0'; | |
| si6->wan_ready = false; |
|
|
||
| if (pd_enabled || multilan_enabled) |
There was a problem hiding this comment.
In gen_zebra_conf(), the OneStack logic sets pd_enabled from isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION) and then wraps the subsequent zebra.conf emission under if (pd_enabled || multilan_enabled). In residential mode (feature not supported) with MULTILAN_FEATURE off, this condition is false and the expected RA/interface config won’t be written. Consider ensuring the non-PD single-LAN zebra.conf path still runs when the feature gate is false.
| if (pd_enabled || multilan_enabled) | |
| #ifdef _ONESTACK_PRODUCT_REQ_ | |
| /* In OneStack builds, always run the zebra.conf emission block so that | |
| * the non-PD single-LAN configuration is still generated even when | |
| * IPv6 delegation is not supported in the current mode. PD-specific | |
| * behavior inside the block remains controlled by pd_enabled. | |
| */ | |
| if (1) | |
| #else | |
| if (pd_enabled || multilan_enabled) | |
| #endif |
|
|
||
| STATIC int serv_ipv6_start(struct serv_ipv6 *si6) | ||
| { | ||
| fprintf(stderr, "Entered serv_ipv6_start \n"); |
There was a problem hiding this comment.
The new unconditional fprintf(stderr, "Entered serv_ipv6_start") debug print will spam logs on every start. Please remove it or gate it behind an existing debug macro/log level to avoid noisy production logs.
| fprintf(stderr, "Entered serv_ipv6_start \n"); |
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| const char** SERVICE_CUSTOM_EVENTS = NULL; | ||
| #endif |
There was a problem hiding this comment.
For OneStack, SERVICE_CUSTOM_EVENTS is declared as a pointer (const char**), but the file can also define a const char* SERVICE_CUSTOM_EVENTS[] array in other preprocessor branches. If _ONESTACK_PRODUCT_REQ_ is ever combined with those branches, this will be a symbol redefinition compile error. Please adjust the preprocessor logic or rename the OneStack variable so SERVICE_CUSTOM_EVENTS is defined exactly once per build.
| #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.
The OneStack feature-gated IPv6 delegation behavior (e.g., runtime branching on isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION) for active LAN selection / config generation) is not covered by the existing unit tests under source/test/service_routed. Please add tests that exercise both feature modes to prevent regressions (at minimum verifying route_unset() and zebra.conf generation behave correctly in business vs residential mode).
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| const char** SERVICE_CUSTOM_EVENTS = NULL; | ||
| #endif | ||
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) | ||
| const char* SERVICE_CUSTOM_EVENTS[] = { |
There was a problem hiding this comment.
OneStack introduces const char** SERVICE_CUSTOM_EVENTS = NULL; but this file also defines const char* SERVICE_CUSTOM_EVENTS[] under CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION. If both macros are enabled, this will not compile (symbol redefinition). Please restructure/guard so only one SERVICE_CUSTOM_EVENTS definition exists for any macro combination.
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| { | ||
| sysevent_get(si6->sefd, si6->setok, "ipv6_prefix_delegation", si6->mso_prefix, sizeof(si6->mso_prefix)); | ||
| sysevent_set(si6->sefd, si6->setok, "ipv6_prefix-divided", "", 0); |
There was a problem hiding this comment.
The new OneStack branch in serv_ipv6_start() (switching to ipv6_prefix_delegation and skipping lan_addr6_set() when delegation is enabled) isn’t covered by existing source/test/service_ipv6 unit tests. Please add tests that mock isFeatureSupportedInCurrentMode() and verify both business vs residential paths.
| #ifdef _CBR_PRODUCT_REQ_ | ||
| STATIC uint64_t helper_ntoh64(const uint64_t *inputval) | ||
| #if defined( _CBR_PRODUCT_REQ_) || defined (_ONESTACK_PRODUCT_REQ_) | ||
| STATIC uint64_t helper_ntoh64(const long long unsigned int *inputval) |
There was a problem hiding this comment.
why we need this change ? this will impact CBRv2 as well
There was a problem hiding this comment.
For Little Endian devices , this is needed, Both CBRv2 and XB10 are little endian devices , ```
root@Docsis-Gateway:# deviceinfo.sh -mo# lscpu | grep -i endian
CGA4332COM
root@Docsis-Gateway:
Byte Order: Little Endian
root@Docsis-Gateway:~#
bb5a727 to
8836593
Compare
8836593 to
7b599ff
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(MULTILAN_FEATURE) || defined(_ONESTACK_PRODUCT_REQ_) | ||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| #endif | ||
| { |
There was a problem hiding this comment.
On OneStack builds, this #if ... || defined(_ONESTACK_PRODUCT_REQ_) forces compilation into the PD/MULTILAN branch even when FEATURE_IPV6_DELEGATION is false at runtime, which also prevents the compile-time fallback branch (the #elif !defined(WAN_MANAGER_UNIFICATION_ENABLED) path) from being compiled at all. This risks skipping the required non-PD routing behavior in residential mode. Restructure so OneStack compiles both behaviors and selects via runtime if/else (e.g., make OneStack a separate #if defined(_ONESTACK_PRODUCT_REQ_) wrapper with an explicit else calling the non-PD logic), while keeping the original compile-time selection for non-OneStack builds.
| } | ||
| #elif !defined(WAN_MANAGER_UNIFICATION_ENABLED) |
There was a problem hiding this comment.
Same issue as route_set(): for OneStack, the #elif !defined(WAN_MANAGER_UNIFICATION_ENABLED) block will never be compiled (because _ONESTACK_PRODUCT_REQ_ makes the first #if true), even though the runtime feature check can be false. This can drop the non-PD route-unset behavior in residential mode. Refactor the preprocessor structure so that OneStack builds compile both code paths and choose at runtime (explicit if/else), instead of using _ONESTACK_PRODUCT_REQ_ in the compile-time condition that controls the #elif.
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| const char** SERVICE_CUSTOM_EVENTS = NULL; | ||
| #endif | ||
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) | ||
| const char* SERVICE_CUSTOM_EVENTS[] = { |
There was a problem hiding this comment.
This introduces a duplicate definition of SERVICE_CUSTOM_EVENTS when both _ONESTACK_PRODUCT_REQ_ and CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION are defined (pointer + array with the same identifier), which will fail compilation. Fix by making the OneStack pointer declaration mutually exclusive with the other SERVICE_CUSTOM_EVENTS[] declarations (e.g., fold it into the same #if/#elif/#else chain), or rename the OneStack-selected pointer (e.g., SERVICE_CUSTOM_EVENTS_PTR) and pass that to sm_register().
| #ifdef WAN_FAILOVER_SUPPORTED | ||
| char last_broadcasted_prefix[64] = {0}; | ||
| #endif | ||
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | ||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| #endif | ||
| { | ||
| sysevent_get(sefd, setok, "previous_ipv6_prefix", orig_prefix, sizeof(orig_prefix)); |
There was a problem hiding this comment.
When WAN_FAILOVER_SUPPORTED is enabled but CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION is defined and _ONESTACK_PRODUCT_REQ_ is not, last_broadcasted_prefix is declared but the only code that uses it is not compiled (it lives in the non-PD branch). This can trigger an unused-variable build warning. Declare last_broadcasted_prefix only within the branch where it’s used, or guard its declaration with the same preprocessor conditions as the usage.
| #if defined( _CBR_PRODUCT_REQ_) || defined (_ONESTACK_PRODUCT_REQ_) | ||
| STATIC uint64_t helper_ntoh64(const long long unsigned int *inputval) | ||
| { | ||
| uint64_t returnval; |
There was a problem hiding this comment.
Changing the parameter type from const uint64_t * to const long long unsigned int * makes the API less portable and can cause type-mismatch warnings/errors depending on how uint64_t is typedef’d on the target toolchain. Prefer keeping the signature as const uint64_t * (and adjust call sites if needed), since the function is explicitly operating on 64-bit values.
There was a problem hiding this comment.
This function is called only in CBR and XB10 devices, Due to Architecture incompatability error, quoted below, this has been changed
In CBRv2, ---> 32bit Arch ----> uint64_t is unsigned long long
In XB10, ---> 64bit Arch ----> uint64_t is unsigned long
/git/source/service_ipv6/service_ipv6.c
| ../../../git/source/service_ipv6/service_ipv6.c: In function 'divide_ipv6_prefix':
| ../../../git/source/service_ipv6/service_ipv6.c:909:36: error: passing argument 1 of 'helper_ntoh64' from incompatible pointer type [-Werror=incompatible-pointer-types]
| 909 | tmp_prefix = helper_ntoh64(&tmp_prefix); // The memcpy is copying in reverse order due to LEndianess
| | ^~~~~~~~~~~
| | |
| | long long unsigned int *
| ../../../git/source/service_ipv6/service_ipv6.c:264:47: note: expected 'const uint64_t *' {aka 'const long unsigned int *'} but argument is of type 'long long unsigned int *'
| 264 | STATIC uint64_t helper_ntoh64(const uint64_t *inputval)
| | ~~~~~~~~~~~~~~~~^~~~~~~~
| ../../../git/source/service_ipv6/service_ipv6.c:922:44: error: passing argument 1 of 'helper_hton64' from incompatible pointer type [-Werror=incompatible-pointer-types]
| 922 | sub_prefix = helper_hton64(&sub_prefix);// The memcpy is copying in reverse order due to LEndianess
| | ^~~~~~~~~~~
| | |
| | long long unsigned int *
| ../../../git/source/service_ipv6/service_ipv6.c:281:47: note: expected 'const uint64_t *' {aka 'const long unsigned int *'} but argument is of type 'long long unsigned int *'
| 281 | STATIC uint64_t helper_hton64(const uint64_t *inputval)
| | ~~~~~~~~~~~~~~~~^~~~~~~~
| ../../../git/source/service_ipv6/service_ipv6.c:950:36: error: passing argument 1 of 'helper_ntoh64' from incompatible pointer type [-Werror=incompatible-pointer-types]
| AM_CPPFLAGS = -I$(top_srcdir)/source/include \ | ||
| -I$(top_srcdir)/source/util/utils \ | ||
| $(DBUS_CFLAGS) | ||
|
|
||
| AM_LDFLAGS = -lsecure_wrapper -lccsp_common $(DBUS_LIBS) |
There was a problem hiding this comment.
Using AM_LDFLAGS to add libraries (-l... and $(DBUS_LIBS)) is non-idiomatic Automake and can lead to link ordering issues. Prefer moving libraries into service_routed_LDADD (or LIBS) and keep *_LDFLAGS for linker flags like -L.../-Wl,....
| 15_hotspot_SOURCES = 15_hotspot.c | ||
| endif | ||
| if ONESTACK_PRODUCT_REQ | ||
| LDFLAGS += -lrdkb_feature_mode_gate |
There was a problem hiding this comment.
Appending -l... to global LDFLAGS affects all targets in this directory and mixes libraries into a flags variable. Prefer adding -lrdkb_feature_mode_gate to the specific program’s LDADD/LIBS (or a target-specific *_LDADD) so only the binaries that include rdkb_feature_mode_gate.h link against it.
| LDFLAGS += -lrdkb_feature_mode_gate | |
| 02_devicemode_LDADD = -lrdkb_feature_mode_gate |
|
|
||
| STATIC int serv_ipv6_stop(struct serv_ipv6 *si6) | ||
| { | ||
| fprintf(stderr, "Entered serv_ipv6_stop \n"); |
There was a problem hiding this comment.
Same as above: avoid unconditional stderr debug prints in the stop path; use the existing logging mechanism or a debug-only guard.
| fprintf(stderr, "Entered serv_ipv6_stop \n"); | |
| #ifdef DEBUG | |
| fprintf(stderr, "Entered serv_ipv6_stop \n"); | |
| #endif |
7b599ff to
9280c3f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if(isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| { | ||
| sysevent_get(sysevent_fd, sysevent_token, "ipv6_prefix_delegation", prefix, sizeof(prefix)); | ||
| } | ||
| else | ||
| { | ||
| sysevent_get(sysevent_fd, sysevent_token, "ipv6_prefix", prefix, sizeof(prefix)); | ||
| } | ||
| } |
There was a problem hiding this comment.
isFeatureSupportedInCurrentMode() is called inside the WAN_FAILOVER_SUPPORTED branch without an _ONESTACK_PRODUCT_REQ_ guard, but the header/library are only included/linked for ONESTACK builds. If WAN_FAILOVER_SUPPORTED is enabled on a non-ONESTACK build this will fail to compile/link. Please wrap these calls with #ifdef _ONESTACK_PRODUCT_REQ_ (and fall back to ipv6_prefix otherwise), or make the feature-gate dependency unconditional for the configs that enable this path.
| if(isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | |
| { | |
| sysevent_get(sysevent_fd, sysevent_token, "ipv6_prefix_delegation", prefix, sizeof(prefix)); | |
| } | |
| else | |
| { | |
| sysevent_get(sysevent_fd, sysevent_token, "ipv6_prefix", prefix, sizeof(prefix)); | |
| } | |
| } | |
| #ifdef _ONESTACK_PRODUCT_REQ_ | |
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | |
| { | |
| sysevent_get(sysevent_fd, sysevent_token, "ipv6_prefix_delegation", prefix, sizeof(prefix)); | |
| } | |
| else | |
| { | |
| sysevent_get(sysevent_fd, sysevent_token, "ipv6_prefix", prefix, sizeof(prefix)); | |
| } | |
| #else | |
| sysevent_get(sysevent_fd, sysevent_token, "ipv6_prefix", prefix, sizeof(prefix)); | |
| #endif | |
| } |
| { | ||
| if(isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| { | ||
| sysevent_get(sysevent_fd, sysevent_token, "ipv6_prefix_delegation", prefix, sizeof(prefix)); | ||
| } | ||
| else |
There was a problem hiding this comment.
applyRoutingRules() is compiled when WAN_FAILOVER_SUPPORTED is enabled, but it now calls isFeatureSupportedInCurrentMode() unconditionally (no _ONESTACK_PRODUCT_REQ_ guard). Since rdkb_feature_mode_gate.h/-lrdkb_feature_mode_gate are only present for ONESTACK builds, this introduces a build break for non-ONESTACK + WAN_FAILOVER configurations. Guard the feature-gate call with _ONESTACK_PRODUCT_REQ_ (and default to ipv6_prefix otherwise).
| { | |
| if(isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | |
| { | |
| sysevent_get(sysevent_fd, sysevent_token, "ipv6_prefix_delegation", prefix, sizeof(prefix)); | |
| } | |
| else | |
| { | |
| #ifdef _ONESTACK_PRODUCT_REQ_ | |
| if(isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | |
| { | |
| sysevent_get(sysevent_fd, sysevent_token, "ipv6_prefix_delegation", prefix, sizeof(prefix)); | |
| } | |
| else | |
| #endif |
| @@ -47,11 +54,29 @@ const char* SERVICE_CUSTOM_EVENTS[] = { | |||
| "dhcpv6_server-restart|/usr/bin/service_ipv6", | |||
| NULL | |||
| }; | |||
| #elif defined (CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) | |||
| #elif defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) | |||
| const char* SERVICE_CUSTOM_EVENTS[] = { | |||
| "dhcpv6_option_changed|/etc/utopia/service.d/service_dhcpv6_server.sh|NULL|"TUPLE_FLAG_EVENT, | |||
| NULL | |||
There was a problem hiding this comment.
Same issue as in 20_routing.c: _ONESTACK_PRODUCT_REQ_ adds a const char** SERVICE_CUSTOM_EVENTS declaration, but the CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION branch defines const char* SERVICE_CUSTOM_EVENTS[]. If both macros are defined this will not compile due to redefinition/type mismatch. Restructure so there is only one SERVICE_CUSTOM_EVENTS definition in any build configuration (or use distinct names for the pointer vs arrays).
| if (i == NELEMS(cmd_ops)) | ||
| fprintf(stderr, "[%s] unknown command: %s\n", PROG_NAME, argv[1]); | ||
|
|
||
| fprintf(stderr, "[%s] -- DHCPv6 start in progress\n", PROG_NAME); |
There was a problem hiding this comment.
The log line "-- DHCPv6 start in progress" is printed unconditionally after processing any command (including stop, restart, or even an unknown command). This is misleading for operators and makes logs noisier. Consider removing it or changing it to a command-agnostic message (or only printing it for the start paths).
| fprintf(stderr, "[%s] -- DHCPv6 start in progress\n", PROG_NAME); | |
| fprintf(stderr, "[%s] -- DHCPv6 command processing complete\n", PROG_NAME); |
7fe7748 to
e1f892c
Compare
e1f892c to
33c34ce
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if defined(MULTILAN_FEATURE) || defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined (_ONESTACK_PRODUCT_REQ_) | ||
| int multilan_enabled = 0; | ||
| int pd_enabled = 0; | ||
|
|
||
| #if defined(MULTILAN_FEATURE) | ||
| multilan_enabled = 1; | ||
| #endif | ||
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) | ||
| pd_enabled = 1; | ||
| #endif | ||
|
|
||
| #ifdef _ONESTACK_PRODUCT_REQ_ | ||
| pd_enabled = isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION); | ||
| #endif | ||
|
|
||
| if (pd_enabled || multilan_enabled) | ||
| { | ||
| get_active_lanif(sefd, setok, l2_insts, &enabled_iface_num); |
There was a problem hiding this comment.
The new #if defined(MULTILAN_FEATURE) || defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) block starting here appears to be closed by an existing #endif shortly after the first few statements (around where lan_addr is fetched). That early close will compile out the opening if/for braces when the condition is false, but the closing braces later in gen_zebra_conf() still compile, causing a build break in configurations without MULTILAN/PD/ONESTACK. Please ensure the #if introduced here is closed at the same place the for loop / surrounding if are closed (near the later } //for (...)), and remove/relocate the premature #endif.
| @@ -275,7 +278,7 @@ STATIC uint64_t helper_ntoh64(const uint64_t *inputval) | |||
| return returnval; | |||
| } | |||
|
|
|||
| STATIC uint64_t helper_hton64(const uint64_t *inputval) | |||
| STATIC uint64_t helper_hton64(const long long unsigned int *inputval) | |||
| { | |||
There was a problem hiding this comment.
helper_ntoh64() / helper_hton64() now take a const long long unsigned int *, but they are called with &tmp_prefix where tmp_prefix is a uint64_t. This introduces an incompatible pointer type conversion (can be a warning or error depending on flags/toolchain). Keeping the parameter as const uint64_t * (as before) avoids the mismatch and still supports the bit-shift logic.
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| { | ||
| sysevent_get(si6->sefd, si6->setok, "ipv6_prefix_delegation", si6->mso_prefix, sizeof(si6->mso_prefix)); | ||
| sysevent_set(si6->sefd, si6->setok, "ipv6_prefix-divided", "", 0); | ||
| } |
There was a problem hiding this comment.
ONESTACK adds new behavior here (reading ipv6_prefix_delegation and resetting ipv6_prefix-divided only when FEATURE_IPV6_DELEGATION is supported). The existing service_ipv6_gtest suite doesn’t exercise this ONESTACK-specific path, so regressions in business vs residential mode selection would be easy to miss. Please consider adding unit coverage that builds with _ONESTACK_PRODUCT_REQ_ and stubs isFeatureSupportedInCurrentMode() to validate both branches (delegation vs non-delegation) set the expected sysevent keys / control flow.
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | |
| { | |
| sysevent_get(si6->sefd, si6->setok, "ipv6_prefix_delegation", si6->mso_prefix, sizeof(si6->mso_prefix)); | |
| sysevent_set(si6->sefd, si6->setok, "ipv6_prefix-divided", "", 0); | |
| } | |
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | |
| { | |
| sysevent_get(si6->sefd, si6->setok, "ipv6_prefix_delegation", si6->mso_prefix, sizeof(si6->mso_prefix)); | |
| sysevent_set(si6->sefd, si6->setok, "ipv6_prefix-divided", "", 0); | |
| } | |
| else | |
| { | |
| /* Fall back to legacy behavior when prefix delegation is not supported. */ | |
| sysevent_get(si6->sefd, si6->setok, "ipv6_prefix", si6->mso_prefix, sizeof(si6->mso_prefix)); | |
| sysevent_set(si6->sefd, si6->setok, "ipv6_prefix-divided", "", 0); | |
| } |
Reason for change: Prefix delegation handling
Test Procedure:
Build OneStack Image
In Business-mode, Check dibbler server is started and server.conf has prefix-delegation class
In Residential-mode, check whether device behaves as a non-CBR device Risks: None
Priority: P1
Is this a User Story (US)?
Have all dependent PRs from other components been listed ?
(List of PRs:
RDKB-63098: [ONESTACK]-Handle IPv6 delegation for business vs residential Partner ID as part of single build provisioning-and-management#230
RDKB-63098: [ONESTACK]-Handle IPv6 delegation for business vs residential Partner ID as part of single build dhcp-manager#79
RDKB-63098: [ONESTACK]-Handle IPv6 delegation for business vs residential Partner ID as part of single build #238
https://github.com/rdk-gdcs/apparmor-profiles/pull/90
https://gerrit.teamccp.com/#/c/947064/)
Does the commit message include both the User Story ticket and the Subtask ticket?
Will be all changes related to the User Story squashed and merged in a single commit?
Has the PR been raised only after completing all changes for the User Story (no partial changes)?
Has code development for the User Story been completed?
If yes, has the Gerrit topic or list of all dependent PRs across components (including meta-layer changes) been shared?
https://gerrit.teamccp.com/#/c/946157/
Is there a validation log available in the Jira ticket for verifying builds with the updated generic-srcrev.inc across all platforms?
RDKB-63098-logs.txt