Skip to content

icmp: return ENOENT when no echo reply is available - #590

Merged
rjarry merged 1 commit into
DPDK:mainfrom
rjarry:icmp-msgsize
Apr 9, 2026
Merged

icmp: return ENOENT when no echo reply is available#590
rjarry merged 1 commit into
DPDK:mainfrom
rjarry:icmp-msgsize

Conversation

@rjarry

@rjarry rjarry commented Apr 8, 2026

Copy link
Copy Markdown
Collaborator

Since commit ab83fb6 ("api: validate response payload size in clients"), the client library rejects responses whose payload size does not match the declared response type. The ICMP and ICMPv6 recv handlers were returning success with an empty payload when no reply had arrived yet, which now triggers EMSGSIZE on the client side.

Here is an example trace (truncated):

+ grcli ping fd00:ba4:1::2 count 3 delay 10
API: accept_conn_cb: new connection fd=20 pid=17607
API: read_cb: ... (GR_HELLO) req_len=132 status=0 (Success) resp_len=0
API: read_cb: ... (GR_IP6_ICMP6_SEND) req_len=26 status=0 (Success) resp_len=0
TRACE: [tx p1] ... / ICMPv6 neigh solicit who has fd00:ba4:1::2? ...
API: read_cb: ... (GR_IP6_ICMP6_RECV) req_len=4 status=0 (Success) resp_len=0
                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: command failed: Message too long (EMSGSIZE)
API: disconnect_client: client pid=17607 disconnected
TRACE: [rx p1] ... / ICMPv6 neigh advert fd00:ba4:1::2 is at ...
TRACE: [tx p1] ... / ICMPv6 echo request id=53710 seq=0, (pkt_len=70)
TRACE: [rx p1] ... / ICMPv6 echo reply id=53710 seq=0, (pkt_len=70)

Return ENOENT from the recv handlers when no matching echo reply is found. Adjust the CLI polling loops to retry on ENOENT and treat it as a normal "not yet" condition rather than a fatal error.

I have checked that no other API handlers return an empty success response when they are expected to return a payload with the following obscure shell pipeline:

sed -En 's/^GR_REQ\((.+), .+, (.+)\)\;$/\1 \2/p' $(git grep -wl GR_REQ) |
grep -v gr_empty | sed -En 's/(.*) struct .*/\1/p' |
while read -r req; do sed -En "s/^.*api_handler\($req, (.+)\);$/\\1/p" $(git grep -wl $req); done | \
while read -r func; do sed -En "/$func\\(/,/^}\$/p" $(git grep -wl $func); done |
grep -c 'api_out(0, 0'

Technical changes

Control layer (ICMP / ICMPv6 recv handlers)

  • get_icmp_response() and get_icmp6_echo_reply() return errno_set_null(ENOENT) when no matching echo reply/error packet is found (instead of returning NULL).
  • icmp_recv() and icmp6_recv() propagate errno via api_out(errno, 0, NULL) on the "no message found" path rather than returning api_out(0, 0, NULL).

CLI layer (polling loops)

  • icmp_send() in modules/ip/cli/icmp.c and modules/ip6/cli/icmp6.c: the reply-wait loop now continues only while gr_api_client_send_recv() returns -ENOENT and timeout remains; previously the loop continued while ret == 0. After the loop, negative return values are treated as fatal only when ret != -ENOENT (ret == -ENOENT is now treated as the expected "no reply yet" condition).

Control-flow simplification

  • get_icmp_response() returns the matched rte_mbuf immediately when an identifier/sequence match is found (replacing an earlier break-based exit).

Workflows and test helpers

  • .github/workflows/check.sh prints the active compiler version and, on unit-test failure, runs each test binary under gdb -batch to capture backtraces before exiting non-zero.

Rationale tied to code history

  • Commit ab83fb6 ("api: validate response payload size in clients") made clients reject responses whose payload size does not match the declared response type. Previously, recv handlers returned a successful zero-length response when no reply existed, which caused clients to see an empty-success response (status=0, resp_len=0) and fail with EMSGSIZE. Returning ENOENT from recv handlers distinguishes "no reply yet" from a successful empty response so CLI polling loops retry instead of failing.

@coderabbitai

This comment was marked as resolved.

@rjarry
rjarry requested a review from david-marchand April 8, 2026 15:35
@rjarry

rjarry commented Apr 8, 2026

Copy link
Copy Markdown
Collaborator Author

@rjarry
rjarry requested review from christophefontaine and removed request for david-marchand April 8, 2026 15:43
coderabbitai[bot]

This comment was marked as resolved.

@rjarry
rjarry force-pushed the icmp-msgsize branch 4 times, most recently from dde1f3a to 85c5709 Compare April 9, 2026 06:55
Comment thread .github/workflows/check.yml
Since commit ab83fb6 ("api: validate response payload size in
clients"), the client library rejects responses whose payload size does
not match the declared response type. The ICMP and ICMPv6 recv handlers
were returning success with an empty payload when no reply had arrived
yet, which now triggers EMSGSIZE on the client side.

Here is an excerpt from a failed test (truncated):

```
+ grcli ping fd00:ba4:1::2 count 3 delay 10
API: accept_conn_cb: new connection fd=20 pid=17607
API: read_cb: ... (GR_HELLO) req_len=132 status=0 (Success) resp_len=0
API: read_cb: ... (GR_IP6_ICMP6_SEND) req_len=26 status=0 (Success) resp_len=0
TRACE: [tx p1] ... / ICMPv6 neigh solicit who has fd00:ba4:1::2? ...
API: read_cb: ... (GR_IP6_ICMP6_RECV) req_len=4 status=0 (Success) resp_len=0
                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: command failed: Message too long (EMSGSIZE)
API: disconnect_client: client pid=17607 disconnected
TRACE: [rx p1] ... / ICMPv6 neigh advert fd00:ba4:1::2 is at ...
TRACE: [tx p1] ... / ICMPv6 echo request id=53710 seq=0, (pkt_len=70)
TRACE: [rx p1] ... / ICMPv6 echo reply id=53710 seq=0, (pkt_len=70)
```

Return ENOENT from the recv handlers when no matching echo reply is
found. Adjust the CLI polling loops to retry on ENOENT and treat it as
a normal "not yet" condition rather than a fatal error.

I have checked that no other API handlers return an empty success
response when they are expected to return a payload with the following
obscure shell pipeline:

```
sed -En 's/^GR_REQ\((.+), .+, (.+)\)\;$/\1 \2/p' $(git grep -wl GR_REQ) |
grep -v gr_empty | sed -En 's/(.*) struct .*/\1/p' |
while read -r req; do sed -En "s/^.*api_handler\($req, (.+)\);$/\\1/p" $(git grep -wl $req); done | \
while read -r func; do sed -En "/$func\\(/,/^}\$/p" $(git grep -wl $func); done |
grep -c 'api_out(0, 0'
```

Signed-off-by: Robin Jarry <rjarry@redhat.com>
Reviewed-by: Christophe Fontaine <cfontain@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
@rjarry
rjarry merged commit 6bf4f96 into DPDK:main Apr 9, 2026
10 checks passed
@rjarry
rjarry deleted the icmp-msgsize branch April 9, 2026 09:58
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