[B.A.T.M.A.N.] [PATCH] batctl: create common header for ICMP packets

[B.A.T.M.A.N.] [PATCH] batctl: create common header for ICMP packets

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

the icmp and the icmp_rr packets share the same initial
fields since they use the same code to be processed and
forwarded.

Extract the common fields and put them into a separate
struct so that future ICMP packets can be easily added
without bloating the packet definition.

However, keep the seqno field outside of the newly created
common header because future ICMP types may require a
bigger sequence number space.

This change breaks compatibility due to fields reordering
in the ICMP headers.

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---

This patch assumes that 

("batman-adv: fix typo in kernel doc")

and

("batman-adv: correctly align the tt_tvlv_data struct")

have been applied to batctl too.


 ping.c       | 24 ++++++++++++++----------
 tcpdump.c    | 27 +++++++++++++++++----------
 traceroute.c | 31 +++++++++++++++++++------------
 3 files changed, 50 insertions(+), 32 deletions(-)

diff --git a/ping.c b/ping.c
index 5d6b7e9..6679604 100644
--- a/ping.c
+++ b/ping.c
@@ -171,11 +171,11 @@ int ping(char *mesh_iface, int argc, char **argv)
 
 	packet_len = sizeof(struct batadv_icmp_packet);
 
-	memcpy(&icmp_packet_out.dst, dst_mac, ETH_ALEN);
-	icmp_packet_out.header.packet_type = BATADV_ICMP;
-	icmp_packet_out.header.version = BATADV_COMPAT_VERSION;
-	icmp_packet_out.msg_type = BATADV_ECHO_REQUEST;
-	icmp_packet_out.header.ttl = 50;
+	memcpy(&icmp_packet_out.icmph.dst, dst_mac, ETH_ALEN);
+	icmp_packet_out.icmph.header.packet_type = BATADV_ICMP;
+	icmp_packet_out.icmph.header.version = BATADV_COMPAT_VERSION;
+	icmp_packet_out.icmph.msg_type = BATADV_ECHO_REQUEST;
+	icmp_packet_out.icmph.header.ttl = 50;
 	icmp_packet_out.seqno = 0;
 
 	if (rr) {
@@ -245,12 +245,14 @@ read_packet:
 		if (htons(seq_counter) != icmp_packet_in.seqno)
 			goto read_packet;
 
-		switch (icmp_packet_in.msg_type) {
+		switch (icmp_packet_in.icmph.msg_type) {
 		case BATADV_ECHO_REPLY:
 			time_delta = end_timer();
 			printf("%zd bytes from %s icmp_seq=%hu ttl=%d time=%.2f ms",
-					read_len, dst_string, ntohs(icmp_packet_in.seqno),
-					icmp_packet_in.header.ttl, time_delta);
+					read_len, dst_string,
+					ntohs(icmp_packet_in.seqno),
+					icmp_packet_in.icmph.header.ttl,
+					time_delta);
 
 			if (read_len == sizeof(struct batadv_icmp_packet_rr)) {
 				if (last_rr_cur == icmp_packet_in.rr_cur
@@ -299,11 +301,13 @@ read_packet:
 			break;
 		case BATADV_PARAMETER_PROBLEM:
 			fprintf(stderr, "Error - the batman adv kernel module version (%d) differs from ours (%d)\n",
-					icmp_packet_in.header.version, BATADV_COMPAT_VERSION);
+					icmp_packet_in.icmph.header.version,
+					BATADV_COMPAT_VERSION);
 			printf("Please make sure to use compatible versions!\n");
 			goto out;
 		default:
-			printf("Unknown message type %d len %zd received\n", icmp_packet_in.msg_type, read_len);
+			printf("Unknown message type %d len %zd received\n",
+			       icmp_packet_in.icmph.msg_type, read_len);
 			break;
 		}
 
diff --git a/tcpdump.c b/tcpdump.c
index 3e932cd..7e0987b 100644
--- a/tcpdump.c
+++ b/tcpdump.c
@@ -361,32 +361,39 @@ static void dump_batman_icmp(unsigned char *packet_buff, ssize_t buff_len, int r
 	if (!time_printed)
 		print_time();
 
-	printf("BAT %s > ", get_name_by_macaddr((struct ether_addr *)icmp_packet->orig, read_opt));
+	printf("BAT %s > ",
+	       get_name_by_macaddr((struct ether_addr *)icmp_packet->icmph.orig,
+				   read_opt));
 
-	name = get_name_by_macaddr((struct ether_addr *)icmp_packet->dst, read_opt);
+	name = get_name_by_macaddr((struct ether_addr *)icmp_packet->icmph.dst,
+				   read_opt);
 
-	switch (icmp_packet->msg_type) {
+	switch (icmp_packet->icmph.msg_type) {
 	case BATADV_ECHO_REPLY:
 		printf("%s: ICMP echo reply, id %hhu, seq %hu, ttl %2d, v %d, length %zu\n",
-			name, icmp_packet->uid, ntohs(icmp_packet->seqno),
-			icmp_packet->header.ttl, icmp_packet->header.version,
+			name, icmp_packet->icmph.uid, ntohs(icmp_packet->seqno),
+			icmp_packet->icmph.header.ttl,
+			icmp_packet->icmph.header.version,
 			(size_t)buff_len - sizeof(struct ether_header));
 		break;
 	case BATADV_ECHO_REQUEST:
 		printf("%s: ICMP echo request, id %hhu, seq %hu, ttl %2d, v %d, length %zu\n",
-			name, icmp_packet->uid, ntohs(icmp_packet->seqno),
-			icmp_packet->header.ttl, icmp_packet->header.version,
+			name, icmp_packet->icmph.uid, ntohs(icmp_packet->seqno),
+			icmp_packet->icmph.header.ttl,
+			icmp_packet->icmph.header.version,
 			(size_t)buff_len - sizeof(struct ether_header));
 		break;
 	case BATADV_TTL_EXCEEDED:
 		printf("%s: ICMP time exceeded in-transit, id %hhu, seq %hu, ttl %2d, v %d, length %zu\n",
-			name, icmp_packet->uid, ntohs(icmp_packet->seqno),
-			icmp_packet->header.ttl, icmp_packet->header.version,
+			name, icmp_packet->icmph.uid, ntohs(icmp_packet->seqno),
+			icmp_packet->icmph.header.ttl,
+			icmp_packet->icmph.header.version,
 			(size_t)buff_len - sizeof(struct ether_header));
 		break;
 	default:
 		printf("%s: ICMP type %hhu, length %zu\n",
-			name, icmp_packet->msg_type, (size_t)buff_len - sizeof(struct ether_header));
+			name, icmp_packet->icmph.msg_type,
+			(size_t)buff_len - sizeof(struct ether_header));
 		break;
 	}
 }
diff --git a/traceroute.c b/traceroute.c
index 38d3796..362d293 100644
--- a/traceroute.c
+++ b/traceroute.c
@@ -129,17 +129,19 @@ int traceroute(char *mesh_iface, int argc, char **argv)
 		goto out;
 	}
 
-	memcpy(&icmp_packet_out.dst, dst_mac, ETH_ALEN);
-	icmp_packet_out.header.version = BATADV_COMPAT_VERSION;
-	icmp_packet_out.header.packet_type = BATADV_ICMP;
-	icmp_packet_out.msg_type = BATADV_ECHO_REQUEST;
+	memcpy(&icmp_packet_out.icmph.dst, dst_mac, ETH_ALEN);
+	icmp_packet_out.icmph.header.version = BATADV_COMPAT_VERSION;
+	icmp_packet_out.icmph.header.packet_type = BATADV_ICMP;
+	icmp_packet_out.icmph.msg_type = BATADV_ECHO_REQUEST;
 	icmp_packet_out.seqno = 0;
 	icmp_packet_out.reserved = 0;
 
 	printf("traceroute to %s (%s), %d hops max, %zu byte packets\n",
 		dst_string, mac_string, TTL_MAX, sizeof(icmp_packet_out));
 
-	for (icmp_packet_out.header.ttl = 1; !dst_reached && icmp_packet_out.header.ttl < TTL_MAX; icmp_packet_out.header.ttl++) {
+	for (icmp_packet_out.icmph.header.ttl = 1;
+	     !dst_reached && icmp_packet_out.icmph.header.ttl < TTL_MAX;
+	     icmp_packet_out.icmph.header.ttl++) {
 		return_mac = NULL;
 		bat_host = NULL;
 
@@ -183,7 +185,7 @@ read_packet:
 			if (htons(seq_counter) != icmp_packet_in.seqno)
 				goto read_packet;
 
-			switch (icmp_packet_in.msg_type) {
+			switch (icmp_packet_in.icmph.msg_type) {
 			case BATADV_ECHO_REPLY:
 				dst_reached = 1;
 				/* fall through */
@@ -191,10 +193,10 @@ read_packet:
 				time_delta[i] = end_timer();
 
 				if (!return_mac) {
-					return_mac = ether_ntoa_long((struct ether_addr *)&icmp_packet_in.orig);
+					return_mac = ether_ntoa_long((struct ether_addr *)&icmp_packet_in.icmph.orig);
 
 					if (read_opt & USE_BAT_HOSTS)
-						bat_host = bat_hosts_find_by_mac((char *)&icmp_packet_in.orig);
+						bat_host = bat_hosts_find_by_mac((char *)&icmp_packet_in.icmph.orig);
 				}
 
 				break;
@@ -203,19 +205,24 @@ read_packet:
 				goto out;
 			case BATADV_PARAMETER_PROBLEM:
 				fprintf(stderr, "Error - the batman adv kernel module version (%d) differs from ours (%d)\n",
-						icmp_packet_in.header.version, BATADV_COMPAT_VERSION);
+						icmp_packet_in.icmph.header.version,
+						BATADV_COMPAT_VERSION);
 				fprintf(stderr, "Please make sure to use compatible versions!\n");
 				goto out;
 			default:
-				printf("Unknown message type %d len %zd received\n", icmp_packet_in.msg_type, read_len);
+				printf("Unknown message type %d len %zd received\n",
+				       icmp_packet_in.icmph.msg_type, read_len);
 				break;
 			}
 		}
 
 		if (!bat_host)
-			printf("%2hhu: %s", icmp_packet_out.header.ttl, (return_mac ? return_mac : "*"));
+			printf("%2hhu: %s", icmp_packet_out.icmph.header.ttl,
+			       (return_mac ? return_mac : "*"));
 		else
-			printf("%2hhu: %s (%s)", icmp_packet_out.header.ttl, bat_host->name, return_mac);
+			printf("%2hhu: %s (%s)",
+			       icmp_packet_out.icmph.header.ttl,
+			       bat_host->name, return_mac);
 
 		for (i = 0; i < NUM_PACKETS; i++) {
 			if (time_delta[i])
-- 
1.8.1.5

Re: [B.A.T.M.A.N.] [PATCH] batctl: create common header for ICMP packets

[Marek Lindner]

On Monday, May 27, 2013 16:55:20 Antonio Quartulli wrote:
>  ping.c       | 24 ++++++++++++++----------
>  tcpdump.c    | 27 +++++++++++++++++----------
>  traceroute.c | 31 +++++++++++++++++++------------
>  3 files changed, 50 insertions(+), 32 deletions(-)

How about adding the packet.h changes to this patch ? That would make the code 
compile.  :)

Cheers,
Marek

Re: [B.A.T.M.A.N.] [PATCH] batctl: create common header for ICMP packets

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

On Wed, May 29, 2013 at 03:41:21PM +0800, Marek Lindner wrote:
> On Monday, May 27, 2013 16:55:20 Antonio Quartulli wrote:
> >  ping.c       | 24 ++++++++++++++----------
> >  tcpdump.c    | 27 +++++++++++++++++----------
> >  traceroute.c | 31 +++++++++++++++++++------------
> >  3 files changed, 50 insertions(+), 32 deletions(-)
> 
> How about adding the packet.h changes to this patch ? That would make the code 
> compile.  :)

oky


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]