ping: add RFC 8335 PROBE request and response functionality by aroeseler · Pull Request #342 · iputils/iputils

aroeseler · 2021-06-22T16:39:42Z

This is a continuation of a project with slang21 and an update to the existing pull request found here: #328

This pull request adds timestamp info to the probe messages, fixes a bug in calculating the ctype of the message, and adds probing by ICMPV6 messages.

Original pull request blurb:
RFC 8335 describes the use of extended echo requests to probe specific network interfaces. This PR adds the -e flag to the ping command to probe specific interfaces and receive responses from those interfaces. Functionality for RFC 8335 was recently included in Linux net-next.

Usage:

ping -e [interface] [address]

Interface can be specified by name, ifindex, or IPV4 or IPV6 address.

ping -e 1 10.0.0.89
PING 10.0.0.89 (10.0.0.89) 56(84) bytes of data.
Interface: 1
Status: ACTIVE IPV4 IPV6
64 bytes from 10.0.0.89: icmp_seq=1 ttl=64 time=2.30 ms

--- 10.0.0.89 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 2.298/2.298/2.298/0.000 ms

This has been tested using interoperability with Wireshark and the net-next linux branch. The IPV4 PROBE response has been merged into the mainline Linux 5.13 version, and the IPV6 PROBE response has been merged into the net-next Linux branch and will be available in Linux 5.14.

Signed-off-by: Andreas Roeseler <[email protected]>

fenner

Thanks for the great start on PROBE support! While I have several comments, I believe all of them are minor.

I have two different test resources to help with testing these changes:

A synthetic probe responder, which responds with a different response to each request, then a timeout, then starts cycling around from the beginning again. This way, you don't have to manually create a situation that invokes a given response; just run against an instance of this responder. It's a standalone script written in python, that uses the scapy packet library. It's available from https://github.com/aristanetworks/probe-tools
tcpdump patches that print PROBE requests/replies, including checking the checksum, etc.

fenner · 2024-01-22T17:30:49Z

+	icp->un.echo.sequence |= htons(1);	/* Set L-bit */
+	WRITE_VERSION(ext.v_rsvd , 2);
+	ext.v_rsvd = htons(ext.v_rsvd);
+	ext.checksum = 0;


You must fill in the checksum later. While it appears that the Linux kernel does not check the checksum, there are other probe implementations that do.

My tcpdump update the-tcpdump-group/tcpdump#1115 will check that it's filled in properly.

fenner · 2024-01-22T17:32:42Z

+	/* These checks mimic kernel checks in dev_valid_name */
+	if (*name == '\0')
+		return -1;
+	if (strlen(name) >= IFNAMSIZ)


This limit only exists in Linux. Imagine running this against a router with an interface name TwentyFiveGigabitEthernet3/0/1

fenner · 2024-01-22T17:33:23Z

+		return -1;
+
+	while (*name) {
+		if (*name == '/' || isspace(*name))


As mentioned above, slashes are common in interface names. RFC8335 only limits interface names by referring to RFC7223, which says that they are yang strings.

fenner · 2024-01-22T17:36:59Z

+			return 1;
+
+		sequence = ntohs(icmph->icmp6_seq);
+		state = icmph->icmp6_seq & 0xe0;


You forgot to shift this right by 5, so the switch below won't hit any of the cases.

fenner · 2024-01-22T17:38:22Z

+			case 4:
+				printf("Error: Multiple Interfaces Satisfy Query\n");
+				break;
+			default:


An invalid code (5 or above) will result in no error, and no output from the "code == 0" case below. This will likely be fairly confusing. Sure, it shouldn't happen, but, let's try to confuse the user less if it does occur.

fenner · 2024-01-22T17:39:58Z

+				if (sequence & ICMP_EXT_ECHOREPLY_IPV4)
+					printf(" IPV4");
+				if (sequence & ICMP_EXT_ECHOREPLY_IPV6)
+					printf(" IPV6");


There's an unusual situation when the status is active, but there is no v4 or v6 configured. I suggest printing out something specific when (!sequence & (ICMP_EXT_ECHOREPLY_IPV4| ICMP_EXT_ECHOREPLY_IPV6)).

fenner · 2024-01-22T17:41:00Z

+				if (sequence & ICMP_EXT_ECHOREPLY_IPV6)
+					printf(" IPV6");
+			printf("\n");
+			}


The else case here is significant: if you sent a query about an interface, and A=0, that is something the user probably really wants to know. Maybe

} else { printf("Status: INACTIVE\n"); }

fenner · 2024-01-22T17:41:54Z

+			default:
+				break;
+		}
+		switch (state) {


The returned state value is only valid if code == 0 and the transmitted L flag is 0; I recommend checking that here.

fenner · 2024-01-22T17:42:19Z

+			default:
+				break;
+		}
+		if (icmph->icmp6_code == 0) {


The returned A/4/6 values are only valid if code=0 and L=1, I recommend checking that here.

fenner · 2024-01-22T17:44:23Z

+	icmph->icmp6_id = rts->ident;
+	/* PROBE messages use only the first 8 bits as sequence number */
+	icmph->icmp6_dataun.icmp6_un_data8[2] = rts->ntransmitted + 1;
+	icmph->icmp6_dataun.icmp6_un_data8[3] = 1;	/* Set L-bit */


I recommend giving the user an argument to control this (e.g., "--remote") - this is one of the most important pieces of PROBE functionality. In the case where the L-bit is clear, the only valid ctype is ICMP_EXT_ECHO_CTYPE_ADDR, but otherwise all the rest of the code applies.

fenner · 2024-01-22T20:41:02Z

Oh, one more thought here: when running the client on a system without the necessary kernel updates (pre-5.14, afaik), the send() returns with EINVAL and the request never gets sent, but there is no indication of a problem.

./ping -e 1 -c 5 127.0.0.1
PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.

--- 127.0.0.1 ping statistics ---
5 packets transmitted, 0 received, 100% packet loss, time 4094ms

100% packet loss is probably not the best way to represent "your system can not send this type of packet".

strace says:

sendto(3, "*\0\353\16\0\0\5\1  \0\0\0\10\3\2\0\0\0\1\261\321\256e\0\0\0\0\"\310\0\0"..., 64, 0, {sa_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINVAL (Invalid argument)

fenner · 2024-01-23T15:58:31Z

If the system is using the raw socket (e.g., the user's gid is not inside net.ipv4.ping_group_range), then the filter installed by ping4_install_filter() still expects the return packet to be ICMP_ECHOREPLY, so will not filter out the extended echo replies for a simultaneous ping with a different ident.

fenner · 2024-03-09T19:01:59Z

I've created issues for all of my comments at https://github.com/aristanetworks/iputils/issues and intend to work on addressing them.

See https://github.com/aristanetworks/iputils/wiki for more information.

ping: add RFC 8335 PROBE request and response functionality

9451dea

Signed-off-by: Andreas Roeseler <[email protected]>

pevik added the new RFC / experimental label Mar 7, 2023

fenner suggested changes Jan 22, 2024

View reviewed changes

Conversation

aroeseler commented Jun 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fenner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fenner commented Jan 22, 2024

Uh oh!

fenner commented Jan 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fenner commented Mar 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aroeseler commented Jun 22, 2021 •

edited

Loading

fenner commented Jan 23, 2024 •

edited

Loading