Skip to content

Conversation

@rjarry
Copy link
Collaborator

@rjarry rjarry commented Oct 27, 2025

  • cli: fix ipv6 ping and traceroute aliases
  • icmp: allow specifying arguments in any order
  • smoke: reduce ping interval for srv6 tests
  • ecmp: fix smoke tests

Summary by CodeRabbit

  • New Features

    • Added dedicated IPv6 ICMP commands: ping6 and traceroute6 for clarity.
  • Documentation

    • Standardized CLI parameter display to grouped, parenthesized syntax for IP/ICMP commands.
  • Tests

    • Updated smoke tests for IPv6 command names and adjusted expectations.
    • Reworked several test topologies and command invocations (namespace/interface layout, routes, and ping options) for clearer, consolidated setups and stricter timing/output flags.

@coderabbitai
Copy link

coderabbitai bot commented Oct 27, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

CLI help text for ICMP commands was changed to use grouped, parenthesized optional-parameter syntax. IPv6 CLI commands were renamed from generic names to IPv6-specific variants (ping -> ping6, traceroute -> traceroute6) with updated argument formats. Smoke tests were updated to match command renames, reworked network namespace/interface layouts in load-balance tests, and tightened ping invocation options/timing in srv6 tests.

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "smoke: fix load-balance tests" refers to a real and significant part of the changeset—specifically the reworks to ip_loadbalance_test.sh and ip_loadbalance_frr_test.sh. However, the PR encompasses broader changes beyond just load-balance tests, including CLI command alias fixes (ping6/traceroute6 renaming), CLI help string reformatting, and updates to srv6 smoke tests. The title captures one important aspect of the change but does not fully represent the scope of modifications across CLI modules and multiple test files.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d974b4d and d66be9f.

📒 Files selected for processing (7)
  • modules/ip/cli/icmp.c (2 hunks)
  • modules/ip6/cli/icmp6.c (2 hunks)
  • smoke/ip6_builtin_icmp_test.sh (1 hunks)
  • smoke/ip_loadbalance_frr_test.sh (2 hunks)
  • smoke/ip_loadbalance_test.sh (1 hunks)
  • smoke/srv6_frr_test.sh (1 hunks)
  • smoke/srv6_test.sh (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Currently the IPv6 ping and traceroute commands are aliased to their
IPv4 versions. This results in an ambiguous tree with multiple nodes
matching the "ping" and "traceroute" tokens but with different subtrees
below them.

Make everything more consistent by using different command names for
IPv6 ICMP commands.

Fixes: 71a80f6 ("ip6: add ping and traceroute command")
Signed-off-by: Robin Jarry <[email protected]>
Reviewed-by: Christophe Fontaine <[email protected]>
Do not be pedantic about argument order for the ping, traceroute, ping6
and ping6 commands.

Signed-off-by: Robin Jarry <[email protected]>
Reviewed-by: Christophe Fontaine <[email protected]>
Do not wait for 3 seconds for each ping command. I had forgotten about
these two tests.

Fixes: 72cc512 ("smoke: reduce test execution time")
Signed-off-by: Robin Jarry <[email protected]>
Reviewed-by: Christophe Fontaine <[email protected]>
These tests were added at the same time than the rework of smoke test
interface/netns names and were lost in translation.

Fix the tests to use the same naming patterns.

Fixes: 329388e ("smoke: homogenize netns names")
Fixes: 99ea3c6 ("smoke: add helper to create ports")
Fixes: b3be6c9 ("smoke: use deterministic interface names")
Fixes: b99eb3a ("smoke: remove the need for stable mac addresses")
Fixes: ba05a4c ("smoke: always set lo up in network namespaces")
Fixes: f083695 ("smoke: use different names for network namespaces")
Signed-off-by: Robin Jarry <[email protected]>
Reviewed-by: Christophe Fontaine <[email protected]>
@christophefontaine christophefontaine merged commit d08da9d into DPDK:main Oct 27, 2025
2 of 3 checks passed
@rjarry rjarry deleted the smoke-ip-lb branch October 27, 2025 14:38
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