From 12e1fd653deab52cef5b1679ba631c35b2a55286 Mon Sep 17 00:00:00 2001 From: Robert Lubos Date: Wed, 16 Sep 2020 15:09:30 +0200 Subject: [PATCH] net: lwm2m: Fix FOTA block transfer with opaque content-format This commit fixes PUSH FOTA when opaque content-format is used. This consists of the following fixes: * Moved `struct block_context` to a private header, so that it can be a part of `struct lwm2m_input_context`. This allows content decoders to make use of the block context data. * Removed faulty `get_length_left` function from the plain text decoder, and replace it with coap_packet_get_payload() to obtain the actual payload size. * Introduce `struct lwm2m_opaque_context` as a part of block context, which allows to keep track of opaque data download progress. * Simplify `lwm2m_write_handler_opaque()` function. It will now only make calls to `engine_get_opaque` - it's the decoder responsibility to update the opaque context according to it's content format (for instance TLV decoder should only update it with the actual opaque data size, not the whole TLV). Signed-off-by: Robert Lubos --- subsys/net/lib/lwm2m/lwm2m_engine.c | 140 +++++++-------------- subsys/net/lib/lwm2m/lwm2m_engine.h | 4 +- subsys/net/lib/lwm2m/lwm2m_object.h | 27 +++- subsys/net/lib/lwm2m/lwm2m_rw_json.c | 4 +- subsys/net/lib/lwm2m/lwm2m_rw_oma_tlv.c | 18 ++- subsys/net/lib/lwm2m/lwm2m_rw_plain_text.c | 48 +++++-- 6 files changed, 125 insertions(+), 116 deletions(-) diff --git a/subsys/net/lib/lwm2m/lwm2m_engine.c b/subsys/net/lib/lwm2m/lwm2m_engine.c index 92d5081b3b4..194b45c92ff 100644 --- a/subsys/net/lib/lwm2m/lwm2m_engine.c +++ b/subsys/net/lib/lwm2m/lwm2m_engine.c @@ -138,18 +138,7 @@ static int sock_nfds; /* TODO: figure out what's correct value */ #define TIMEOUT_BLOCKWISE_TRANSFER_MS (MSEC_PER_SEC * 30) -struct block_context { - struct coap_block_context ctx; - int64_t timestamp; - uint32_t remaining_len; - uint32_t expected; - uint8_t token[8]; - uint8_t tkl; - uint8_t opaque_header_len; - bool last_block : 1; -}; - -static struct block_context block1_contexts[NUM_BLOCK1_CONTEXT]; +static struct lwm2m_block_context block1_contexts[NUM_BLOCK1_CONTEXT]; /* write-attribute related definitons */ static const char * const LWM2M_ATTR_STR[] = { "pmin", "pmax", @@ -243,8 +232,8 @@ enum coap_block_size lwm2m_default_block_size(void) return COAP_BLOCK_256; } -static int -init_block_ctx(const uint8_t *token, uint8_t tkl, struct block_context **ctx) +static int init_block_ctx(const uint8_t *token, uint8_t tkl, + struct lwm2m_block_context **ctx) { int i; int64_t timestamp; @@ -276,16 +265,15 @@ init_block_ctx(const uint8_t *token, uint8_t tkl, struct block_context **ctx) memcpy((*ctx)->token, token, tkl); coap_block_transfer_init(&(*ctx)->ctx, lwm2m_default_block_size(), 0); (*ctx)->timestamp = timestamp; - (*ctx)->remaining_len = 0; - (*ctx)->opaque_header_len = 0; (*ctx)->expected = 0; (*ctx)->last_block = false; + memset(&(*ctx)->opaque, 0, sizeof((*ctx)->opaque)); return 0; } -static int -get_block_ctx(const uint8_t *token, uint8_t tkl, struct block_context **ctx) +static int get_block_ctx(const uint8_t *token, uint8_t tkl, + struct lwm2m_block_context **ctx) { int i; @@ -308,7 +296,7 @@ get_block_ctx(const uint8_t *token, uint8_t tkl, struct block_context **ctx) return 0; } -static void free_block_ctx(struct block_context *ctx) +static void free_block_ctx(struct lwm2m_block_context *ctx) { if (ctx == NULL) { return; @@ -2213,9 +2201,11 @@ static int lwm2m_read_handler(struct lwm2m_engine_obj_inst *obj_inst, } size_t lwm2m_engine_get_opaque_more(struct lwm2m_input_context *in, - uint8_t *buf, size_t buflen, bool *last_block) + uint8_t *buf, size_t buflen, + struct lwm2m_opaque_context *opaque, + bool *last_block) { - uint32_t in_len = in->opaque_len; + uint32_t in_len = opaque->remaining; uint16_t remaining = in->in_cpkt->max_len - in->offset; if (in_len > buflen) { @@ -2226,9 +2216,9 @@ size_t lwm2m_engine_get_opaque_more(struct lwm2m_input_context *in, in_len = remaining; } - in->opaque_len -= in_len; + opaque->remaining -= in_len; remaining -= in_len; - if (in->opaque_len == 0U || remaining == 0) { + if (opaque->remaining == 0U || remaining == 0) { *last_block = true; } @@ -2245,80 +2235,44 @@ static int lwm2m_write_handler_opaque(struct lwm2m_engine_obj_inst *obj_inst, struct lwm2m_engine_res *res, struct lwm2m_engine_res_inst *res_inst, struct lwm2m_input_context *in, - void *data_ptr, size_t data_len, - struct block_context *block_ctx) + void *data_ptr, size_t data_len) { size_t len = 1; - bool last_pkt_block = false, first_read = true; - int block_num = 0; + bool last_pkt_block = false; int ret = 0; - size_t total_size = 0; bool last_block = true; + struct lwm2m_opaque_context opaque_ctx = { 0 }; - if (block_ctx != NULL) { - block_num = block_ctx->ctx.current / - coap_block_size_to_bytes(block_ctx->ctx.block_size); - last_block = block_ctx->last_block; + if (in->block_ctx != NULL) { + last_block = in->block_ctx->last_block; + + /* Restore the opaque context from the block context, if used. + */ + opaque_ctx = in->block_ctx->opaque; } while (!last_pkt_block && len > 0) { - if (first_read && block_num == 0) { - len = engine_get_opaque(in, (uint8_t *)data_ptr, - data_len, &last_pkt_block); - if (len == 0) { - /* ignore empty content and continue */ - return 0; - } - - if (block_ctx != NULL) { - block_ctx->remaining_len = in->opaque_len; - /* engine_get_opaque returns only the actual - * data length, not the header length, so we - * need to calculate it. - */ - block_ctx->opaque_header_len = - block_ctx->ctx.total_size - - (block_ctx->remaining_len + len); - } - - first_read = false; - } else { - if (block_ctx != NULL) { - in->opaque_len = block_ctx->remaining_len; - } - - len = lwm2m_engine_get_opaque_more(in, (uint8_t *)data_ptr, - data_len, - &last_pkt_block); - - if (block_ctx != NULL) { - block_ctx->remaining_len = in->opaque_len; - } - } - + len = engine_get_opaque(in, (uint8_t *)data_ptr, + data_len, &opaque_ctx, &last_pkt_block); if (len == 0) { - return -EINVAL; + /* ignore empty content and continue */ + return 0; } - - if (block_ctx != NULL) { - /* We shall only report the actual total data length, - * excluding the header size (if any). - */ - total_size = block_ctx->ctx.total_size - - block_ctx->opaque_header_len; - } - if (res->post_write_cb) { ret = res->post_write_cb( obj_inst->obj_inst_id, res->res_id, res_inst->res_inst_id, data_ptr, len, - last_pkt_block && last_block, total_size); + last_pkt_block && last_block, opaque_ctx.len); if (ret < 0) { return ret; } } } + if (in->block_ctx != NULL) { + in->block_ctx->opaque = opaque_ctx; + } + return ret; } @@ -2329,7 +2283,6 @@ int lwm2m_write_handler(struct lwm2m_engine_obj_inst *obj_inst, struct lwm2m_engine_obj_field *obj_field, struct lwm2m_message *msg) { - struct block_context *block_ctx = NULL; void *data_ptr = NULL; size_t data_len = 0; size_t len = 0; @@ -2337,8 +2290,6 @@ int lwm2m_write_handler(struct lwm2m_engine_obj_inst *obj_inst, int64_t temp64 = 0; int32_t temp32 = 0; int ret = 0; - uint8_t token[8]; - uint8_t tkl = 0U; bool last_block = true; if (!obj_inst || !res || !res_inst || !obj_field || !msg) { @@ -2361,19 +2312,14 @@ int lwm2m_write_handler(struct lwm2m_engine_obj_inst *obj_inst, } if (res->post_write_cb) { - /* Get block1 option for checking MORE block flag */ - ret = coap_get_option_int(msg->in.in_cpkt, COAP_OPTION_BLOCK1); - if (ret >= 0) { + if (msg->in.block_ctx != NULL) { /* Get block_ctx for total_size (might be zero) */ - tkl = coap_header_get_token(msg->in.in_cpkt, token); - if (tkl && !get_block_ctx(token, tkl, &block_ctx)) { - total_size = block_ctx->ctx.total_size; - LOG_DBG("BLOCK1: total:%zu current:%zu" - " last:%u", - block_ctx->ctx.total_size, - block_ctx->ctx.current, - block_ctx->last_block); - } + total_size = msg->in.block_ctx->ctx.total_size; + LOG_DBG("BLOCK1: total:%zu current:%zu" + " last:%u", + msg->in.block_ctx->ctx.total_size, + msg->in.block_ctx->ctx.current, + msg->in.block_ctx->last_block); } } @@ -2383,8 +2329,7 @@ int lwm2m_write_handler(struct lwm2m_engine_obj_inst *obj_inst, case LWM2M_RES_TYPE_OPAQUE: ret = lwm2m_write_handler_opaque(obj_inst, res, res_inst, &msg->in, - data_ptr, data_len, - block_ctx); + data_ptr, data_len); if (ret < 0) { return ret; } @@ -3330,7 +3275,7 @@ static int handle_request(struct coap_packet *request, int observe = -1; /* default to -1, 0 = ENABLE, 1 = DISABLE */ bool well_known = false; int block_opt, block_num; - struct block_context *block_ctx = NULL; + struct lwm2m_block_context *block_ctx = NULL; enum coap_block_size block_size; uint16_t payload_len = 0U; bool last_block = false; @@ -3543,6 +3488,8 @@ static int handle_request(struct coap_packet *request, goto error; } + msg->in.block_ctx = block_ctx; + if (block_num < block_ctx->expected) { LOG_WRN("Block already handled %d, expected %d", block_num, block_ctx->expected); @@ -4518,8 +4465,7 @@ static int lwm2m_engine_init(const struct device *dev) { int ret = 0; - (void)memset(block1_contexts, 0, - sizeof(struct block_context) * NUM_BLOCK1_CONTEXT); + (void)memset(block1_contexts, 0, sizeof(block1_contexts)); /* start sock receive thread */ k_thread_create(&engine_thread_data, diff --git a/subsys/net/lib/lwm2m/lwm2m_engine.h b/subsys/net/lib/lwm2m/lwm2m_engine.h index 94fe10993b4..9c55e17edfc 100644 --- a/subsys/net/lib/lwm2m/lwm2m_engine.h +++ b/subsys/net/lib/lwm2m/lwm2m_engine.h @@ -102,7 +102,9 @@ int lwm2m_engine_get_resource(char *pathstr, void lwm2m_engine_get_binding(char *binding); size_t lwm2m_engine_get_opaque_more(struct lwm2m_input_context *in, - uint8_t *buf, size_t buflen, bool *last_block); + uint8_t *buf, size_t buflen, + struct lwm2m_opaque_context *opaque, + bool *last_block); int lwm2m_security_inst_id_to_index(uint16_t obj_inst_id); int lwm2m_security_index_to_inst_id(int index); diff --git a/subsys/net/lib/lwm2m/lwm2m_object.h b/subsys/net/lib/lwm2m/lwm2m_object.h index 444fd6deec9..ef5955ab965 100644 --- a/subsys/net/lib/lwm2m/lwm2m_object.h +++ b/subsys/net/lib/lwm2m/lwm2m_object.h @@ -363,6 +363,21 @@ static inline void init_res_instance(struct lwm2m_engine_res_inst *ri, } } +struct lwm2m_opaque_context { + size_t len; + size_t remaining; +}; + +struct lwm2m_block_context { + struct coap_block_context ctx; + struct lwm2m_opaque_context opaque; + int64_t timestamp; + uint32_t expected; + uint8_t token[8]; + uint8_t tkl; + bool last_block : 1; +}; + struct lwm2m_output_context { const struct lwm2m_writer *writer; struct coap_packet *out_cpkt; @@ -378,8 +393,8 @@ struct lwm2m_input_context { /* current position in buffer */ uint16_t offset; - /* length of incoming opaque */ - size_t opaque_len; + /* Corresponding block context. NULL if block transfer is not used. */ + struct lwm2m_block_context *block_ctx; /* private output data */ void *user_data; @@ -490,7 +505,9 @@ struct lwm2m_reader { size_t (*get_bool)(struct lwm2m_input_context *in, bool *value); size_t (*get_opaque)(struct lwm2m_input_context *in, - uint8_t *buf, size_t buflen, bool *last_block); + uint8_t *buf, size_t buflen, + struct lwm2m_opaque_context *opaque, + bool *last_block); size_t (*get_objlnk)(struct lwm2m_input_context *in, struct lwm2m_objlnk *value); }; @@ -725,10 +742,12 @@ static inline size_t engine_get_bool(struct lwm2m_input_context *in, static inline size_t engine_get_opaque(struct lwm2m_input_context *in, uint8_t *buf, size_t buflen, + struct lwm2m_opaque_context *opaque, bool *last_block) { if (in->reader->get_opaque) { - return in->reader->get_opaque(in, buf, buflen, last_block); + return in->reader->get_opaque(in, buf, buflen, + opaque, last_block); } return 0; diff --git a/subsys/net/lib/lwm2m/lwm2m_rw_json.c b/subsys/net/lib/lwm2m/lwm2m_rw_json.c index 00996e6b4ae..bd905f4feda 100644 --- a/subsys/net/lib/lwm2m/lwm2m_rw_json.c +++ b/subsys/net/lib/lwm2m/lwm2m_rw_json.c @@ -684,7 +684,9 @@ static size_t get_bool(struct lwm2m_input_context *in, bool *value) } static size_t get_opaque(struct lwm2m_input_context *in, - uint8_t *value, size_t buflen, bool *last_block) + uint8_t *value, size_t buflen, + struct lwm2m_opaque_context *opaque, + bool *last_block) { /* TODO */ return 0; diff --git a/subsys/net/lib/lwm2m/lwm2m_rw_oma_tlv.c b/subsys/net/lib/lwm2m/lwm2m_rw_oma_tlv.c index 18fbe74bb04..0652be9ff15 100644 --- a/subsys/net/lib/lwm2m/lwm2m_rw_oma_tlv.c +++ b/subsys/net/lib/lwm2m/lwm2m_rw_oma_tlv.c @@ -758,13 +758,23 @@ static size_t get_bool(struct lwm2m_input_context *in, bool *value) } static size_t get_opaque(struct lwm2m_input_context *in, - uint8_t *value, size_t buflen, bool *last_block) + uint8_t *value, size_t buflen, + struct lwm2m_opaque_context *opaque, + bool *last_block) { struct oma_tlv tlv; + size_t size; - oma_tlv_get(&tlv, in, false); - in->opaque_len = tlv.length; - return lwm2m_engine_get_opaque_more(in, value, buflen, last_block); + /* Get the TLV header only on first read. */ + if (opaque->remaining == 0) { + size = oma_tlv_get(&tlv, in, false); + + opaque->len = tlv.length; + opaque->remaining = tlv.length; + } + + return lwm2m_engine_get_opaque_more(in, value, buflen, + opaque, last_block); } static size_t get_objlnk(struct lwm2m_input_context *in, diff --git a/subsys/net/lib/lwm2m/lwm2m_rw_plain_text.c b/subsys/net/lib/lwm2m/lwm2m_rw_plain_text.c index d1c54461e01..23aeff14712 100644 --- a/subsys/net/lib/lwm2m/lwm2m_rw_plain_text.c +++ b/subsys/net/lib/lwm2m/lwm2m_rw_plain_text.c @@ -202,11 +202,6 @@ static size_t put_objlnk(struct lwm2m_output_context *out, value->obj_inst); } -static int get_length_left(struct lwm2m_input_context *in) -{ - return in->in_cpkt->offset - in->offset; -} - static size_t plain_text_read_number(struct lwm2m_input_context *in, int64_t *value1, int64_t *value2, @@ -275,7 +270,9 @@ static size_t get_s64(struct lwm2m_input_context *in, int64_t *value) static size_t get_string(struct lwm2m_input_context *in, uint8_t *value, size_t buflen) { - uint16_t in_len = get_length_left(in); + uint16_t in_len; + + coap_packet_get_payload(in->in_cpkt, &in_len); if (in_len > buflen) { /* TODO: generate warning? */ @@ -332,10 +329,43 @@ static size_t get_bool(struct lwm2m_input_context *in, } static size_t get_opaque(struct lwm2m_input_context *in, - uint8_t *value, size_t buflen, bool *last_block) + uint8_t *value, size_t buflen, + struct lwm2m_opaque_context *opaque, + bool *last_block) { - in->opaque_len = get_length_left(in); - return lwm2m_engine_get_opaque_more(in, value, buflen, last_block); + uint16_t in_len; + + if (opaque->remaining == 0) { + coap_packet_get_payload(in->in_cpkt, &in_len); + + if (in->block_ctx != NULL) { + uint32_t block_num = + in->block_ctx->ctx.current / + coap_block_size_to_bytes( + in->block_ctx->ctx.block_size); + + if (block_num == 0) { + opaque->len = in->block_ctx->ctx.total_size; + } + + if (opaque->len == 0) { + /* No size1 option provided, use current + * payload size. This will reset on next packet + * received. + */ + opaque->remaining = in_len; + } else { + opaque->remaining = opaque->len; + } + + } else { + opaque->len = in_len; + opaque->remaining = in_len; + } + } + + return lwm2m_engine_get_opaque_more(in, value, buflen, + opaque, last_block); }