MAC: iotthd-3413 Review updates

Author: Jarkko Paso

source/MAC/IEEE802_15_4/mac_defines.h

@@ -295,21 +295,6 @@ typedef struct protocol_interface_rf_mac_setup {
 #define MAC_FCF_SRC_ADDR_MASK               0xc000
 #define MAC_FCF_SRC_ADDR_SHIFT              14
 
-#define MAC_FCF_DST_MODE                    0x0C
-#define MAC_FCF_DST_ADDR_NONE               0x00
-#define MAC_FCF_DST_16_BITS                 0x08
-#define MAC_FCF_DST_64_BITS                 0x0C
-#define MAC_FCF_SRC_MODE                    0xC0
-#define MAC_FCF_SEQUENCE_COMPRESSION        0x01
-#define MAC_FCF_PAN_ID_COMPRESSION          0x40
-#define MAC_FCF_SRC_64_BITS                 0xC0
-#define SHIFT_SEQ_COMP_FIELD                (0)
-#define SHIFT_VERSION_FIELD                 (4)
-#define SHIFT_PANID_COMP_FIELD              (6)
-#define VERSION_FIELD_MASK                  0x30
-#define OFFSET_DST_PAN_ID                   (3)
-#define OFFSET_DST_ADDR                     (5)
-
 /* MAC supported frame types */
 #define FC_BEACON_FRAME         0x00
 #define FC_DATA_FRAME           0x01

source/MAC/IEEE802_15_4/mac_mcps_sap.c

@@ -2063,9 +2063,7 @@ static mac_pre_build_frame_t *mcps_sap_pd_req_queue_read(protocol_interface_rf_m
 
 void mcps_sap_pre_parsed_frame_buffer_free(mac_pre_parsed_frame_t *buf)
 {
-    if (buf) {
-        ns_dyn_mem_free(buf);
-    }
+    ns_dyn_mem_free(buf);
 }
 
 mac_pre_parsed_frame_t *mcps_sap_pre_parsed_frame_buffer_get(const uint8_t *data_ptr, uint16_t frame_length)

source/MAC/IEEE802_15_4/mac_pd_sap.c

@@ -572,7 +572,7 @@ static int8_t mac_data_interface_tx_done_by_ack_cb(protocol_interface_rf_mac_set
     return 0;
 }
 
-static bool mac_pd_sap_ack_validation(protocol_interface_rf_mac_setup_s *rf_ptr, mac_fcf_sequence_t *fcf_dsn, const uint8_t *data_ptr)
+static bool mac_pd_sap_ack_validation(protocol_interface_rf_mac_setup_s *rf_ptr, const mac_fcf_sequence_t *fcf_dsn, const uint8_t *data_ptr)
 {
     if (!rf_ptr->active_pd_data_request || !rf_ptr->active_pd_data_request->fcf_dsn.ackRequested) {
         return false; //No active Data request anymore or no ACK request for current TX
@@ -628,32 +628,32 @@ static bool mac_pd_sap_ack_validation(protocol_interface_rf_mac_setup_s *rf_ptr,
     return true;
 }
 
-static int8_t mac_pd_sap_validate_fcf(protocol_interface_rf_mac_setup_s *rf_ptr, mac_fcf_sequence_t fcf_read, arm_pd_sap_generic_ind_t *pd_data_ind)
+static int8_t mac_pd_sap_validate_fcf(protocol_interface_rf_mac_setup_s *rf_ptr, const mac_fcf_sequence_t *fcf_read, arm_pd_sap_generic_ind_t *pd_data_ind)
 {
-    switch (fcf_read.frametype) {
+    switch (fcf_read->frametype) {
         case FC_DATA_FRAME:
-            if (fcf_read.SrcAddrMode == MAC_ADDR_MODE_NONE) {
+            if (fcf_read->SrcAddrMode == MAC_ADDR_MODE_NONE) {
                 return -1;
-            } else if (fcf_read.DstAddrMode == MAC_ADDR_MODE_NONE && fcf_read.frameVersion != MAC_FRAME_VERSION_2015) {
+            } else if (fcf_read->DstAddrMode == MAC_ADDR_MODE_NONE && fcf_read->frameVersion != MAC_FRAME_VERSION_2015) {
                 return -1;
             }
             break;
         case FC_BEACON_FRAME:
-            if (fcf_read.SrcAddrMode == MAC_ADDR_MODE_NONE || fcf_read.DstAddrMode != MAC_ADDR_MODE_NONE) {
+            if (fcf_read->SrcAddrMode == MAC_ADDR_MODE_NONE || fcf_read->DstAddrMode != MAC_ADDR_MODE_NONE) {
                 return -1;
             }
             break;
         case FC_ACK_FRAME:
             // Only accept version 2015 Acks
-            if (fcf_read.frameVersion != MAC_FRAME_VERSION_2015) {
+            if (fcf_read->frameVersion != MAC_FRAME_VERSION_2015) {
                 return -1;
             }
             //Validate Ack doesn't request Ack
-            if (fcf_read.ackRequested) {
+            if (fcf_read->ackRequested) {
                 return -1;
             }
             //Validate ACK
-            if (!mac_pd_sap_ack_validation(rf_ptr, &fcf_read, pd_data_ind->data_ptr)) {
+            if (!mac_pd_sap_ack_validation(rf_ptr, fcf_read, pd_data_ind->data_ptr)) {
                 return -1;
             }
             break;
@@ -665,150 +665,117 @@ static int8_t mac_pd_sap_validate_fcf(protocol_interface_rf_mac_setup_s *rf_ptr,
     return 0;
 }
 
-static bool mac_pd_sap_panid_filter_common(const uint8_t *panid_start, uint8_t *pan_id, uint8_t frame_type)
+static bool mac_pd_sap_panid_filter_common(const uint8_t *mac_header, const mac_fcf_sequence_t *fcf_read, uint16_t own_pan_id)
 {
-    // PHY driver shouldn't drop received Beacon frames as they might be used by load balancing
-    if (frame_type == MAC_FRAME_BEACON) {
+    // Beacon frames shouldn't be dropped as they might be used by load balancing
+    if (fcf_read->frametype == MAC_FRAME_BEACON) {
         return true;
     }
-    bool retval = true;
-    uint8_t cmp_table[2] = {0xff, 0xff};
-    if (!(pan_id[0] == 0xff && pan_id[1] == 0xff)) {
-        if (memcmp((uint8_t *)panid_start, (uint8_t *) cmp_table, 2)) {
-            retval = false;
-        }
-        if (!retval) {
-            for (uint8_t i = 0; i < 2; i++) {
-                cmp_table[1 - i] = panid_start[i];
-            }
-            if (!memcmp(pan_id, cmp_table, 2)) {
-                retval = true;
-            }
-        }
+    if (own_pan_id == 0xffff) {
+        return true;
+    }
+    uint16_t dst_pan_id = mac_header_get_dst_panid(fcf_read, mac_header, 0xffff);
+    if (dst_pan_id == 0xffff) {
+        return true;
+    }
+    if (own_pan_id == dst_pan_id) {
+        return true;
     }
-    return retval;
+    return false;
 }
 
-static bool mac_pd_sap_panid_v2_filter(const uint8_t *ptr, uint8_t *pan_id, uint8_t dst_mode, uint8_t src_mode, uint8_t seq_compressed, uint8_t panid_compressed, uint8_t frame_type)
+static bool mac_pd_sap_panid_v2_filter(const uint8_t *ptr, const mac_fcf_sequence_t *fcf_read, uint16_t pan_id)
 {
-    if ((dst_mode == MAC_FCF_DST_ADDR_NONE) && (frame_type == FC_DATA_FRAME || frame_type == FC_CMD_FRAME)) {
+    if ((fcf_read->DstAddrMode == MAC_ADDR_MODE_NONE) && (fcf_read->frametype == FC_DATA_FRAME || fcf_read->frametype == FC_CMD_FRAME)) {
         return true;
     }
-    if ((dst_mode == MAC_FCF_DST_64_BITS) && (src_mode == MAC_FCF_SRC_64_BITS) && panid_compressed) {
+    if ((fcf_read->DstAddrMode == MAC_ADDR_MODE_64_BIT) && (fcf_read->SrcAddrMode == MAC_ADDR_MODE_64_BIT) && fcf_read->intraPan) {
         return true;
     }
-    if (seq_compressed) {
-        ptr--;
-    }
-    return mac_pd_sap_panid_filter_common(ptr, pan_id, frame_type);
+    return mac_pd_sap_panid_filter_common(ptr, fcf_read, pan_id);
 }
 
-static bool mac_pd_sap_addr_filter_common(const uint8_t *ptr, uint8_t addr_mode, uint8_t *mac_64bit_addr, uint8_t *mac_16bit_addr)
+static bool mac_pd_sap_addr_filter_common(const uint8_t *mac_header, const mac_fcf_sequence_t *fcf_read, uint8_t *mac_64bit_addr, uint16_t mac_16bit_addr)
 {
     uint8_t cmp_table[8] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
-    bool retval = true;
-    switch (addr_mode) {
-        case MAC_FCF_DST_16_BITS:
-            if (memcmp((uint8_t *)ptr, (uint8_t *) cmp_table, 2)) {
-                retval = false;
-            }
-            if (!retval) {
-                for (uint8_t i = 0; i < 2; i++) {
-                    cmp_table[1 - i] = ptr[i];
-                }
+    uint8_t dst_addr[8];
+    mac_header_get_dst_address(fcf_read, mac_header, dst_addr);
 
-                if (!memcmp((uint8_t *)mac_16bit_addr, (uint8_t *) cmp_table, 2)) {
-                    retval = true;
-                }
+    switch (fcf_read->DstAddrMode) {
+        case MAC_ADDR_MODE_16_BIT:
+            if (!memcmp(dst_addr, cmp_table, 2)) {
+                return true;
+            }
+            uint8_t temp[2];
+            common_write_16_bit(mac_16bit_addr, temp);
+            if (!memcmp(temp, dst_addr, 2)) {
+                return true;
             }
             break;
-        case MAC_FCF_DST_64_BITS:
-            if (memcmp((uint8_t *)ptr, (uint8_t *) cmp_table, 8)) {
-                retval = false;
+        case MAC_ADDR_MODE_64_BIT:
+            if (!memcmp(dst_addr, cmp_table, 8)) {
+                return true;
             }
-            if (!retval) {
-                for (uint8_t i = 0; i < 8; i++) {
-                    cmp_table[7 - i] = ptr[i];
-                }
-
-                if (!memcmp((uint8_t *)mac_64bit_addr, (uint8_t *) cmp_table, 8)) {
-                    retval = true;
-                }
+            if (!memcmp(mac_64bit_addr, dst_addr, 8)) {
+                return true;
             }
             break;
-        case MAC_FCF_DST_ADDR_NONE:
-            retval = true;
+        case MAC_ADDR_MODE_NONE:
+            return true;
             break;
         default:
-            retval = false;
             break;
     }
-    return retval;
+    return false;
 }
 
-static bool mac_pd_sap_addr_v2_filter(const uint8_t *ptr, uint8_t *mac_64bit_addr, uint8_t *mac_16bit_addr, uint8_t dst_mode, uint8_t seq_compressed, uint8_t panid_compressed)
+static bool mac_pd_sap_addr_v2_filter(const uint8_t *mac_header, const mac_fcf_sequence_t *fcf_read, uint8_t *mac_64bit_addr, uint16_t mac_16bit_addr)
 {
-    if (seq_compressed) {
-        ptr--;
-    }
-    if (panid_compressed) {
-        ptr -= 2;
-    }
-    return mac_pd_sap_addr_filter_common(ptr, dst_mode, mac_64bit_addr, mac_16bit_addr);
+    return mac_pd_sap_addr_filter_common(mac_header, fcf_read, mac_64bit_addr, mac_16bit_addr);
 }
 
-static bool mac_pd_sap_rx_filter(const uint8_t *mac_header, uint8_t phy_filter_mask, uint8_t *mac_64bit_addr, uint16_t mac_16bit_addr, uint16_t pan_id)
+static bool mac_pd_sap_rx_filter(const uint8_t *mac_header, const mac_fcf_sequence_t *fcf_read, uint8_t phy_filter_mask, uint8_t *mac_64bit_addr, uint16_t mac_16bit_addr, uint16_t pan_id)
 {
-    uint8_t dst_mode = (mac_header[1] & MAC_FCF_DST_MODE);
-    uint8_t src_mode = (mac_header[1] & MAC_FCF_SRC_MODE);
-    uint8_t seq_compressed = ((mac_header[1] & MAC_FCF_SEQUENCE_COMPRESSION) >> SHIFT_SEQ_COMP_FIELD);
-    uint8_t panid_compressed = ((mac_header[0] & MAC_FCF_PAN_ID_COMPRESSION) >> SHIFT_PANID_COMP_FIELD);
-    uint8_t frame_type = mac_header[0] & MAC_FCF_FRAME_TYPE_MASK;
-    uint8_t version = ((mac_header[1] & VERSION_FIELD_MASK) >> SHIFT_VERSION_FIELD);
+    uint8_t version = fcf_read->frameVersion;
+
     if (version == MAC_FRAME_VERSION_2015 && !(phy_filter_mask & (1 << MAC_FRAME_VERSION_2))) {
-        uint8_t temp[2];
-        common_write_16_bit(pan_id, temp);
-        if (!mac_pd_sap_panid_v2_filter(mac_header + OFFSET_DST_PAN_ID, temp, dst_mode, src_mode, seq_compressed, panid_compressed, frame_type)) {
+        if (!mac_pd_sap_panid_v2_filter(mac_header, fcf_read, pan_id)) {
             return false;
         }
-        common_write_16_bit(mac_16bit_addr, temp);
-        if (!mac_pd_sap_addr_v2_filter(mac_header + OFFSET_DST_ADDR, mac_64bit_addr, temp, dst_mode, seq_compressed, panid_compressed)) {
+        if (!mac_pd_sap_addr_v2_filter(mac_header, fcf_read, mac_64bit_addr, mac_16bit_addr)) {
             return false;
         }
     }
     return true;
 }
 
-static int8_t mac_pd_sap_generate_ack(protocol_interface_rf_mac_setup_s *rf_ptr, mac_fcf_sequence_t fcf_read, arm_pd_sap_generic_ind_t *pd_data_ind)
+static int8_t mac_pd_sap_generate_ack(protocol_interface_rf_mac_setup_s *rf_ptr, const mac_fcf_sequence_t *fcf_read, arm_pd_sap_generic_ind_t *pd_data_ind)
 {
-    //Generate ACK when Extension is enabled and ACK is requested
-    if (rf_ptr->mac_extension_enabled && fcf_read.ackRequested && fcf_read.frameVersion == MAC_FRAME_VERSION_2015) {
-        //SEND ACK here
-        if (rf_ptr->mac_ack_tx_active) {
-            return -1;
-        }
+    //Generate ACK when Extension is enabled and ACK is requested only for version 2 frames.
+    if (!rf_ptr->mac_extension_enabled || !fcf_read->ackRequested || (fcf_read->frameVersion != MAC_FRAME_VERSION_2015)) {
+        return 0;
+    }
+    if (rf_ptr->mac_ack_tx_active) {
+        return -1;
+    }
 
-        mcps_ack_data_payload_t ack_payload;
-        mac_api_t *mac_api = get_sw_mac_api(rf_ptr);
-        mac_api->enhanced_ack_data_req_cb(mac_api, &ack_payload, pd_data_ind->dbm, pd_data_ind->link_quality);
-        //Calculate Delta time
+    mcps_ack_data_payload_t ack_payload;
+    mac_api_t *mac_api = get_sw_mac_api(rf_ptr);
+    mac_api->enhanced_ack_data_req_cb(mac_api, &ack_payload, pd_data_ind->dbm, pd_data_ind->link_quality);
+    //Calculate Delta time
 
-        if (mcps_generic_ack_build(rf_ptr, &fcf_read, pd_data_ind->data_ptr, &ack_payload) != 0) {
-            return -1;
-        }
-    }
-    return 0;
+    return mcps_generic_ack_build(rf_ptr, fcf_read, pd_data_ind->data_ptr, &ack_payload);
 }
 
-static mac_pre_parsed_frame_t *mac_pd_sap_allocate_receive_buffer(protocol_interface_rf_mac_setup_s *rf_ptr, mac_fcf_sequence_t fcf_read, arm_pd_sap_generic_ind_t *pd_data_ind)
+static mac_pre_parsed_frame_t *mac_pd_sap_allocate_receive_buffer(protocol_interface_rf_mac_setup_s *rf_ptr, const mac_fcf_sequence_t *fcf_read, arm_pd_sap_generic_ind_t *pd_data_ind)
 {
     mac_pre_parsed_frame_t *buffer = mcps_sap_pre_parsed_frame_buffer_get(pd_data_ind->data_ptr, pd_data_ind->data_len);
     if (!buffer) {
         return NULL;
     }
 
     //Copy Pre Parsed values
-    buffer->fcf_dsn = fcf_read;
+    buffer->fcf_dsn = *fcf_read;
     buffer->timestamp = mac_pd_sap_get_phy_rx_time(rf_ptr);
     buffer->ack_pendinfg_status = mac_data_interface_read_last_ack_pending_status(rf_ptr);
     /* Set default flags */
@@ -896,21 +863,22 @@ int8_t mac_pd_sap_data_cb(void *identifier, arm_phy_sap_msg_t *message)
         if (pd_data_ind->data_len < 3) {
             return -1;
         }
+
         mac_fcf_sequence_t fcf_read;
         const uint8_t *ptr = mac_header_parse_fcf_dsn(&fcf_read, pd_data_ind->data_ptr);
 
-        mac_pre_parsed_frame_t *buffer = mac_pd_sap_allocate_receive_buffer(rf_ptr, fcf_read, pd_data_ind);
+        mac_pre_parsed_frame_t *buffer = mac_pd_sap_allocate_receive_buffer(rf_ptr, &fcf_read, pd_data_ind);
         if (buffer && mac_filter_modify_link_quality(rf_ptr->mac_interface_id, buffer) == 1) {
             goto ERROR_HANDLER;
         }
         if (!rf_ptr->macProminousMode) {
-            if (mac_pd_sap_validate_fcf(rf_ptr, fcf_read, pd_data_ind)) {
+            if (mac_pd_sap_validate_fcf(rf_ptr, &fcf_read, pd_data_ind)) {
                 goto ERROR_HANDLER;
             }
-            if (!mac_pd_sap_rx_filter(pd_data_ind->data_ptr, rf_ptr->mac_frame_filters, rf_ptr->mac64, rf_ptr->mac_short_address, rf_ptr->pan_id)) {
+            if (!mac_pd_sap_rx_filter(pd_data_ind->data_ptr, &fcf_read, rf_ptr->mac_frame_filters, rf_ptr->mac64, rf_ptr->mac_short_address, rf_ptr->pan_id)) {
                 goto ERROR_HANDLER;
             }
-            if (mac_pd_sap_generate_ack(rf_ptr, fcf_read, pd_data_ind)) {
+            if (mac_pd_sap_generate_ack(rf_ptr, &fcf_read, pd_data_ind)) {
                 goto ERROR_HANDLER;
             }
             if (buffer) {