shell/gnrc_icmpv6_echo: separate ICMPv6 echo sending / parsing from shell command #18934

benpicco · 2022-11-18T17:10:47Z

Contribution description

Sending and parsing ICMPv6 echo messages is deeply intertwined with the _gnrc_icmpv6_ping shell command.

This makes re-using the code e.g. for implementing traceroute very hard.
Therefore move the code to a common place in gnrc_icmpv6_echo.c so it can be re-used by other (shell) functions.

Testing procedure

ping should function as before, no logic changes intended.

> ping ff02::1
12 bytes from fe80::90a7:a6ff:fe4b:2e32%6: icmp_seq=0 ttl=64 time=0.498 ms
12 bytes from fe80::7837:fcff:fe7d:1aaf%6: icmp_seq=0 ttl=64 time=0.760 ms (DUP!)
12 bytes from fe80::90a7:a6ff:fe4b:2e32%6: icmp_seq=1 ttl=64 time=0.187 ms
12 bytes from fe80::7837:fcff:fe7d:1aaf%6: icmp_seq=1 ttl=64 time=0.347 ms (DUP!)
12 bytes from fe80::90a7:a6ff:fe4b:2e32%6: icmp_seq=2 ttl=64 time=0.137 ms

--- ff02::1 PING statistics ---
3 packets transmitted, 3 packets received, 2 duplicates, 0% packet loss
round-trip min/avg/max = 0.137/0.385/0.760 ms

Issues/PRs references

riot-ci · 2022-11-19T10:52:09Z

Murdock results

✔️ PASSED

d415c16 examples/paho-mqtt: re-add stm32mp157c-dk2 to Makefile.ci

Success	Failures	Total	Runtime
6785	0	6785	13m:24s

Artifacts

Documentation preview

sys/include/net/gnrc/icmpv6/echo.h

sys/net/gnrc/network_layer/icmpv6/echo/gnrc_icmpv6_echo.c

miri64

Last round of change requests. I think after that we can merge => Please squash directly.

sys/include/net/gnrc/icmpv6/echo.h

sys/net/gnrc/network_layer/icmpv6/echo/gnrc_icmpv6_echo.c

benpicco · 2022-12-14T17:21:36Z

@miri64 ping :)

miri64 · 2023-01-16T12:33:50Z

bors merge

18863: boards/esp32s2-mini: add definition for ESP32 S2 Mini r=benpicco a=benpicco 18934: shell/gnrc_icmpv6_echo: separate ICMPv6 echo sending / parsing from shell command r=miri64 a=benpicco Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de> Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>

miri64 · 2023-01-16T12:49:40Z

bors cancel
bors merge

bors · 2023-01-16T12:49:42Z

Canceled.

18863: boards/esp32s2-mini: add definition for ESP32 S2 Mini r=miri64 a=benpicco 18934: shell/gnrc_icmpv6_echo: separate ICMPv6 echo sending / parsing from shell command r=miri64 a=benpicco 19154: tests/gcoap_dns: Remove duplicate r=miri64 a=Teufelchen1 ### Contribution description Removes a duplicated test. Originally, the test should have looked different and not be a duplicate. However, the behavior the test aimed at is no longer present anyway, hence it can be removed - rather than fixed. Further insights can be provided by `@miri64` Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de> Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com> Co-authored-by: Teufelchen1 <bennet.blischke@haw-hamburg.de>

bors · 2023-01-16T13:20:07Z

Build failed (retrying...):

Murdock

18934: shell/gnrc_icmpv6_echo: separate ICMPv6 echo sending / parsing from shell command r=miri64 a=benpicco 19060: SUBSYSTEMS.md: migrate subsystem list from wiki r=maribu a=jia200x 19154: tests/gcoap_dns: Remove duplicate r=miri64 a=Teufelchen1 ### Contribution description Removes a duplicated test. Originally, the test should have looked different and not be a duplicate. However, the behavior the test aimed at is no longer present anyway, hence it can be removed - rather than fixed. Further insights can be provided by `@miri64` Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de> Co-authored-by: Jose Alamos <jose@alamos.cc> Co-authored-by: Teufelchen1 <bennet.blischke@haw-hamburg.de>

maribu · 2023-01-16T13:32:21Z

bors cancel
bors merge

Come on, bors. We have have four PRs in the merge train! :)

🚂 🚋 🚋 🚋 🚋

bors · 2023-01-16T13:32:24Z

Canceled.

18934: shell/gnrc_icmpv6_echo: separate ICMPv6 echo sending / parsing from shell command r=maribu a=benpicco 19060: SUBSYSTEMS.md: migrate subsystem list from wiki r=maribu a=jia200x 19154: tests/gcoap_dns: Remove duplicate r=miri64 a=Teufelchen1 ### Contribution description Removes a duplicated test. Originally, the test should have looked different and not be a duplicate. However, the behavior the test aimed at is no longer present anyway, hence it can be removed - rather than fixed. Further insights can be provided by `@miri64` Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de> Co-authored-by: Jose Alamos <jose@alamos.cc> Co-authored-by: Teufelchen1 <bennet.blischke@haw-hamburg.de>

bors · 2023-01-16T14:03:00Z

Build failed (retrying...):

Murdock

benpicco · 2023-01-16T15:34:09Z

bors cancel

bors · 2023-01-16T15:34:11Z

Canceled.

benpicco · 2023-01-17T01:34:58Z

bors retry

benpicco · 2023-01-17T09:13:40Z

bors retry

benpicco · 2023-01-17T09:14:15Z

bors merge

benpicco · 2023-01-17T09:14:33Z

bors ping

18934: shell/gnrc_icmpv6_echo: separate ICMPv6 echo sending / parsing from shell command r=benpicco a=benpicco Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>

bors · 2023-01-17T09:14:35Z

pong

benpicco · 2023-01-17T09:35:37Z

let's see if this PR was what caused the last merge train to fail

bors · 2023-01-17T09:57:45Z

Build failed:

Murdock

miri64 · 2023-01-18T12:16:22Z

bors cancel
bors merge

bors · 2023-01-18T13:47:50Z

Canceled.

benpicco · 2023-01-19T12:20:42Z

bors merge

bors · 2023-01-19T15:38:42Z

Build succeeded:

benpicco requested review from miri64, haukepetersen, PeterKietzmann and cgundogan as code owners November 18, 2022 17:10

benpicco requested a review from maribu November 18, 2022 17:10

github-actions bot added Area: network Area: Networking Area: sys Area: System labels Nov 18, 2022

benpicco force-pushed the gnrc_icmpv6_echo_send branch 2 times, most recently from 92cb896 to 7d4ef96 Compare November 18, 2022 20:57

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 18, 2022

miri64 requested changes Nov 19, 2022

View reviewed changes

sys/include/net/gnrc/icmpv6/echo.h Outdated Show resolved Hide resolved

benpicco removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 19, 2022

miri64 reviewed Nov 19, 2022

View reviewed changes

sys/net/gnrc/network_layer/icmpv6/echo/gnrc_icmpv6_echo.c Outdated Show resolved Hide resolved

miri64 requested changes Nov 19, 2022

View reviewed changes

sys/include/net/gnrc/icmpv6/echo.h Outdated Show resolved Hide resolved

sys/net/gnrc/network_layer/icmpv6/echo/gnrc_icmpv6_echo.c Show resolved Hide resolved

benpicco force-pushed the gnrc_icmpv6_echo_send branch from a48d772 to 06ba5ee Compare November 19, 2022 20:58

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 19, 2022

benpicco force-pushed the gnrc_icmpv6_echo_send branch from 06ba5ee to df49280 Compare November 19, 2022 21:50

miri64 approved these changes Jan 16, 2023

View reviewed changes

bors bot added a commit that referenced this pull request Jan 17, 2023

Merge #18934

f7599f6

18934: shell/gnrc_icmpv6_echo: separate ICMPv6 echo sending / parsing from shell command r=benpicco a=benpicco Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>

benpicco force-pushed the gnrc_icmpv6_echo_send branch from df49280 to 3849ade Compare January 17, 2023 11:26

github-actions bot added the Area: examples Area: Example Applications label Jan 17, 2023

miri64 added Area: sys Area: System and removed Area: sys Area: System labels Jan 18, 2023

benpicco and others added 3 commits January 18, 2023 14:47

gnrc_icmpv6_echo: implement gnrc_icmpv6_echo_{send, req_handle}()

f6c1ab3

shell/gnrc_icmpv6_echo: make use of gnrc_icmpv6_echo helper functions

b6de4d1

examples/paho-mqtt: re-add stm32mp157c-dk2 to Makefile.ci

d415c16

benpicco force-pushed the gnrc_icmpv6_echo_send branch from 3849ade to d415c16 Compare January 18, 2023 13:47

benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 18, 2023

bors bot merged commit e248f95 into RIOT-OS:master Jan 19, 2023

benpicco deleted the gnrc_icmpv6_echo_send branch January 19, 2023 15:55

MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023

shell/gnrc_icmpv6_echo: separate ICMPv6 echo sending / parsing from shell command #18934

shell/gnrc_icmpv6_echo: separate ICMPv6 echo sending / parsing from shell command #18934

Uh oh!

Conversation

benpicco commented Nov 18, 2022

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

riot-ci commented Nov 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Murdock results

Artifacts

Uh oh!

Uh oh!

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

benpicco commented Dec 14, 2022

Uh oh!

miri64 commented Jan 16, 2023

Uh oh!

miri64 commented Jan 16, 2023

Uh oh!

bors bot commented Jan 16, 2023

Uh oh!

bors bot commented Jan 16, 2023

Uh oh!

maribu commented Jan 16, 2023

Uh oh!

bors bot commented Jan 16, 2023

Uh oh!

bors bot commented Jan 16, 2023

Uh oh!

benpicco commented Jan 16, 2023

Uh oh!

bors bot commented Jan 16, 2023

Uh oh!

benpicco commented Jan 17, 2023

Uh oh!

benpicco commented Jan 17, 2023

Uh oh!

benpicco commented Jan 17, 2023

Uh oh!

benpicco commented Jan 17, 2023

Uh oh!

bors bot commented Jan 17, 2023

Uh oh!

benpicco commented Jan 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors bot commented Jan 17, 2023

Uh oh!

miri64 commented Jan 18, 2023

Uh oh!

bors bot commented Jan 18, 2023

Uh oh!

benpicco commented Jan 19, 2023

Uh oh!

bors bot commented Jan 19, 2023

Uh oh!

Uh oh!

riot-ci commented Nov 19, 2022 •

edited

Loading

benpicco commented Jan 17, 2023 •

edited

Loading