RDKB-63098: Handle IPv6 delegation for business vs residential Partner ID as part of single build #215
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| /* | ||
| * If not stated otherwise in this file or this component's Licenses.txt file the | ||
|
Check failure on line 2 in source/firewall/firewall_ipv6.c
|
||
| * following copyright and licenses apply: | ||
| * | ||
| * Copyright 2015 RDK Management | ||
|
|
@@ -98,6 +98,9 @@ | |
| #include <net/if.h> | ||
| #endif | ||
|
|
||
| #ifdef _ONESTACK_PRODUCT_REQ_ | ||
| #include <rdkb_feature_mode_gate.h> | ||
| #endif | ||
| void* bus_handle ; | ||
| int sysevent_fd; | ||
| char sysevent_ip[19]; | ||
|
|
@@ -862,11 +865,17 @@ | |
| } | ||
| #endif /*_HUB4_PRODUCT_REQ_*/ | ||
|
|
||
| #if (defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && !defined(_CBR_PRODUCT_REQ_)) || defined(_ONESTACK_PRODUCT_REQ_) | ||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| if ( isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION) == true) | ||
| #endif | ||
| { | ||
|
|
||
| /*Add a simple logic here to make traffic allowed for lan interfaces | ||
| * exclude primary lan*/ | ||
| prepare_ipv6_multinet(fp); | ||
| } | ||
| #endif | ||
| #if !defined(_XER5_PRODUCT_REQ_) && !defined (_SCER11BEL_PRODUCT_REQ_) && !defined(_COSA_QCA_ARM_) //wan0 is not applicable for XER5 | ||
| /* not allow ping wan0 from brlan0 */ | ||
| int i; | ||
|
|
@@ -1705,9 +1714,17 @@ | |
| FIREWALL_DEBUG("Exiting prepare_ipv6_firewall \n"); | ||
| } | ||
|
|
||
| #if (defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) && !defined(_CBR_PRODUCT_REQ_)) \ | ||
| || defined(_ONESTACK_PRODUCT_REQ_) | ||
| int prepare_ipv6_multinet(FILE *fp) | ||
| { | ||
| #ifdef _ONESTACK_PRODUCT_REQ_ | ||
| if (!isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||
| { | ||
| /* PD feature disabled */ | ||
| return 0; | ||
| } | ||
| #endif | ||
|
Comment on lines
+1719
to
+1727
There was a problem hiding this comment. Within prepare_ipv6_multinet(), the code later does a do/while loop that formats with the first token from strtok(active_insts, " "). If the sysevent value "multinet-instances" is empty, strtok returns NULL and that loop will dereference a NULL pointer. Add an early return when the first token is NULL (or switch to a while(token != NULL) loop). |
||
| char active_insts[32] = {0}; | ||
| char lan_pd_if[128] = {0}; | ||
| char *p = NULL; | ||
|
Comment on lines
+1719
to
1730
There was a problem hiding this comment. In |
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||
| /************************************************************************************ | ||||||
|
Check failure on line 1 in source/service_ipv6/service_ipv6.c
|
||||||
| If not stated otherwise in this file or this component's Licenses.txt file the | ||||||
| following copyright and licenses apply: | ||||||
|
|
||||||
|
|
@@ -53,6 +53,9 @@ | |||||
| #include "ccsp_memory.h" | ||||||
| #endif | ||||||
|
|
||||||
| #ifdef _ONESTACK_PRODUCT_REQ_ | ||||||
| #include <rdkb_feature_mode_gate.h> | ||||||
| #endif | ||||||
| #ifdef _HUB4_PRODUCT_REQ_ | ||||||
| #include "ccsp_dm_api.h" | ||||||
| #include "ccsp_custom.h" | ||||||
|
|
@@ -391,10 +394,21 @@ | |||||
| DHCPV6S_SYSCFG_GETI(DHCPV6S_NAME, "pool", cfg->index, "", 0, "X_RDKCENTRAL_COM_DNSServersEnabled", cfg->X_RDKCENTRAL_COM_DNSServersEnabled); | ||||||
|
|
||||||
| #ifdef MULTILAN_FEATURE | ||||||
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | ||||||
|
There was a problem hiding this comment. Inconsistent spacing in preprocessor directive: There are two spaces between '||' and 'defined(ONESTACK_PRODUCT_REQ)'. For consistency with other preprocessor directives in the codebase, use a single space.
Suggested change
|
||||||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||||||
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION) == true) | ||||||
|
There was a problem hiding this comment. Inconsistent boolean comparison style: This explicit comparison with 'true' is inconsistent with other calls to isFeatureSupportedInCurrentMode() in this PR. For example, line 647 in service_routed.c omits the '== true'. For consistency, either use the explicit comparison everywhere or omit it everywhere. If the function returns a boolean type, the explicit comparison is redundant. |
||||||
| #endif | ||||||
| { | ||||||
| DHCPV6S_SYSCFG_GETS(DHCPV6S_NAME, "pool", cfg->index, "", 0, "IAInterface", iface_name); | ||||||
| } | ||||||
|
Comment on lines
+397
to
+403
There was a problem hiding this comment. New ONESTACK runtime gating changes which syscfg key is read (IAInterface vs Interface), but the unit tests under source/test/service_ipv6 don’t appear to cover the ONESTACK path. Please add tests that mock isFeatureSupportedInCurrentMode() for both enabled/disabled and verify the expected DHCPV6S_SYSCFG_GETS key is used. |
||||||
| #endif | ||||||
| #if !defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | ||||||
|
There was a problem hiding this comment. Inconsistent spacing in preprocessor directive: There are two spaces between '||' and 'defined(ONESTACK_PRODUCT_REQ)'. For consistency with other preprocessor directives in the codebase, use a single space.
Suggested change
|
||||||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||||||
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION) == false) | ||||||
|
There was a problem hiding this comment. Inconsistent boolean comparison style: This explicit comparison with 'false' is inconsistent with other calls to isFeatureSupportedInCurrentMode() in this PR. For example, line 462 in service_routed.c uses the negation operator '!'. For consistency, use the negation operator '!' instead of explicit comparison with 'false'.
Suggested change
|
||||||
| #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 instead of spaces. The surrounding code uses spaces (4 spaces per indentation level). Replace the tab with spaces for consistency.
Suggested change
|
||||||
| } | ||||||
|
Comment on lines
+397
to
+411
There was a problem hiding this comment. ONESTACK-specific behavior is now introduced in get_dhcpv6s_pool_cfg() to choose between IAInterface vs Interface based on FEATURE_IPV6_DELEGATION, but the existing service_ipv6 unit tests don’t build with ONESTACK_PRODUCT_REQ and don’t validate either branch. Add unit tests that exercise both feature states (mocking isFeatureSupportedInCurrentMode) to ensure the correct syscfg key is read. |
||||||
| #endif | ||||||
| #else | ||||||
| DHCPV6S_SYSCFG_GETS(DHCPV6S_NAME, "pool", cfg->index, "", 0, "IAInterface", cfg->interface); | ||||||
|
|
@@ -606,7 +620,7 @@ | |||||
| STATIC int get_active_lanif(struct serv_ipv6 *si6, unsigned int insts[], unsigned int *num) | ||||||
| { | ||||||
| int i = 0; | ||||||
| #if !defined(MULTILAN_FEATURE) || defined CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION || defined(_ONESTACK_PRODUCT_REQ_) | ||||||
| char active_insts[32] = {0}; | ||||||
| char lan_pd_if[128] = {0}; | ||||||
| char *p = NULL; | ||||||
|
|
@@ -628,7 +642,11 @@ | |||||
| unsigned int max_active_if_count = 0; | ||||||
| int primary_l3_instance = 0; | ||||||
|
|
||||||
| #if defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | ||||||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||||||
| if (isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||||||
| #endif | ||||||
| { | ||||||
|
Comment on lines
+645
to
+649
There was a problem hiding this comment. get_active_lanif() now adds ONESTACK-specific runtime branching based on FEATURE_IPV6_DELEGATION. The existing service_ipv6 unit tests don’t appear to cover this new feature-gated behavior; please add tests for ONESTACK builds that exercise both branches (feature enabled vs disabled) and assert the expected sysevent/syscfg calls. |
||||||
| syscfg_get(NULL, "lan_pd_interfaces", lan_pd_if, sizeof(lan_pd_if)); | ||||||
| if (lan_pd_if[0] == '\0') { | ||||||
| *num = 0; | ||||||
|
|
@@ -648,7 +666,14 @@ | |||||
|
|
||||||
| p = strtok(NULL, " "); | ||||||
| } | ||||||
| } | ||||||
| #endif | ||||||
| #if !defined(CISCO_CONFIG_DHCPV6_PREFIX_DELEGATION) || defined(_ONESTACK_PRODUCT_REQ_) | ||||||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||||||
| if (!isFeatureSupportedInCurrentMode(FEATURE_IPV6_DELEGATION)) | ||||||
| #endif | ||||||
| { | ||||||
|
|
||||||
| /* Get active bridge count from PSM */ | ||||||
| if (!bus_handle) { | ||||||
| fprintf(stderr, "DBUS not connected, returning \n"); | ||||||
|
|
@@ -718,6 +743,7 @@ | |||||
| } | ||||||
| /* Set active IPv6 instances */ | ||||||
| sysevent_set(si6->sefd, si6->setok, "ipv6_active_inst", active_if_list, 0); | ||||||
| } | ||||||
| #endif | ||||||
|
|
||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent boolean comparison style: This explicit comparison with 'true' is inconsistent with other calls to isFeatureSupportedInCurrentMode() in this PR. For example, line 433 in service_routed.c omits the '== true'. For consistency, either use the explicit comparison everywhere or omit it everywhere. If the function returns a boolean type, the explicit comparison is redundant.