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

[tukken-comcast]

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

[Copilot]

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.

[Copilot]

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

lib/rdk/networkConnectionRecovery.sh:243

• Same issue for IPv6: if $gwIp_interface is empty, ping6 -I will treat -c as the interface name and the probe will fail, producing an empty/unparseable packet-loss result.

      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"

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

lib/rdk/networkConnectionRecovery.sh:239

• In checkPacketLoss(V4), default-route parsing can yield an interface name when the route is of the form default dev <if> (no via), which makes gwIp non-IP and causes ping to fail / packet loss to become unparseable. Also, building ping -I $gwIp_interface breaks when the interface field is missing/empty (it will consume -c as the interface). Parsing via/dev explicitly and only adding -I when an interface is present avoids both failure modes.

    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"

lib/rdk/networkConnectionRecovery.sh:244

• In checkPacketLoss(V6), the same route parsing issue exists as V4: if the default route does not contain via, gwIp becomes non-IP (often the interface), and ping6 -I $gwIp_interface misbehaves when $gwIp_interface is empty. Parse via/dev explicitly and only add -I when an interface is present.

    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

[bpunnuru]

Looks good

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

lib/rdk/networkConnectionRecovery.sh:239

• In checkPacketLoss(V4), the interface is parsed as field 5 from ip route, and then used unconditionally with ping -I. For some valid default-route formats (e.g. default dev eth0 ...), field positions differ and $gwIp_interface can be empty/incorrect, which can make ping -I treat -c as the interface and break packet-loss detection.

    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"

lib/rdk/networkConnectionRecovery.sh:243

• In checkPacketLoss(V6), ping6 -I $gwIp_interface is built even if the route parse fails to produce an interface (or the default route format doesn’t place the interface in field 5). This can turn into ping6 -I -c ... and cause packet loss to be treated as unmeasurable.

    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"

lib/rdk/networkConnectionRecovery.sh:250

• gwResponse captures only stdout from ping. If ping fails due to invalid arguments (e.g., malformed -I), gwResponse becomes empty and packetLoss parses as empty, leading to malformed telemetry lines like TELEMETRY_GATEWAY_PACKET_LOSS:,$gwIp. Consider capturing stderr and forcing a numeric fallback when the packet-loss parse fails.

    gwResponse=$($pingCmd -c "$pingCount" -i "$pingInterval" "$gwIp")
    packetLoss=$(echo "$gwResponse" | grep "packet"|awk '{print $7}'|cut -d'%' -f1)