Skip to content

RDKB-58910, RDKB-60010 : Move the WAN IPV6 configuration from brlan0#69

Open
S-Parthiban-Selvaraj wants to merge 17 commits intodevelopfrom
feature/wan_Ipv6_address_from_IAPD
Open

RDKB-58910, RDKB-60010 : Move the WAN IPV6 configuration from brlan0#69
S-Parthiban-Selvaraj wants to merge 17 commits intodevelopfrom
feature/wan_Ipv6_address_from_IAPD

Conversation

@S-Parthiban-Selvaraj
Copy link
Copy Markdown
Contributor

@S-Parthiban-Selvaraj S-Parthiban-Selvaraj commented Sep 18, 2025

Reason for change:
[NTP] Remove sky specific workarounds from the NTP script .

Test Procedure:
Updated in Jira.

Risks: none
Priority: P1

This PR is dependent on the following related PRs:
rdkcentral/telco-voice-manager#5
#69
rdkcentral/wan-manager#79
rdkcentral/provisioning-and-management#127
rdkcentral/xconf-client#20
rdkcentral/test-and-diagnostic#172
https://github.com/rdk-gdcs/firewall/pull/5
rdkcentral/sysint-broadband#34

Reason for change:
[NTP] Remove sky specific workarounds from the NTP script .

Test Procedure:
Updated in Jira.

Risks: none
Priority: P1

Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Copilot AI review requested due to automatic review settings November 7, 2025 18:36
Copy link
Copy Markdown
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI review requested due to automatic review settings November 24, 2025 20:38
@jonathanwu-csv jonathanwu-csv force-pushed the feature/wan_Ipv6_address_from_IAPD branch from 98f1db8 to a20ac26 Compare November 24, 2025 20:38
Copy link
Copy Markdown
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Reason for change: Remove Sky specific workaround from NTP script
Test Procedure: None
Risks: None
Priority: P1

Signed-off-by: Jonathan Wu <Jonathan_Wu@comcast.com>
@jonathanwu-csv jonathanwu-csv force-pushed the feature/wan_Ipv6_address_from_IAPD branch from a20ac26 to 7ea7a04 Compare November 24, 2025 21:52
Copilot AI review requested due to automatic review settings November 25, 2025 16:58
Copy link
Copy Markdown
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@S-Parthiban-Selvaraj S-Parthiban-Selvaraj force-pushed the feature/wan_Ipv6_address_from_IAPD branch 2 times, most recently from a44a4bd to d1e188c Compare January 16, 2026 14:14
Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
@S-Parthiban-Selvaraj S-Parthiban-Selvaraj force-pushed the feature/wan_Ipv6_address_from_IAPD branch from d1e188c to 89263a8 Compare January 16, 2026 14:16
@S-Parthiban-Selvaraj S-Parthiban-Selvaraj marked this pull request as ready for review April 15, 2026 17:03
@S-Parthiban-Selvaraj S-Parthiban-Selvaraj requested review from a team as code owners April 15, 2026 17:03
Copilot AI review requested due to automatic review settings April 15, 2026 17:03
Copy link
Copy Markdown
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 1 out of 1 changed files in this pull request and generated 1 comment.


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

@@ -572,19 +545,6 @@
echo "interface ignore wildcard" >> $NTP_CONF_TMP
echo "interface listen 127.0.0.1" >> $NTP_CONF_TMP
echo "interface listen ::1" >> $NTP_CONF_TMP
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

sysevent set ntp_ipv6_listen was removed from service_start(), but the ipv6_connection_state) handler later reads ntp_ipv6_listen to decide whether to restart NTP. With the flag never being set to "set", the condition can remain true and trigger unnecessary/repeated restarts even when the prefix is unchanged. Either restore updating ntp_ipv6_listen (set/unset) in service_start() or remove/adjust the downstream logic to no longer depend on this flag.

Suggested change
echo "interface listen ::1" >> $NTP_CONF_TMP
echo "interface listen ::1" >> $NTP_CONF_TMP
sysevent set ntp_ipv6_listen set

Copilot uses AI. Check for mistakes.
# Setting Time status as Unsynchronized
syscfg set ntp_status 2

if [ "$BOX_TYPE" = "HUB4" ] || [ "$BOX_TYPE" = "SR300" ] || [ "$BOX_TYPE" = "SE501" ] || [ "$BOX_TYPE" = "WNXL11BWL" ] || [ "$BOX_TYPE" = "SR213" ] || [ "$LANIPV6Support" = "true" ]; then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to handle here as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is handling the ipv6 sysevent,
I didn't touch this sysevent.

Copilot AI review requested due to automatic review settings April 24, 2026 13:19
Copy link
Copy Markdown
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 3 out of 3 changed files in this pull request and generated 3 comments.


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

elif [ "$BOX_TYPE" = "SCER11BEL" -a "$LANIPV6Support" = "true" ]; then
dropbear -E -s -b /etc/sshbanner.txt $commandString -r $DROPBEAR_PARAMS_1 -r $DROPBEAR_PARAMS_2 -a -P $PID_FILE -K 60 $USE_DEVKEYS 2>>$CONSOLEFILE
else
dropbear -E -s -b /etc/sshbanner.txt -a -r $DROPBEAR_PARAMS_1 -r $DROPBEAR_PARAMS_2 -p [$CM_IP]:22 -P $PID_FILE $USE_DEVKEYS 2>>$CONSOLEFILE
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

The SCER11BEL + LANIPv6GUASupport=true path was removed, so SCER11BEL now falls through to the generic dropbear ... -p [$CM_IP]:22 invocation. If $CM_IP is empty (e.g., WAN IPv6 GUA not yet configured at the time this script runs), this will produce an invalid listen spec ([]:22) and can prevent dropbear from starting. Consider reusing get_listen_params/LISTEN_PARAMS for this case, or explicitly guarding for an empty $CM_IP (bind to localhost and rely on a later restart), and/or ensure the service waits for a global address before launching dropbear on SCER11BEL.

Suggested change
dropbear -E -s -b /etc/sshbanner.txt -a -r $DROPBEAR_PARAMS_1 -r $DROPBEAR_PARAMS_2 -p [$CM_IP]:22 -P $PID_FILE $USE_DEVKEYS 2>>$CONSOLEFILE
if [ -n "$CM_IP" ] ; then
dropbear -E -s -b /etc/sshbanner.txt -a -r $DROPBEAR_PARAMS_1 -r $DROPBEAR_PARAMS_2 -p [$CM_IP]:22 -P $PID_FILE $USE_DEVKEYS 2>>$CONSOLEFILE
else
echo_t "[utopia]: CM_IP is empty; starting dropbear on localhost until a valid global address is available."
dropbear -E -s -b /etc/sshbanner.txt -a -r $DROPBEAR_PARAMS_1 -r $DROPBEAR_PARAMS_2 -p 127.0.0.1:22 -P $PID_FILE $USE_DEVKEYS 2>>$CONSOLEFILE
fi

Copilot uses AI. Check for mistakes.
Comment on lines 544 to 548
# interface rules can't be written to quick sync conf file so write here after quick sync conf file creation.
echo "interface ignore wildcard" >> $NTP_CONF_TMP
echo "interface listen 127.0.0.1" >> $NTP_CONF_TMP
echo "interface listen ::1" >> $NTP_CONF_TMP
#SHARMAN-2301
#This change is for UK MAP-T SR213. Since we will not have any of the global IP on WAN interface, We need to add the IPv6 interface (currently brlan0) to the config file
if [ "$BOX_TYPE" = "SR213" ] || [ "$LANIPV6Support" = "true" ]; then
MAPT_STATS=$(sysevent get mapt_config_flag)
echo_t "SERVICE_NTPD : MAPT_STATS=$MAPT_STATS"
if [ x"$MAPT_STATS" = x"set" ]; then
IPV4_CONN_STATE=$(sysevent get ipv4_connection_state)
echo_t "SERVICE_NTPD : IPV4_CONN_STATE=$IPV4_CONN_STATE"
if [ x"$IPV4_CONN_STATE" != x"up" ]; then
echo "interface listen $NTPD_IPV6_INTERFACE" >> $NTP_CONF_TMP
fi
fi
fi

Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

ipv6_connection_state handling later in this script gates restarts on sysevent get ntp_ipv6_listen, but this PR removed the only place where ntp_ipv6_listen was set/unset while generating ntp.conf. As a result, ntp_ipv6_listen will remain empty and the ipv6_connection_state handler condition will continue to evaluate true, causing repeated/unnecessary service_start restarts whenever the event fires. Either restore setting ntp_ipv6_listen (and align it with the current interface listen behavior), or simplify the ipv6_connection_state condition to only compare the prefix/state you still maintain.

Copilot uses AI. Check for mistakes.
Comment on lines 49 to 54
NTP_CONF_TMP=/tmp/ntp.conf
NTP_CONF_QUICK_SYNC=/tmp/ntp_quick_sync.conf
LOCKFILE=/var/tmp/service_ntpd.pid
BIN=ntpd
WAN_IPv6_UP=0
QUICK_SYNC_PID=""
QUICK_SYNC_DONE=0
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

The PR title targets moving WAN IPv6 configuration off brlan0, but the PR description is focused on removing Sky-specific NTP workarounds. Since this change set also touches the SSH daemon start logic and DHCP server functions, please update the PR description (or title) so it accurately reflects the scope and intent of the changes in this repo.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants