rebase - #432
Open
nhanasi wants to merge 182 commits into
Open
Conversation
Sysint 4.2.1 release with network updates
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
RDK-59247: cleaning up network scripts.
…ts To C Implementation (#410) * Update Start_MaintenanceTasks.sh * Update lib/rdk/Start_MaintenanceTasks.sh Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update lib/rdk/Start_MaintenanceTasks.sh Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Start_MaintenanceTasks.sh * Update Start_MaintenanceTasks.sh * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Start_MaintenanceTasks.sh * Update Start_MaintenanceTasks.sh * Update Start_MaintenanceTasks.sh * Update Start_MaintenanceTasks.sh * Update Start_MaintenanceTasks.sh --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Saranya2421 <saranya.suvi@gmail.com>
Sysint 4.2.2 release for logupload migration to C
|
I have read the CLA Document and I hereby sign the CLA 12 out of 13 committers have signed the CLA. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR consolidates multiple changes from a rebase operation, including network script refactoring, log upload binary migration, and configuration updates.
Changes:
- Deleted
updateGlobalIPInfo.shand refactored its functionality intoNM_Dispatcher.shandNM_preDown.sh - Added support for a binary log upload implementation with fallback to script-based approach in
Start_MaintenanceTasks.sh - Added
com.comcast.viper_ipato warehouse testing exceptions inwh_api_5.conf
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/rdk/wh_api_5.conf | Added viper_ipa app to warehouse testing exception list |
| lib/rdk/updateGlobalIPInfo.sh | Deleted file - functionality moved to NM dispatcher scripts |
| lib/rdk/Start_MaintenanceTasks.sh | Integrated binary-based log upload with fallback to shell script |
| lib/rdk/NM_preDown.sh | Refactored IP deletion logic from updateGlobalIPInfo.sh |
| lib/rdk/NM_Dispatcher.sh | Refactored IP addition logic from updateGlobalIPInfo.sh |
| CHANGELOG.md | Updated changelog for version 4.2.2 with merged PRs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+60
to
+68
| if [ "x$mode" == "xipv6" ]; then | ||
| if [[ $addr == fc* || $addr == fd* ]]; then | ||
| return 1 | ||
| fi | ||
| elif [ "x$mode" == "xipv4" ]; then | ||
| autoIPTrunc=$(echo $addr | cut -d "." -f1-2) | ||
| if [ "$autoIPTrunc" == "169.254" ]; then | ||
| return 1 | ||
| fi |
There was a problem hiding this comment.
The check_valid_IPaddress function has inconsistent implementations between NM_preDown.sh and NM_Dispatcher.sh. In NM_preDown.sh, it uses POSIX-compliant single equals (=) and a case statement for IPv6 pattern matching. In NM_Dispatcher.sh, it uses bash-specific double equals (==) and double bracket conditionals. These functions should be identical to maintain consistency and predictable behavior across both scripts.
Suggested change
| if [ "x$mode" == "xipv6" ]; then | |
| if [[ $addr == fc* || $addr == fd* ]]; then | |
| return 1 | |
| fi | |
| elif [ "x$mode" == "xipv4" ]; then | |
| autoIPTrunc=$(echo $addr | cut -d "." -f1-2) | |
| if [ "$autoIPTrunc" == "169.254" ]; then | |
| return 1 | |
| fi | |
| if [ "x$mode" = "xipv6" ]; then | |
| case "$addr" in | |
| fc*|fd*) | |
| return 1 | |
| ;; | |
| esac | |
| elif [ "x$mode" = "xipv4" ]; then | |
| autoIPTrunc=$(echo "$addr" | cut -d "." -f1-2) | |
| if [ "$autoIPTrunc" = "169.254" ]; then | |
| return 1 | |
| fi |
Comment on lines
+188
to
+193
| sh $LOGUPLOAD_SCRIPT "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" "$TriggerType" 2>/dev/null | ||
| result=$? | ||
| fi | ||
| else | ||
| logUploadLog "logupload binary not found at $LOG_UPLOAD_BIN_PATH...executing script" | ||
| sh $LOGUPLOAD_SCRIPT "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" "$TriggerType" 2>&1 |
There was a problem hiding this comment.
Inconsistent parameter passed to binary vs script. The binary is called with "ondemand" as the 7th argument (line 181), but when falling back to the script (line 188), it's called with "$TriggerType" which is the numeric value 5. This parameter mismatch could cause different behavior between the binary and script paths.
Suggested change
| sh $LOGUPLOAD_SCRIPT "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" "$TriggerType" 2>/dev/null | |
| result=$? | |
| fi | |
| else | |
| logUploadLog "logupload binary not found at $LOG_UPLOAD_BIN_PATH...executing script" | |
| sh $LOGUPLOAD_SCRIPT "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" "$TriggerType" 2>&1 | |
| sh $LOGUPLOAD_SCRIPT "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" "ondemand" 2>/dev/null | |
| result=$? | |
| fi | |
| else | |
| logUploadLog "logupload binary not found at $LOG_UPLOAD_BIN_PATH...executing script" | |
| sh $LOGUPLOAD_SCRIPT "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" "ondemand" 2>&1 |
| return 0 | ||
| else | ||
| logUploadLog "Binary execution failed with result=$result; falling back to script" | ||
| nice -n 19 sh $LOGUPLOAD_SCRIPT "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" & |
There was a problem hiding this comment.
Trailing whitespace after the ampersand on this line. This is inconsistent with line 216 which has the same command structure without trailing whitespace.
Suggested change
| nice -n 19 sh $LOGUPLOAD_SCRIPT "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" & | |
| nice -n 19 sh $LOGUPLOAD_SCRIPT "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" & |
Comment on lines
+200
to
+216
| nice -n 19 "$LOG_UPLOAD_BIN_PATH" "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" >> /opt/logs/dcmscript.log & | ||
| bg_pid=$! | ||
| wait $bg_pid | ||
| result=$? | ||
| if [ "$result" -eq 0 ]; then | ||
| logUploadLog "Binary execution succeeded" | ||
| return 0 | ||
| else | ||
| logUploadLog "Binary execution failed with result=$result; falling back to script" | ||
| nice -n 19 sh $LOGUPLOAD_SCRIPT "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" & | ||
| bg_pid=$! | ||
| wait $bg_pid | ||
| result=$? | ||
| fi | ||
| else | ||
| logUploadLog "logupload binary not found at $LOG_UPLOAD_BIN_PATH...executing script" | ||
| nice -n 19 sh $LOGUPLOAD_SCRIPT "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" & |
There was a problem hiding this comment.
The binary call on line 200 is missing the trigger type parameter (7th argument) that the script receives on line 216. When the binary succeeds, no trigger type is passed, but when it fails and falls back to the script, no trigger type is passed either. However, in the on-demand path (line 181), the binary receives "ondemand" as the 7th argument. This inconsistency between regular and on-demand execution paths could lead to different behavior.
Comment on lines
+90
to
+98
| echo "$addr" > /tmp/.$mode$ESTB_INTERFACE | ||
| refresh_devicedetails "estb_ip" | ||
| elif [[ "$ifc" == "$MOCA_INTERFACE" || "$ifc" == "$MOCA_INTERFACE:0" ]]; then | ||
| NMdispatcherLog "Updating MoCA IP" | ||
| echo "$addr" > /tmp/.$mode$MOCA_INTERFACE | ||
| refresh_devicedetails "moca_ip" | ||
| elif [[ "$ifc" == "$WIFI_INTERFACE" || "$ifc" == "$WIFI_INTERFACE:0" ]]; then | ||
| NMdispatcherLog "Updating Wi-Fi IP" | ||
| echo "$addr" > /tmp/.$mode$WIFI_INTERFACE |
There was a problem hiding this comment.
The echo "$addr" > /tmp/.$mode$ESTB_INTERFACE (and similar for MOCA_INTERFACE and WIFI_INTERFACE) writes to a predictable path in /tmp as root without any protection against symlink attacks. A local attacker who can create a symlink at /tmp/.$mode$ESTB_INTERFACE (or the other variants) can cause this script to truncate or overwrite an arbitrary file (e.g., /etc/shadow), leading to privilege escalation or data corruption. Use a safer pattern for temporary files (e.g., a dedicated directory with restricted permissions or APIs that open files with O_NOFOLLOW/proper checks) so that writes cannot be redirected via attacker-controlled symlinks.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (1)
lib/rdk/readBTAddress-generic.sh:31
- readBTAddress-generic.sh may echo an empty string when BLUETOOTH_ENABLED is false because bluetooth_mac is never initialized. Callers (e.g., getDeviceDetails.sh) typically expect a stable default value (like 00:00:00:00:00:00).
Comment on lines
+48
to
+50
| if [ -f /lib/rdk/utils-vendor.sh ]; then | ||
| . $RDK_PATH/utils-vendor.sh | ||
| fi |
Comment on lines
284
to
+288
| bluetooth_mac="00:00:00:00:00:00" | ||
| if [ "$BLUETOOTH_ENABLED" = "true" ]; then | ||
| bluetooth_mac=$(getDeviceBluetoothMac) | ||
| if [ -f /lib/rdk/readBTAddress-vendor.sh ]; then | ||
| bluetooth_mac=`sh /lib/rdk/readBTAddress-vendor.sh` | ||
| else | ||
| bluetooth_mac=`sh /lib/rdk/readBTAddress-generic.sh` |
Comment on lines
+118
to
+122
| for f in /opt/secure/NetworkManager/system-connections/*; do | ||
| if grep -q "type=wifi" "$f"; then | ||
| rm -f "$f" | ||
| fi | ||
| done |
Comment on lines
95
to
99
| /bin/timestamp | ||
| uptime | ||
| uptime | ||
| run_top_command | ||
| log_disk_usage | ||
| cpu_statistics | ||
|
|
Comment on lines
+158
to
+170
| # Create a named pipe | ||
| PIPE=$(mktemp -u) | ||
| if ! mkfifo "$PIPE" 2>/dev/null; then | ||
| echo_t "STUNNEL: ERROR - Failed to create named pipe" | ||
| fi | ||
|
|
||
| # Open the pipe using the available FD, with error handling | ||
| if ! eval "exec $FD_NUMBER<>$PIPE" 2>/dev/null; then | ||
| echo_t "STUNNEL: ERROR - Failed to open pipe with file descriptor" | ||
| fi | ||
|
|
||
| # Removing the pipe after opening | ||
| rm "$PIPE" |
| rm "$PIPE" | ||
|
|
||
| # Writing passcode to open file descriptor | ||
| echo "$(eval "$PASSCODE")" >&$FD_NUMBER & |
Comment on lines
+101
to
+107
| if [ "x$PROCESS_NAME" == "xdeepSleepMgrMain" ]; then | ||
| # Message data is actual metadata header in case of trigger from deepSleep manager process | ||
| # This change is needed since there are data clouds in different deployment which are not flexible to accomodate any deviations in data format | ||
| strjson="{\"searchResult\":[{\"Time\":\"$currentTime\"},{\"process_name\":\"$PROCESS_NAME\"},{\"mac\":\"$estb_mac\"},{\"Version\":\"$software_version\"},{\"PartnerId\":\"$partnerId\"},{\"$MSG_DATA\":\"1\"}]}" | ||
| else | ||
| strjson="{\"searchResult\":[{\"process_name\":\"$PROCESS_NAME\"},{\"mac\":\"$estb_mac\"},{\"Version\":\"$software_version\"},{\"msgTime\":\"$currentTime\"},{\"PartnerId\":\"$partnerId\"},{\"logEntry\":\"$MSG_DATA\"}]}" | ||
| fi |
Comment on lines
31
to
34
| Environment="LOG_PATH=/opt/logs" | ||
| Environment="RDK_PATH=/lib/rdk" | ||
| ExecStart=/lib/rdk/backup_logs.sh | ||
| ExecStart=/usr/bin/backup_logs | ||
| RemainAfterExit=yes |
Comment on lines
+49
to
+51
| - RDKEMW-15490 : Removing reboot script reference from sysint [`#500`](https://github.com/rdkcentral/sysint/pull/500) | ||
| - 5.0.0 release changelog updates [`711601e`](https://github.com/rdkcentral/sysint/commit/711601e303a449f3ea34d5f858f4d7c5d873c9c1) | ||
| - Merge tag '4.5.5' into develop [`654be5f`](https://github.com/rdkcentral/sysint/commit/654be5fcc3473060f765481e5cd9dd4f84f65d95) |
* RDKEMW-21159: avoid thermal call in deepsleep * Apply suggestions from code review --------- Signed-off-by: apatel859 <Amit_Patel5@comcast.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 38 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (1)
lib/rdk/readBTAddress-generic.sh:31
- If
BLUETOOTH_ENABLEDis nottrue,bluetooth_macis never set and the script prints an empty line. Callers likegetDeviceDetails.shassign this output tobluetooth_mac, which would overwrite the intended default (00:00:00:00:00:00) with empty.
Comment on lines
+48
to
+50
| if [ -f /lib/rdk/utils-vendor.sh ]; then | ||
| . $RDK_PATH/utils-vendor.sh | ||
| fi |
Comment on lines
+268
to
+274
| for i in {1..9}; do | ||
| if ([ "$packetsLostipv4" -ge $((i*10)) ] && [ "$packetsLostipv4" -lt $((i*10+10)) ]) || ([ "$packetsLostipv6" -ge $((i*10)) ] && [ "$packetsLostipv6" -lt $((i*10+10)) ]); then | ||
| echo "$(/bin/timestamp) Current Packet loss is WIFIV_WARN_PL_"$((i*10))"PERC" >> "$logsFile" | ||
| t2CountNotify "WIFIV_WARN_PL_"$((i*10))"PERC" | ||
| break | ||
| fi | ||
| done |
Comment on lines
+42
to
+45
| if [ "$INTERFACE" != "$ETHERNET_INTERFACE" ] && [ "$INTERFACE" != "$WIFI_INTERFACE" ]; then | ||
| Log "INFO: Link-local not started for $INTERFACE (only eth0 and wlan0 allowed)" | ||
| exit 0 | ||
| fi |
Comment on lines
+61
to
+66
| get_bs_val() { | ||
| key="$1" | ||
| # Extract RHS after '=' and trim whitespace | ||
| grep -m1 -E "^[[:space:]]*$key=" "$BOOTSTRAP" 2>/dev/null | \ | ||
| cut -d'=' -f2- | sed 's/^[[:space:]]*//; s/[[:space:]]*$//' | ||
| } |
Comment on lines
+25
to
27
| Type=simple | ||
| ExecStart= /bin/sh -c '/lib/rdk/NM_Bootstrap.sh' | ||
|
|
Comment on lines
+158
to
+173
| # Create a named pipe | ||
| PIPE=$(mktemp -u) | ||
| if ! mkfifo "$PIPE" 2>/dev/null; then | ||
| echo_t "STUNNEL: ERROR - Failed to create named pipe" | ||
| fi | ||
|
|
||
| # Open the pipe using the available FD, with error handling | ||
| if ! eval "exec $FD_NUMBER<>$PIPE" 2>/dev/null; then | ||
| echo_t "STUNNEL: ERROR - Failed to open pipe with file descriptor" | ||
| fi | ||
|
|
||
| # Removing the pipe after opening | ||
| rm "$PIPE" | ||
|
|
||
| # Writing passcode to open file descriptor | ||
| echo "$(eval "$PASSCODE")" >&$FD_NUMBER & |
Comment on lines
+20
to
+23
| # Purpose: This script is used to backup the Logs | ||
| # Scope: RDK devices | ||
| # Usage: This script is triggered by systemd service | ||
| ############################################################################## |
Comment on lines
48
to
53
| /bin/systemctl stop dump-backup.service | ||
| /bin/systemctl stop dnsmasq.service | ||
| /bin/systemctl stop syslog.socket | ||
| /bin/systemctl stop wpeframework.service | ||
| if [ "$DOBBY_ENABLED" == "true" ]; then | ||
| /bin/systemctl stop dobby.service | ||
| fi |
…twork Connectivity Prior to Migration Completion (#571) Co-authored-by: mtirum011 <madhubabu_tirumala@comcast.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 39 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
lib/rdk/readBTAddress-generic.sh:31
- This script can output an empty string when Bluetooth is disabled, which then propagates to
getDeviceDetails.shand results in a missing Bluetooth MAC instead of a stable default. Also,$RDK_PATHmay be unset here, causing. $RDK_PATH/utils.shto fail. Initialize a default MAC and only override it when Bluetooth is enabled and the helper is available.
Comment on lines
+268
to
+274
| for i in {1..9}; do | ||
| if ([ "$packetsLostipv4" -ge $((i*10)) ] && [ "$packetsLostipv4" -lt $((i*10+10)) ]) || ([ "$packetsLostipv6" -ge $((i*10)) ] && [ "$packetsLostipv6" -lt $((i*10+10)) ]); then | ||
| echo "$(/bin/timestamp) Current Packet loss is WIFIV_WARN_PL_"$((i*10))"PERC" >> "$logsFile" | ||
| t2CountNotify "WIFIV_WARN_PL_"$((i*10))"PERC" | ||
| break | ||
| fi | ||
| done |
Comment on lines
+118
to
+121
| for f in /opt/secure/NetworkManager/system-connections/*; do | ||
| if grep -q "type=wifi" "$f"; then | ||
| rm -f "$f" | ||
| fi |
Comment on lines
+158
to
+170
| # Create a named pipe | ||
| PIPE=$(mktemp -u) | ||
| if ! mkfifo "$PIPE" 2>/dev/null; then | ||
| echo_t "STUNNEL: ERROR - Failed to create named pipe" | ||
| fi | ||
|
|
||
| # Open the pipe using the available FD, with error handling | ||
| if ! eval "exec $FD_NUMBER<>$PIPE" 2>/dev/null; then | ||
| echo_t "STUNNEL: ERROR - Failed to open pipe with file descriptor" | ||
| fi | ||
|
|
||
| # Removing the pipe after opening | ||
| rm "$PIPE" |
Comment on lines
101
to
106
| extract_stunnel_client_cert | ||
|
|
||
| if [ ! -f $CERT_FILE -o ! -f $CA_FILE ]; then | ||
| if [ ! -f $CERT_PATH -o ! -f $CA_FILE ]; then | ||
| echo_t "STUNNEL: Required cert/CA file not found. Exiting..." | ||
| t2CountNotify "SHORTS_STUNNEL_CERT_FAILURE" | ||
| exit 1 |
…f develop (#572) Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 39 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (7)
lib/rdk/readBTAddress-generic.sh:31
bluetooth_macis echoed even when Bluetooth is disabled, but it’s never initialized in that case, so callers can receive an empty value. Initialize it to a predictable default and quote the output.
lib/rdk/getDeviceDetails.sh:50- The existence check uses
/lib/rdk/utils-vendor.sh, but the script sources$RDK_PATH/utils-vendor.sh. IfRDK_PATHdiffers from/lib/rdk, this will fail even though the file exists. Use the same path for both the check and sourcing.
if [ -f /lib/rdk/utils-vendor.sh ]; then
. $RDK_PATH/utils-vendor.sh
fi
lib/rdk/NM_Bootstrap.sh:122
- If
/opt/secure/NetworkManager/system-connections/*matches no files, the glob remains literal andgrepwill error (and can also result in attempting to delete a non-existent literal path). Guard the loop for the empty-glob case.
for f in /opt/secure/NetworkManager/system-connections/*; do
if grep -q "type=wifi" "$f"; then
rm -f "$f"
fi
done
lib/rdk/timesyncd-conf-update.sh:66
get_bs_valbuilds agrep -Eregex using an unescaped key (e.g.,Device.Time.NTPServer1), so.will match any character and can accidentally match the wrong line inbootstrap.ini. Escape regex metacharacters (at least.) before building the pattern.
get_bs_val() {
key="$1"
# Extract RHS after '=' and trim whitespace
grep -m1 -E "^[[:space:]]*$key=" "$BOOTSTRAP" 2>/dev/null | \
cut -d'=' -f2- | sed 's/^[[:space:]]*//; s/[[:space:]]*$//'
}
lib/rdk/startLinkLocal.sh:44
- The log message says only "eth0 and wlan0" are allowed, but the actual allowlist is
$ETHERNET_INTERFACEand$WIFI_INTERFACE. Update the message to reflect what the code enforces.
if [ "$INTERFACE" != "$ETHERNET_INTERFACE" ] && [ "$INTERFACE" != "$WIFI_INTERFACE" ]; then
Log "INFO: Link-local not started for $INTERFACE (only eth0 and wlan0 allowed)"
exit 0
lib/rdk/startStunnel.sh:170
- Using
mktemp -uto pick a FIFO path is vulnerable to TOCTOU races (another process can create that path beforemkfifo). Create a private temp directory withmktemp -dand create the FIFO inside it, failing hard if any step fails.
# Create a named pipe
PIPE=$(mktemp -u)
if ! mkfifo "$PIPE" 2>/dev/null; then
echo_t "STUNNEL: ERROR - Failed to create named pipe"
fi
# Open the pipe using the available FD, with error handling
if ! eval "exec $FD_NUMBER<>$PIPE" 2>/dev/null; then
echo_t "STUNNEL: ERROR - Failed to open pipe with file descriptor"
fi
# Removing the pipe after opening
rm "$PIPE"
lib/rdk/alertSystem.sh:107
strjsonis built by interpolating unescaped user-provided strings (PROCESS_NAME,MSG_DATA, etc.). If these contain quotes, backslashes, or newlines the JSON becomes invalid and can be used for injection. Escape JSON string values, and sanitize the dynamic key used for the deepSleepMgrMain case.
strjson="{\"searchResult\":[{\"Time\":\"$currentTime\"},{\"process_name\":\"$PROCESS_NAME\"},{\"mac\":\"$estb_mac\"},{\"Version\":\"$software_version\"},{\"PartnerId\":\"$partnerId\"},{\"$MSG_DATA\":\"1\"}]}"
else
strjson="{\"searchResult\":[{\"process_name\":\"$PROCESS_NAME\"},{\"mac\":\"$estb_mac\"},{\"Version\":\"$software_version\"},{\"msgTime\":\"$currentTime\"},{\"PartnerId\":\"$partnerId\"},{\"logEntry\":\"$MSG_DATA\"}]}"
fi
Release 6.0.1 6.0.1
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (7)
lib/rdk/timesyncd-conf-update.sh:65
- In get_bs_val(), the grep pattern treats the key as an ERE. Since the keys contain dots (e.g., Device.Time.NTPServer1), this can match unintended lines ('.' matches any char). Use an exact key comparison (e.g., awk on the LHS) to avoid regex interpretation.
get_bs_val() {
key="$1"
# Extract RHS after '=' and trim whitespace
grep -m1 -E "^[[:space:]]*$key=" "$BOOTSTRAP" 2>/dev/null | \
cut -d'=' -f2- | sed 's/^[[:space:]]*//; s/[[:space:]]*$//'
lib/rdk/startStunnel.sh:173
- This writes PASSCODE via
eval, which is unnecessary and can execute arbitrary input if PASSCODE is ever user-controlled. Also, PASSCODE is not set anywhere in this script, so this currently writes an empty line.
# Writing passcode to open file descriptor
echo "$(eval "$PASSCODE")" >&$FD_NUMBER &
lib/rdk/readBTAddress-generic.sh:31
- If BLUETOOTH_ENABLED is not true, bluetooth_mac is never set and the script echoes an empty value. Since callers treat this output as the device BT MAC, initialize a safe default and only override when Bluetooth is enabled and the helper exists.
lib/rdk/startLinkLocal.sh:44 - The log message says only "eth0 and wlan0" are allowed, but the actual restriction is based on $ETHERNET_INTERFACE and $WIFI_INTERFACE from device.properties. This can be misleading on platforms where those map to different names.
if [ "$INTERFACE" != "$ETHERNET_INTERFACE" ] && [ "$INTERFACE" != "$WIFI_INTERFACE" ]; then
Log "INFO: Link-local not started for $INTERFACE (only eth0 and wlan0 allowed)"
exit 0
lib/rdk/NM_Bootstrap.sh:122
- When /opt/secure/NetworkManager/system-connections is empty, the glob expands to a literal '*' and grep will emit errors. Guard against non-existent matches before grepping/removing.
for f in /opt/secure/NetworkManager/system-connections/*; do
if grep -q "type=wifi" "$f"; then
rm -f "$f"
fi
done
lib/rdk/system_info_collector.sh:98
- log_disk_usage() is still defined but no longer called (it was removed from the Yocto logging section). This leaves dead code and drops disk-usage diagnostics from the output. Either remove the function or call it here.
echo "Logging for Yocto platforms..."
/bin/timestamp
uptime
run_top_command
cpu_statistics
CHANGELOG.md:62
- This PR deletes rebootNow.sh, but there are still in-repo references that will break at build/runtime (e.g., Makefile:86 symlinks /lib/rdk/rebootNow.sh, and lib/rdk/userInitiatedFWDnld.sh / lib/rdk/factory-reset.sh invoke /rebootNow.sh). Either keep a compat wrapper/symlink to the new implementation, or update all call sites to the new reboot binary/API.
- RDKEMW-15490 : Removing reboot script reference from sysint [`#500`](https://github.com/rdkcentral/sysint/pull/500)
Comment on lines
+134
to
+151
| #Function to find available fd at this point in time | ||
| get_next_fd() { | ||
| local fd=3 # Start checking from FD 3 (since 0, 1, 2 are standard in/out/err) | ||
| local max_fd=20 # Maximum FD to check | ||
|
|
||
| # Try to redirect to /dev/null and check if the FD is free | ||
| while [ $fd -le $max_fd ]; do | ||
| if ! { true >&$fd; } 2>/dev/null; then | ||
| echo "$fd" # Output the available FD (goes to standard output) | ||
| return 0 # Set exit status to success | ||
| fi | ||
| fd=$((fd + 1)) | ||
| done | ||
|
|
||
| # Log failure if no free FD is found | ||
| echo "Error: No available file descriptor found up to FD $max_fd" >&2 | ||
| return 1 # Set exit status to failure | ||
| } |
…nly/IPv6-only networks (#581) * DELIA-70624: updated to reset counters only if gateway is available for ipv6. Signed-off-by: Balaji Punnuru <Balaji_Punnuru@comcast.com> * DELIA-70624: Added debug in the script Signed-off-by: Balaji Punnuru <Balaji_Punnuru@comcast.com> * DELIA-70624: networkConnectionRecovery: presence-aware packet-loss recovery trigger Recovery now fires only when no routed IP stack has acceptable connectivity and at least one routed stack is at/above the reassociate tolerance. This fixes the IPv4-only / IPv6-only case where a stack with no default route left packetsLost at 0 and suppressed recovery. Also folded in reliability/logging fixes: - clear stale gwIp on the V6 test-hook path - guard packet-loss comparisons against unparseable ping output - emit SYST_WARN_GW100PERC_PACKETLOSS to the logs file on every run - ping the IPv4 default-route interface explicitly (ping -I) - log total ipv4/ipv6 packet loss when above threshold - simplify the trigger to anyGood/anyBad classification * DELIA-70624: networkConnectionRecovery: add log() helper and per-call packet-loss fix - Introduce a log() helper that prepends the timestamp and appends to $logsFile, and convert the timestamped echo call sites to use it. - Rename packetsLostipv4/packetsLostipv6 globals to ipv4PacketLoss/ipv6PacketLoss for clarity. - Reset the per-call packetLoss to "" at the top of checkPacketLoss and guard the packet-loss telemetry block with [ -n "$packetLoss" ], so a routeless or unparseable ping no longer reuses the other family's value or triggers integer-comparison errors. - Fix a "[" spacing bug in checkWifiDrvErrors. Telemetry markers and t2CountNotify calls are unchanged. * DELIA-70624: networkConnectionRecovery: log route/loss snapshot on state change Emit a "network state changed" line with the per-stack route-present flags and packet-loss values only when the snapshot differs from the previous run (persisted in /tmp/.ncr_laststate), so field logs capture every transition without printing on every run. Debugging aid for customer-site triage. * DELIA-70624: remove debug logging and make script POSIX/busybox-sh safe - Drop the temporary DEBUG_NCR field-debug logging; retain the "network state changed" transition log (now unprefixed). - checkWifiDrvErrors(): $dir already holds the full debugfs path, so remove the duplicated /sys/kernel/debug/ieee80211/ prefix from the log messages, and capture the cat exit status so the failure log reports the real status instead of the enclosing test's result. - Replace bashisms with POSIX/busybox-ash equivalents: source -> ., [[ =~ ]] -> case, [[ ]] -> [ ], and {1..9} -> explicit list (the brace form never expands under busybox ash, which had left the WIFIV_WARN_PL_20..90PERC packet-loss markers non-functional on target). - Minor whitespace cleanup in checkDnsFile(). * DELIA-70624: restore 100% packet-loss triage marker and fix marker case Reinstate the field-triage log marker for 100% packet loss that was lost when checkPacketLoss() moved to a tolerance-based recovery decision. The new log-only marker "100% Packet loss is observed on all routed IP stacks" fires when every routed IP stack shows exactly 100% loss. It is independent of WifiReassociateTolerance and does not alter the recovery/return path, so recovery behaviour is unchanged. Unlike the legacy dual-stack "...for both ipv4 and ipv6" print, it is also emitted correctly on IPv4-only and IPv6-only networks. Triage associates the old and new strings to the same 100%-packet-loss marker for trend continuity across images. Also capitalise the "Packet loss more than 10% observed" marker (was lowercase "packet loss ...") since triage marker matching is case-sensitive. --------- Signed-off-by: Balaji Punnuru <Balaji_Punnuru@comcast.com> Co-authored-by: Balaji Punnuru <Balaji_Punnuru@comcast.com>
Sysint release for 8.4 hotfix
* RDK-61784:Model specific sshkeys for RDKE active platforms Reason for change:Switch keys only in prod builds Test Procedure: Build and verify. Risks: None Priority: P1 * Update Start_MaintenanceTasks.sh * Revert "Rebase " --------- Co-authored-by: NareshM1702 <risingphoenix785@gmail.com>
…nly/IPv6-only networks (#570) * DELIA-70624: Updated Script to reset counters only if gateway is enabled. Signed-off-by: Balaji Punnuru <Balaji_Punnuru@comcast.com> * DELIA-70624: networkConnectionRecovery: presence-aware packet-loss recovery trigger Recovery now fires only when no routed IP stack has acceptable connectivity and at least one routed stack is at/above the reassociate tolerance. This fixes the IPv4-only / IPv6-only case where a stack with no default route left packetsLost at 0 and suppressed recovery. Also folded in reliability/logging fixes: - clear stale gwIp on the V6 test-hook path - guard packet-loss comparisons against unparseable ping output - emit SYST_WARN_GW100PERC_PACKETLOSS to the logs file on every run - ping the IPv4 default-route interface explicitly (ping -I) - log total ipv4/ipv6 packet loss when above threshold - simplify the trigger to anyGood/anyBad classification * DELIA-70624: networkConnectionRecovery: add log() helper and per-call packet-loss fix - Introduce a log() helper that prepends the timestamp and appends to $logsFile, and convert the timestamped echo call sites to use it. - Rename packetsLostipv4/packetsLostipv6 globals to ipv4PacketLoss/ipv6PacketLoss for clarity. - Reset the per-call packetLoss to "" at the top of checkPacketLoss and guard the packet-loss telemetry block with [ -n "$packetLoss" ], so a routeless or unparseable ping no longer reuses the other family's value or triggers integer-comparison errors. - Fix a "[" spacing bug in checkWifiDrvErrors. Telemetry markers and t2CountNotify calls are unchanged. * DELIA-70624: networkConnectionRecovery: log route/loss snapshot on state change Emit a "network state changed" line with the per-stack route-present flags and packet-loss values only when the snapshot differs from the previous run (persisted in /tmp/.ncr_laststate), so field logs capture every transition without printing on every run. Debugging aid for customer-site triage. * DELIA-70624: remove debug logging and make script POSIX/busybox-sh safe - Drop the temporary DEBUG_NCR field-debug logging; retain the "network state changed" transition log (now unprefixed). - checkWifiDrvErrors(): $dir already holds the full debugfs path, so remove the duplicated /sys/kernel/debug/ieee80211/ prefix from the log messages, and capture the cat exit status so the failure log reports the real status instead of the enclosing test's result. - Replace bashisms with POSIX/busybox-ash equivalents: source -> ., [[ =~ ]] -> case, [[ ]] -> [ ], and {1..9} -> explicit list (the brace form never expands under busybox ash, which had left the WIFIV_WARN_PL_20..90PERC packet-loss markers non-functional on target). - Minor whitespace cleanup in checkDnsFile(). * DELIA-70624: restore 100% packet-loss triage marker and fix marker case Reinstate the field-triage log marker for 100% packet loss that was lost when checkPacketLoss() moved to a tolerance-based recovery decision. The new log-only marker "100% Packet loss is observed on all routed IP stacks" fires when every routed IP stack shows exactly 100% loss. It is independent of WifiReassociateTolerance and does not alter the recovery/return path, so recovery behaviour is unchanged. Unlike the legacy dual-stack "...for both ipv4 and ipv6" print, it is also emitted correctly on IPv4-only and IPv6-only networks. Triage associates the old and new strings to the same 100%-packet-loss marker for trend continuity across images. Also capitalise the "Packet loss more than 10% observed" marker (was lowercase "packet loss ...") since triage marker matching is case-sensitive. --------- Signed-off-by: Balaji Punnuru <Balaji_Punnuru@comcast.com> Co-authored-by: Balaji Punnuru <Balaji_Punnuru@comcast.com>
…rty files in /etc (#582) Co-authored-by: mtirum011 <madhubabu_tirumala@comcast.com> Co-authored-by: nhanasi <navihansi@gmail.com>
Sysint Release for 6.0.2 develop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.