Multiple fixes in packet handling

- add and handle layer 2 packet type correctly in data packets
- don't produce garbage packets if a packet could not be decoded or was not a
  data packet
- handle beacon/connection request/connection parameters handshake
- digipeater cycle timeout does not reset beacon timer anymore. This prevented
  any beacon transmission.
- Reset the connection timeout when empty packets are received

impl/src/layer2/connection.c

@@ -100,6 +100,9 @@ result_t connection_handle_packet(connection_ctx_t *ctx, const uint8_t *buf, siz
 			break;
 	}
 
+	data_packet->payload_type = L2_PAYLOAD_TYPE_INVALID;
+	data_packet->payload_len = 0;
+
 	// check the CRC
 	size_t packet_size = buf_len - crc_sizeof_key(PAYLOAD_CRC_SCHEME);
 
@@ -122,7 +125,8 @@ result_t connection_handle_packet(connection_ctx_t *ctx, const uint8_t *buf, siz
 		return OK;
 	}
 
-	if(!ham64_is_equal(&header.dst_addr, &ctx->my_addr)) {
+	if(ham64_get_addr_type(&header.dst_addr) != HAM64_ADDR_TYPE_BROADCAST
+			&& !ham64_is_equal(&header.dst_addr, &ctx->my_addr)) {
 		char fmt_dst_addr[HAM64_FMT_MAX_LEN];
 		char fmt_my_addr[HAM64_FMT_MAX_LEN];
 
@@ -157,29 +161,41 @@ static result_t handle_conn_mgmt(
 
 	uint8_t packet_type = payload[0];
 
-	if(packet_type == CONN_MGMT_TYPE_BEACON) {
+	switch(packet_type) {
-		if(ctx->conn_state == CONN_STATE_CONNECTING) {
+		case CONN_MGMT_TYPE_BEACON:
-			LOG(LVL_INFO, "Received beacon; queueing connection request.");
+			if(ctx->conn_state == CONN_STATE_CONNECTING) {
+				LOG(LVL_INFO, "Received beacon; queueing connection request.");
 
-			// enqueue a connection request packet
+				// enqueue a connection request packet
-			layer2_packet_header_t conn_request_header;
+				layer2_packet_header_t conn_request_header;
 
-			conn_request_header.tx_request = true;
+				conn_request_header.tx_request = true;
-			conn_request_header.dst_addr = ctx->peer_addr;
+				conn_request_header.dst_addr = ctx->peer_addr;
-			conn_request_header.src_addr = ctx->my_addr;
+				conn_request_header.src_addr = ctx->my_addr;
-			conn_request_header.msg_type = L2_MSG_TYPE_CONN_MGMT;
+				conn_request_header.msg_type = L2_MSG_TYPE_CONN_MGMT;
-			conn_request_header.rx_seq_nr = 0;
+				conn_request_header.rx_seq_nr = 0;
-			conn_request_header.tx_seq_nr = 0;
+				conn_request_header.tx_seq_nr = 0;
 
-			// create a persistent copy of the packet data.
+				// create a persistent copy of the packet data.
-			uint8_t packetbuf[1];
+				uint8_t packetbuf[1];
-			packetbuf[0] = CONN_MGMT_TYPE_CONNECTION_REQUEST;
+				packetbuf[0] = CONN_MGMT_TYPE_CONNECTION_REQUEST;
 
-			connection_enqueue_packet(ctx, &conn_request_header, packetbuf, 1);
+				connection_enqueue_packet(ctx, &conn_request_header, packetbuf, 1);
-		} else {
+			} else {
-			LOG(LVL_WARN, "Beacons are ignored in states other than CONNECTING.");
+				LOG(LVL_WARN, "Beacons are ignored in states other than CONNECTING.");
-			return ERR_INVALID_STATE;
+				return ERR_INVALID_STATE;
-		}
+			}
+			break;
+
+		case CONN_MGMT_TYPE_CONNECTION_PARAMETERS:
+			LOG(LVL_INFO, "Connection parameters received! -> connection established");
+			ctx->next_expected_seq = 1; // connection parameters are packet 0
+			ctx->conn_state = CONN_STATE_ESTABLISHED;
+			break;
+
+		default:
+			LOG(LVL_WARN, "Ignored connection management type %d", packet_type);
+			break;
 	}
 
 	return OK;
@@ -231,6 +247,10 @@ result_t connection_handle_packet_prechecked(
 	switch(header->msg_type) {
 		case L2_MSG_TYPE_EMPTY:
 			LOG(LVL_DEBUG, "Empty packet: accepted ACK for %u.", ctx->last_acked_seq);
+
+			// empty packets also reset the timeout timer
+			ctx->last_rx_time = get_hires_time();
+
 			// handle the acknowledgement internally
 			connection_handle_ack(ctx, header->rx_seq_nr, false);
 			return OK; // do not ACK
@@ -318,7 +338,11 @@ result_t connection_enqueue_packet(
 }
 
 
-result_t connection_enqueue_data_packet(connection_ctx_t *ctx, uint8_t *buf, size_t buf_len)
+result_t connection_enqueue_data_packet(
+		connection_ctx_t *ctx,
+		layer2_payload_type_t payload_type,
+		uint8_t *buf,
+		size_t buf_len)
 {
 	// check the connection state
 	switch(ctx->conn_state) {
@@ -343,7 +367,12 @@ result_t connection_enqueue_data_packet(connection_ctx_t *ctx, uint8_t *buf, siz
 	header.tx_request = 0;
 	header.tx_seq_nr = ctx->next_seq_nr;
 
-	ERR_CHECK(connection_enqueue_packet(ctx, &header, buf, buf_len));
+	uint8_t packet_with_type[buf_len + 1];
+
+	packet_with_type[0] = (uint8_t)payload_type;
+	memcpy(packet_with_type+1, buf, buf_len);
+
+	ERR_CHECK(connection_enqueue_packet(ctx, &header, packet_with_type, sizeof(packet_with_type)));
 
 	ctx->next_seq_nr++;
 	ctx->next_seq_nr &= SEQ_NR_MASK;
@@ -531,8 +560,11 @@ void connection_handle_ack(connection_ctx_t *ctx, uint8_t acked_seq, bool do_ack
 		packet_queue_delete(&ctx->packet_queue, packets_to_remove);
 
 		// send the next requested packet (all previous ones were deleted above).
-		assert(ctx->next_packet_index >= packets_to_remove);
+		if(ctx->next_packet_index >= packets_to_remove) {
-		ctx->next_packet_index -= packets_to_remove;
+			ctx->next_packet_index -= packets_to_remove;
+		} else {
+			ctx->next_packet_index = 0;
+		}
 
 		packets_available = packet_queue_get_used_space(&ctx->packet_queue);
 

impl/src/layer2/connection.h

@@ -138,10 +138,15 @@ result_t connection_enqueue_packet(
 
 /*!\brief Enqueue a data packet for transmission.
  * \param ctx       The connection context.
+ * \param payload_type Type of the payload.
  * \param buf       Pointer to the data buffer.
  * \param buf_len   Length of the data.
  */
-result_t connection_enqueue_data_packet(connection_ctx_t *ctx, uint8_t *buf, size_t buf_len);
+result_t connection_enqueue_data_packet(
+		connection_ctx_t *ctx,
+		layer2_payload_type_t payload_type,
+		uint8_t *buf,
+		size_t buf_len);
 
 /*!\brief Check if there is free space in the TX packet queue.
  * \param ctx       The connection context.

impl/src/layer2/connection_list.c

@@ -1,10 +1,14 @@
 #include <string.h>
 #include <assert.h>
 
+#define LOGGER_MODULE_NAME "clist"
+#include "logger.h"
+
 #include "connection_list.h"
 
 #include "layer2/connection.h"
 #include "results.h"
+#include "utils.h"
 
 #define SEQ_NR_MASK 0xF
 
@@ -213,7 +217,7 @@ result_t connection_list_enqueue_packet(connection_list_t *list, uint8_t *data,
 		return ERR_INVALID_ADDRESS;
 	}
 
-	return connection_enqueue_data_packet(&ptr->connection, data, data_len);
+	return connection_enqueue_data_packet(&ptr->connection, L2_PAYLOAD_TYPE_IPV6, data, data_len);
 }
 
 

impl/src/layer2/digipeater.c

@@ -186,7 +186,7 @@ result_t digipeater_fill_packet_queues_from_tundev(digipeater_ctx_t *ctx, int tu
 	// first check if any queue is already full, so we don't have to drop packets
 	// from the TUN device queue.
 	if(!connection_list_can_enqueue_packet(&ctx->conn_list)) {
-		LOG(LVL_DEBUG, "No free space in queues.");
+		LOG(LVL_DEBUG, "No connection or no free space in queues.");
 		return OK; // do nothing
 	}
 
@@ -215,6 +215,9 @@ result_t digipeater_fill_packet_queues_from_tundev(digipeater_ctx_t *ctx, int tu
 			} else if(ret == 0) {
 				// no more data, should not happen
 				break;
+			} else if(ret < 4) {
+				LOG(LVL_ERR, "Not enough data from TUN read() to check packet type!");
+				return ERR_SYSCALL;
 			}
 
 			uint16_t flags = *(uint16_t*)packetbuf;
@@ -222,9 +225,13 @@ result_t digipeater_fill_packet_queues_from_tundev(digipeater_ctx_t *ctx, int tu
 			LOG(LVL_DUMP, "TUN Flags: 0x%04x", flags);
 			LOG(LVL_DUMP, "TUN Proto: 0x%04x", proto);
 
+			uint8_t *packet_data = packetbuf + 4;
+			size_t packet_length = ret - 4;
+
+			// note: octets are swapped in case statements
 			switch(proto) {
-				case 0x86dd: // IPv6
+				case 0xdd86: // IPv6
-					ERR_CHECK(connection_list_enqueue_packet(&ctx->conn_list, packetbuf, ret));
+					WARN_ON_ERR(connection_list_enqueue_packet(&ctx->conn_list, packet_data, packet_length));
 					break;
 
 				default:
@@ -258,6 +265,7 @@ size_t digipeater_encode_next_packet(digipeater_ctx_t *ctx, uint8_t *buf, size_t
 	switch(ctx->state) {
 		case DIGIPEATER_STATE_BEACON:
 			packet_size = encode_beacon_packet(ctx, buf, buf_len);
+			ctx->next_beacon_time += HRTIME_MS(BEACON_INTERVAL_MS);
 			*end_burst = true;
 			break;
 
@@ -318,7 +326,6 @@ result_t digipeater_end_cycle(digipeater_ctx_t *ctx)
 	uint64_t now = get_hires_time();
 
 	if(now >= ctx->next_beacon_time) {
-		ctx->next_beacon_time += HRTIME_MS(BEACON_INTERVAL_MS);
 		ctx->state = DIGIPEATER_STATE_BEACON;
 	} else {
 		// TODO: adjust the time based on connection activity; right now this results

impl/src/results.h

@@ -49,4 +49,12 @@ typedef enum {
 		} \
 	} while(0);
 
+#define WARN_ON_ERR(call) \
+	do { \
+		result_t err_check_result = call; \
+		if(err_check_result != OK) { \
+			LOG(LVL_WARN, "Error ignored at %s:%d: %d", __FILE__, __LINE__, err_check_result); \
+		} \
+	} while(0);
+
 #endif // RESULTS_H

impl/test/layer2_over_udp/l2udptest_client.c

@@ -21,6 +21,7 @@
 #include <errno.h>
 
 #include "layer2/ham64.h"
+#include "results.h"
 #include "utils.h"
 
 #define LOGGER_MODULE_NAME "main"
@@ -314,12 +315,25 @@ int main(void)
 				} else if(ret == 0) {
 					// no more data, should not happen
 					break;
+				} else if(ret < 4) {
+					LOG(LVL_ERR, "Not enough data from TUN read() to check packet type!");
+					return ERR_SYSCALL;
 				}
 
-				LOG(LVL_DUMP, "TUN Flags: 0x%04x", *(uint16_t*)packetbuf);
+				uint16_t flags = *(uint16_t*)packetbuf;
-				LOG(LVL_DUMP, "TUN Proto: 0x%04x", *((uint16_t*)packetbuf + 1));
+				uint16_t proto = *((uint16_t*)packetbuf + 1);
+				LOG(LVL_DUMP, "TUN Flags: 0x%04x", flags);
+				LOG(LVL_DUMP, "TUN Proto: 0x%04x", proto);
 
-				RESULT_CHECK(connection_enqueue_data_packet(&l2conn, packetbuf, ret));
+				uint8_t *packet_data = packetbuf + 4;
+				size_t packet_length = ret - 4;
+
+				if(proto != 0xdd86) {
+					LOG(LVL_WARN, "Non-IPv6 packet ignored. Proto: 0x%04x", proto);
+					continue;
+				}
+
+				RESULT_CHECK(connection_enqueue_data_packet(&l2conn, L2_PAYLOAD_TYPE_IPV6, packet_data, packet_length));
 			}
 		}
 
@@ -411,6 +425,10 @@ int main(void)
 					LOG(LVL_ERR, "Packet not in the expected sequence.");
 					break;
 
+				case ERR_INVALID_STATE:
+					LOG(LVL_WARN, "Packet ignored due to invalid state.");
+					break;
+
 				default: // all other errors
 					LOG(LVL_ERR, "layer2_rx_handle_packet() returned error code %u.", result);
 					break;