[git commit] ping: populate icmp_id field for "simple" ping too

[Denys Vlasenko]

commit: https://git.busybox.net/busybox/commit/?id=4d5acd2d4264d0a754d3d11c94825fd69d0c7837
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

The ICMP RFC says that identifier and sequence number may be zero.
Having them zero for a Echo message, along with a data of zero's
as well will result in a Echo reply message with only zero's.

Some NAT implementations seem to get the checksum wrong on these
packages. Setting a checksum of 0x0 instead of 0xffff.

Through NAT:
  Internet Control Message Protocol
      Type: 0 (Echo (ping) reply)
      Code: 0
      Checksum: 0x0000 [incorrect, should be 0xffff]
      Identifier (BE): 0 (0x0000)
      Identifier (LE): 0 (0x0000)
      Sequence number (BE): 0 (0x0000)
      Sequence number (LE): 0 (0x0000)
      Data (56 bytes)
          Data: 000000000000000000000000000000000000000000000000...
          [Length: 56]

Without NAT:
  Internet Control Message Protocol
      Type: 0 (Echo (ping) reply)
      Code: 0
      Checksum: 0xffff [correct]
      Identifier (BE): 0 (0x0000)
      Identifier (LE): 0 (0x0000)
      Sequence number (BE): 0 (0x0000)
      Sequence number (LE): 0 (0x0000)
      [Request frame: 189]
      [Response time: 0.024 ms]
      Data (56 bytes)
          Data: 000000000000000000000000000000000000000000000000...
          [Length: 56]

And this in turn will make some hardware MAC checksum offloading
engines drop the packet.

(This was seen with a Synopsis MAC, the same one used in for instance the
stmmac Ethernet driver in the linux kernel.)

This change can be seen as a workaround for bugs in other layers.
But just setting an identifier for the Echo message packet will
avoid prodding the hornets nest.

function                                             old     new   delta
common_ping_main                                     424     500     +76

Signed-off-by: Jonas Danielsson <jonasdn at axis.com>
Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 networking/ping.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/networking/ping.c b/networking/ping.c
index cfe6826..d8767a3 100644
--- a/networking/ping.c
+++ b/networking/ping.c
@@ -186,6 +186,7 @@ create_icmp_socket(void)
 struct globals {
 	char *hostname;
 	char packet[DEFDATALEN + MAXIPLEN + MAXICMPLEN];
+	uint16_t myid;
 } FIX_ALIASING;
 #define G (*(struct globals*)bb_common_bufsiz1)
 #define INIT_G() do { setup_common_bufsiz(); } while (0)
@@ -204,6 +205,7 @@ static void ping4(len_and_sockaddr *lsa)
 	pkt = (struct icmp *) G.packet;
 	/*memset(pkt, 0, sizeof(G.packet)); already is */
 	pkt->icmp_type = ICMP_ECHO;
+	pkt->icmp_id = G.myid;
 	pkt->icmp_cksum = inet_cksum((uint16_t *) pkt, sizeof(G.packet));
 
 	xsendto(pingsock, G.packet, DEFDATALEN + ICMP_MINLEN, &lsa->u.sa, lsa->len);
@@ -228,6 +230,8 @@ static void ping4(len_and_sockaddr *lsa)
 			struct iphdr *iphdr = (struct iphdr *) G.packet;
 
 			pkt = (struct icmp *) (G.packet + (iphdr->ihl << 2));	/* skip ip hdr */
+			if (pkt->icmp_id != G.myid)
+				continue; /* not our ping */
 			if (pkt->icmp_type == ICMP_ECHOREPLY)
 				break;
 		}
@@ -246,6 +250,7 @@ static void ping6(len_and_sockaddr *lsa)
 	pkt = (struct icmp6_hdr *) G.packet;
 	/*memset(pkt, 0, sizeof(G.packet)); already is */
 	pkt->icmp6_type = ICMP6_ECHO_REQUEST;
+	pkt->icmp6_id = G.myid;
 
 	sockopt = offsetof(struct icmp6_hdr, icmp6_cksum);
 	setsockopt_int(pingsock, SOL_RAW, IPV6_CHECKSUM, sockopt);
@@ -269,6 +274,8 @@ static void ping6(len_and_sockaddr *lsa)
 			continue;
 		}
 		if (c >= ICMP_MINLEN) {	/* icmp6_hdr */
+			if (pkt->icmp6_id != G.myid)
+				continue; /* not our ping */
 			if (pkt->icmp6_type == ICMP6_ECHO_REPLY)
 				break;
 		}
@@ -317,6 +324,7 @@ static int common_ping_main(sa_family_t af, char **argv)
 	alarm(5); /* give the host 5000ms to respond */
 
 	create_icmp_socket(lsa);
+	G.myid = (uint16_t) getpid();
 #if ENABLE_PING6
 	if (lsa->u.sa.sa_family == AF_INET6)
 		ping6(lsa);