Skip to content

RDKB-63098: [ONESTACK]-Handle IPv6 delegation for business vs residential Partner ID as part of single build #79

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
rirfha948 wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

RDKB-63098: [ONESTACK]-Handle IPv6 delegation for business vs residential Partner ID as part of single build #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d77496
RDKB-63098: OneStack - IPV6 PREFIX DELEGATION handling.
rirfha948 Feb 26, 2026
2483635
Review comment addressed
rirfha948 Mar 1, 2026
60afb1b
Review comment addressed -patch2
rirfha948 Mar 5, 2026
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
1 change: 1 addition & 0 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ AM_CONDITIONAL(VOICE_MTA_SUPPORT, test x"$voice_support" = x"yes")
# Check ra_monitor_support
AM_CONDITIONAL([RA_MONITOR_SUPPORT], [test "$RA_MONITOR_SUPPORT" = "yes"])

Copilot AI Mar 2, 2026

Copy link

Choose a reason for hiding this comment

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

AM_CONDITIONAL([ONESTACK_PRODUCT_REQ], ...) only controls Automake conditionals; it does not define a C preprocessor macro. Since the code changes are guarded by _ONESTACK_PRODUCT_REQ_, you likely need to either AC_DEFINE a matching macro (or arrange for CPPFLAGS to define it) when this configure option is enabled, otherwise the ONESTACK-specific code path won’t be compiled.

Suggested change
AS_IF([test "x$ONESTACK_PRODUCT_REQ" = "xtrue"], [
AC_DEFINE([_ONESTACK_PRODUCT_REQ_], [1],
[Define to 1 if OneStack product requirements are enabled.])
])

Copilot uses AI. Check for mistakes.
AM_CONDITIONAL([ONESTACK_PRODUCT_REQ], [test "x$ONESTACK_PRODUCT_REQ" = "xtrue"])

Copilot AI Mar 5, 2026

Copy link

Choose a reason for hiding this comment

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

The new AM_CONDITIONAL([ONESTACK_PRODUCT_REQ], ...) is not referenced anywhere in the repo’s Makefile.am files, so it currently has no effect on the build. If this is meant to control compilation for Onestack, wire it into the relevant Makefile(s) (e.g., add a CPPFLAGS/CFLAGS define when ONESTACK_PRODUCT_REQ is true), or remove this conditional to avoid misleading configuration.

Suggested change
AM_CONDITIONAL([ONESTACK_PRODUCT_REQ], [test "x$ONESTACK_PRODUCT_REQ" = "xtrue"])

Copilot uses AI. Check for mistakes.
# Checks for header files.
AC_CHECK_HEADERS([stdlib.h string.h unistd.h])

Expand Down
9 changes: 8 additions & 1 deletion source/DHCPMgrUtils/dhcpmgr_v6_lease_handler.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@
#include "dhcpmgr_recovery_handler.h"
#include "dhcpmgr_custom_options.h"
#include "ifl.h"

#if defined(_ONESTACK_PRODUCT_REQ_)

Copilot AI Mar 5, 2026

Copy link

Choose a reason for hiding this comment

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

This code is guarded by _ONESTACK_PRODUCT_REQ_, but the build-system change introduces an Automake conditional named ONESTACK_PRODUCT_REQ (without leading/trailing underscores) and does not define the _ONESTACK_PRODUCT_REQ_ preprocessor symbol. Unless _ONESTACK_PRODUCT_REQ_ is already provided externally via compiler flags, this block will never compile; consider aligning the names and ensuring the C macro is defined (e.g., via CPPFLAGS/CFLAGS or a config header that is actually included).

Suggested change
#if defined(_ONESTACK_PRODUCT_REQ_)
#if defined(ONESTACK_PRODUCT_REQ) || defined(_ONESTACK_PRODUCT_REQ_)

Copilot uses AI. Check for mistakes.
#define COSA_DML_PREF_DELEGATION_SYSEVENT_NAME "ipv6_prefix_delegation"
#define COSA_DML_WANIface_PREF_DELEGATION_SYSEVENT_NAME "ipv6_%s_prefix_delegation"
#endif
Comment thread
rirfha948 marked this conversation as resolved.
#define COSA_DML_WANIface_PREF_SYSEVENT_NAME "tr_%s_dhcpv6_client_v6pref"
#define COSA_DML_WANIface_PREF_IAID_SYSEVENT_NAME "tr_%s_dhcpv6_client_pref_iaid"
#define COSA_DML_WANIface_PREF_T1_SYSEVENT_NAME "tr_%s_dhcpv6_client_pref_t1"
Expand Down Expand Up @@ -287,6 +290,10 @@ static void ConfigureIpv6Sysevents(PCOSA_DML_DHCPCV6_FULL pDhcp6c)

IPv6Events eventv6[] = {
{ia_pd_prefix, COSA_DML_WANIface_PREF_SYSEVENT_NAME},
#if defined(_ONESTACK_PRODUCT_REQ_)
{ia_pd_prefix, COSA_DML_PREF_DELEGATION_SYSEVENT_NAME},
{ia_pd_prefix, COSA_DML_WANIface_PREF_DELEGATION_SYSEVENT_NAME},

Copilot AI Feb 26, 2026

Copy link

Choose a reason for hiding this comment

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

This line uses a tab character for indentation, which is inconsistent with the surrounding code that uses spaces. The codebase appears to use spaces for indentation (as seen in lines 294, 298-302). Please use consistent spacing to match the rest of the file.

Suggested change
{ia_pd_prefix, COSA_DML_WANIface_PREF_DELEGATION_SYSEVENT_NAME},
{ia_pd_prefix, COSA_DML_WANIface_PREF_DELEGATION_SYSEVENT_NAME},

Copilot uses AI. Check for mistakes.
Comment thread
rirfha948 marked this conversation as resolved.
#endif
Comment on lines +293 to +296

Copilot AI Feb 26, 2026

Copy link

Choose a reason for hiding this comment

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

According to the PR description, this change should "Handle IPv6 delegation for business vs residential Partner ID as part of single build" with different behavior in Business-mode versus Residential-mode. However, the current implementation unconditionally adds the prefix delegation sysevent whenever ONESTACK_PRODUCT_REQ is defined, without any runtime check to distinguish between business and residential modes. If runtime mode differentiation is required as suggested by the PR description, the code should use the rdkb_feature_mode_gate.h header to check the device mode and conditionally set the sysevent accordingly. Otherwise, if compile-time differentiation is sufficient, the PR description should be clarified.

Copilot uses AI. Check for mistakes.
{iapd_iaid, COSA_DML_WANIface_PREF_IAID_SYSEVENT_NAME},
{iapd_t1, COSA_DML_WANIface_PREF_T1_SYSEVENT_NAME},
{iapd_t2, COSA_DML_WANIface_PREF_T2_SYSEVENT_NAME},
Expand Down