Skip to content

cli: merge ping{,6} and traceroute{,6} commands - #577

Merged
rjarry merged 1 commit into
DPDK:mainfrom
rjarry:ping
Apr 1, 2026
Merged

cli: merge ping{,6} and traceroute{,6} commands#577
rjarry merged 1 commit into
DPDK:mainfrom
rjarry:ping

Conversation

@rjarry

@rjarry rjarry commented Mar 26, 2026

Copy link
Copy Markdown
Collaborator

The IPv4 and IPv6 ping/traceroute commands share the same argument structure. Introduce a cli_icmp_ops dispatch table in the infra CLI so that a single "ping" and "traceroute" command handles both address families, selected automatically based on the destination address format.

Register per-family callbacks via cli_icmp_ops_register() instead of creating separate CLI contexts. Remove the ping6 and traceroute6 commands from the CLI and update smoke tests accordingly.

Command consolidation via dispatch table

Adds struct cli_icmp_ops and registration API in modules/infra/cli/gr_cli_l3.h:

  • struct cli_icmp_ops { addr_family_t af; cmd_cb_t ping; cmd_cb_t traceroute; STAILQ_ENTRY(cli_icmp_ops) next; }
  • void cli_icmp_ops_register(struct cli_icmp_ops *);

Implements a centralized ICMP CLI context in modules/infra/cli/icmp.c:

  • New icmp CLI context registers two subcommands, "ping" and "traceroute".
  • A static STAILQ of cli_icmp_ops handlers is maintained; cli_icmp_ops_register() asserts non-NULL callbacks and unique af before inserting.
  • Command handlers parse the DEST argument with arg_ip(..., "DEST", ..., ops->af) and dispatch to the first registered ops whose address-family parse succeeds.
  • If no registered handler accepts the parsed DEST, errno is set to ENOPROTOOPT and the command returns CMD_ERROR.
  • The centralized context provides schema/help for DEST, VRF, IFACE, COUNT/DELAY, and IDENT and is registered via a constructor.

IPv4 and IPv6 implementation refactoring

  • modules/ip/cli/icmp.c

    • Removed per-context registration; replaced with static struct cli_icmp_ops ops = { .af = GR_AF_IP4, .ping = ping, .traceroute = traceroute } and register via cli_icmp_ops_register(&ops).
    • Replaced arg_ip4/arg_ip parsing target name to "DEST" to match centralized dispatcher.
    • ICMPv4 send/recv logic (icmp_send, ping, traceroute) kept; only CLI wiring changed.
  • modules/ip6/cli/icmp6.c

    • Removed ctx_init/cli_context for ping6/traceroute6; added static struct cli_icmp_ops ops = { .af = GR_AF_IP6, .ping = ping, .traceroute = traceroute } and register via cli_icmp_ops_register(&ops).
    • Replaced per-command CLI schema with centralized DEST-based dispatch; per-family parsing uses arg_ip6(..., "DEST", ...).

Build, docs and test updates

  • modules/infra/cli/meson.build: icmp.c added to the cli_src file list so centralized context is compiled.
  • docs/meson.build: per-command man-page list (grcli_commands) updated to remove ping6/traceroute6 and include only the consolidated commands (ping/traceroute).
  • Smoke tests updated to invoke consolidated commands:
    • smoke/ip6_builtin_icmp_test.sh: replaced grcli ping6/traceroute6 with grcli ping/traceroute (IPv6 addresses unchanged).
    • smoke/ospf6_frr_manualtest.sh: replaced final grcli ping6 call with grcli ping (IPv6 target unchanged).

@coderabbitai

coderabbitai Bot commented Mar 26, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 01813460-4a9b-4cbc-9831-4d93b6704f37

📥 Commits

Reviewing files that changed from the base of the PR and between b5aeacd and dec15f3.

📒 Files selected for processing (8)
  • docs/meson.build
  • modules/infra/cli/gr_cli_l3.h
  • modules/infra/cli/icmp.c
  • modules/infra/cli/meson.build
  • modules/ip/cli/icmp.c
  • modules/ip6/cli/icmp6.c
  • smoke/ip6_builtin_icmp_test.sh
  • smoke/ospf6_frr_manualtest.sh
✅ Files skipped from review due to trivial changes (3)
  • modules/infra/cli/meson.build
  • docs/meson.build
  • smoke/ospf6_frr_manualtest.sh
🚧 Files skipped from review as they are similar to previous changes (2)
  • smoke/ip6_builtin_icmp_test.sh
  • modules/infra/cli/gr_cli_l3.h

📝 Walkthrough

Walkthrough

This PR centralizes ICMP CLI handling by introducing a generic ICMP CLI module that registers address-family-specific handlers via a new struct cli_icmp_ops and cli_icmp_ops_register(). IPv4 and IPv6 implementations now register ops instead of their own command contexts; the dispatcher chooses the handler based on the parsed destination address family. ping6/traceroute6 usages and command definitions were removed or replaced with unified ping/traceroute commands, and build and test scripts were updated to include and invoke the new module.


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.

The IPv4 and IPv6 ping/traceroute commands share the same argument
structure. Introduce a cli_icmp_ops dispatch table in the infra CLI
so that a single "ping" and "traceroute" command handles both address
families, selected automatically based on the destination address
format.

Register per-family callbacks via cli_icmp_ops_register() instead of
creating separate CLI contexts. Remove the ping6 and traceroute6
commands from the CLI and update smoke tests accordingly.

Signed-off-by: Robin Jarry <rjarry@redhat.com>
Reviewed-by: Christophe Fontaine <cfontain@redhat.com>
@rjarry
rjarry merged commit 6c305ab into DPDK:main Apr 1, 2026
6 checks passed
@rjarry
rjarry deleted the ping branch April 1, 2026 15:51
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