Skip to content

Conversation

@smuppand
Copy link
Contributor

This PR improves the Ethernet validation flow to be less disruptive, less flaky, and easier to debug on both full userland images (NetworkManager/systemd-networkd) and minimal/kernel-only style images.
What changed

  • Ethernet/run.sh
    • Adds a fast-path: if an interface already has Link detected=yes and a valid IPv4 (non 169.254.x.x), the test skips bring-up + DHCP and directly validates connectivity via ping.
    • Keeps existing behavior: if a network manager is active, the script does not run udhcpc and instead waits for IP.
    • Improves debug logs for triage (interface snapshot + ping output).
  • functestlib.sh
    • Updates try_dhcp_client_safe() to skip DHCP when a valid IP already exists.
    • If systemctl is available and NetworkManager/systemd-networkd is active, it waits for IP rather than fighting the manager.
    • Retains the safe udhcpc script flow for minimal images and adds extra logging for DHCP decisions/outcome.
  • Why
    • Avoids re-running link bring-up/DHCP when the interface is already configured.
    • Prevents conflicts with network managers on full images.
    • Maintains usability on kernel/minimal environments where systemctl may not exist.
    • Better logging helps debug intermittent link/IP issues quickly in CI/LAVA.

Copy link

@bhargav0610 bhargav0610 left a comment

Choose a reason for hiding this comment

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

LGTM

@vnarapar
Copy link
Contributor

Please move 31f25f0 to first as we will see undefined function if we pick ec82b4d as head

@smuppand
Copy link
Contributor Author

Please move 31f25f0 to first as we will see undefined function if we pick ec82b4d as head

Nice catch. I’ve moved the necessary commit to the head.

@vnarapar
Copy link
Contributor

Can you remove this commit 62a7eef as we are no longer relying on user input? Also please update the readme

- In try_dhcp_client_safe(), detect a valid (non link-local) IPv4 early and
  return success without launching DHCP again.
- When systemctl is available and NetworkManager/systemd-networkd is active,
  wait for IP instead of running udhcpc to avoid fighting the manager.
- Keep the udhcpc safe-script behavior for minimal images while improving
  debug visibility around DHCP decisions and outcomes.

Signed-off-by: Srikanth Muppandam <smuppand@qti.qualcomm.com>
@smuppand
Copy link
Contributor Author

Can you remove this commit 62a7eef as we are no longer relying on user input? Also please update the readme

We cannot remove this commit because we need some of the changes it contains. I have pushed the latest updates.

@smuppand smuppand requested a review from vnarapar December 22, 2025 16:18
- Add a fast-path: if Link detected=yes and a valid (non link-local) IPv4 is
  already present, skip link bring-up + IP acquisition and run ping directly.
- Keep existing manager-aware behavior: when NetworkManager/systemd-networkd
  is active (via systemctl), don’t run udhcpc; only wait for IP.
- Improve debug logs (ip-link / ethtool / ping output) for easier LAVA triage.
- Preserve PASS/FAIL/SKIP summary reporting per interface.

Signed-off-by: Srikanth Muppandam <smuppand@qti.qualcomm.com>
Signed-off-by: Srikanth Muppandam <smuppand@qti.qualcomm.com>
- Document auto-detection of Ethernet interfaces (not hardcoded eth0)
- Mention manager-aware IP acquisition (NetworkManager/systemd-networkd)
  and the DHCP behavior when no manager is active
- Describe fast-path when link + valid IPv4 already exist
- Add new config knobs (LINK_TIMEOUT_S, IP_TIMEOUT_S, PING_* etc.)
- Update result/summary expectations and sample logs
- Note interface rename usage (e.g., end0)

Signed-off-by: Srikanth Muppandam <smuppand@qti.qualcomm.com>
Copy link
Contributor

@vnarapar vnarapar left a comment

Choose a reason for hiding this comment

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

LGTM

else
# Decide SKIP/FAIL post-failure
carrier="?"
if [ -r "/sys/class/net/$iface/carrier" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

@smuppand hope Lines196- 199is not required as same is given at Line175 -> 178


log_info "Link bring-up failed for $iface. Diagnostics:"
ip link show "$iface" 2>/dev/null | while IFS= read -r l; do [ -n "$l" ] && log_info "[ip-link] $l"; done
ethtool "$iface" 2>/dev/null | sed -n '1,80p' | while IFS= read -r l; do [ -n "$l" ] && log_info "[ethtool] $l"; done
Copy link
Contributor

Choose a reason for hiding this comment

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

@smuppand 201 to 203 lines are also not required as they are checked at Line#180,181 and not operations performed inbetween

ip link set "$iface" up >/dev/null 2>&1 || true

# If link is down/no cable, don't try DHCP
if command -v is_link_up >/dev/null 2>&1; then
Copy link
Contributor

Choose a reason for hiding this comment

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

if condition not required as is_link_up is helper function

log_warn "$iface link is down; skipping DHCP"
return 1
fi
elif command -v ethIsLinkUp >/dev/null 2>&1; then
Copy link
Contributor

Choose a reason for hiding this comment

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

if condition not required as ethIsLinkUp is helper function

return 1
fi
elif command -v ethIsLinkUp >/dev/null 2>&1; then
if ! ethIsLinkUp "$iface"; then
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT

fi

# 2) If helper exists and says up, accept it (but don't fail early)
if command -v is_link_up >/dev/null 2>&1; then
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT

init_autoneg=$(ethGetAutonegState "$iface" 2>/dev/null || echo "unknown")

# Bring interface up using existing helper (admin-up)
if command -v bringup_interface >/dev/null 2>&1; then
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT

@vnarapar vnarapar merged commit 08b0b34 into qualcomm-linux:main Dec 23, 2025
8 checks passed
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.

3 participants