Skip to content

RDKEMW-21581: Network connection recovery script should handle IPv4-only/IPv6-only networks#570

Open
tukken-comcast wants to merge 5 commits into
developfrom
topic/RDKEMW-21581
Open

RDKEMW-21581: Network connection recovery script should handle IPv4-only/IPv6-only networks#570
tukken-comcast wants to merge 5 commits into
developfrom
topic/RDKEMW-21581

Conversation

@tukken-comcast

Copy link
Copy Markdown

Reason for change: fix network connection recovery script
Test Procedure: See ticket
Priority: P1
Risk: Medium

Balaji Punnuru and others added 4 commits July 14, 2026 15:14
…led.

Signed-off-by: Balaji Punnuru <Balaji_Punnuru@comcast.com>
…covery 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
… 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.
…ate 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.
@tukken-comcast
tukken-comcast requested a review from a team as a code owner July 14, 2026 17:38
Copilot AI review requested due to automatic review settings July 14, 2026 17:38

Copilot AI left a comment

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.

Pull request overview

Updates the RDK network connection recovery script to make packet-loss based recovery decisions correctly on IPv4-only, IPv6-only, and dual-stack networks.

Changes:

  • Refactors logging by adding a log() helper and converting many direct echo ... >> $logsFile calls to log ....
  • Updates packet-loss probing and recovery decision logic to evaluate “bad vs good connectivity” per routed IP stack (IPv4/IPv6), rather than requiring both stacks to be present.
  • Adds lightweight state tracking (/tmp/.ncr_laststate) to log route/loss transitions only when they change.

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

Comment thread lib/rdk/networkConnectionRecovery.sh Outdated
Copilot AI review requested due to automatic review settings July 15, 2026 09:56

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

lib/rdk/networkConnectionRecovery.sh:244

  • pingCmd is built as a single string including -I $gwIp_interface, but gwIp_interface can be empty if ip route parsing fails. In that case the script would attempt to run ping -I -c ..., which errors and makes packetLoss unparseable. Build the base command first and only append -I <iface> when the interface value is non-empty.
    if [ "$version" = "V4" ] ; then
      gwIp=$(/sbin/ip -4 route | awk '/default/ { print $3 }' | head -n1 | awk '{print $1;}')
      gwIp_interface=$(/sbin/ip -4 route | awk '/default/ { print $5 }' | head -n1 | awk '{print $1;}')
      pingCmd="ping -I $gwIp_interface"
    elif [ "$version" = "V6" ] ; then
      gwIp=$(/sbin/ip -6 route | awk '/default/ { print $3 }' | head -n1 | awk '{print $1;}')
      gwIp_interface=$(/sbin/ip -6 route | awk '/default/ { print $5 }' | head -n1 | awk '{print $1;}')
      pingCmd="ping6 -I $gwIp_interface"
    fi

Comment thread lib/rdk/networkConnectionRecovery.sh Outdated
- 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().
Copilot AI review requested due to automatic review settings July 15, 2026 10:12

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread lib/rdk/networkConnectionRecovery.sh
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.

2 participants