[commits] [alfred] master: alfred: Use well defined structs to create packets (ed38711)

18 Jan 2013

Repository : ssh://git@open-mesh.org/alfred
On branch  : master
...

commit ed3871171223562bbd2da1dd4354916a04382210
Author: Sven Eckelmann sven@open-mesh.com
Date:   Fri Jan 11 19:13:17 2013 +0100
alfred: Use well defined structs to create packets
The current packets sent over the network or over the unix socket were only
    defined in the code but not by any struct. This makes it hard to find the right
    place in the code to change when working with the struct. It is also makes it
    easy to accidently use the wrong position when reading or writing from/to the
    buffer.
Structs are introduced to define the fixed entries of a packet. The struct also
    defines the position of the dynamic part followed by it using zero-length
    arrays.
Signed-off-by: Sven Eckelmann sven@open-mesh.com
...

ed3871171223562bbd2da1dd4354916a04382210
 alfred.h    |    4 ----
 client.c    |   47 +++++++++++++++++++--------------------
 packet.h    |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 recv.c      |   51 ++++++++++++++++++++++++------------------
 send.c      |   43 ++++++++++++++++++------------------
 unix_sock.c |   59 ++++++++++++++++++++++++++-----------------------
 6 files changed, 170 insertions(+), 105 deletions(-)

diff --git a/alfred.h b/alfred.h
index 6632571..21f0078 100644
--- a/alfred.h
+++ b/alfred.h
@@ -44,8 +44,6 @@ enum data_source {
    SOURCE_SYNCED = 2,
 };
-struct alfred_data;
-
 struct dataset {
    struct alfred_data data;
    unsigned char *buf;
@@ -95,8 +93,6 @@ struct globals {
 #define debugMalloc(size, num)	malloc(size)
 #define debugFree(ptr, num)	free(ptr)
-#define ALFRED_HEADLEN		sizeof(struct alfred_packet)
-
 #define MAX_PAYLOAD ((1 << 16) - 1)
extern const struct in6_addr in6addr_localmcast;
diff --git a/client.c b/client.c
index c7780cb..f1629ab 100644
--- a/client.c
+++ b/client.c
@@ -34,34 +34,36 @@
 int alfred_client_request_data(struct globals *globals)
 {
    unsigned char buf[MAX_PAYLOAD], *pos;
-	struct alfred_packet *packet;
+	struct alfred_request_v0 *request;
+	struct alfred_push_data_v0 *push;
    struct alfred_data *data;
    int ret, len, headlen, data_len, i;
if (unix_sock_open_client(globals, ALFRED_SOCK_PATH))
    	return -1;
-	packet = (struct alfred_packet *)buf;
-	len = sizeof(*packet) + 1;
+	request = (struct alfred_request_v0 *)buf;
+	len = sizeof(*request);
-	packet->type = ALFRED_REQUEST;
-	packet->version = ALFRED_VERSION;
-	packet->length = htons(1);
-	*((uint8_t *)(packet + 1)) = globals->clientmode_arg;
+	request->header.type = ALFRED_REQUEST;
+	request->header.version = ALFRED_VERSION;
+	request->header.length = htons(1);
+	request->requested_type = globals->clientmode_arg;
ret = write(globals->unix_sock, buf, len);
    if (ret != len)
    	fprintf(stderr, "%s: only wrote %d of %d bytes: %s\n",
    		__func__, ret, len, strerror(errno));
-	headlen = sizeof(*packet) + sizeof(*data);
+	headlen = sizeof(*push) + sizeof(*data);
+	push = (struct alfred_push_data_v0 *)buf;
    while ((ret = read(globals->unix_sock, buf, headlen)) > 0) {
    	/* too short */
    	if (ret < headlen)
    		break;
-		data = (struct alfred_data *)(packet + 1);
-		data_len = ntohs(data->length);
+		data = push->data;
+		data_len = ntohs(data->header.length);
/* would it fit? it should! */
    	if (data_len > (int)(sizeof(buf) - headlen))
@@ -74,7 +76,7 @@ int alfred_client_request_data(struct globals *globals)
    	if (ret < data_len)
    		break;
-		pos = (uint8_t *)(data + 1);
+		pos = data->data;
printf("{ "%02x:%02x:%02x:%02x:%02x:%02x", "",
    	       data->source[0], data->source[1],
@@ -102,16 +104,16 @@ int alfred_client_request_data(struct globals *globals)
 int alfred_client_set_data(struct globals *globals)
 {
    unsigned char buf[MAX_PAYLOAD];
-	struct alfred_packet *packet;
+	struct alfred_push_data_v0 *push;
    struct alfred_data *data;
    int ret, len;
if (unix_sock_open_client(globals, ALFRED_SOCK_PATH))
    	return -1;
-	packet = (struct alfred_packet *)buf;
-	data = (struct alfred_data *)(packet + 1);
-	len = sizeof(*packet) + sizeof(*data);
+	push = (struct alfred_push_data_v0 *)buf;
+	data = push->data;
+	len = sizeof(*push) + sizeof(*data);
    while (!feof(stdin)) {
    	ret = fread(&buf[len], 1, sizeof(buf) - len, stdin);
    	len += ret;
@@ -120,16 +122,15 @@ int alfred_client_set_data(struct globals *globals)
    		break;
    }
-	packet->type = ALFRED_PUSH_DATA;
-	packet->version = ALFRED_VERSION;
-	packet->length = htons(len - sizeof(*packet));
+	push->header.type = ALFRED_PUSH_DATA;
+	push->header.version = ALFRED_VERSION;
+	push->header.length = htons(len - sizeof(*push));
/* we leave data->source "empty" */
    memset(data->source, 0, sizeof(data->source));
-	data->type = globals->clientmode_arg;
-	data->version = globals->clientmode_version;
-	data->length = htons(len - sizeof(*packet) - sizeof(*data));
-
+	data->header.type = globals->clientmode_arg;
+	data->header.version = globals->clientmode_version;
+	data->header.length = htons(len - sizeof(*push) - sizeof(*data));
ret = write(globals->unix_sock, buf, len);
    if (ret != len)
@@ -139,5 +140,3 @@ int alfred_client_set_data(struct globals *globals)
    unix_sock_close(globals);
    return 0;
 }
-
-
diff --git a/packet.h b/packet.h
index 5b8bfb5..3579a3d 100644
--- a/packet.h
+++ b/packet.h
@@ -24,25 +24,84 @@
#define __packed __attribute__ ((packed))
-struct alfred_data {
-	uint8_t source[ETH_ALEN];
+/* basic blocks */
+
+/**
+ * struct alfred_tlv - Type (Version) Length part of a TLV
+ * @type: Type of the data
+ * @version: Version of the data
+ * @length: Length of the data without the alfred_tlv header
+ */
+struct alfred_tlv {
    uint8_t type;
    uint8_t version;
    uint16_t length;
 } __packed;
-struct alfred_packet {
-	uint8_t type;
-	uint8_t version;
-	uint16_t length;
+/**
+ * struct alfred_data - Data block header
+ * @source: Mac address of the original source of the data
+ * @header: TLV-header for the data
+ * @data: "length" number of bytes followed by the header
+ */
+struct alfred_data {
+	uint8_t source[ETH_ALEN];
+	struct alfred_tlv header;
+	/* flexible data block */
+	__extension__ uint8_t data[0];
 } __packed;
+/**
+ * enum alfred_packet_type - Types of packet stored in the main alfred_tlv
+ * @ALFRED_PUSH_DATA: Packet is an alfred_push_data_v*
+ * @ALFRED_ANNOUNCE_MASTER: Packet is an alfred_announce_master_v*
+ * @ALFRED_REQUEST: Packet is an alfred_request_v*
+ */
 enum alfred_packet_type {
    ALFRED_PUSH_DATA = 0,
    ALFRED_ANNOUNCE_MASTER = 1,
    ALFRED_REQUEST = 2,
 };
+/* packets */
+
+/**
+ * struct alfred_push_data_v0 - Packet to push data blocks to another
+ * @header: TLV header describing the complete packet
+ * @data: multiple "alfred_data" blocks of arbitrary size (accumulated size
+ *  stored in "header.length")
+ *
+ * alfred_push_data_v0 packets are always sent using unicast
+ */
+struct alfred_push_data_v0 {
+	struct alfred_tlv header;
+	/* flexible data block */
+	__extension__  struct alfred_data data[0];
+} __packed;
+
+/**
+ * struct alfred_announce_master_v0 - Hello packet sent by an alfred master
+ * @header: TLV header describing the complete packet
+ *
+ * Each alfred daemon running in master mode sends it using multicast. The
+ * receiver has to calculate the source using the network header
+ */
+struct alfred_announce_master_v0 {
+	struct alfred_tlv header;
+} __packed;
+
+/**
+ * struct alfred_request_v0 - Request for a specific type
+ * @header: TLV header describing the complete packet
+ * @requested_type: data type which is requested
+ *
+ * Sent as unicast to the node storing it
+ */
+struct alfred_request_v0 {
+	struct alfred_tlv header;
+	uint8_t requested_type;
+} __packed;
+
 #define ALFRED_VERSION			0
 #define ALFRED_PORT			0x4242
 #define ALFRED_MAX_RESERVED_TYPE	64
diff --git a/recv.c b/recv.c
index 37bcc3d..7d4d4bf 100644
--- a/recv.c
+++ b/recv.c
@@ -35,7 +35,7 @@
static int process_alfred_push_data(struct globals *globals,
    			    struct in6_addr *source,
-				    struct alfred_packet *packet)
+				    struct alfred_push_data_v0 *push)
 {
    int len, data_len;
    struct alfred_data *data;
@@ -48,12 +48,12 @@ static int process_alfred_push_data(struct globals *globals,
    if (ret < 0)
    	goto err;
-	len = ntohs(packet->length);
-	pos = (uint8_t *)(packet + 1);
+	len = ntohs(push->header.length);
+	pos = (uint8_t *)push->data;
while (len > (int)sizeof(*data)) {
    	data = (struct alfred_data *)pos;
-		data_len = ntohs(data->length);
+		data_len = ntohs(data->header.length);
/* check if enough data is available */
    	if ((int)(data_len + sizeof(*data)) > len)
@@ -89,8 +89,8 @@ static int process_alfred_push_data(struct globals *globals,
    	if (!dataset->buf)
    		goto err;
-		dataset->data.length = data_len;
-		memcpy(dataset->buf, (data + 1), data_len);
+		dataset->data.header.length = data_len;
+		memcpy(dataset->buf, data->data, data_len);
/* if the sender is also the the source of the dataset, we
    	 * got a first hand dataset. */
@@ -107,20 +107,27 @@ err:
    return -1;
 }
-static int process_alfred_announce_master(struct globals *globals,
-					  struct in6_addr *source,
-					  struct alfred_packet *packet)
+static int
+process_alfred_announce_master(struct globals *globals,
+			       struct in6_addr *source,
+			       struct alfred_announce_master_v0 *announce)
 {
    struct server *server;
    struct ether_addr *macaddr;
    struct ether_addr mac;
    int ret;
+	int len;
+
+	len = ntohs(announce->header.length);
ret = ipv6_to_mac(source, &mac);
    if (ret < 0)
    	return -1;
-	if (packet->version != ALFRED_VERSION)
+	if (announce->header.version != ALFRED_VERSION)
+		return -1;
+
+	if (len != (sizeof(*announce) - sizeof(announce->header)))
    	return -1;
server = hash_find(globals->server_hash, &mac);
@@ -159,21 +166,19 @@ static int process_alfred_announce_master(struct globals *globals,
static int process_alfred_request(struct globals *globals,
    			  struct in6_addr *source,
-				  struct alfred_packet *packet)
+				  struct alfred_request_v0 *request)
 {
-	uint8_t type;
    int len;
-	len = ntohs(packet->length);
+	len = ntohs(request->header.length);
-	if (packet->version != ALFRED_VERSION)
+	if (request->header.version != ALFRED_VERSION)
    	return -1;
-	if (len != 1)
+	if (len != (sizeof(*request) - sizeof(request->header)))
    	return -1;
-	type = *((uint8_t *)(packet + 1));
-	push_data(globals, source, SOURCE_SYNCED, type);
+	push_data(globals, source, SOURCE_SYNCED, request->requested_type);
return 0;
 }
@@ -183,7 +188,7 @@ int recv_alfred_packet(struct globals *globals)
 {
    uint8_t buf[MAX_PAYLOAD];
    ssize_t length;
-	struct alfred_packet *packet;
+	struct alfred_tlv *packet;
    struct sockaddr_in6 source;
    socklen_t sourcelen;
@@ -196,7 +201,7 @@ int recv_alfred_packet(struct globals *globals)
    	return -1;
    }
-	packet = (struct alfred_packet *)buf;
+	packet = (struct alfred_tlv *)buf;
/* drop packets not sent over link-local ipv6 */
    if (!is_ipv6_eui64(&source.sin6_addr))
@@ -217,14 +222,16 @@ int recv_alfred_packet(struct globals *globals)
switch (packet->type) {
    case ALFRED_PUSH_DATA:
-		process_alfred_push_data(globals, &source.sin6_addr, packet);
+		process_alfred_push_data(globals, &source.sin6_addr,
+					 (struct alfred_push_data_v0 *)packet);
    	break;
    case ALFRED_ANNOUNCE_MASTER:
    	process_alfred_announce_master(globals, &source.sin6_addr,
-					       packet);
+					       (struct alfred_announce_master_v0 *)packet);
    	break;
    case ALFRED_REQUEST:
-		process_alfred_request(globals, &source.sin6_addr, packet);
+		process_alfred_request(globals, &source.sin6_addr,
+				       (struct alfred_request_v0 *)packet);
    	break;
    default:
    	/* unknown packet type */
diff --git a/send.c b/send.c
index 8b52ed1..01bc592 100644
--- a/send.c
+++ b/send.c
@@ -35,11 +35,11 @@
int announce_master(struct globals *globals)
 {
-	struct alfred_packet announcement;
+	struct alfred_announce_master_v0 announcement;
-	announcement.type = ALFRED_ANNOUNCE_MASTER;
-	announcement.version = ALFRED_VERSION;
-	announcement.length = htons(0);
+	announcement.header.type = ALFRED_ANNOUNCE_MASTER;
+	announcement.header.version = ALFRED_VERSION;
+	announcement.header.length = htons(0);
send_alfred_packet(globals, &in6addr_localmcast, &announcement,
    		   sizeof(announcement));
@@ -52,13 +52,13 @@ int push_data(struct globals *globals, struct in6_addr *destination,
 {
    struct hash_it_t *hashit = NULL;
    uint8_t buf[MAX_PAYLOAD];
-	struct alfred_packet *packet;
+	struct alfred_push_data_v0 *push;
    struct alfred_data *data;
    uint16_t total_length = 0;
-	packet = (struct alfred_packet *)buf;
-	packet->type = ALFRED_PUSH_DATA;
-	packet->version = ALFRED_VERSION;
+	push = (struct alfred_push_data_v0 *)buf;
+	push->header.type = ALFRED_PUSH_DATA;
+	push->header.version = ALFRED_VERSION;
while (NULL != (hashit = hash_iterate(globals->data_hash, hashit))) {
    	struct dataset *dataset = hashit->bucket->data;
@@ -66,32 +66,33 @@ int push_data(struct globals *globals, struct in6_addr *destination,
    	if (dataset->data_source > max_source_level)
    		continue;
-		if (type_filter >= 0 && dataset->data.type != type_filter)
+		if (type_filter >= 0 &&
+		    dataset->data.header.type != type_filter)
    		continue;
/* would the packet be too big? send so far aggregated data
    	 * first */
-		if (total_length + dataset->data.length + sizeof(*data) >
-		    MAX_PAYLOAD - ALFRED_HEADLEN) {
-			packet->length = htons(total_length);
-			send_alfred_packet(globals, destination, packet,
-					   sizeof(*packet) + total_length);
+		if (total_length + dataset->data.header.length + sizeof(*data) >
+		    MAX_PAYLOAD - sizeof(*push)) {
+			push->header.length = htons(total_length);
+			send_alfred_packet(globals, destination, push,
+					   sizeof(*push) + total_length);
    		total_length = 0;
    	}
data = (struct alfred_data *)
-		       (buf + sizeof(*packet) + total_length);
+		       (buf + sizeof(*push) + total_length);
    	memcpy(data, &dataset->data, sizeof(*data));
-		data->length = htons(data->length);
-		memcpy((data + 1), dataset->buf, dataset->data.length);
+		data->header.length = htons(data->header.length);
+		memcpy(data->data, dataset->buf, dataset->data.header.length);
-		total_length += dataset->data.length + sizeof(*data);
+		total_length += dataset->data.header.length + sizeof(*data);
    }
    /* send the final packet */
    if (total_length) {
-		packet->length = htons(total_length);
-		send_alfred_packet(globals, destination, packet,
-				   sizeof(*packet) + total_length);
+		push->header.length = htons(total_length);
+		send_alfred_packet(globals, destination, push,
+				   sizeof(*push) + total_length);
    }
return 0;
diff --git a/unix_sock.c b/unix_sock.c
index 4a5df12..27dee54 100644
--- a/unix_sock.c
+++ b/unix_sock.c
@@ -91,19 +91,19 @@ int unix_sock_open_client(struct globals *globals, char *path)
static int unix_sock_add_data(struct globals *globals,
-			      struct alfred_packet *packet)
+			      struct alfred_push_data_v0 *push)
 {
    struct alfred_data *data;
    struct dataset *dataset;
    int len, data_len;
-	len = ntohs(packet->length);
+	len = ntohs(push->header.length);
if (len < (int)sizeof(*data))
    	return -1;
-	data = (struct alfred_data *)(packet + 1);
-	data_len = ntohs(data->length);
+	data = push->data;
+	data_len = ntohs(data->header.length);
    memcpy(data->source, &globals->hwaddr, sizeof(globals->hwaddr));
if ((int)(data_len + sizeof(*data)) > len)
@@ -127,33 +127,34 @@ static int unix_sock_add_data(struct globals *globals,
    dataset->last_seen = time(NULL);
/* free old buffer */
-	if (dataset->buf)
-		free(dataset->buf);
+	free(dataset->buf);
dataset->buf = malloc(data_len);
    /* that's not good */
    if (!dataset->buf)
    	return -1;
-	dataset->data.length = data_len;
-	memcpy(dataset->buf, (data + 1), data_len);
+	dataset->data.header.length = data_len;
+	memcpy(dataset->buf, data->data, data_len);
return 0;
 }
static int unix_sock_req_data(struct globals *globals,
-			      struct alfred_packet *packet, int client_sock)
+			      struct alfred_request_v0 *request,
+			      int client_sock)
 {
    struct hash_it_t *hashit = NULL;
    struct timeval tv, last_check, now;
    fd_set fds;
-	int ret, len, type;
+	int ret, len;
    uint8_t buf[MAX_PAYLOAD];
+	struct alfred_push_data_v0 *push;
-	len = ntohs(packet->length);
+	len = ntohs(request->header.length);
-	if (len != 1)
+	if (len != (sizeof(*request) - sizeof(request->header)))
    	return -1;
/* no server to send the request to, only give back what we have now. */
@@ -165,7 +166,7 @@ static int unix_sock_req_data(struct globals *globals,
    	goto send_reply;
send_alfred_packet(globals, &globals->best_server->address,
-			   packet, sizeof(*packet) + len);
+			   request, sizeof(*request));
/* process incoming packets ... */
    FD_ZERO(&fds);
@@ -195,27 +196,26 @@ send_reply:
/* send some data back through the unix socket */
-	type = *((uint8_t *)(packet + 1));
-	packet = (struct alfred_packet *)buf;
-	packet->type = ALFRED_PUSH_DATA;
-	packet->version = ALFRED_VERSION;
+	push = (struct alfred_push_data_v0 *)buf;
+	push->header.type = ALFRED_PUSH_DATA;
+	push->header.version = ALFRED_VERSION;
while (NULL != (hashit = hash_iterate(globals->data_hash, hashit))) {
    	struct dataset *dataset = hashit->bucket->data;
    	struct alfred_data *data;
-		if (dataset->data.type != type)
+		if (dataset->data.header.type != request->requested_type)
    		continue;
-		data = (struct alfred_data *)(packet + 1);
+		data = push->data;
    	memcpy(data, &dataset->data, sizeof(*data));
-		data->length = htons(data->length);
-		memcpy((data + 1), dataset->buf, dataset->data.length);
+		data->header.length = htons(data->header.length);
+		memcpy(data->data, dataset->buf, dataset->data.header.length);
-		packet->length = htons(dataset->data.length + sizeof(*packet));
+		len = dataset->data.header.length + sizeof(*data);
+		push->header.length = htons(len);
-		write(client_sock, buf,
-		      sizeof(*packet) + sizeof(*data) + dataset->data.length);
+		write(client_sock, buf, sizeof(*push) + len);
    }
return 0;
@@ -226,7 +226,7 @@ int unix_sock_read(struct globals *globals)
    int client_sock;
    struct sockaddr_un sun_addr;
    socklen_t sun_size = sizeof(sun_addr);
-	struct alfred_packet *packet;
+	struct alfred_tlv *packet;
    uint8_t buf[MAX_PAYLOAD];
    int length, headsize, ret = -1;
@@ -251,7 +251,7 @@ int unix_sock_read(struct globals *globals)
    if (length < headsize)
    	goto err;
-	packet = (struct alfred_packet *)buf;
+	packet = (struct alfred_tlv *)buf;
if ((length - headsize) < ((int)ntohs(packet->length)))
    	goto err;
@@ -263,10 +263,13 @@ int unix_sock_read(struct globals *globals)
switch (packet->type) {
    case ALFRED_PUSH_DATA:
-		ret = unix_sock_add_data(globals, packet);
+		ret = unix_sock_add_data(globals,
+					 (struct alfred_push_data_v0 *)packet);
    	break;
    case ALFRED_REQUEST:
-		ret = unix_sock_req_data(globals, packet, client_sock);
+		ret = unix_sock_req_data(globals,
+					 (struct alfred_request_v0 *)packet,
+					 client_sock);
    	break;
    default:
    	/* unknown packet type */

    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

[commits] [alfred] master: alfred: Use well defined structs to create packets (ed38711)