From a8c96b82b159ac4dfc7b83915e2e8825d3cf37e3 Mon Sep 17 00:00:00 2001 From: Antti Yli-Tokola Date: Thu, 23 Apr 2020 13:14:50 +0300 Subject: [PATCH 1/2] Update mbed-coap to version 5.1.4 Add also 4.13 (Request Entity Too Large) responses to duplicate info list. Add client library configurations for DEFAULT_RESPONSE_TIMEOUT and SN_COAP_DUPLICATION_MAX_TIME_MSGS_STORED. Increased the default timeouts of DEFAULT_RESPONSE_TIMEOUT and SN_COAP_DUPLICATION_MAX_TIME_MSGS_STORED to 300 seconds. These two are critical parameters for low-bandwidth high-latency networks. The defaults should be more geared towards such networks that are likely to have issues with transmissions. The increased defaults can increase the runtime HEAP usage when there is a lot of duplicates or retransmissions. --- features/frameworks/mbed-coap/CHANGELOG.md | 8 ++++++++ .../frameworks/mbed-coap/mbed-coap/sn_config.h | 15 ++++++++++++--- .../mbed-coap/source/sn_coap_protocol.c | 8 ++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/features/frameworks/mbed-coap/CHANGELOG.md b/features/frameworks/mbed-coap/CHANGELOG.md index a2a2750579..17aafbd00e 100644 --- a/features/frameworks/mbed-coap/CHANGELOG.md +++ b/features/frameworks/mbed-coap/CHANGELOG.md @@ -1,5 +1,13 @@ # Change Log +## [v5.1.4](https://github.com/ARMmbed/mbed-coap/releases/tag/v5.1.4) + +- Add also 4.13 (Request Entity Too Large) responses to duplicate info list. +- Add client library configurations for `DEFAULT_RESPONSE_TIMEOUT` and `SN_COAP_DUPLICATION_MAX_TIME_MSGS_STORED`. +- Increased the default timeouts of `DEFAULT_RESPONSE_TIMEOUT` and `SN_COAP_DUPLICATION_MAX_TIME_MSGS_STORED` to 300 seconds. + * These two are critical parameters for low-bandwidth high-latency networks. The defaults should be more geared towards such networks that are likely to have issues with transmissions. + * The increased defaults can increase the runtime HEAP usage when there is a lot of duplicates or retransmissions. + ## [v5.1.3](https://github.com/ARMmbed/mbed-coap/releases/tag/v5.1.3) - Fix potential integer overflow when calculating CoAP data packet size: IOTCLT-3748 CVE-2019-17211 - mbed-coap integer overflow diff --git a/features/frameworks/mbed-coap/mbed-coap/sn_config.h b/features/frameworks/mbed-coap/mbed-coap/sn_config.h index f04ffe80c7..a5c45caf43 100644 --- a/features/frameworks/mbed-coap/mbed-coap/sn_config.h +++ b/features/frameworks/mbed-coap/mbed-coap/sn_config.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016 ARM Limited. All rights reserved. + * Copyright (c) 2020 ARM Limited. All rights reserved. * SPDX-License-Identifier: Apache-2.0 * Licensed under the Apache License, Version 2.0 (the License); you may * not use this file except in compliance with the License. @@ -98,6 +98,11 @@ * \brief Sets the CoAP re-send interval in seconds. * By default is 10 seconds. */ + +#ifdef MBED_CONF_MBED_CLIENT_DEFAULT_RESPONSE_TIMEOUT +#define DEFAULT_RESPONSE_TIMEOUT MBED_CONF_MBED_CLIENT_DEFAULT_RESPONSE_TIMEOUT +#endif + #ifndef DEFAULT_RESPONSE_TIMEOUT #define DEFAULT_RESPONSE_TIMEOUT 10 /**< Default re-sending timeout as seconds */ #endif @@ -219,8 +224,12 @@ * \brief Maximum time in seconds howe long message is kept for duplicate detection. * By default 60 seconds. */ +#ifdef MBED_CONF_MBED_CLIENT_SN_COAP_DUPLICATION_MAX_TIME_MSGS_STORED +#define SN_COAP_DUPLICATION_MAX_TIME_MSGS_STORED MBED_CONF_MBED_CLIENT_SN_COAP_DUPLICATION_MAX_TIME_MSGS_STORED +#endif + #ifndef SN_COAP_DUPLICATION_MAX_TIME_MSGS_STORED -#define SN_COAP_DUPLICATION_MAX_TIME_MSGS_STORED 60 /** RESPONSE_TIMEOUT * RESPONSE_RANDOM_FACTOR * (2 ^ MAX_RETRANSMIT - 1) + the expected maximum round trip time **/ +#define SN_COAP_DUPLICATION_MAX_TIME_MSGS_STORED 300 /** RESPONSE_TIMEOUT * RESPONSE_RANDOM_FACTOR * (2 ^ MAX_RETRANSMIT - 1) + the expected maximum round trip time **/ #endif /** @@ -234,7 +243,7 @@ #endif #ifndef SN_COAP_BLOCKWISE_MAX_TIME_DATA_STORED -#define SN_COAP_BLOCKWISE_MAX_TIME_DATA_STORED 60 /**< Maximum time in seconds of data (messages and payload) to be stored for blockwising */ +#define SN_COAP_BLOCKWISE_MAX_TIME_DATA_STORED 300 /**< Maximum time in seconds of data (messages and payload) to be stored for blockwising */ #endif /** diff --git a/features/frameworks/mbed-coap/source/sn_coap_protocol.c b/features/frameworks/mbed-coap/source/sn_coap_protocol.c index e3e156ead3..0b86107f0e 100644 --- a/features/frameworks/mbed-coap/source/sn_coap_protocol.c +++ b/features/frameworks/mbed-coap/source/sn_coap_protocol.c @@ -759,6 +759,14 @@ sn_coap_hdr_s *sn_coap_protocol_parse(struct coap_s *handle, sn_nsdl_addr_s *src goto cleanup; } +#if SN_COAP_DUPLICATION_MAX_MSGS_COUNT + // copy data buffer to duplicate list for resending purposes + if (!sn_coap_protocol_update_duplicate_package_data(handle, src_addr_ptr, resp, packet_data_size, packet_data_ptr)) { + tr_error("sn_coap_protocol_parse - failed to update duplicate info!"); + goto cleanup; + } +#endif + sn_coap_parser_release_allocated_coap_msg_mem(handle, resp); handle->sn_coap_tx_callback(packet_data_ptr, packet_data_size, src_addr_ptr, param); From fab40925434959f03e29ad34d3f04785e1d0d510 Mon Sep 17 00:00:00 2001 From: Teemu Takaluoma Date: Mon, 27 Apr 2020 11:11:02 +0300 Subject: [PATCH 2/2] Update mbed-coap to version 5.1.5 BLOCK2 code-branch was missing handling for duplicate packets. As part of the fix, added also a call to update the duplicate package data via a new function sn_coap_protocol_update_duplicate_package_data_all. The new implementation handles all CoAP messages, not just those with COAP_MSG_TYPE_ACKNOWLEDGEMENT. --- features/frameworks/mbed-coap/.mbedignore | 1 + features/frameworks/mbed-coap/CHANGELOG.md | 5 ++ .../mbed-coap/source/sn_coap_protocol.c | 63 ++++++++++++++----- 3 files changed, 52 insertions(+), 17 deletions(-) diff --git a/features/frameworks/mbed-coap/.mbedignore b/features/frameworks/mbed-coap/.mbedignore index 4140aabf01..5ea7d98d1c 100644 --- a/features/frameworks/mbed-coap/.mbedignore +++ b/features/frameworks/mbed-coap/.mbedignore @@ -1,2 +1,3 @@ test/* +test_modules/* unittest/* diff --git a/features/frameworks/mbed-coap/CHANGELOG.md b/features/frameworks/mbed-coap/CHANGELOG.md index 17aafbd00e..f774e0db88 100644 --- a/features/frameworks/mbed-coap/CHANGELOG.md +++ b/features/frameworks/mbed-coap/CHANGELOG.md @@ -1,5 +1,10 @@ # Change Log +## [v5.1.5](https://github.com/ARMmbed/mbed-coap/releases/tag/v5.1.5) + +- Added handling for duplicate message handling for Block2 messages. Previously CoAP was silently ignoring the duplicate messages. Now proper response will be sent. +- Added extended version of `sn_coap_protocol_update_duplicate_package_data`, `sn_coap_protocol_update_duplicate_package_data_all` which will handle all CoAP data in the list. + ## [v5.1.4](https://github.com/ARMmbed/mbed-coap/releases/tag/v5.1.4) - Add also 4.13 (Request Entity Too Large) responses to duplicate info list. diff --git a/features/frameworks/mbed-coap/source/sn_coap_protocol.c b/features/frameworks/mbed-coap/source/sn_coap_protocol.c index 0b86107f0e..c49a7e1012 100644 --- a/features/frameworks/mbed-coap/source/sn_coap_protocol.c +++ b/features/frameworks/mbed-coap/source/sn_coap_protocol.c @@ -53,6 +53,8 @@ static coap_duplication_info_s *sn_coap_protocol_linked_list_duplication_info_se static void sn_coap_protocol_linked_list_duplication_info_remove_old_ones(struct coap_s *handle); static void sn_coap_protocol_duplication_info_free(struct coap_s *handle, coap_duplication_info_s *duplication_info_ptr); static bool sn_coap_protocol_update_duplicate_package_data(const struct coap_s *handle, const sn_nsdl_addr_s *dst_addr_ptr, const sn_coap_hdr_s *coap_msg_ptr, const int16_t data_size, const uint8_t *dst_packet_data_ptr); +static bool sn_coap_protocol_update_duplicate_package_data_all(const struct coap_s *handle, const sn_nsdl_addr_s *dst_addr_ptr, const sn_coap_hdr_s *coap_msg_ptr, const int16_t data_size, const uint8_t *dst_packet_data_ptr); + #endif #if SN_COAP_BLOCKWISE_ENABLED || SN_COAP_MAX_BLOCKWISE_PAYLOAD_SIZE /* If Message blockwising is not enabled, this part of code will not be compiled */ @@ -690,6 +692,8 @@ sn_coap_hdr_s *sn_coap_protocol_parse(struct coap_s *handle, sn_nsdl_addr_s *src tr_debug("sn_coap_protocol_parse - send ack for duplicate message"); handle->sn_coap_tx_callback(response->packet_ptr, response->packet_len, response->address, response->param); + } else { + tr_error("sn_coap_protocol_parse - response not yet build"); } } @@ -2235,6 +2239,20 @@ static sn_coap_hdr_s *sn_coap_handle_blockwise_message(struct coap_s *handle, sn } sn_coap_builder_2(dst_ack_packet_data_ptr, src_coap_blockwise_ack_msg_ptr, handle->sn_coap_block_data_size); + +#if SN_COAP_DUPLICATION_MAX_MSGS_COUNT + // copy coap data buffer to duplicate list for resending purposes + if (!sn_coap_protocol_update_duplicate_package_data_all(handle, + src_addr_ptr, + src_coap_blockwise_ack_msg_ptr, + dst_packed_data_needed_mem, + dst_ack_packet_data_ptr)) { + sn_coap_parser_release_allocated_coap_msg_mem(handle, src_coap_blockwise_ack_msg_ptr); + handle->sn_coap_protocol_free(dst_ack_packet_data_ptr); + return NULL; + } +#endif + handle->sn_coap_tx_callback(dst_ack_packet_data_ptr, dst_packed_data_needed_mem, src_addr_ptr, param); handle->sn_coap_protocol_free(dst_ack_packet_data_ptr); @@ -2434,27 +2452,38 @@ static sn_coap_hdr_s *sn_coap_protocol_copy_header(struct coap_s *handle, const #if SN_COAP_DUPLICATION_MAX_MSGS_COUNT static bool sn_coap_protocol_update_duplicate_package_data(const struct coap_s *handle, - const sn_nsdl_addr_s *dst_addr_ptr, - const sn_coap_hdr_s *coap_msg_ptr, - const int16_t data_size, - const uint8_t *dst_packet_data_ptr) + const sn_nsdl_addr_s *dst_addr_ptr, + const sn_coap_hdr_s *coap_msg_ptr, + const int16_t data_size, + const uint8_t *dst_packet_data_ptr) { if (coap_msg_ptr->msg_type == COAP_MSG_TYPE_ACKNOWLEDGEMENT && handle->sn_coap_duplication_buffer_size != 0) { - coap_duplication_info_s* info = sn_coap_protocol_linked_list_duplication_info_search(handle, - dst_addr_ptr, - coap_msg_ptr->msg_id); + return sn_coap_protocol_update_duplicate_package_data_all(handle, dst_addr_ptr, coap_msg_ptr, data_size, dst_packet_data_ptr); + } + return true; +} - /* Update package data to duplication info struct if it's not there yet */ - if (info && info->packet_ptr == NULL) { - info->packet_ptr = handle->sn_coap_protocol_malloc(data_size); - if (info->packet_ptr) { - memcpy(info->packet_ptr, dst_packet_data_ptr, data_size); - info->packet_len = data_size; - } else { - tr_error("sn_coap_protocol_update_duplication_package_data - failed to allocate duplication info!"); - return false; - } +static bool sn_coap_protocol_update_duplicate_package_data_all(const struct coap_s *handle, + const sn_nsdl_addr_s *dst_addr_ptr, + const sn_coap_hdr_s *coap_msg_ptr, + const int16_t data_size, + const uint8_t *dst_packet_data_ptr) +{ + coap_duplication_info_s* info = sn_coap_protocol_linked_list_duplication_info_search(handle, + dst_addr_ptr, + coap_msg_ptr->msg_id); + + /* Update package data to duplication info struct if it's not there yet */ + if (info && info->packet_ptr == NULL) { + info->packet_ptr = handle->sn_coap_protocol_malloc(data_size); + if (info->packet_ptr) { + tr_debug("sn_coap_protocol_update_duplication_package_data - added to duplicate list!"); + memcpy(info->packet_ptr, dst_packet_data_ptr, data_size); + info->packet_len = data_size; + } else { + tr_error("sn_coap_protocol_update_duplication_package_data - failed to allocate duplication info!"); + return false; } } return true;