Skip to content

Conversation

@S-Parthiban-Selvaraj
Copy link
Contributor

@S-Parthiban-Selvaraj S-Parthiban-Selvaraj commented Nov 15, 2024

This PR moves WAN IPv6 configuration from the LAN bridge to the WAN interface itself, addressing an issue where IANA is not assigned by the BNG (Broadband Network Gateway). The changes enable the creation of IPv6 addresses on WAN interfaces from delegated prefixes when IANA addresses are not provided.

Key changes:

Removed IPv6 configuration from LAN bridge and moved it to WAN interfaces
Added new function to construct WAN IPv6 addresses from IAPD (IA Prefix Delegation)
Updated IPv6 utility functions to work with virtual interfaces instead of interface names
Cleaned up LAN-specific IPv6 handling code and legacy bridge mode logic

This PR is dependent on the following related PRs:
rdkcentral/telco-voice-manager#5
rdkcentral/utopia#69
#79
rdkcentral/provisioning-and-management#127

S-Parthiban-Selvaraj and others added 30 commits July 16, 2024 14:26
Reason for change:  Solving Build errors

Test Procedure:
    Updated in Jira.

Risks: none
Priority: P1

Signed-off-by: parthiban.selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: parthiban.selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: parthiban.selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
…eint after WFO

Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Fixing Virtual interface name set.

Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
…ive VISM for products use the same default name for all interfaces

Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
…nager

Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: S-Parthiban-Selvaraj <parthiban.selvaraj@sky.uk>
@S-Parthiban-Selvaraj S-Parthiban-Selvaraj changed the title RDKB-50962 : This is the change to create WAN ipv6 address from the delegated pref… RDKB-58910 : Move the WAN IPV6 configuration from LAN bridge Jun 6, 2025
S-Parthiban-Selvaraj and others added 6 commits June 6, 2025 13:09
Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
LakshminarayananShenbagaraj pushed a commit that referenced this pull request Aug 19, 2025
New Tags:

https://github.com/rdkcentral/RdkWanManager/releases/tag/v2.7.0

What's Changed
RDKCOM-5173 REFPLTB-3128: [TDK][AUTO][RPI4][CELLULAR MANAGER] Switching back to ethwan from cellular mode not working as expected by @ksaipr036
RDKB-57254 : MAP-T Unification by @S-Parthiban-Selvaraj in

Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves WAN IPv6 configuration from the LAN bridge to the WAN interface itself, addressing an issue where IANA is not assigned by the BNG (Broadband Network Gateway). The changes enable the creation of IPv6 addresses on WAN interfaces from delegated prefixes when IANA addresses are not provided.

Key changes:

  • Removed IPv6 configuration from LAN bridge and moved it to WAN interfaces
  • Added new function to construct WAN IPv6 addresses from IAPD (IA Prefix Delegation)
  • Updated IPv6 utility functions to work with virtual interfaces instead of interface names
  • Cleaned up LAN-specific IPv6 handling code and legacy bridge mode logic

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
wanmgr_sysevents.h Removed LAN sync support structures and duplicate sysevent registrations
wanmgr_sysevents.c Cleaned up LAN state variables and duplicate sysevent handling
wanmgr_net_utils.h/c Refactored IPv6 utility function to use virtual interface pointer
wanmgr_ipc.h/c Updated IHC messaging to use virtual interface structure
wanmgr_interface_sm.c Major refactoring of IPv6 address handling and state management
wanmgr_dhcpv6_apis.h/c Added new function to construct WAN addresses from IAPD
wanmgr_dhcp_event_handler.c Updated to use new IPv6 utility functions
dhcpv6c_msg_apis.c Added validation for prefix assignment
wanmgr_rdkbus_common.h Removed obsolete bridge interface definition
Comments suppressed due to low confidence (1)

source/WanManager/wanmgr_dhcpv6_apis.c:1

  • This condition appears incorrect. It checks for neither address nor prefix assigned, but the comment suggests it should handle the case where only IAPD is received. The condition should be !pDhcpv6Data->addrAssigned && pDhcpv6Data->prefixAssigned.
/*

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@S-Parthiban-Selvaraj S-Parthiban-Selvaraj marked this pull request as ready for review January 20, 2026 11:13
@S-Parthiban-Selvaraj S-Parthiban-Selvaraj requested a review from a team as a code owner January 20, 2026 11:13
Copilot AI review requested due to automatic review settings January 20, 2026 11:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 623 to 624
sysevent_set_options(sysevent_msg_fd, sysevent_msg_token, SYSEVENT_IPV6_TOGGLE, TUPLE_FLAG_EVENT);
sysevent_setnotification(sysevent_msg_fd, sysevent_msg_token, SYSEVENT_IPV6_TOGGLE, &default_route_change_event_asyncid);
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Duplicate sysevent registration for SYSEVENT_IPV6_TOGGLE. Lines 620-621 are identical to lines 623-624, causing the same event to be registered twice with the same parameters.

Suggested change
sysevent_set_options(sysevent_msg_fd, sysevent_msg_token, SYSEVENT_IPV6_TOGGLE, TUPLE_FLAG_EVENT);
sysevent_setnotification(sysevent_msg_fd, sysevent_msg_token, SYSEVENT_IPV6_TOGGLE, &default_route_change_event_asyncid);

Copilot uses AI. Check for mistakes.
if(dad_flag == 0 || route_flag == 0)
{
CcspTraceError(("%s %d dad_flag[%d] route_flag[%d] Failed \n", __FUNCTION__, __LINE__,dad_flag,route_flag));
CcspTraceError(("%s %d dad_flag[%s] route_flag[%s] Failed \n", __FUNCTION__, __LINE__,dad_flag ?"SUCCESS":"FAILED",route_flag?"SUCCESS":"FAILED"));
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Format string expects integer values (%d) but provides strings. The format specifiers should be %s to match the ternary expressions that output strings.

Copilot uses AI. Check for mistakes.
{
/* In an IPv6 lease, if only IAPD is received and we never received IANA,
* We can use the received IAPD to construct a Ipv6 /128 address which can be used for managerment and voice ...
* If we reach this point, only IAPD has been received. Canculate Wan Ipv6 address
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Corrected spelling of 'Canculate' to 'Calculate'.

Suggested change
* If we reach this point, only IAPD has been received. Canculate Wan Ipv6 address
* If we reach this point, only IAPD has been received. Calculate Wan Ipv6 address

Copilot uses AI. Check for mistakes.
{
/* In an IPv6 lease, if only IAPD is received and we never received IANA,
* We can use the received IAPD to construct a Ipv6 /128 address which can be used for managerment and voice ...
* If we reach this point, only IAPD has been received. Canculate Wan Ipv6 address
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Corrected spelling of 'Canculate' to 'Calculate'.

Suggested change
* If we reach this point, only IAPD has been received. Canculate Wan Ipv6 address
* If we reach this point, only IAPD has been received. Calculate Wan Ipv6 address

Copilot uses AI. Check for mistakes.
// Convert prefix to binary format
if (inet_pton(AF_INET6, iapd_prefix, &prefix) != 1)
{
fprintf(stderr, "Invalid IPv6 prefix\n");
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Using fprintf to stderr for error logging is inconsistent with the rest of the codebase which uses CcspTraceError. The line immediately following (1626) already logs the error properly using CcspTraceError, making this fprintf redundant.

Suggested change
fprintf(stderr, "Invalid IPv6 prefix\n");

Copilot uses AI. Check for mistakes.
return -1;
}

prefix.s6_addr[7] += 0x01; // Use next subnet for WAN. First /64 will be used for LAN.
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Hardcoded array index 7 for subnet increment lacks explanation. Consider adding a comment explaining why byte 7 is used for /56 to /64 subnet splitting, or define a constant for clarity (e.g., IPV6_SUBNET_BYTE_INDEX).

Copilot uses AI. Check for mistakes.
pDhcpv6Data->prefixAssigned = leaseInfo->prefixAssigned;
pDhcpv6Data->domainNameAssigned = leaseInfo->domainNameAssigned;
pDhcpv6Data->ipv6_TimeOffset = leaseInfo->ipv6_TimeOffset;
if(!pDhcpv6Data->addrAssigned && !pDhcpv6Data->prefixAssigned)
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Logic error: the condition checks if both addrAssigned and prefixAssigned are false, but according to the comment and context, the intention is to construct a WAN address when IANA is not assigned but IAPD is received. The condition should be if(!pDhcpv6Data->addrAssigned && pDhcpv6Data->prefixAssigned) to match the stated behavior.

Suggested change
if(!pDhcpv6Data->addrAssigned && !pDhcpv6Data->prefixAssigned)
if(!pDhcpv6Data->addrAssigned && pDhcpv6Data->prefixAssigned)

Copilot uses AI. Check for mistakes.
WanMgr_Rbus_EventPublishHandler(param_name, "", RBUS_STRING);
WanManager_UpdateInterfaceStatus (p_VirtIf, WANMGR_IFACE_CONNECTION_IPV6_DOWN);

//Disable accept_ra
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Comment lacks proper spacing. Should be '// Disable accept_ra' with space after '//'.

Suggested change
//Disable accept_ra
// Disable accept_ra

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings January 20, 2026 11:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

* In this scenario, we should not consider IANA or IAPD as deleted.
* If we reach this point, only IAPD has been renewed. Use the previous IANA details. */
* If we reach this point, only IAPD has been renewed. Use the previous IANA details.
*/
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Corrected spelling of 'Canculate' to 'Calculate'.

Copilot uses AI. Check for mistakes.
Comment on lines +110 to +113
* If we reach this point, only IAPD has been received. Canculate Wan Ipv6 address
*/

CcspTraceInfo(("IANA is not assigned by DHCPV6. Constructing WAN address from the IAPD for Wan Interface \n"));
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The term 'Wan' should be written as 'WAN' (all caps) for consistency with standard networking terminology and the rest of the codebase.

Suggested change
* If we reach this point, only IAPD has been received. Canculate Wan Ipv6 address
*/
CcspTraceInfo(("IANA is not assigned by DHCPV6. Constructing WAN address from the IAPD for Wan Interface \n"));
* If we reach this point, only IAPD has been received. Canculate WAN Ipv6 address
*/
CcspTraceInfo(("IANA is not assigned by DHCPV6. Constructing WAN address from the IAPD for WAN Interface \n"));

Copilot uses AI. Check for mistakes.
if(dad_flag == 0 || route_flag == 0)
{
CcspTraceError(("%s %d dad_flag[%d] route_flag[%d] Failed \n", __FUNCTION__, __LINE__,dad_flag,route_flag));
CcspTraceError(("%s %d dad_flag[%s] route_flag[%s] Failed \n", __FUNCTION__, __LINE__,dad_flag ?"SUCCESS":"FAILED",route_flag?"SUCCESS":"FAILED"));
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Missing spaces around the ternary operator's '?' and after commas in the format string arguments. This should be formatted as: dad_flag ? \"SUCCESS\" : \"FAILED\", route_flag ? \"SUCCESS\" : \"FAILED\"

Suggested change
CcspTraceError(("%s %d dad_flag[%s] route_flag[%s] Failed \n", __FUNCTION__, __LINE__,dad_flag ?"SUCCESS":"FAILED",route_flag?"SUCCESS":"FAILED"));
CcspTraceError(("%s %d dad_flag[%s] route_flag[%s] Failed \n", __FUNCTION__, __LINE__, dad_flag ? "SUCCESS" : "FAILED", route_flag ? "SUCCESS" : "FAILED"));

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants