From 4189e2624297e6a7f92239774cc4f5aff562916f Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Tue, 1 Oct 2024 15:24:02 +0300 Subject: [PATCH 01/29] [nrf fromtree] net: lwm2m: Remove hostname_verify flag from context Use security mode (PSK or X509) to detect if we should set the socket option to verify hostname. PSK security mode cannot verify hostnames as this information is coming in the certificate, so don't set the options. Signed-off-by: Seppo Takalo (cherry picked from commit 73a3438b8204a61211b5f78786799a8e6ba77afa) --- include/zephyr/net/lwm2m.h | 2 -- subsys/net/lib/lwm2m/lwm2m_engine.c | 2 +- subsys/net/lib/lwm2m/lwm2m_message_handling.c | 1 - tests/net/lib/lwm2m/lwm2m_engine/src/main.c | 9 +++------ 4 files changed, 4 insertions(+), 10 deletions(-) diff --git a/include/zephyr/net/lwm2m.h b/include/zephyr/net/lwm2m.h index d69e508c8f3..8a2cbf557d0 100644 --- a/include/zephyr/net/lwm2m.h +++ b/include/zephyr/net/lwm2m.h @@ -245,8 +245,6 @@ struct lwm2m_ctx { char *desthostname; /** Destination hostname length */ uint16_t desthostnamelen; - /** Flag to indicate if hostname verification is enabled */ - bool hostname_verify; /** Custom load_credentials function. * Client can set load_credentials function as a way of overriding diff --git a/subsys/net/lib/lwm2m/lwm2m_engine.c b/subsys/net/lib/lwm2m/lwm2m_engine.c index 03d978fb913..833b72217f1 100644 --- a/subsys/net/lib/lwm2m/lwm2m_engine.c +++ b/subsys/net/lib/lwm2m/lwm2m_engine.c @@ -1070,7 +1070,7 @@ int lwm2m_set_default_sockopt(struct lwm2m_ctx *ctx) } } - if (ctx->hostname_verify && (ctx->desthostname != NULL)) { + if (ctx->desthostname != NULL && lwm2m_security_mode(ctx) == LWM2M_SECURITY_CERT) { /** store character at len position */ tmp = ctx->desthostname[ctx->desthostnamelen]; diff --git a/subsys/net/lib/lwm2m/lwm2m_message_handling.c b/subsys/net/lib/lwm2m/lwm2m_message_handling.c index 1b136fd8fdc..95347719206 100644 --- a/subsys/net/lib/lwm2m/lwm2m_message_handling.c +++ b/subsys/net/lib/lwm2m/lwm2m_message_handling.c @@ -3324,7 +3324,6 @@ int lwm2m_parse_peerinfo(char *url, struct lwm2m_ctx *client_ctx, bool is_firmwa /** copy url pointer to be used in socket */ client_ctx->desthostname = url + off; client_ctx->desthostnamelen = len; - client_ctx->hostname_verify = true; #endif #else diff --git a/tests/net/lib/lwm2m/lwm2m_engine/src/main.c b/tests/net/lib/lwm2m/lwm2m_engine/src/main.c index d982f3aecae..fc1f8dea2de 100644 --- a/tests/net/lib/lwm2m/lwm2m_engine/src/main.c +++ b/tests/net/lib/lwm2m/lwm2m_engine/src/main.c @@ -105,7 +105,6 @@ ZTEST(lwm2m_engine, test_start_stop) ctx.load_credentials = NULL; ctx.desthostname = host_name; ctx.desthostnamelen = strlen(host_name); - ctx.hostname_verify = true; ctx.use_dtls = true; ret = lwm2m_engine_start(&ctx); @@ -436,7 +435,6 @@ ZTEST(lwm2m_engine, test_security) ctx.load_credentials = NULL; ctx.desthostname = host_name; ctx.desthostnamelen = strlen(host_name); - ctx.hostname_verify = true; ctx.use_dtls = false; lwm2m_security_mode_fake.return_val = LWM2M_SECURITY_NOSEC; @@ -452,9 +450,8 @@ ZTEST(lwm2m_engine, test_security) lwm2m_security_mode_fake.return_val = LWM2M_SECURITY_PSK; zassert_equal(lwm2m_engine_start(&ctx), 0); zassert_equal(z_impl_zsock_setsockopt_fake.arg2_history[0], TLS_SEC_TAG_LIST); - zassert_equal(z_impl_zsock_setsockopt_fake.arg2_history[1], TLS_HOSTNAME); - zassert_equal(z_impl_zsock_setsockopt_fake.arg2_history[2], TLS_PEER_VERIFY); - zassert_equal(z_impl_zsock_setsockopt_fake.arg2_history[3], TLS_CIPHERSUITE_LIST); + zassert_equal(z_impl_zsock_setsockopt_fake.arg2_history[1], TLS_PEER_VERIFY); + zassert_equal(z_impl_zsock_setsockopt_fake.arg2_history[2], TLS_CIPHERSUITE_LIST); zassert_true(tls_credential_delete_fake.call_count > 3); zassert_true(tls_credential_add_fake.call_count == 2); zassert_equal(tls_credential_add_fake.arg1_history[0], TLS_CREDENTIAL_PSK_ID); @@ -464,7 +461,7 @@ ZTEST(lwm2m_engine, test_security) RESET_FAKE(z_impl_zsock_setsockopt); RESET_FAKE(tls_credential_add); lwm2m_security_mode_fake.return_val = LWM2M_SECURITY_CERT; - ctx.hostname_verify = false; + ctx.desthostname = NULL; zassert_equal(lwm2m_engine_start(&ctx), 0); zassert_equal(z_impl_zsock_setsockopt_fake.arg2_history[0], TLS_SEC_TAG_LIST); zassert_equal(z_impl_zsock_setsockopt_fake.arg2_history[1], TLS_PEER_VERIFY); From f8d32d256a2bb6db7207e95698bb5f8fb5c4a77a Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Wed, 2 Oct 2024 17:22:57 +0300 Subject: [PATCH 02/29] [nrf fromtree] net: lwm2m: Add shell command for listing resources Add shell command for listing multiple objects, resources or resource instances. Signed-off-by: Seppo Takalo (cherry picked from commit 8068cb2567508d760e71f04042f0301fdd081403) --- doc/connectivity/networking/api/lwm2m.rst | 3 + subsys/net/lib/lwm2m/lwm2m_shell.c | 123 +++++++++++++++++++++- 2 files changed, 125 insertions(+), 1 deletion(-) diff --git a/doc/connectivity/networking/api/lwm2m.rst b/doc/connectivity/networking/api/lwm2m.rst index b0d1a04f0de..cc48086613f 100644 --- a/doc/connectivity/networking/api/lwm2m.rst +++ b/doc/connectivity/networking/api/lwm2m.rst @@ -789,6 +789,9 @@ required actions from the server side. resume :LwM2M engine thread resume lock :Lock the LwM2M registry unlock :Unlock the LwM2M registry + obs : List observations + ls : ls [PATH] + List objects, instances, resources diff --git a/subsys/net/lib/lwm2m/lwm2m_shell.c b/subsys/net/lib/lwm2m/lwm2m_shell.c index 03e8cff7471..59749763263 100644 --- a/subsys/net/lib/lwm2m/lwm2m_shell.c +++ b/subsys/net/lib/lwm2m/lwm2m_shell.c @@ -56,7 +56,8 @@ LOG_MODULE_REGISTER(LOG_MODULE_NAME); #define LWM2M_HELP_OBSERV "List observations" #define LWM2M_HELP_CACHE "cache PATH NUM\nEnable data cache for resource\n" \ "PATH is LwM2M path\n" \ - "NUM how many elements to cache\n" \ + "NUM how many elements to cache\n" +#define LWM2M_HELP_LS "ls [PATH]\nList objects, instances, resources\n" static void send_cb(enum lwm2m_send_status status) { @@ -712,6 +713,125 @@ static int cmd_observations(const struct shell *sh, size_t argc, char **argv) return 0; } +static int print_object_instance(const struct shell *sh, struct lwm2m_engine_obj_inst *oi) +{ + struct lwm2m_engine_obj *obj; + + if (!oi) { + return -ENOEXEC; + } + + obj = oi->obj; + + for (int i = 0; i < oi->resource_count; i++) { + struct lwm2m_engine_res *re = &oi->resources[i]; + + for (int j = 0; j < re->res_inst_count; j++) { + struct lwm2m_engine_res_inst *ri = &re->res_instances[j]; + + if (ri->data_ptr && ri->data_len > 0 && + ri->res_inst_id != RES_INSTANCE_NOT_CREATED) { + struct lwm2m_engine_obj_field *field = + lwm2m_get_engine_obj_field(obj, re->res_id); + char path[LWM2M_MAX_PATH_STR_SIZE]; + + if (re->multi_res_inst) { + snprintf(path, sizeof(path), "%hu/%hu/%hu/%hu", obj->obj_id, + oi->obj_inst_id, re->res_id, ri->res_inst_id); + } else { + snprintf(path, sizeof(path), "%hu/%hu/%hu", obj->obj_id, + oi->obj_inst_id, re->res_id); + } + switch (field->data_type) { + case LWM2M_RES_TYPE_STRING: + shell_print(sh, "%s : %s", path, (char *)ri->data_ptr); + break; + case LWM2M_RES_TYPE_U8: + case LWM2M_RES_TYPE_S8: + case LWM2M_RES_TYPE_BOOL: + shell_print(sh, "%s : %u", path, *(uint8_t *)ri->data_ptr); + break; + case LWM2M_RES_TYPE_U16: + case LWM2M_RES_TYPE_S16: + shell_print(sh, "%s : %u", path, *(uint16_t *)ri->data_ptr); + break; + case LWM2M_RES_TYPE_U32: + case LWM2M_RES_TYPE_S32: + shell_print(sh, "%s : %u", path, *(uint32_t *)ri->data_ptr); + break; + case LWM2M_RES_TYPE_S64: + case LWM2M_RES_TYPE_TIME: + shell_print(sh, "%s : %lld", path, + *(int64_t *)ri->data_ptr); + break; + case LWM2M_RES_TYPE_FLOAT: + shell_print(sh, "%s : %lf", path, *(double *)ri->data_ptr); + break; + case LWM2M_RES_TYPE_OPAQUE: + shell_print(sh, "%s : OPAQUE(%hu/%hu)", path, ri->data_len, + ri->max_data_len); + break; + } + } + } + } + return 0; +} + +static int print_object(const struct shell *sh, struct lwm2m_engine_obj *obj) +{ + if (!obj) { + return -ENOEXEC; + } + struct lwm2m_engine_obj_inst *oi = next_engine_obj_inst(obj->obj_id, -1); + + for (int i = 0; i < obj->instance_count; i++) { + print_object_instance(sh, oi); + oi = next_engine_obj_inst(obj->obj_id, oi->obj_inst_id); + } + return 0; +} + +static int print_all_objs(const struct shell *sh) +{ + struct lwm2m_engine_obj *obj; + + SYS_SLIST_FOR_EACH_CONTAINER(lwm2m_engine_obj_list(), obj, node) { + print_object(sh, obj); + } + return 0; +} + +static int cmd_ls(const struct shell *sh, size_t argc, char **argv) +{ + struct lwm2m_obj_path path; + int ret; + + if (argc < 2) { + return print_all_objs(sh); + } + + ret = lwm2m_string_to_path(argv[1], &path, '/'); + if (ret < 0) { + return -ENOEXEC; + } + + if (path.level == LWM2M_PATH_LEVEL_NONE) { + return print_all_objs(sh); + } else if (path.level == LWM2M_PATH_LEVEL_OBJECT) { + struct lwm2m_engine_obj *obj = lwm2m_engine_get_obj(&path); + + return print_object(sh, obj); + } else if (path.level == LWM2M_PATH_LEVEL_OBJECT_INST) { + struct lwm2m_engine_obj_inst *oi = lwm2m_engine_get_obj_inst(&path); + + return print_object_instance(sh, oi); + } else if (path.level == LWM2M_PATH_LEVEL_RESOURCE) { + return cmd_read(sh, argc, argv); + } + return -ENOEXEC; +} + SHELL_STATIC_SUBCMD_SET_CREATE( sub_lwm2m, SHELL_COND_CMD_ARG(CONFIG_LWM2M_VERSION_1_1, send, NULL, @@ -730,6 +850,7 @@ SHELL_STATIC_SUBCMD_SET_CREATE( SHELL_CMD_ARG(lock, NULL, LWM2M_HELP_LOCK, cmd_lock, 1, 0), SHELL_CMD_ARG(unlock, NULL, LWM2M_HELP_UNLOCK, cmd_unlock, 1, 0), SHELL_CMD_ARG(obs, NULL, LWM2M_HELP_OBSERV, cmd_observations, 1, 0), + SHELL_CMD_ARG(ls, NULL, LWM2M_HELP_LS, cmd_ls, 1, 1), SHELL_SUBCMD_SET_END); SHELL_COND_CMD_ARG_REGISTER(CONFIG_LWM2M_SHELL, lwm2m, &sub_lwm2m, LWM2M_HELP_CMD, NULL, 1, 0); From 6a95e9b99adaa7ae9dc74b8b430ca559b0481204 Mon Sep 17 00:00:00 2001 From: Brandon Allen Date: Thu, 17 Oct 2024 12:35:09 -0400 Subject: [PATCH 03/29] [nrf fromtree] net: lib: lwm2m: lwm2m_rw_senml_cbor: only assign time on get_s64() success Currently GCC complains that temp64 may be used uninitialized in this function. Adds a check to ensure time is valid before assignining and fixes GCC warning. Signed-off-by: Brandon Allen (cherry picked from commit bb24c83d706bce0274c2d6cdc3bd472ca2906c9d) --- subsys/net/lib/lwm2m/lwm2m_rw_senml_cbor.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/subsys/net/lib/lwm2m/lwm2m_rw_senml_cbor.c b/subsys/net/lib/lwm2m/lwm2m_rw_senml_cbor.c index c371840b113..2429c60864f 100644 --- a/subsys/net/lib/lwm2m/lwm2m_rw_senml_cbor.c +++ b/subsys/net/lib/lwm2m/lwm2m_rw_senml_cbor.c @@ -601,7 +601,9 @@ static int get_time(struct lwm2m_input_context *in, time_t *value) int ret; ret = get_s64(in, &temp64); - *value = (time_t)temp64; + if (ret == 0) { + *value = (time_t)temp64; + } return ret; } From 535cd0d963ad52710768695ce3f3bb6948557046 Mon Sep 17 00:00:00 2001 From: Jeroen Broersen Date: Fri, 25 Oct 2024 18:30:23 +0200 Subject: [PATCH 04/29] [nrf fromtree] net: lwm2m: Add TLS_ECDHE_ECDSA_WITH_AES_128_CCM_8 to cipher list Add TLS_ECDHE_ECDSA_WITH_AES_128_CCM_8 to the list for use with x509 certificates. The LWM2M v1.1 specification says that a LWM2M client which used X509 certificates must support this ciphersuite and additional ciphersuites may be supported. Signed-off-by: Jeroen Broersen (cherry picked from commit f889c1ababfbef49d215b04e75dc05a0a30bfa3e) --- subsys/net/lib/lwm2m/lwm2m_engine.c | 1 + 1 file changed, 1 insertion(+) diff --git a/subsys/net/lib/lwm2m/lwm2m_engine.c b/subsys/net/lib/lwm2m/lwm2m_engine.c index 833b72217f1..bafa1be4517 100644 --- a/subsys/net/lib/lwm2m/lwm2m_engine.c +++ b/subsys/net/lib/lwm2m/lwm2m_engine.c @@ -1023,6 +1023,7 @@ static const int cipher_list_psk[] = { }; static const int cipher_list_cert[] = { + MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_128_CCM_8, MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, }; From 68691be40fe97ba932112eedb4be32685a4e1f11 Mon Sep 17 00:00:00 2001 From: Pieter De Gendt Date: Wed, 13 Nov 2024 08:39:54 +0100 Subject: [PATCH 05/29] [nrf fromtree] tests: net: lib: lwm2m: interop: Fix strip-with-multi-characters (B005) All strip functions remove any of the provided characters instead of a prefix/suffix. This is likely a bug. See https://docs.astral.sh/ruff/rules/strip-with-multi-characters/ Signed-off-by: Pieter De Gendt (cherry picked from commit 175bfb4bc9e59371a8101af96c6ff853ad18dab6) --- tests/net/lib/lwm2m/interop/pytest/leshan.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/net/lib/lwm2m/interop/pytest/leshan.py b/tests/net/lib/lwm2m/interop/pytest/leshan.py index 024bc6775c0..916661a8a4e 100644 --- a/tests/net/lib/lwm2m/interop/pytest/leshan.py +++ b/tests/net/lib/lwm2m/interop/pytest/leshan.py @@ -455,7 +455,7 @@ def next_event(self, event: str): for line in self._it: if not line.startswith('data: '): continue - data = json.loads(line.lstrip('data: ')) + data = json.loads(line.removeprefix('data: ')) if event == 'SEND' or (event == 'NOTIFICATION' and data['kind'] == 'composite'): return Leshan.parse_composite(data['val']) if event == 'NOTIFICATION': From 337a496b950f086812311f165c615c2b11b45f50 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 11 Nov 2024 14:35:30 +0200 Subject: [PATCH 06/29] [nrf fromtree] tests: coap_client: Improve socket stubs * Use sys_rand_get() and seed the CoAP library, so our MIDs are random. * Set socket events per socket, so we don't accidentally receive on wrong socket * Reply with correct tokens Signed-off-by: Seppo Takalo (cherry picked from commit ca67f51170a53f32ff9c0769d085fcbc47b905da) --- tests/net/lib/coap_client/prj.conf | 1 + tests/net/lib/coap_client/src/main.c | 91 +++++++++++++++++++-------- tests/net/lib/coap_client/src/stubs.c | 20 +++--- tests/net/lib/coap_client/src/stubs.h | 7 +-- 4 files changed, 77 insertions(+), 42 deletions(-) diff --git a/tests/net/lib/coap_client/prj.conf b/tests/net/lib/coap_client/prj.conf index c7265029a21..8405a56f800 100644 --- a/tests/net/lib/coap_client/prj.conf +++ b/tests/net/lib/coap_client/prj.conf @@ -1,3 +1,4 @@ #Testing CONFIG_ZTEST=y CONFIG_ZTEST_STACK_SIZE=4096 +CONFIG_TEST_RANDOM_GENERATOR=y diff --git a/tests/net/lib/coap_client/src/main.c b/tests/net/lib/coap_client/src/main.c index f06a0ce646c..e066f86bae6 100644 --- a/tests/net/lib/coap_client/src/main.c +++ b/tests/net/lib/coap_client/src/main.c @@ -11,7 +11,7 @@ #include "stubs.h" -LOG_MODULE_REGISTER(coap_client_test); +LOG_MODULE_REGISTER(coap_client_test, LOG_LEVEL_DBG); DEFINE_FFF_GLOBALS; #define FFF_FAKES_LIST(FAKE) @@ -23,11 +23,14 @@ DEFINE_FFF_GLOBALS; (CONFIG_COAP_INIT_ACK_TIMEOUT_MS + CONFIG_COAP_INIT_ACK_TIMEOUT_MS / 2) #define VALID_MESSAGE_ID BIT(31) +#define TOKEN_OFFSET 4 static int16_t last_response_code; static const char *test_path = "test"; static uint32_t messages_needing_response[2]; +static uint8_t last_token[2][COAP_TOKEN_MAX_LEN]; +static const uint8_t empty_token[COAP_TOKEN_MAX_LEN] = {0}; static struct coap_client client; @@ -61,6 +64,27 @@ static void set_next_pending_message_id(uint16_t id) } } +static void store_token(uint8_t *buf) +{ + for (int i = 0; i < ARRAY_SIZE(last_token); i++) { + if (memcmp(last_token[i], empty_token, 8) == 0) { + memcpy(last_token[i], buf + TOKEN_OFFSET, COAP_TOKEN_MAX_LEN); + return; + } + } +} + +static void restore_token(uint8_t *buf) +{ + for (int i = 0; i < ARRAY_SIZE(last_token); i++) { + if (memcmp(last_token[i], empty_token, 8) != 0) { + memcpy(buf + TOKEN_OFFSET, last_token[i], COAP_TOKEN_MAX_LEN); + memset(last_token[i], 0, COAP_TOKEN_MAX_LEN); + return; + } + } +} + static ssize_t z_impl_zsock_recvfrom_custom_fake(int sock, void *buf, size_t max_len, int flags, struct sockaddr *src_addr, socklen_t *addrlen) { @@ -74,10 +98,11 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake(int sock, void *buf, size_t max ack_data[2] = (uint8_t)(last_message_id >> 8); ack_data[3] = (uint8_t)last_message_id; + restore_token(ack_data); memcpy(buf, ack_data, sizeof(ack_data)); - clear_socket_events(ZSOCK_POLLIN); + clear_socket_events(sock, ZSOCK_POLLIN); return sizeof(ack_data); } @@ -90,14 +115,14 @@ static ssize_t z_impl_zsock_sendto_custom_fake(int sock, void *buf, size_t len, last_message_id |= ((uint8_t *)buf)[2] << 8; last_message_id |= ((uint8_t *)buf)[3]; - type = (((uint8_t *)buf)[0] & 0x30) >> 4; + store_token(buf); set_next_pending_message_id(last_message_id); LOG_INF("Latest message ID: %d", last_message_id); if (type == 0) { - set_socket_events(ZSOCK_POLLIN); + set_socket_events(sock, ZSOCK_POLLIN); } return 1; @@ -111,6 +136,7 @@ static ssize_t z_impl_zsock_sendto_custom_fake_no_reply(int sock, void *buf, siz last_message_id |= ((uint8_t *)buf)[2] << 8; last_message_id |= ((uint8_t *)buf)[3]; + store_token(buf); set_next_pending_message_id(last_message_id); LOG_INF("Latest message ID: %d", last_message_id); @@ -128,6 +154,7 @@ static ssize_t z_impl_zsock_sendto_custom_fake_echo(int sock, void *buf, size_t last_message_id |= ((uint8_t *)buf)[2] << 8; last_message_id |= ((uint8_t *)buf)[3]; + store_token(buf); set_next_pending_message_id(last_message_id); LOG_INF("Latest message ID: %d", last_message_id); @@ -144,7 +171,7 @@ static ssize_t z_impl_zsock_sendto_custom_fake_echo(int sock, void *buf, size_t z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake; - set_socket_events(ZSOCK_POLLIN); + set_socket_events(sock, ZSOCK_POLLIN); return 1; } @@ -160,6 +187,7 @@ static ssize_t z_impl_zsock_sendto_custom_fake_echo_next_req(int sock, void *buf last_message_id |= ((uint8_t *)buf)[2] << 8; last_message_id |= ((uint8_t *)buf)[3]; + store_token(buf); set_next_pending_message_id(last_message_id); LOG_INF("Latest message ID: %d", last_message_id); @@ -184,7 +212,7 @@ static ssize_t z_impl_zsock_sendto_custom_fake_echo_next_req(int sock, void *buf z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake; - set_socket_events(ZSOCK_POLLIN); + set_socket_events(sock, ZSOCK_POLLIN); return 1; } @@ -202,10 +230,11 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_response(int sock, void *buf, s ack_data[2] = (uint8_t)(last_message_id >> 8); ack_data[3] = (uint8_t)last_message_id; + restore_token(ack_data); memcpy(buf, ack_data, sizeof(ack_data)); - clear_socket_events(ZSOCK_POLLIN); + clear_socket_events(sock, ZSOCK_POLLIN); return sizeof(ack_data); } @@ -216,7 +245,7 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_empty_ack(int sock, void *buf, { uint16_t last_message_id = 0; - static uint8_t ack_data[] = {0x68, 0x00, 0x00, 0x00, 0x00, 0x00, + static uint8_t ack_data[] = {0x60, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; last_message_id = get_next_pending_message_id(); @@ -247,7 +276,7 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_unmatching(int sock, void *buf, memcpy(buf, ack_data, sizeof(ack_data)); - clear_socket_events(ZSOCK_POLLIN); + clear_socket_events(sock, ZSOCK_POLLIN); return sizeof(ack_data); } @@ -267,13 +296,14 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_echo(int sock, void *buf, size_ ack_data[2] = (uint8_t)(last_message_id >> 8); ack_data[3] = (uint8_t)last_message_id; + restore_token(ack_data); memcpy(buf, ack_data, sizeof(ack_data)); z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_response; z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_echo; - clear_socket_events(ZSOCK_POLLIN); + clear_socket_events(sock, ZSOCK_POLLIN); return sizeof(ack_data); } @@ -293,19 +323,23 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_echo_next_req(int sock, void *b ack_data[2] = (uint8_t)(last_message_id >> 8); ack_data[3] = (uint8_t)last_message_id; + restore_token(ack_data); memcpy(buf, ack_data, sizeof(ack_data)); z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_response; z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_echo_next_req; - clear_socket_events(ZSOCK_POLLIN); + clear_socket_events(sock, ZSOCK_POLLIN); return sizeof(ack_data); } +extern void net_coap_init(void); + static void *suite_setup(void) { + net_coap_init(); coap_client_init(&client, NULL); return NULL; @@ -315,6 +349,8 @@ static void test_setup(void *data) { int i; + k_mutex_lock(&client.lock, K_FOREVER); + /* Register resets */ DO_FOREACH_FAKE(RESET_FAKE); /* reset common FFF internal structures */ @@ -322,13 +358,19 @@ static void test_setup(void *data) z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake; z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake; - clear_socket_events(ZSOCK_POLLIN | ZSOCK_POLLOUT | ZSOCK_POLLERR); + clear_socket_events(0, ZSOCK_POLLIN | ZSOCK_POLLOUT | ZSOCK_POLLERR); + clear_socket_events(1, ZSOCK_POLLIN | ZSOCK_POLLOUT | ZSOCK_POLLERR); for (i = 0; i < ARRAY_SIZE(messages_needing_response); i++) { messages_needing_response[i] = 0; } + memset(&client.requests, 0, sizeof(client.requests)); + memset(last_token, 0, sizeof(last_token)); + last_response_code = 0; + + k_mutex_unlock(&client.lock); } void coap_callback(int16_t code, size_t offset, const uint8_t *payload, size_t len, bool last_block, @@ -396,7 +438,7 @@ ZTEST(coap_client, test_resend_request) ret = coap_client_req(&client, 0, &address, &client_request, NULL); zassert_true(ret >= 0, "Sending request failed, %d", ret); k_sleep(K_MSEC(MORE_THAN_ACK_TIMEOUT_MS)); - set_socket_events(ZSOCK_POLLIN | ZSOCK_POLLOUT); + set_socket_events(client.fd, ZSOCK_POLLIN | ZSOCK_POLLOUT); k_sleep(K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); @@ -551,7 +593,7 @@ ZTEST(coap_client, test_no_response) client_request.len = strlen(short_payload); z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; - set_socket_events(ZSOCK_POLLOUT); + set_socket_events(client.fd, ZSOCK_POLLOUT); k_sleep(K_MSEC(1)); @@ -628,7 +670,7 @@ ZTEST(coap_client, test_multiple_requests) ret = coap_client_req(&client, 0, &address, &req2, NULL); zassert_true(ret >= 0, "Sending request failed, %d", ret); - set_socket_events(ZSOCK_POLLIN); + set_socket_events(client.fd, ZSOCK_POLLIN); while (last_response_code == 0 && retry > 0) { retry--; k_sleep(K_MSEC(1)); @@ -636,7 +678,7 @@ ZTEST(coap_client, test_multiple_requests) zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); last_response_code = 0; - set_socket_events(ZSOCK_POLLIN); + set_socket_events(client.fd, ZSOCK_POLLIN); zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); zassert_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); @@ -665,7 +707,7 @@ ZTEST(coap_client, test_unmatching_tokens) client_request.len = strlen(short_payload); z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_unmatching; - set_socket_events(ZSOCK_POLLIN | ZSOCK_POLLOUT); + set_socket_events(client.fd, ZSOCK_POLLIN | ZSOCK_POLLOUT); k_sleep(K_MSEC(1)); @@ -681,9 +723,6 @@ ZTEST(coap_client, test_multiple_clients) { int ret; int retry = MORE_THAN_EXCHANGE_LIFETIME_MS; - static struct coap_client client2 = { - .fd = 2, - }; struct k_sem sem1, sem2; struct sockaddr address = {0}; struct coap_client_request req1 = { @@ -710,17 +749,17 @@ ZTEST(coap_client, test_multiple_clients) k_sleep(K_MSEC(1)); LOG_INF("Sending requests"); - ret = coap_client_req(&client, 1, &address, &req1, NULL); + ret = coap_client_req(&client, client.fd, &address, &req1, NULL); zassert_true(ret >= 0, "Sending request failed, %d", ret); - ret = coap_client_req(&client2, 2, &address, &req2, NULL); + ret = coap_client_req(&client2, client2.fd, &address, &req2, NULL); zassert_true(ret >= 0, "Sending request failed, %d", ret); while (last_response_code == 0 && retry > 0) { retry--; k_sleep(K_MSEC(1)); } - set_socket_events(ZSOCK_POLLIN); + set_socket_events(client2.fd, ZSOCK_POLLIN); k_sleep(K_SECONDS(1)); @@ -746,7 +785,7 @@ ZTEST(coap_client, test_poll_err) }; z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; - set_socket_events(ZSOCK_POLLERR); + set_socket_events(client.fd, ZSOCK_POLLERR); k_sleep(K_MSEC(1)); @@ -776,7 +815,7 @@ ZTEST(coap_client, test_poll_err_after_response) zassert_ok(k_sem_init(&sem1, 0, 1)); z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; - set_socket_events(ZSOCK_POLLIN); + set_socket_events(client.fd, ZSOCK_POLLIN); k_sleep(K_MSEC(1)); @@ -787,6 +826,6 @@ ZTEST(coap_client, test_poll_err_after_response) zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); - set_socket_events(ZSOCK_POLLERR); + set_socket_events(client.fd, ZSOCK_POLLERR); zassert_not_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); } diff --git a/tests/net/lib/coap_client/src/stubs.c b/tests/net/lib/coap_client/src/stubs.c index 63307e27fb7..f3451df054f 100644 --- a/tests/net/lib/coap_client/src/stubs.c +++ b/tests/net/lib/coap_client/src/stubs.c @@ -10,7 +10,6 @@ LOG_MODULE_DECLARE(coap_client_test); DEFINE_FAKE_VALUE_FUNC(uint32_t, z_impl_sys_rand32_get); -DEFINE_FAKE_VOID_FUNC(z_impl_sys_rand_get, void *, size_t); DEFINE_FAKE_VALUE_FUNC(ssize_t, z_impl_zsock_recvfrom, int, void *, size_t, int, struct sockaddr *, socklen_t *); DEFINE_FAKE_VALUE_FUNC(ssize_t, z_impl_zsock_sendto, int, void *, size_t, int, @@ -22,21 +21,17 @@ struct zsock_pollfd { short revents; }; -static short my_events; +static short my_events[NUM_FD]; -void set_socket_events(short events) +void set_socket_events(int fd, short events) { - my_events |= events; + __ASSERT_NO_MSG(fd < NUM_FD); + my_events[fd] |= events; } -void clear_socket_events(short events) +void clear_socket_events(int fd, short events) { - my_events &= ~events; -} - -int z_impl_zsock_socket(int family, int type, int proto) -{ - return 0; + my_events[fd] &= ~events; } int z_impl_zsock_poll(struct zsock_pollfd *fds, int nfds, int poll_timeout) @@ -44,7 +39,8 @@ int z_impl_zsock_poll(struct zsock_pollfd *fds, int nfds, int poll_timeout) int events = 0; k_sleep(K_MSEC(1)); for (int i = 0; i < nfds; i++) { - fds[i].revents = my_events & (fds[i].events | ZSOCK_POLLERR | ZSOCK_POLLHUP); + fds[i].revents = + my_events[fds[i].fd] & (fds[i].events | ZSOCK_POLLERR | ZSOCK_POLLHUP); if (fds[i].revents) { events++; } diff --git a/tests/net/lib/coap_client/src/stubs.h b/tests/net/lib/coap_client/src/stubs.h index 9a9f929ce76..c3024536b54 100644 --- a/tests/net/lib/coap_client/src/stubs.h +++ b/tests/net/lib/coap_client/src/stubs.h @@ -36,12 +36,12 @@ #define ZSOCK_POLLNVAL 0x20 /** @} */ +#define NUM_FD 2 -void set_socket_events(short events); -void clear_socket_events(short events); +void set_socket_events(int fd, short events); +void clear_socket_events(int fd, short events); DECLARE_FAKE_VALUE_FUNC(uint32_t, z_impl_sys_rand32_get); -DECLARE_FAKE_VOID_FUNC(z_impl_sys_rand_get, void *, size_t); DECLARE_FAKE_VALUE_FUNC(ssize_t, z_impl_zsock_recvfrom, int, void *, size_t, int, struct sockaddr *, socklen_t *); DECLARE_FAKE_VALUE_FUNC(ssize_t, z_impl_zsock_sendto, int, void*, size_t, int, @@ -50,7 +50,6 @@ DECLARE_FAKE_VALUE_FUNC(ssize_t, z_impl_zsock_sendto, int, void*, size_t, int, #define DO_FOREACH_FAKE(FUNC) \ do { \ FUNC(z_impl_sys_rand32_get) \ - FUNC(z_impl_sys_rand_get) \ FUNC(z_impl_zsock_recvfrom) \ FUNC(z_impl_zsock_sendto) \ } while (0) From bd5832f870173b96031ae3dadb74279700dcd345 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 11 Nov 2024 14:36:46 +0200 Subject: [PATCH 07/29] [nrf fromtree] tests: coap_client: Add test for duplicate response Add test where we receive same UDP packet twice. Signed-off-by: Seppo Takalo (cherry picked from commit 83bc1fcb46b6c32d324c9c4d0fa697b51dfb5a0e) --- tests/net/lib/coap_client/CMakeLists.txt | 2 +- tests/net/lib/coap_client/src/main.c | 55 ++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/tests/net/lib/coap_client/CMakeLists.txt b/tests/net/lib/coap_client/CMakeLists.txt index d7f0c0cce5e..07a4d8a9827 100644 --- a/tests/net/lib/coap_client/CMakeLists.txt +++ b/tests/net/lib/coap_client/CMakeLists.txt @@ -26,7 +26,7 @@ add_compile_definitions(CONFIG_COAP_CLIENT_MESSAGE_HEADER_SIZE=48) add_compile_definitions(CONFIG_COAP_CLIENT_STACK_SIZE=1024) add_compile_definitions(CONFIG_COAP_CLIENT_THREAD_PRIORITY=10) add_compile_definitions(CONFIG_COAP_LOG_LEVEL=4) -add_compile_definitions(CONFIG_COAP_INIT_ACK_TIMEOUT_MS=10) +add_compile_definitions(CONFIG_COAP_INIT_ACK_TIMEOUT_MS=100) add_compile_definitions(CONFIG_COAP_CLIENT_MAX_REQUESTS=2) add_compile_definitions(CONFIG_COAP_CLIENT_MAX_INSTANCES=2) add_compile_definitions(CONFIG_COAP_MAX_RETRANSMIT=4) diff --git a/tests/net/lib/coap_client/src/main.c b/tests/net/lib/coap_client/src/main.c index e066f86bae6..d03160ca53f 100644 --- a/tests/net/lib/coap_client/src/main.c +++ b/tests/net/lib/coap_client/src/main.c @@ -335,6 +335,30 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_echo_next_req(int sock, void *b return sizeof(ack_data); } +static ssize_t z_impl_zsock_recvfrom_custom_fake_duplicate_response(int sock, void *buf, + size_t max_len, int flags, + struct sockaddr *src_addr, + socklen_t *addrlen) +{ + uint8_t token[TOKEN_OFFSET + COAP_TOKEN_MAX_LEN]; + + uint16_t last_message_id = get_next_pending_message_id(); + + restore_token(token); + + set_next_pending_message_id(last_message_id); + set_next_pending_message_id(last_message_id); + store_token(token); + store_token(token); + + int ret = z_impl_zsock_recvfrom_custom_fake(sock, buf, max_len, flags, src_addr, addrlen); + + set_socket_events(sock, ZSOCK_POLLIN); + z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake; + + return ret; +} + extern void net_coap_init(void); static void *suite_setup(void) @@ -829,3 +853,34 @@ ZTEST(coap_client, test_poll_err_after_response) set_socket_events(client.fd, ZSOCK_POLLERR); zassert_not_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); } + +ZTEST(coap_client, test_duplicate_response) +{ + int ret = 0; + struct k_sem sem; + struct sockaddr address = {0}; + struct coap_client_request client_request = { + .method = COAP_METHOD_GET, + .confirmable = true, + .path = test_path, + .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, + .cb = coap_callback, + .payload = short_payload, + .len = strlen(short_payload), + .user_data = &sem, + }; + + zassert_ok(k_sem_init(&sem, 0, 2)); + z_impl_zsock_recvfrom_fake.custom_fake = + z_impl_zsock_recvfrom_custom_fake_duplicate_response; + + k_sleep(K_MSEC(1)); + LOG_INF("Send request"); + ret = coap_client_req(&client, 0, &address, &client_request, NULL); + zassert_true(ret >= 0, "Sending request failed, %d", ret); + + zassert_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); + + zassert_equal(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)), -EAGAIN, ""); +} From ab4be5299ef96a749130c3e96dd818574dcd474a Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 11 Nov 2024 16:42:52 +0200 Subject: [PATCH 08/29] [nrf fromtree] tests: coap_client: Add test when separate response is lost Test a scenario where Ack is received but the actual response is not coming. Signed-off-by: Seppo Takalo (cherry picked from commit 120aabbb8f3bdc97e6338b0590cd738d0795615b) --- tests/net/lib/coap_client/src/main.c | 47 +++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/tests/net/lib/coap_client/src/main.c b/tests/net/lib/coap_client/src/main.c index d03160ca53f..1ed77cbfc62 100644 --- a/tests/net/lib/coap_client/src/main.c +++ b/tests/net/lib/coap_client/src/main.c @@ -21,7 +21,7 @@ DEFINE_FFF_GLOBALS; #define MORE_THAN_LONG_EXCHANGE_LIFETIME_MS 4 * LONG_ACK_TIMEOUT_MS #define MORE_THAN_ACK_TIMEOUT_MS \ (CONFIG_COAP_INIT_ACK_TIMEOUT_MS + CONFIG_COAP_INIT_ACK_TIMEOUT_MS / 2) - +#define COAP_SEPARATE_TIMEOUT (6000 * 3) /* Needs a safety marging, tests run faster than -rt */ #define VALID_MESSAGE_ID BIT(31) #define TOKEN_OFFSET 4 @@ -33,6 +33,9 @@ static uint8_t last_token[2][COAP_TOKEN_MAX_LEN]; static const uint8_t empty_token[COAP_TOKEN_MAX_LEN] = {0}; static struct coap_client client; +static struct coap_client client2 = { + .fd = 1, +}; static char *short_payload = "testing"; static char *long_payload = LOREM_IPSUM_SHORT; @@ -260,6 +263,18 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_empty_ack(int sock, void *buf, return sizeof(ack_data); } +static ssize_t z_impl_zsock_recvfrom_custom_fake_only_ack(int sock, void *buf, size_t max_len, + int flags, struct sockaddr *src_addr, + socklen_t *addrlen) +{ + int ret; + + ret = z_impl_zsock_recvfrom_custom_fake_empty_ack(sock, buf, max_len, flags, src_addr, + addrlen); + clear_socket_events(sock, ZSOCK_POLLIN); + return ret; +} + static ssize_t z_impl_zsock_recvfrom_custom_fake_unmatching(int sock, void *buf, size_t max_len, int flags, struct sockaddr *src_addr, socklen_t *addrlen) @@ -659,6 +674,36 @@ ZTEST(coap_client, test_separate_response) zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); } +ZTEST(coap_client, test_separate_response_lost) +{ + int ret = 0; + struct k_sem sem; + struct sockaddr address = {0}; + struct coap_client_request client_request = { + .method = COAP_METHOD_GET, + .confirmable = true, + .path = test_path, + .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, + .cb = coap_callback, + .payload = short_payload, + .len = strlen(short_payload), + .user_data = &sem, + }; + + zassert_ok(k_sem_init(&sem, 0, 1)); + z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_only_ack; + set_socket_events(client.fd, ZSOCK_POLLOUT); + + k_sleep(K_MSEC(1)); + + LOG_INF("Send request"); + ret = coap_client_req(&client, 0, &address, &client_request, NULL); + zassert_true(ret >= 0, "Sending request failed, %d", ret); + + zassert_ok(k_sem_take(&sem, K_MSEC(COAP_SEPARATE_TIMEOUT))); + zassert_equal(last_response_code, -ETIMEDOUT, ""); +} + ZTEST(coap_client, test_multiple_requests) { int ret = 0; From effb7a61cb1922d7662c5143fb5eb4314c67d1b7 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Tue, 12 Nov 2024 13:32:53 +0200 Subject: [PATCH 09/29] [nrf fromtree] tests: coap_client: Improve retry testcases Add testcases for testing client's retry behaviour. Signed-off-by: Seppo Takalo (cherry picked from commit 5559a520c9b0c619a32f306666a1618caed7c238) --- tests/net/lib/coap_client/src/main.c | 57 ++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 7 deletions(-) diff --git a/tests/net/lib/coap_client/src/main.c b/tests/net/lib/coap_client/src/main.c index 1ed77cbfc62..330ad689b97 100644 --- a/tests/net/lib/coap_client/src/main.c +++ b/tests/net/lib/coap_client/src/main.c @@ -220,6 +220,22 @@ static ssize_t z_impl_zsock_sendto_custom_fake_echo_next_req(int sock, void *buf return 1; } +static ssize_t z_impl_zsock_sendto_custom_fake_block(int sock, void *buf, size_t len, int flags, + const struct sockaddr *dest_addr, + socklen_t addrlen) +{ + errno = EAGAIN; + return -1; +} + +static ssize_t z_impl_zsock_sendto_custom_fake_err(int sock, void *buf, size_t len, int flags, + const struct sockaddr *dest_addr, + socklen_t addrlen) +{ + errno = ENETDOWN; + return -1; +} + static ssize_t z_impl_zsock_recvfrom_custom_fake_response(int sock, void *buf, size_t max_len, int flags, struct sockaddr *src_addr, socklen_t *addrlen) @@ -452,6 +468,30 @@ ZTEST(coap_client, test_get_request) LOG_INF("Test done"); } +ZTEST(coap_client, test_request_block) +{ + int ret = 0; + struct sockaddr address = {0}; + struct coap_client_request client_request = { + .method = COAP_METHOD_GET, + .confirmable = true, + .path = test_path, + .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, + .cb = coap_callback, + .payload = short_payload, + .len = strlen(short_payload), + }; + + z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_block; + + k_sleep(K_MSEC(1)); + + LOG_INF("Send request"); + ret = coap_client_req(&client, 0, &address, &client_request, NULL); + zassert_equal(ret, -EAGAIN, ""); +} + + ZTEST(coap_client, test_resend_request) { int ret = 0; @@ -462,14 +502,18 @@ ZTEST(coap_client, test_resend_request) .path = test_path, .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, .cb = coap_callback, - .payload = NULL, - .len = 0 + .payload = short_payload, + .len = strlen(short_payload), }; - client_request.payload = short_payload; - client_request.len = strlen(short_payload); + int (*sendto_fakes[])(int, void *, size_t, int, const struct sockaddr *, socklen_t) = { + z_impl_zsock_sendto_custom_fake_no_reply, + z_impl_zsock_sendto_custom_fake_block, + z_impl_zsock_sendto_custom_fake, + }; - z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; + SET_CUSTOM_FAKE_SEQ(z_impl_zsock_sendto, sendto_fakes, ARRAY_SIZE(sendto_fakes)); + set_socket_events(client.fd, ZSOCK_POLLOUT); k_sleep(K_MSEC(1)); @@ -477,11 +521,10 @@ ZTEST(coap_client, test_resend_request) ret = coap_client_req(&client, 0, &address, &client_request, NULL); zassert_true(ret >= 0, "Sending request failed, %d", ret); k_sleep(K_MSEC(MORE_THAN_ACK_TIMEOUT_MS)); - set_socket_events(client.fd, ZSOCK_POLLIN | ZSOCK_POLLOUT); k_sleep(K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); - zassert_equal(z_impl_zsock_sendto_fake.call_count, 2); + zassert_equal(z_impl_zsock_sendto_fake.call_count, 3); LOG_INF("Test done"); } From 7bb1a14e333279639528dfead0ad7eb87c2ed7b9 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Tue, 12 Nov 2024 13:38:11 +0200 Subject: [PATCH 10/29] [nrf fromtree] tests: coap_client: Add testcase for Ack failure Add testcase where sending Ack to incomming Confirmable message fails. This should be reported to application as now the server is unaware that transmission have succeeded, so we cannot thread it as success. Signed-off-by: Seppo Takalo (cherry picked from commit fc51fa491ff70574092fb9e5fd9c5d7e4caa2985) --- tests/net/lib/coap_client/src/main.c | 35 ++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/net/lib/coap_client/src/main.c b/tests/net/lib/coap_client/src/main.c index 330ad689b97..7554ffcd7ab 100644 --- a/tests/net/lib/coap_client/src/main.c +++ b/tests/net/lib/coap_client/src/main.c @@ -747,6 +747,41 @@ ZTEST(coap_client, test_separate_response_lost) zassert_equal(last_response_code, -ETIMEDOUT, ""); } +ZTEST(coap_client, test_separate_response_ack_fail) +{ + int ret = 0; + struct k_sem sem; + struct sockaddr address = {0}; + struct coap_client_request client_request = { + .method = COAP_METHOD_GET, + .confirmable = true, + .path = test_path, + .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, + .cb = coap_callback, + .payload = short_payload, + .len = strlen(short_payload), + .user_data = &sem, + }; + zassert_ok(k_sem_init(&sem, 0, 1)); + + int (*sendto_fakes[])(int, void *, size_t, int, const struct sockaddr *, socklen_t) = { + z_impl_zsock_sendto_custom_fake, + z_impl_zsock_sendto_custom_fake_err, + }; + + SET_CUSTOM_FAKE_SEQ(z_impl_zsock_sendto, sendto_fakes, ARRAY_SIZE(sendto_fakes)); + z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_empty_ack; + + k_sleep(K_MSEC(1)); + + LOG_INF("Send request"); + ret = coap_client_req(&client, 0, &address, &client_request, NULL); + zassert_true(ret >= 0, "Sending request failed, %d", ret); + + zassert_ok(k_sem_take(&sem, K_MSEC(COAP_SEPARATE_TIMEOUT))); + zassert_equal(last_response_code, -ENETDOWN, ""); +} + ZTEST(coap_client, test_multiple_requests) { int ret = 0; From 5edd70f0dedf4bd77fc25f6b49a7e7d33bfbda0d Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Tue, 12 Nov 2024 15:01:23 +0200 Subject: [PATCH 11/29] [nrf fromtree] tests: coap_client: Test for operating on socket while another fails CoAP client should be able to push data through functioning socket while another sockets is failing or reporting poll() errors. Signed-off-by: Seppo Takalo (cherry picked from commit 237b26c44a1be61cfb04305bbf6f92fe116ab729) --- tests/net/lib/coap_client/src/main.c | 45 ++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/tests/net/lib/coap_client/src/main.c b/tests/net/lib/coap_client/src/main.c index 7554ffcd7ab..69e5b7dc9bd 100644 --- a/tests/net/lib/coap_client/src/main.c +++ b/tests/net/lib/coap_client/src/main.c @@ -395,7 +395,8 @@ extern void net_coap_init(void); static void *suite_setup(void) { net_coap_init(); - coap_client_init(&client, NULL); + zassert_ok(coap_client_init(&client, NULL)); + zassert_ok(coap_client_init(&client2, NULL)); return NULL; } @@ -891,8 +892,6 @@ ZTEST(coap_client, test_multiple_clients) zassert_ok(k_sem_init(&sem1, 0, 1)); zassert_ok(k_sem_init(&sem2, 0, 1)); - zassert_ok(coap_client_init(&client2, NULL)); - k_sleep(K_MSEC(1)); LOG_INF("Sending requests"); @@ -977,6 +976,46 @@ ZTEST(coap_client, test_poll_err_after_response) zassert_not_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); } +ZTEST(coap_client, test_poll_err_on_another_sock) +{ + int ret = 0; + struct k_sem sem1, sem2; + struct sockaddr address = {0}; + struct coap_client_request client_request = { + .method = COAP_METHOD_GET, + .confirmable = true, + .path = test_path, + .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, + .cb = coap_callback, + .payload = short_payload, + .len = strlen(short_payload), + .user_data = &sem1 + }; + struct coap_client_request request2 = client_request; + + request2.user_data = &sem2; + + zassert_ok(k_sem_init(&sem1, 0, 1)); + zassert_ok(k_sem_init(&sem2, 0, 1)); + + z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; + set_socket_events(client.fd, ZSOCK_POLLERR); + + k_sleep(K_MSEC(1)); + + ret = coap_client_req(&client2, client2.fd, &address, &request2, NULL); + zassert_true(ret >= 0, "Sending request failed, %d", ret); + ret = coap_client_req(&client, client.fd, &address, &client_request, NULL); + zassert_true(ret >= 0, "Sending request failed, %d", ret); + + set_socket_events(client2.fd, ZSOCK_POLLIN); + + zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_equal(last_response_code, -EIO, ""); + zassert_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, ""); +} + ZTEST(coap_client, test_duplicate_response) { int ret = 0; From 1ac36d977057df63f972230f6e529b78e4becb73 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Tue, 12 Nov 2024 16:07:54 +0200 Subject: [PATCH 12/29] [nrf fromtree] tests: coap_client: Add testcase for observation Add test for ongoing observation and cancellation. Signed-off-by: Seppo Takalo (cherry picked from commit 05a6ba678efc6e4a49b70c84669d32006a98bbdc) --- tests/net/lib/coap_client/src/main.c | 52 ++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/net/lib/coap_client/src/main.c b/tests/net/lib/coap_client/src/main.c index 69e5b7dc9bd..6c4b624798c 100644 --- a/tests/net/lib/coap_client/src/main.c +++ b/tests/net/lib/coap_client/src/main.c @@ -390,6 +390,18 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_duplicate_response(int sock, vo return ret; } +static ssize_t z_impl_zsock_recvfrom_custom_fake_observe(int sock, void *buf, size_t max_len, + int flags, struct sockaddr *src_addr, + socklen_t *addrlen) +{ + int ret = z_impl_zsock_recvfrom_custom_fake_duplicate_response(sock, buf, max_len, flags, + src_addr, addrlen); + + set_next_pending_message_id(get_next_pending_message_id() + 1); + z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_observe; + return ret; +} + extern void net_coap_init(void); static void *suite_setup(void) @@ -1046,3 +1058,43 @@ ZTEST(coap_client, test_duplicate_response) zassert_equal(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)), -EAGAIN, ""); } + +ZTEST(coap_client, test_observe) +{ + struct k_sem sem; + struct sockaddr address = {0}; + struct coap_client_option options = { + .code = COAP_OPTION_OBSERVE, + .value[0] = 0, + .len = 1, + }; + struct coap_client_request client_request = { + .method = COAP_METHOD_GET, + .confirmable = true, + .path = test_path, + .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, + .cb = coap_callback, + .payload = short_payload, + .len = strlen(short_payload), + .options = &options, + .num_options = 1, + .user_data = &sem, + }; + + zassert_ok(k_sem_init(&sem, 0, 1)); + z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_observe; + + k_sleep(K_MSEC(1)); + + zassert_ok(coap_client_req(&client, 0, &address, &client_request, NULL)); + + zassert_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + + coap_client_cancel_requests(&client); + zassert_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_equal(last_response_code, -ECANCELED, ""); + + zassert_not_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); +} From a29009d7dcee4b03e480b3edc2f5740dfa8c6eee Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Tue, 12 Nov 2024 16:18:46 +0200 Subject: [PATCH 13/29] [nrf fromtree] tests: coap_client: Add testcase for receiving RST When server responds with CoAP RESET, we should inform the client and stop the request. Signed-off-by: Seppo Takalo (cherry picked from commit 107dc9b96dd0627805a25f5ec91407bfb86bc00c) --- tests/net/lib/coap_client/src/main.c | 44 ++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/net/lib/coap_client/src/main.c b/tests/net/lib/coap_client/src/main.c index 6c4b624798c..c44fd30fbdc 100644 --- a/tests/net/lib/coap_client/src/main.c +++ b/tests/net/lib/coap_client/src/main.c @@ -279,6 +279,25 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_empty_ack(int sock, void *buf, return sizeof(ack_data); } +static ssize_t z_impl_zsock_recvfrom_custom_fake_rst(int sock, void *buf, size_t max_len, int flags, + struct sockaddr *src_addr, socklen_t *addrlen) +{ + uint16_t last_message_id = 0; + + static uint8_t rst_data[] = {0x70, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + + last_message_id = get_next_pending_message_id(); + + rst_data[2] = (uint8_t)(last_message_id >> 8); + rst_data[3] = (uint8_t)last_message_id; + + memcpy(buf, rst_data, sizeof(rst_data)); + clear_socket_events(sock, ZSOCK_POLLIN); + + return sizeof(rst_data); +} + static ssize_t z_impl_zsock_recvfrom_custom_fake_only_ack(int sock, void *buf, size_t max_len, int flags, struct sockaddr *src_addr, socklen_t *addrlen) @@ -1098,3 +1117,28 @@ ZTEST(coap_client, test_observe) zassert_not_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); } + +ZTEST(coap_client, test_request_rst) +{ + struct k_sem sem; + struct sockaddr address = {0}; + struct coap_client_request client_request = { + .method = COAP_METHOD_GET, + .confirmable = true, + .path = test_path, + .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, + .cb = coap_callback, + .payload = short_payload, + .len = strlen(short_payload), + .user_data = &sem, + }; + + zassert_ok(k_sem_init(&sem, 0, 1)); + k_sleep(K_MSEC(1)); + z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_rst; + + zassert_ok(coap_client_req(&client, 0, &address, &client_request, NULL)); + + zassert_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_equal(last_response_code, -ECONNRESET, ""); +} From d3d5eee8a7fe9c83b422d58519926464f80d618c Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 11 Nov 2024 14:19:49 +0200 Subject: [PATCH 14/29] [nrf fromtree] net: lib: coap_client: Don't clear internal structures on response When response is received and handled, don't just clear the structure but instead mark it as ongoing=false. So if we later on receive a duplicate response for it, we can still respond with Ack or Rst. This is achieved by using release_internal_request() when we don't expect any response for it and reset_internal_request() when we really fill up a new request. Signed-off-by: Seppo Takalo (cherry picked from commit 41ee35ae8b52095cce830ac3381b5ffa3c4162b4) --- subsys/net/lib/coap/coap_client.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/subsys/net/lib/coap/coap_client.c b/subsys/net/lib/coap/coap_client.c index ebf99bade9f..43cb6416d9a 100644 --- a/subsys/net/lib/coap/coap_client.c +++ b/subsys/net/lib/coap/coap_client.c @@ -61,16 +61,24 @@ static int receive(int sock, void *buf, size_t max_len, int flags, return err; } +/** Reset all fields to zero. + * Use when a new request is filled in. + */ static void reset_internal_request(struct coap_client_internal_request *request) { - request->offset = 0; - request->last_id = 0; - request->last_response_id = -1; + *request = (struct coap_client_internal_request){ + .last_response_id = -1, + }; +} + +/** Release a request structure. + * Use when a request is no longer needed, but we might still receive + * responses for it, which must be handled. + */ +static void release_internal_request(struct coap_client_internal_request *request) +{ request->request_ongoing = false; - request->is_observe = false; request->pending.timeout = 0; - request->recv_blk_ctx = (struct coap_block_context){ 0 }; - request->send_blk_ctx = (struct coap_block_context){ 0 }; } static int coap_client_schedule_poll(struct coap_client *client, int sock, @@ -417,6 +425,7 @@ int coap_client_req(struct coap_client *client, int sock, const struct sockaddr coap_pending_cycle(&internal_req->pending); internal_req->is_observe = coap_request_is_observe(&internal_req->request); + LOG_DBG("Request is_observe %d", internal_req->is_observe); } ret = send_request(sock, internal_req->request.data, internal_req->request.offset, 0, @@ -513,7 +522,7 @@ static void coap_client_resend_handler(struct coap_client *client) ret = resend_request(client, &client->requests[i]); if (ret < 0) { report_callback_error(&client->requests[i], ret); - reset_internal_request(&client->requests[i]); + release_internal_request(&client->requests[i]); } } } @@ -745,7 +754,7 @@ static int handle_response(struct coap_client *client, const struct coap_packet return 0; } report_callback_error(internal_req, -ECONNRESET); - reset_internal_request(internal_req); + release_internal_request(internal_req); return 0; } @@ -931,7 +940,7 @@ static int handle_response(struct coap_client *client, const struct coap_packet } fail: if (ret < 0 || !internal_req->is_observe) { - reset_internal_request(internal_req); + release_internal_request(internal_req); } return ret; } @@ -949,7 +958,7 @@ static void cancel_requests_with(struct coap_client *client, int error) * request was cancelled anyway. */ report_callback_error(&client->requests[i], error); - reset_internal_request(&client->requests[i]); + release_internal_request(&client->requests[i]); } } k_mutex_unlock(&client->lock); From 03c3ded35cc5b726e0a77061f965e8848f45a301 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 11 Nov 2024 14:21:29 +0200 Subject: [PATCH 15/29] [nrf fromtree] net: lib: coap_client: Parse incoming MID only once Incomming Message-ID is already parsed, use it as a parameter to get_request_with_mid(). Signed-off-by: Seppo Takalo (cherry picked from commit 7b0cce4418403e046e710009c591fa21e6aaed3c) --- subsys/net/lib/coap/coap_client.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/subsys/net/lib/coap/coap_client.c b/subsys/net/lib/coap/coap_client.c index 43cb6416d9a..1f6b93c2221 100644 --- a/subsys/net/lib/coap/coap_client.c +++ b/subsys/net/lib/coap/coap_client.c @@ -30,9 +30,8 @@ static void cancel_requests_with(struct coap_client *client, int error); static int recv_response(struct coap_client *client, struct coap_packet *response, bool *truncated); static int handle_response(struct coap_client *client, const struct coap_packet *response, bool response_truncated); -static struct coap_client_internal_request *get_request_with_mid( - struct coap_client *client, const struct coap_packet *resp); - +static struct coap_client_internal_request *get_request_with_mid(struct coap_client *client, + uint16_t mid); static int send_request(int sock, const void *buf, size_t len, int flags, const struct sockaddr *dest_addr, socklen_t addrlen) @@ -701,14 +700,12 @@ static struct coap_client_internal_request *get_request_with_token( return NULL; } -static struct coap_client_internal_request *get_request_with_mid( - struct coap_client *client, const struct coap_packet *resp) +static struct coap_client_internal_request *get_request_with_mid(struct coap_client *client, + uint16_t mid) { - uint16_t mid = coap_header_get_id(resp); - for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) { if (client->requests[i].request_ongoing) { - if (client->requests[i].last_id == mid) { + if (client->requests[i].last_id == (int)mid) { return &client->requests[i]; } } @@ -717,7 +714,6 @@ static struct coap_client_internal_request *get_request_with_mid( return NULL; } - static bool find_echo_option(const struct coap_packet *response, struct coap_option *option) { return coap_find_options(response, COAP_OPTION_ECHO, option, 1); @@ -748,7 +744,7 @@ static int handle_response(struct coap_client *client, const struct coap_packet const uint8_t *payload = coap_packet_get_payload(response, &payload_len); if (response_type == COAP_TYPE_RESET) { - internal_req = get_request_with_mid(client, response); + internal_req = get_request_with_mid(client, response_id); if (!internal_req) { LOG_WRN("No matching request for RESET"); return 0; @@ -761,7 +757,7 @@ static int handle_response(struct coap_client *client, const struct coap_packet /* Separate response coming */ if (payload_len == 0 && response_type == COAP_TYPE_ACK && response_code == COAP_CODE_EMPTY) { - internal_req = get_request_with_mid(client, response); + internal_req = get_request_with_mid(client, response_id); if (!internal_req) { LOG_WRN("No matching request for ACK"); return 0; From 4569c8d211d30a375886e5ef9038f14b11a44392 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 11 Nov 2024 17:14:50 +0200 Subject: [PATCH 16/29] [nrf fromtree] net: lib: coap_client: Don't match zero length tokens If our internal structure is cleared, don't match tokens. Signed-off-by: Seppo Takalo (cherry picked from commit 934c74f26e8f2d25bfb7b4ac517db251b2197780) --- subsys/net/lib/coap/coap_client.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/subsys/net/lib/coap/coap_client.c b/subsys/net/lib/coap/coap_client.c index 1f6b93c2221..9518da30e74 100644 --- a/subsys/net/lib/coap/coap_client.c +++ b/subsys/net/lib/coap/coap_client.c @@ -687,6 +687,9 @@ static struct coap_client_internal_request *get_request_with_token( for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) { if (client->requests[i].request_ongoing || !exchange_lifetime_exceeded(&client->requests[i])) { + if (client->requests[i].request_tkl == 0) { + continue; + } if (client->requests[i].request_tkl != response_tkl) { continue; } From 76834f979b050d7675533ada1b1766dff43cf44d Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 11 Nov 2024 14:25:04 +0200 Subject: [PATCH 17/29] [nrf fromtree] net: lib: coap_client: Drop duplicate responses When response is already handled, don't forward anymore responses to the client application. Signed-off-by: Seppo Takalo (cherry picked from commit a1368a7ff707af009575dac8799799b2319cad50) --- subsys/net/lib/coap/coap_client.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/subsys/net/lib/coap/coap_client.c b/subsys/net/lib/coap/coap_client.c index 9518da30e74..57d7d4af4a3 100644 --- a/subsys/net/lib/coap/coap_client.c +++ b/subsys/net/lib/coap/coap_client.c @@ -844,6 +844,15 @@ static int handle_response(struct coap_client *client, const struct coap_packet } } + if (!internal_req->request_ongoing) { + if (internal_req->is_observe) { + (void) send_rst(client, response); + return 0; + } + LOG_DBG("Drop request, already handled"); + goto fail; + } + if (internal_req->pending.timeout != 0) { coap_pending_clear(&internal_req->pending); } From f56220316838bbb25f6d69135fa8dc9bf96f13a8 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 11 Nov 2024 16:44:41 +0200 Subject: [PATCH 18/29] [nrf fromtree] net: lib: coap_client: Return -errno from send_request() Return the -errno when zsock_sendto() or zsock_recvfrom() fails, so rest of the code can deal with return values, instead of separately comparing errno and return value. Signed-off-by: Seppo Takalo (cherry picked from commit 48434a3c1b16c44a410b07a50905681af506ea76) --- subsys/net/lib/coap/coap_client.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/subsys/net/lib/coap/coap_client.c b/subsys/net/lib/coap/coap_client.c index 57d7d4af4a3..9766c698a83 100644 --- a/subsys/net/lib/coap/coap_client.c +++ b/subsys/net/lib/coap/coap_client.c @@ -36,12 +36,16 @@ static struct coap_client_internal_request *get_request_with_mid(struct coap_cli static int send_request(int sock, const void *buf, size_t len, int flags, const struct sockaddr *dest_addr, socklen_t addrlen) { + int ret; + LOG_HEXDUMP_DBG(buf, len, "Send CoAP Request:"); if (addrlen == 0) { - return zsock_sendto(sock, buf, len, flags, NULL, 0); + ret = zsock_sendto(sock, buf, len, flags, NULL, 0); } else { - return zsock_sendto(sock, buf, len, flags, dest_addr, addrlen); + ret = zsock_sendto(sock, buf, len, flags, dest_addr, addrlen); } + + return ret >= 0 ? ret : -errno; } static int receive(int sock, void *buf, size_t max_len, int flags, @@ -57,7 +61,7 @@ static int receive(int sock, void *buf, size_t max_len, int flags, if (err > 0) { LOG_HEXDUMP_DBG(buf, err, "Receive CoAP Response:"); } - return err; + return err >= 0 ? err : -errno; } /** Reset all fields to zero. @@ -493,13 +497,12 @@ static int resend_request(struct coap_client *client, client->socklen); if (ret > 0) { ret = 0; - } else if (ret == -1 && errno == EAGAIN) { + } else if (ret == -EAGAIN) { /* Restore the pending structure, retry later */ internal_req->pending = tmp; /* Not a fatal socket error, will trigger a retry */ ret = 0; } else { - ret = -errno; LOG_ERR("Failed to resend request, %d", ret); } } else { From 5237d64fc3537669adff9ed971566c2d168836df Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 11 Nov 2024 16:46:33 +0200 Subject: [PATCH 19/29] [nrf fromtree] net: lib: coap_client: Drop duplicate MID only after responding with Ack Even if we receive duplicate confirmable message, we should still respond with the Ack. Just don't deliver the second callback. This is achieved by moving the MID deduplication to after Ack handling. Signed-off-by: Seppo Takalo (cherry picked from commit c0eb260c2c53b183aa4ce4a5d1d3ad20516a0b08) --- subsys/net/lib/coap/coap_client.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/subsys/net/lib/coap/coap_client.c b/subsys/net/lib/coap/coap_client.c index 9766c698a83..d2a07301581 100644 --- a/subsys/net/lib/coap/coap_client.c +++ b/subsys/net/lib/coap/coap_client.c @@ -781,14 +781,6 @@ static int handle_response(struct coap_client *client, const struct coap_packet return 0; } - /* MID-based deduplication */ - if (response_id == internal_req->last_response_id) { - LOG_WRN("Duplicate MID, dropping"); - goto fail; - } - - internal_req->last_response_id = response_id; - /* Received echo option */ if (find_echo_option(response, &client->echo_option)) { /* Resend request with echo option */ @@ -847,13 +839,21 @@ static int handle_response(struct coap_client *client, const struct coap_packet } } + /* MID-based deduplication */ + if (response_id == internal_req->last_response_id) { + LOG_WRN("Duplicate MID, dropping"); + return 0; + } + + internal_req->last_response_id = response_id; + if (!internal_req->request_ongoing) { if (internal_req->is_observe) { (void) send_rst(client, response); return 0; } LOG_DBG("Drop request, already handled"); - goto fail; + return 0; } if (internal_req->pending.timeout != 0) { From 1db79238e9ef7ef67d1d554d0eaa6fc482476968 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 11 Nov 2024 16:48:42 +0200 Subject: [PATCH 20/29] [nrf fromtree] net: lib: coap_client: All error cases should be reported to callback When the client fails when parsing the response and we stop proceeding, we should report that to the application. Signed-off-by: Seppo Takalo (cherry picked from commit f72d634826f5c45ee7ed54b4658c3391ae83fe54) --- subsys/net/lib/coap/coap_client.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/subsys/net/lib/coap/coap_client.c b/subsys/net/lib/coap/coap_client.c index d2a07301581..9aaf2b6b267 100644 --- a/subsys/net/lib/coap/coap_client.c +++ b/subsys/net/lib/coap/coap_client.c @@ -950,7 +950,10 @@ static int handle_response(struct coap_client *client, const struct coap_packet } } fail: - if (ret < 0 || !internal_req->is_observe) { + if (ret < 0) { + report_callback_error(internal_req, ret); + } + if (!internal_req->is_observe) { release_internal_request(internal_req); } return ret; From ec45a9777a28d2c5d833ba67a2f133c94fb6cf7a Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Tue, 12 Nov 2024 15:00:05 +0200 Subject: [PATCH 21/29] [nrf fromtree] net: lib: coap_client: Stop polling on unneeded sockets poll() only for sockets that have traffic ongoing or have some lifetime left. On socket failures during a poll(), stop listening for the socket. Application can recover by reconnecting the socket. Signed-off-by: Seppo Takalo (cherry picked from commit f0c6efe7980002eb72e5ad009a8b822d2a43846d) --- subsys/net/lib/coap/coap_client.c | 56 ++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/subsys/net/lib/coap/coap_client.c b/subsys/net/lib/coap/coap_client.c index 9aaf2b6b267..505a9932311 100644 --- a/subsys/net/lib/coap/coap_client.c +++ b/subsys/net/lib/coap/coap_client.c @@ -532,6 +532,17 @@ static void coap_client_resend_handler(struct coap_client *client) k_mutex_unlock(&client->lock); } +static struct coap_client *get_client(int sock) +{ + for (int i = 0; i < num_clients; i++) { + if (clients[i]->fd == sock) { + return clients[i]; + } + } + + return NULL; +} + static int handle_poll(void) { int ret = 0; @@ -541,10 +552,16 @@ static int handle_poll(void) /* Use periodic timeouts */ for (int i = 0; i < num_clients; i++) { - fds[i].fd = clients[i]->fd; - fds[i].events = (has_ongoing_exchange(clients[i]) ? ZSOCK_POLLIN : 0) | - (has_timeout_expired(clients[i]) ? ZSOCK_POLLOUT : 0); - fds[i].revents = 0; + short events = (has_ongoing_exchange(clients[i]) ? ZSOCK_POLLIN : 0) | + (has_timeout_expired(clients[i]) ? ZSOCK_POLLOUT : 0); + + if (events == 0) { + /* Skip this socket */ + continue; + } + fds[nfds].fd = clients[i]->fd; + fds[nfds].events = events; + fds[nfds].revents = 0; nfds++; } @@ -559,42 +576,49 @@ static int handle_poll(void) } for (int i = 0; i < nfds; i++) { + struct coap_client *client = get_client(fds[i].fd); + + if (!client) { + LOG_ERR("No client found for socket %d", fds[i].fd); + continue; + } + if (fds[i].revents & ZSOCK_POLLOUT) { - coap_client_resend_handler(clients[i]); + coap_client_resend_handler(client); } if (fds[i].revents & ZSOCK_POLLIN) { struct coap_packet response; bool response_truncated = false; - ret = recv_response(clients[i], &response, &response_truncated); + ret = recv_response(client, &response, &response_truncated); if (ret < 0) { if (ret == -EAGAIN) { continue; } LOG_ERR("Error receiving response"); - cancel_requests_with(clients[i], -EIO); + cancel_requests_with(client, -EIO); continue; } - k_mutex_lock(&clients[i]->lock, K_FOREVER); - ret = handle_response(clients[i], &response, response_truncated); + k_mutex_lock(&client->lock, K_FOREVER); + ret = handle_response(client, &response, response_truncated); if (ret < 0) { LOG_ERR("Error handling response"); } - k_mutex_unlock(&clients[i]->lock); + k_mutex_unlock(&client->lock); } if (fds[i].revents & ZSOCK_POLLERR) { LOG_ERR("Error in poll for socket %d", fds[i].fd); - cancel_requests_with(clients[i], -EIO); + cancel_requests_with(client, -EIO); } if (fds[i].revents & ZSOCK_POLLHUP) { LOG_ERR("Error in poll: POLLHUP for socket %d", fds[i].fd); - cancel_requests_with(clients[i], -EIO); + cancel_requests_with(client, -EIO); } if (fds[i].revents & ZSOCK_POLLNVAL) { LOG_ERR("Error in poll: POLLNVAL - fd %d not open", fds[i].fd); - cancel_requests_with(clients[i], -EIO); + cancel_requests_with(client, -EIO); } } @@ -974,6 +998,12 @@ static void cancel_requests_with(struct coap_client *client, int error) report_callback_error(&client->requests[i], error); release_internal_request(&client->requests[i]); } + /* If our socket has failed, clear all requests, even completed ones, + * so that our handle_poll() does not poll() anymore for this socket. + */ + if (error == -EIO) { + reset_internal_request(&client->requests[i]); + } } k_mutex_unlock(&client->lock); From c1fd0f318c5bc2b25e19408b001e2935e45b0251 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Wed, 13 Nov 2024 13:24:28 +0200 Subject: [PATCH 22/29] [nrf fromtree] net: lib: coap_client: Add API to cancel specific request Add a new API to cancel just one, or mathing requests, instead of cancelling all ongoing requests. Signed-off-by: Seppo Takalo (cherry picked from commit b3f3bce23eaa854f0eb03e2c92e1c342b9a4ef4a) --- include/zephyr/net/coap_client.h | 15 ++++ subsys/net/lib/coap/coap_client.c | 35 +++++++++ tests/net/lib/coap_client/src/main.c | 108 +++++++++++++++++++++++++++ 3 files changed, 158 insertions(+) diff --git a/include/zephyr/net/coap_client.h b/include/zephyr/net/coap_client.h index 1d68661da3b..bbeebb2d26c 100644 --- a/include/zephyr/net/coap_client.h +++ b/include/zephyr/net/coap_client.h @@ -158,6 +158,21 @@ int coap_client_req(struct coap_client *client, int sock, const struct sockaddr */ void coap_client_cancel_requests(struct coap_client *client); +/** + * @brief Cancel matching requests. + * + * This function cancels all CoAP client request that matches the given request. + * The request is matched based on the method, path, callback and user_data, if provided. + * Any field set to NULL is considered a wildcard. + * + * (struct coap_client_request){0} cancels all requests. + * (struct coap_client_request){.method = COAP_METHOD_GET} cancels all GET requests. + * + * @param client Pointer to the CoAP client instance. + * @param req Pointer to the CoAP client request to be canceled. + */ +void coap_client_cancel_request(struct coap_client *client, struct coap_client_request *req); + /** * @brief Initialise a Block2 option to be added to a request * diff --git a/subsys/net/lib/coap/coap_client.c b/subsys/net/lib/coap/coap_client.c index 505a9932311..468505f5eb5 100644 --- a/subsys/net/lib/coap/coap_client.c +++ b/subsys/net/lib/coap/coap_client.c @@ -1016,6 +1016,41 @@ void coap_client_cancel_requests(struct coap_client *client) k_sleep(K_MSEC(COAP_PERIODIC_TIMEOUT)); } +static bool requests_match(struct coap_client_request *a, struct coap_client_request *b) +{ + /* enum coap_method does not have value for zero, so differentiate valid values */ + if (a->method && b->method && a->method != b->method) { + return false; + } + if (a->path && b->path && strcmp(a->path, b->path) != 0) { + return false; + } + if (a->cb && b->cb && a->cb != b->cb) { + return false; + } + if (a->user_data && b->user_data && a->user_data != b->user_data) { + return false; + } + /* It is intentional that (struct coap_client_request){0} matches all */ + return true; +} + +void coap_client_cancel_request(struct coap_client *client, struct coap_client_request *req) +{ + k_mutex_lock(&client->lock, K_FOREVER); + + for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) { + if (client->requests[i].request_ongoing && + requests_match(&client->requests[i].coap_request, req)) { + LOG_DBG("Cancelling request %d", i); + report_callback_error(&client->requests[i], -ECANCELED); + release_internal_request(&client->requests[i]); + } + } + + k_mutex_unlock(&client->lock); +} + void coap_client_recv(void *coap_cl, void *a, void *b) { int ret; diff --git a/tests/net/lib/coap_client/src/main.c b/tests/net/lib/coap_client/src/main.c index c44fd30fbdc..2f3fa28ebff 100644 --- a/tests/net/lib/coap_client/src/main.c +++ b/tests/net/lib/coap_client/src/main.c @@ -1142,3 +1142,111 @@ ZTEST(coap_client, test_request_rst) zassert_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); zassert_equal(last_response_code, -ECONNRESET, ""); } + +ZTEST(coap_client, test_cancel) +{ + struct k_sem sem1, sem2; + struct sockaddr address = {0}; + struct coap_client_request req1 = { + .method = COAP_METHOD_GET, + .confirmable = true, + .path = test_path, + .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, + .cb = coap_callback, + .payload = short_payload, + .len = strlen(short_payload), + .user_data = &sem1 + }; + struct coap_client_request req2 = req1; + + req2.user_data = &sem2; + + k_sem_init(&sem1, 0, 1); + k_sem_init(&sem2, 0, 1); + + z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; + + k_sleep(K_MSEC(1)); + + zassert_ok(coap_client_req(&client, 0, &address, &req1, NULL)); + zassert_ok(coap_client_req(&client, 0, &address, &req2, NULL)); + + k_sleep(K_SECONDS(1)); + + coap_client_cancel_request(&client, &req1); + zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_not_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_equal(last_response_code, -ECANCELED, ""); + + set_socket_events(client.fd, ZSOCK_POLLIN); /* First response is the cancelled one */ + zassert_not_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + set_socket_events(client.fd, ZSOCK_POLLIN); + zassert_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, ""); +} + +ZTEST(coap_client, test_cancel_match) +{ + struct k_sem sem1, sem2; + struct sockaddr address = {0}; + struct coap_client_request req1 = { + .method = COAP_METHOD_GET, + .confirmable = true, + .path = test_path, + .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, + .cb = coap_callback, + .payload = short_payload, + .len = strlen(short_payload), + .user_data = &sem1 + }; + struct coap_client_request req2 = req1; + + req2.user_data = &sem2; + req2.path = "another"; + + k_sem_init(&sem1, 0, 1); + k_sem_init(&sem2, 0, 1); + + z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; + + k_sleep(K_MSEC(1)); + + zassert_ok(coap_client_req(&client, 0, &address, &req1, NULL)); + zassert_ok(coap_client_req(&client, 0, &address, &req2, NULL)); + + k_sleep(K_SECONDS(1)); + + /* match only one */ + coap_client_cancel_request(&client, &(struct coap_client_request) { + .path = test_path + }); + zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_not_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_equal(last_response_code, -ECANCELED, ""); + + zassert_ok(coap_client_req(&client, 0, &address, &req1, NULL)); + + /* should not match */ + coap_client_cancel_request(&client, &(struct coap_client_request) { + .path = test_path, + .user_data = &sem2, + }); + zassert_not_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_not_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + + /* match both (all GET queries) */ + coap_client_cancel_request(&client, &(struct coap_client_request) { + .method = COAP_METHOD_GET, + }); + zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + + zassert_ok(coap_client_req(&client, 0, &address, &req1, NULL)); + zassert_ok(coap_client_req(&client, 0, &address, &req2, NULL)); + + /* match both (wildcard)*/ + coap_client_cancel_request(&client, &(struct coap_client_request) {0}); + zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + +} From 764f5344f3660ce53eddca31e97eae08f7e1ce41 Mon Sep 17 00:00:00 2001 From: Andi Gerl Date: Fri, 8 Nov 2024 15:23:28 -0500 Subject: [PATCH 23/29] [nrf fromtree] net: lwm2m: add set_socketoptions cb to pull context LwM2M context The pull context LwM2M client's set_socketoptions callback is currently unused and can't be set by a user. Add a public API to set the pull context's client's set_socketoptions callback. Signed-off-by: Andi Gerl (cherry picked from commit 9c2421444ba51c812eaa579e8f7a859e73b78bfa) --- include/zephyr/net/lwm2m.h | 14 +++++++++++++- subsys/net/lib/lwm2m/lwm2m_pull_context.c | 6 +++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/include/zephyr/net/lwm2m.h b/include/zephyr/net/lwm2m.h index 8a2cbf557d0..c080018e657 100644 --- a/include/zephyr/net/lwm2m.h +++ b/include/zephyr/net/lwm2m.h @@ -182,6 +182,7 @@ enum lwm2m_rd_client_event { typedef void (*lwm2m_ctx_event_cb_t)(struct lwm2m_ctx *ctx, enum lwm2m_rd_client_event event); +typedef int (*lwm2m_set_sockopt_cb_t)(struct lwm2m_ctx *client_ctx); /** * @brief Different traffic states of the LwM2M socket. @@ -258,7 +259,7 @@ struct lwm2m_ctx { * a callback that is called after a socket is created and before * connect. */ - int (*set_socketoptions)(struct lwm2m_ctx *client_ctx); + lwm2m_set_sockopt_cb_t set_socketoptions; /** Flag to indicate if context should use DTLS. * Enabled via the use of coaps:// protocol prefix in connection @@ -1621,6 +1622,17 @@ int lwm2m_security_mode(struct lwm2m_ctx *ctx); */ int lwm2m_set_default_sockopt(struct lwm2m_ctx *ctx); +/** + * @brief Set the @ref lwm2m_ctx::set_socketoptions callback for the pull context's client. + * + * This allows setting specific socket options on the socket that is used to pull + * firmware updates. The callback will be called after the pull context socket has been + * created and before it will connect. + * + * @param[in] set_sockopt_cb A callback function to set sockopts for the pull context client. + */ +void lwm2m_pull_context_set_sockopt_callback(lwm2m_set_sockopt_cb_t set_sockopt_cb); + #ifdef __cplusplus } #endif diff --git a/subsys/net/lib/lwm2m/lwm2m_pull_context.c b/subsys/net/lib/lwm2m/lwm2m_pull_context.c index 356f19705f5..8081c3793ed 100644 --- a/subsys/net/lib/lwm2m/lwm2m_pull_context.c +++ b/subsys/net/lib/lwm2m/lwm2m_pull_context.c @@ -446,7 +446,6 @@ int lwm2m_pull_context_start_transfer(char *uri, struct requesting_object req, k context.result_cb = req.result_cb; context.write_cb = req.write_cb; - (void)memset(&context.firmware_ctx, 0, sizeof(struct lwm2m_ctx)); (void)memset(&context.block_ctx, 0, sizeof(struct coap_block_context)); context.firmware_ctx.sock_fd = -1; @@ -454,3 +453,8 @@ int lwm2m_pull_context_start_transfer(char *uri, struct requesting_object req, k return 0; } + +void lwm2m_pull_context_set_sockopt_callback(lwm2m_set_sockopt_cb_t set_sockopt_cb) +{ + context.firmware_ctx.set_socketoptions = set_sockopt_cb; +} From 53e0353d3c18e4e7a8f9d00a42dbca74f19fd2f1 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 18 Nov 2024 15:28:38 +0200 Subject: [PATCH 24/29] [nrf fromtree] tests: coap_client: Proper slow-down Use real-time scheduler with 100x speedup, so timeouts are accurate enough, but still fast for tests to run. Signed-off-by: Seppo Takalo (cherry picked from commit 34a6d5a5dc834f26f06fb318b9c9a21c286a7227) --- tests/net/lib/coap_client/CMakeLists.txt | 2 +- tests/net/lib/coap_client/boards/native_sim.conf | 1 + tests/net/lib/coap_client/src/main.c | 15 ++++++++++++--- tests/net/lib/coap_client/src/stubs.c | 5 ++++- 4 files changed, 18 insertions(+), 5 deletions(-) create mode 100644 tests/net/lib/coap_client/boards/native_sim.conf diff --git a/tests/net/lib/coap_client/CMakeLists.txt b/tests/net/lib/coap_client/CMakeLists.txt index 07a4d8a9827..74d11c90fbf 100644 --- a/tests/net/lib/coap_client/CMakeLists.txt +++ b/tests/net/lib/coap_client/CMakeLists.txt @@ -26,7 +26,7 @@ add_compile_definitions(CONFIG_COAP_CLIENT_MESSAGE_HEADER_SIZE=48) add_compile_definitions(CONFIG_COAP_CLIENT_STACK_SIZE=1024) add_compile_definitions(CONFIG_COAP_CLIENT_THREAD_PRIORITY=10) add_compile_definitions(CONFIG_COAP_LOG_LEVEL=4) -add_compile_definitions(CONFIG_COAP_INIT_ACK_TIMEOUT_MS=100) +add_compile_definitions(CONFIG_COAP_INIT_ACK_TIMEOUT_MS=1000) add_compile_definitions(CONFIG_COAP_CLIENT_MAX_REQUESTS=2) add_compile_definitions(CONFIG_COAP_CLIENT_MAX_INSTANCES=2) add_compile_definitions(CONFIG_COAP_MAX_RETRANSMIT=4) diff --git a/tests/net/lib/coap_client/boards/native_sim.conf b/tests/net/lib/coap_client/boards/native_sim.conf new file mode 100644 index 00000000000..0843e94acbd --- /dev/null +++ b/tests/net/lib/coap_client/boards/native_sim.conf @@ -0,0 +1 @@ +CONFIG_NATIVE_SIM_SLOWDOWN_TO_REAL_TIME=y diff --git a/tests/net/lib/coap_client/src/main.c b/tests/net/lib/coap_client/src/main.c index 2f3fa28ebff..72b84a60f5e 100644 --- a/tests/net/lib/coap_client/src/main.c +++ b/tests/net/lib/coap_client/src/main.c @@ -8,7 +8,9 @@ #include #include #include - +#if defined(CONFIG_NATIVE_SIM_SLOWDOWN_TO_REAL_TIME) +#include "timer_model.h" +#endif #include "stubs.h" LOG_MODULE_REGISTER(coap_client_test, LOG_LEVEL_DBG); @@ -16,12 +18,12 @@ LOG_MODULE_REGISTER(coap_client_test, LOG_LEVEL_DBG); DEFINE_FFF_GLOBALS; #define FFF_FAKES_LIST(FAKE) -#define LONG_ACK_TIMEOUT_MS 200 +#define LONG_ACK_TIMEOUT_MS (2 * CONFIG_COAP_INIT_ACK_TIMEOUT_MS) #define MORE_THAN_EXCHANGE_LIFETIME_MS 4 * CONFIG_COAP_INIT_ACK_TIMEOUT_MS #define MORE_THAN_LONG_EXCHANGE_LIFETIME_MS 4 * LONG_ACK_TIMEOUT_MS #define MORE_THAN_ACK_TIMEOUT_MS \ (CONFIG_COAP_INIT_ACK_TIMEOUT_MS + CONFIG_COAP_INIT_ACK_TIMEOUT_MS / 2) -#define COAP_SEPARATE_TIMEOUT (6000 * 3) /* Needs a safety marging, tests run faster than -rt */ +#define COAP_SEPARATE_TIMEOUT (6000 * 2) /* Needs a safety marging, tests run faster than -rt */ #define VALID_MESSAGE_ID BIT(31) #define TOKEN_OFFSET 4 @@ -425,6 +427,13 @@ extern void net_coap_init(void); static void *suite_setup(void) { +#if defined(CONFIG_NATIVE_SIM_SLOWDOWN_TO_REAL_TIME) + /* It is enough that some slow-down is happening on sleeps, it does not have to be + * real time + */ + hwtimer_set_rt_ratio(100.0); + k_sleep(K_MSEC(1)); +#endif net_coap_init(); zassert_ok(coap_client_init(&client, NULL)); zassert_ok(coap_client_init(&client2, NULL)); diff --git a/tests/net/lib/coap_client/src/stubs.c b/tests/net/lib/coap_client/src/stubs.c index f3451df054f..b3dbfc6df75 100644 --- a/tests/net/lib/coap_client/src/stubs.c +++ b/tests/net/lib/coap_client/src/stubs.c @@ -7,7 +7,7 @@ #include #include -LOG_MODULE_DECLARE(coap_client_test); +LOG_MODULE_DECLARE(coap_client_test, LOG_LEVEL_DBG); DEFINE_FAKE_VALUE_FUNC(uint32_t, z_impl_sys_rand32_get); DEFINE_FAKE_VALUE_FUNC(ssize_t, z_impl_zsock_recvfrom, int, void *, size_t, int, struct sockaddr *, @@ -45,6 +45,9 @@ int z_impl_zsock_poll(struct zsock_pollfd *fds, int nfds, int poll_timeout) events++; } } + if (events == 0) { + k_sleep(K_MSEC(poll_timeout)); + } return events; } From 7f33e227c2b931740a6fb491935d299aa904b61b Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 18 Nov 2024 15:36:16 +0200 Subject: [PATCH 25/29] [nrf fromtree] tests: coap_client: Refactor tests Refactor tests to be a bit shorter, so its easier to read and copy-paste for a new testcase All idioms like "ret = somecall(); zasser.." are replaced with just "zassert_ok(some_call());" Commonly used structures are global and initialized once. To avoid cross-test side-effects, suite_after-function is added to cleanup all requests. Signed-off-by: Seppo Takalo (cherry picked from commit d64748cc5247645dbbc469d9f1ecf3f80dca561d) --- tests/net/lib/coap_client/src/main.c | 594 ++++++--------------------- 1 file changed, 119 insertions(+), 475 deletions(-) diff --git a/tests/net/lib/coap_client/src/main.c b/tests/net/lib/coap_client/src/main.c index 72b84a60f5e..a620b1985e0 100644 --- a/tests/net/lib/coap_client/src/main.c +++ b/tests/net/lib/coap_client/src/main.c @@ -27,20 +27,48 @@ DEFINE_FFF_GLOBALS; #define VALID_MESSAGE_ID BIT(31) #define TOKEN_OFFSET 4 +void coap_callback(int16_t code, size_t offset, const uint8_t *payload, size_t len, bool last_block, + void *user_data); + static int16_t last_response_code; -static const char *test_path = "test"; +static const char test_path[] = "test"; static uint32_t messages_needing_response[2]; static uint8_t last_token[2][COAP_TOKEN_MAX_LEN]; static const uint8_t empty_token[COAP_TOKEN_MAX_LEN] = {0}; +K_SEM_DEFINE(sem1, 0, 1); +K_SEM_DEFINE(sem2, 0, 1); static struct coap_client client; static struct coap_client client2 = { .fd = 1, }; -static char *short_payload = "testing"; -static char *long_payload = LOREM_IPSUM_SHORT; +static const char short_payload[] = "testing"; +static const char long_payload[] = LOREM_IPSUM_SHORT; +static struct coap_client_request short_request = { + .method = COAP_METHOD_GET, + .confirmable = true, + .path = test_path, + .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, + .cb = coap_callback, + .payload = short_payload, + .len = sizeof(short_payload) - 1, + .user_data = &sem1, +}; +static struct coap_client_request long_request = { + .method = COAP_METHOD_GET, + .confirmable = true, + .path = test_path, + .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, + .cb = coap_callback, + .payload = long_payload, + .len = sizeof(long_payload) - 1, + .user_data = &sem2, +}; +static struct sockaddr dst_address; + + static uint16_t get_next_pending_message_id(void) { @@ -423,6 +451,16 @@ static ssize_t z_impl_zsock_recvfrom_custom_fake_observe(int sock, void *buf, si return ret; } +void coap_callback(int16_t code, size_t offset, const uint8_t *payload, size_t len, bool last_block, + void *user_data) +{ + LOG_INF("CoAP response callback, %d", code); + last_response_code = code; + if (user_data) { + k_sem_give((struct k_sem *) user_data); + } +} + extern void net_coap_init(void); static void *suite_setup(void) @@ -463,90 +501,38 @@ static void test_setup(void *data) memset(&client.requests, 0, sizeof(client.requests)); memset(last_token, 0, sizeof(last_token)); - last_response_code = 0; + k_sem_reset(&sem1); + k_sem_reset(&sem2); k_mutex_unlock(&client.lock); } -void coap_callback(int16_t code, size_t offset, const uint8_t *payload, size_t len, bool last_block, - void *user_data) +static void test_after(void *data) { - LOG_INF("CoAP response callback, %d", code); - last_response_code = code; - if (user_data) { - k_sem_give((struct k_sem *) user_data); - } + coap_client_cancel_requests(&client); + coap_client_cancel_requests(&client2); } -ZTEST_SUITE(coap_client, NULL, suite_setup, test_setup, NULL, NULL); +ZTEST_SUITE(coap_client, NULL, suite_setup, test_setup, test_after, NULL); ZTEST(coap_client, test_get_request) { - int ret = 0; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = NULL, - .len = 0 - }; - - client_request.payload = short_payload; - client_request.len = strlen(short_payload); - - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &short_request, NULL)); k_sleep(K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); - LOG_INF("Test done"); } ZTEST(coap_client, test_request_block) { - int ret = 0; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = short_payload, - .len = strlen(short_payload), - }; - z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_block; - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - zassert_equal(ret, -EAGAIN, ""); + zassert_equal(coap_client_req(&client, 0, &dst_address, &short_request, NULL), -EAGAIN, ""); } - ZTEST(coap_client, test_resend_request) { - int ret = 0; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = short_payload, - .len = strlen(short_payload), - }; - int (*sendto_fakes[])(int, void *, size_t, int, const struct sockaddr *, socklen_t) = { z_impl_zsock_sendto_custom_fake_no_reply, z_impl_zsock_sendto_custom_fake_block, @@ -556,86 +542,43 @@ ZTEST(coap_client, test_resend_request) SET_CUSTOM_FAKE_SEQ(z_impl_zsock_sendto, sendto_fakes, ARRAY_SIZE(sendto_fakes)); set_socket_events(client.fd, ZSOCK_POLLOUT); - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &short_request, NULL)); k_sleep(K_MSEC(MORE_THAN_ACK_TIMEOUT_MS)); k_sleep(K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); zassert_equal(z_impl_zsock_sendto_fake.call_count, 3); - LOG_INF("Test done"); } ZTEST(coap_client, test_echo_option) { - int ret = 0; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = NULL, - .len = 0 - }; - - client_request.payload = short_payload; - client_request.len = strlen(short_payload); - z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_echo; - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &short_request, NULL)); k_sleep(K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); - LOG_INF("Test done"); } ZTEST(coap_client, test_echo_option_next_req) { - int ret = 0; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = NULL, - .len = 0 - }; - - client_request.payload = short_payload; - client_request.len = strlen(short_payload); + struct coap_client_request req = short_request; z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_echo_next_req; - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &short_request, NULL)); k_sleep(K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); char *payload = "echo testing"; - client_request.method = COAP_METHOD_POST; - client_request.payload = payload; - client_request.len = strlen(payload); + req.method = COAP_METHOD_POST; + req.payload = payload; + req.len = strlen(payload); LOG_INF("Send next request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &req, NULL)); k_sleep(K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); @@ -643,51 +586,15 @@ ZTEST(coap_client, test_echo_option_next_req) ZTEST(coap_client, test_get_no_path) { - int ret = 0; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = NULL, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = NULL, - .len = 0 - }; + struct coap_client_request req = short_request; - client_request.payload = short_payload; - client_request.len = strlen(short_payload); - - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - - zassert_equal(ret, -EINVAL, "Get request without path"); + req.path = NULL; + zassert_equal(coap_client_req(&client, 0, &dst_address, &req, NULL), -EINVAL, ""); } ZTEST(coap_client, test_send_large_data) { - int ret = 0; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = NULL, - .len = 0 - }; - - client_request.payload = long_payload; - client_request.len = strlen(long_payload); - - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &long_request, NULL)); k_sleep(K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); @@ -695,35 +602,16 @@ ZTEST(coap_client, test_send_large_data) ZTEST(coap_client, test_no_response) { - int ret = 0; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = NULL, - .len = 0 - }; struct coap_transmission_parameters params = { .ack_timeout = LONG_ACK_TIMEOUT_MS, .coap_backoff_percent = 200, .max_retransmission = 0 }; - client_request.payload = short_payload; - client_request.len = strlen(short_payload); - z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; set_socket_events(client.fd, ZSOCK_POLLOUT); - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, ¶ms); - - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &short_request, ¶ms)); k_sleep(K_MSEC(MORE_THAN_LONG_EXCHANGE_LIFETIME_MS)); zassert_equal(last_response_code, -ETIMEDOUT, "Unexpected response"); @@ -731,28 +619,9 @@ ZTEST(coap_client, test_no_response) ZTEST(coap_client, test_separate_response) { - int ret = 0; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = NULL, - .len = 0 - }; - - client_request.payload = short_payload; - client_request.len = strlen(short_payload); - z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_empty_ack; - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &short_request, NULL)); k_sleep(K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); @@ -760,50 +629,24 @@ ZTEST(coap_client, test_separate_response) ZTEST(coap_client, test_separate_response_lost) { - int ret = 0; - struct k_sem sem; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = short_payload, - .len = strlen(short_payload), - .user_data = &sem, - }; + struct coap_client_request req = short_request; + + req.user_data = &sem1; - zassert_ok(k_sem_init(&sem, 0, 1)); z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_only_ack; set_socket_events(client.fd, ZSOCK_POLLOUT); - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &req, NULL)); - zassert_ok(k_sem_take(&sem, K_MSEC(COAP_SEPARATE_TIMEOUT))); + zassert_ok(k_sem_take(&sem1, K_MSEC(COAP_SEPARATE_TIMEOUT))); zassert_equal(last_response_code, -ETIMEDOUT, ""); } ZTEST(coap_client, test_separate_response_ack_fail) { - int ret = 0; - struct k_sem sem; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = short_payload, - .len = strlen(short_payload), - .user_data = &sem, - }; - zassert_ok(k_sem_init(&sem, 0, 1)); + struct coap_client_request req = short_request; + + req.user_data = &sem1; int (*sendto_fakes[])(int, void *, size_t, int, const struct sockaddr *, socklen_t) = { z_impl_zsock_sendto_custom_fake, @@ -813,95 +656,47 @@ ZTEST(coap_client, test_separate_response_ack_fail) SET_CUSTOM_FAKE_SEQ(z_impl_zsock_sendto, sendto_fakes, ARRAY_SIZE(sendto_fakes)); z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_empty_ack; - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &req, NULL)); - zassert_ok(k_sem_take(&sem, K_MSEC(COAP_SEPARATE_TIMEOUT))); + zassert_ok(k_sem_take(&sem1, K_MSEC(COAP_SEPARATE_TIMEOUT))); zassert_equal(last_response_code, -ENETDOWN, ""); } ZTEST(coap_client, test_multiple_requests) { - int ret = 0; - int retry = MORE_THAN_EXCHANGE_LIFETIME_MS; - struct k_sem sem1, sem2; - - struct sockaddr address = {0}; - struct coap_client_request req1 = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = short_payload, - .len = strlen(short_payload), - .user_data = &sem1 - }; - struct coap_client_request req2 = req1; + struct coap_client_request req1 = short_request; + struct coap_client_request req2 = short_request; + req1.user_data = &sem1; req2.user_data = &sem2; - k_sem_init(&sem1, 0, 1); - k_sem_init(&sem2, 0, 1); - z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &req1, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); - - ret = coap_client_req(&client, 0, &address, &req2, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &req1, NULL)); + zassert_ok(coap_client_req(&client, 0, &dst_address, &req2, NULL)); set_socket_events(client.fd, ZSOCK_POLLIN); - while (last_response_code == 0 && retry > 0) { - retry--; - k_sleep(K_MSEC(1)); - } + zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); last_response_code = 0; set_socket_events(client.fd, ZSOCK_POLLIN); - zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); zassert_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); } ZTEST(coap_client, test_unmatching_tokens) { - int ret = 0; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = NULL, - .len = 0 - }; struct coap_transmission_parameters params = { .ack_timeout = LONG_ACK_TIMEOUT_MS, .coap_backoff_percent = 200, .max_retransmission = 0 }; - client_request.payload = short_payload; - client_request.len = strlen(short_payload); - z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_unmatching; set_socket_events(client.fd, ZSOCK_POLLIN | ZSOCK_POLLOUT); - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, ¶ms); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &short_request, ¶ms)); k_sleep(K_MSEC(MORE_THAN_LONG_EXCHANGE_LIFETIME_MS)); zassert_equal(last_response_code, -ETIMEDOUT, "Unexpected response"); @@ -909,45 +704,14 @@ ZTEST(coap_client, test_unmatching_tokens) ZTEST(coap_client, test_multiple_clients) { - int ret; - int retry = MORE_THAN_EXCHANGE_LIFETIME_MS; - struct k_sem sem1, sem2; - struct sockaddr address = {0}; - struct coap_client_request req1 = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = short_payload, - .len = strlen(short_payload), - .user_data = &sem1 - }; - struct coap_client_request req2 = req1; + struct coap_client_request req1 = short_request; + struct coap_client_request req2 = long_request; + req1.user_data = &sem1; req2.user_data = &sem2; - req2.payload = long_payload; - req2.len = strlen(long_payload); - - zassert_ok(k_sem_init(&sem1, 0, 1)); - zassert_ok(k_sem_init(&sem2, 0, 1)); - - k_sleep(K_MSEC(1)); - LOG_INF("Sending requests"); - ret = coap_client_req(&client, client.fd, &address, &req1, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); - - ret = coap_client_req(&client2, client2.fd, &address, &req2, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); - - while (last_response_code == 0 && retry > 0) { - retry--; - k_sleep(K_MSEC(1)); - } - set_socket_events(client2.fd, ZSOCK_POLLIN); - - k_sleep(K_SECONDS(1)); + zassert_ok(coap_client_req(&client, client.fd, &dst_address, &req1, NULL)); + zassert_ok(coap_client_req(&client2, client2.fd, &dst_address, &req2, NULL)); /* ensure we got both responses */ zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); @@ -958,26 +722,10 @@ ZTEST(coap_client, test_multiple_clients) ZTEST(coap_client, test_poll_err) { - int ret = 0; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = short_payload, - .len = strlen(short_payload), - }; - z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; set_socket_events(client.fd, ZSOCK_POLLERR); - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &short_request, NULL)); k_sleep(K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)); zassert_equal(last_response_code, -EIO, "Unexpected response"); @@ -985,29 +733,10 @@ ZTEST(coap_client, test_poll_err) ZTEST(coap_client, test_poll_err_after_response) { - int ret = 0; - struct k_sem sem1; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = short_payload, - .len = strlen(short_payload), - .user_data = &sem1 - }; - - zassert_ok(k_sem_init(&sem1, 0, 1)); z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; set_socket_events(client.fd, ZSOCK_POLLIN); - k_sleep(K_MSEC(1)); - - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &short_request, NULL)); zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); @@ -1018,35 +747,17 @@ ZTEST(coap_client, test_poll_err_after_response) ZTEST(coap_client, test_poll_err_on_another_sock) { - int ret = 0; - struct k_sem sem1, sem2; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = short_payload, - .len = strlen(short_payload), - .user_data = &sem1 - }; - struct coap_client_request request2 = client_request; - - request2.user_data = &sem2; + struct coap_client_request req1 = short_request; + struct coap_client_request req2 = short_request; - zassert_ok(k_sem_init(&sem1, 0, 1)); - zassert_ok(k_sem_init(&sem2, 0, 1)); + req1.user_data = &sem1; + req2.user_data = &sem2; z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; set_socket_events(client.fd, ZSOCK_POLLERR); - k_sleep(K_MSEC(1)); - - ret = coap_client_req(&client2, client2.fd, &address, &request2, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); - ret = coap_client_req(&client, client.fd, &address, &client_request, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client2, client2.fd, &dst_address, &req2, NULL)); + zassert_ok(coap_client_req(&client, client.fd, &dst_address, &req1, NULL)); set_socket_events(client2.fd, ZSOCK_POLLIN); @@ -1058,45 +769,25 @@ ZTEST(coap_client, test_poll_err_on_another_sock) ZTEST(coap_client, test_duplicate_response) { - int ret = 0; - struct k_sem sem; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = short_payload, - .len = strlen(short_payload), - .user_data = &sem, - }; - - zassert_ok(k_sem_init(&sem, 0, 2)); z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_duplicate_response; - k_sleep(K_MSEC(1)); - LOG_INF("Send request"); - ret = coap_client_req(&client, 0, &address, &client_request, NULL); - zassert_true(ret >= 0, "Sending request failed, %d", ret); + zassert_ok(coap_client_req(&client, 0, &dst_address, &short_request, NULL)); - zassert_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); zassert_equal(last_response_code, COAP_RESPONSE_CODE_OK, "Unexpected response"); - zassert_equal(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)), -EAGAIN, ""); + zassert_equal(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS)), -EAGAIN, ""); } ZTEST(coap_client, test_observe) { - struct k_sem sem; - struct sockaddr address = {0}; struct coap_client_option options = { .code = COAP_OPTION_OBSERVE, .value[0] = 0, .len = 1, }; - struct coap_client_request client_request = { + struct coap_client_request req = { .method = COAP_METHOD_GET, .confirmable = true, .path = test_path, @@ -1106,79 +797,47 @@ ZTEST(coap_client, test_observe) .len = strlen(short_payload), .options = &options, .num_options = 1, - .user_data = &sem, + .user_data = &sem1, }; - zassert_ok(k_sem_init(&sem, 0, 1)); z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_observe; - k_sleep(K_MSEC(1)); - zassert_ok(coap_client_req(&client, 0, &address, &client_request, NULL)); + zassert_ok(coap_client_req(&client, 0, &dst_address, &req, NULL)); - zassert_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); - zassert_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); - zassert_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); coap_client_cancel_requests(&client); - zassert_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); zassert_equal(last_response_code, -ECANCELED, ""); - zassert_not_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_not_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); } ZTEST(coap_client, test_request_rst) { - struct k_sem sem; - struct sockaddr address = {0}; - struct coap_client_request client_request = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = short_payload, - .len = strlen(short_payload), - .user_data = &sem, - }; - - zassert_ok(k_sem_init(&sem, 0, 1)); - k_sleep(K_MSEC(1)); z_impl_zsock_recvfrom_fake.custom_fake = z_impl_zsock_recvfrom_custom_fake_rst; - zassert_ok(coap_client_req(&client, 0, &address, &client_request, NULL)); + zassert_ok(coap_client_req(&client, 0, &dst_address, &short_request, NULL)); - zassert_ok(k_sem_take(&sem, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); + zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); zassert_equal(last_response_code, -ECONNRESET, ""); } ZTEST(coap_client, test_cancel) { - struct k_sem sem1, sem2; - struct sockaddr address = {0}; - struct coap_client_request req1 = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = short_payload, - .len = strlen(short_payload), - .user_data = &sem1 - }; - struct coap_client_request req2 = req1; + struct coap_client_request req1 = short_request; + struct coap_client_request req2 = short_request; + req1.user_data = &sem1; req2.user_data = &sem2; - k_sem_init(&sem1, 0, 1); - k_sem_init(&sem2, 0, 1); - z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; - k_sleep(K_MSEC(1)); - - zassert_ok(coap_client_req(&client, 0, &address, &req1, NULL)); - zassert_ok(coap_client_req(&client, 0, &address, &req2, NULL)); + zassert_ok(coap_client_req(&client, 0, &dst_address, &req1, NULL)); + zassert_ok(coap_client_req(&client, 0, &dst_address, &req2, NULL)); k_sleep(K_SECONDS(1)); @@ -1196,32 +855,17 @@ ZTEST(coap_client, test_cancel) ZTEST(coap_client, test_cancel_match) { - struct k_sem sem1, sem2; - struct sockaddr address = {0}; - struct coap_client_request req1 = { - .method = COAP_METHOD_GET, - .confirmable = true, - .path = test_path, - .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, - .cb = coap_callback, - .payload = short_payload, - .len = strlen(short_payload), - .user_data = &sem1 - }; - struct coap_client_request req2 = req1; + struct coap_client_request req1 = short_request; + struct coap_client_request req2 = short_request; + req1.user_data = &sem1; req2.user_data = &sem2; req2.path = "another"; - k_sem_init(&sem1, 0, 1); - k_sem_init(&sem2, 0, 1); - z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; - k_sleep(K_MSEC(1)); - - zassert_ok(coap_client_req(&client, 0, &address, &req1, NULL)); - zassert_ok(coap_client_req(&client, 0, &address, &req2, NULL)); + zassert_ok(coap_client_req(&client, 0, &dst_address, &req1, NULL)); + zassert_ok(coap_client_req(&client, 0, &dst_address, &req2, NULL)); k_sleep(K_SECONDS(1)); @@ -1233,7 +877,7 @@ ZTEST(coap_client, test_cancel_match) zassert_not_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); zassert_equal(last_response_code, -ECANCELED, ""); - zassert_ok(coap_client_req(&client, 0, &address, &req1, NULL)); + zassert_ok(coap_client_req(&client, 0, &dst_address, &req1, NULL)); /* should not match */ coap_client_cancel_request(&client, &(struct coap_client_request) { @@ -1250,8 +894,8 @@ ZTEST(coap_client, test_cancel_match) zassert_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); zassert_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); - zassert_ok(coap_client_req(&client, 0, &address, &req1, NULL)); - zassert_ok(coap_client_req(&client, 0, &address, &req2, NULL)); + zassert_ok(coap_client_req(&client, 0, &dst_address, &req1, NULL)); + zassert_ok(coap_client_req(&client, 0, &dst_address, &req2, NULL)); /* match both (wildcard)*/ coap_client_cancel_request(&client, &(struct coap_client_request) {0}); From 312b2c9d3e271d1150c3fe8eac64484e4afae043 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 18 Nov 2024 15:37:48 +0200 Subject: [PATCH 26/29] [nrf fromtree] tests: coap_client: Add test for non-confirmable request Add test for sending multiple non-confirmable requests. Signed-off-by: Seppo Takalo (cherry picked from commit 23345d203e677cada602707635752f20e73e19a7) --- tests/net/lib/coap_client/src/main.c | 32 ++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/net/lib/coap_client/src/main.c b/tests/net/lib/coap_client/src/main.c index a620b1985e0..02d7693a7a0 100644 --- a/tests/net/lib/coap_client/src/main.c +++ b/tests/net/lib/coap_client/src/main.c @@ -903,3 +903,35 @@ ZTEST(coap_client, test_cancel_match) zassert_ok(k_sem_take(&sem2, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); } + +ZTEST(coap_client, test_non_confirmable) +{ + struct coap_client_request req = { + .method = COAP_METHOD_GET, + .confirmable = false, + .path = test_path, + .fmt = COAP_CONTENT_FORMAT_TEXT_PLAIN, + .cb = coap_callback, + .payload = short_payload, + .len = strlen(short_payload), + .user_data = &sem1 + }; + + z_impl_zsock_sendto_fake.custom_fake = z_impl_zsock_sendto_custom_fake_no_reply; + set_socket_events(client.fd, ZSOCK_POLLOUT); + + for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) { + zassert_ok(coap_client_req(&client, 0, &dst_address, &req, NULL)); + } + zassert_equal(coap_client_req(&client, 0, &dst_address, &req, NULL), -EAGAIN, ""); + + k_sleep(K_MSEC(MORE_THAN_LONG_EXCHANGE_LIFETIME_MS)); + + for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) { + zassert_ok(coap_client_req(&client, 0, &dst_address, &req, NULL)); + } + zassert_equal(coap_client_req(&client, 0, &dst_address, &req, NULL), -EAGAIN, ""); + + /* No callbacks from non-confirmable */ + zassert_not_ok(k_sem_take(&sem1, K_MSEC(MORE_THAN_EXCHANGE_LIFETIME_MS))); +} From 83b90e74f043d24825071c7534275b03b195bbc0 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 18 Nov 2024 15:43:03 +0200 Subject: [PATCH 27/29] [nrf fromtree] net: lib: coap_client: Fix timeout for separate response When waiting for response after receiving the empty Ack, client actually used way too timeout. CoAP timeout only holds the timeout value in ms. t0 is the starting time. Signed-off-by: Seppo Takalo (cherry picked from commit 6c169668e9230740a780c688704c023ec43ccc98) --- subsys/net/lib/coap/coap_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subsys/net/lib/coap/coap_client.c b/subsys/net/lib/coap/coap_client.c index 468505f5eb5..30dcbb42005 100644 --- a/subsys/net/lib/coap/coap_client.c +++ b/subsys/net/lib/coap/coap_client.c @@ -793,7 +793,7 @@ static int handle_response(struct coap_client *client, const struct coap_packet return 0; } internal_req->pending.t0 = k_uptime_get(); - internal_req->pending.timeout = internal_req->pending.t0 + COAP_SEPARATE_TIMEOUT; + internal_req->pending.timeout = COAP_SEPARATE_TIMEOUT; internal_req->pending.retries = 0; return 1; } From f6004fc43165439050478ae17c336b1f11cbd0b1 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 18 Nov 2024 15:47:01 +0200 Subject: [PATCH 28/29] [nrf fromtree] net: lib: coap_client: Release non-confirmable requests Non-confirmable CoAP requests need lifetime tracking as well so we can free the structure after a timeout. Signed-off-by: Seppo Takalo (cherry picked from commit 2066cf6a3d17fc1a1733fab3e47fe8969145df6b) --- subsys/net/lib/coap/coap_client.c | 53 ++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/subsys/net/lib/coap/coap_client.c b/subsys/net/lib/coap/coap_client.c index 30dcbb42005..a3c56281819 100644 --- a/subsys/net/lib/coap/coap_client.c +++ b/subsys/net/lib/coap/coap_client.c @@ -105,6 +105,10 @@ static bool exchange_lifetime_exceeded(struct coap_client_internal_request *inte return true; } + if (internal_req->pending.t0 == 0) { + return true; + } + time_since_t0 = k_uptime_get() - internal_req->pending.t0; exchange_lifetime = (internal_req->pending.params.ack_timeout * COAP_EXCHANGE_LIFETIME_FACTOR); @@ -364,7 +368,7 @@ int coap_client_req(struct coap_client *client, int sock, const struct sockaddr /* Don't allow changing to a different socket if there is already request ongoing. */ if (client->fd != sock && has_ongoing_request(client)) { ret = -EALREADY; - goto out; + goto release; } /* Don't allow changing to a different address if there is already request ongoing. */ @@ -373,7 +377,7 @@ int coap_client_req(struct coap_client *client, int sock, const struct sockaddr if (has_ongoing_request(client)) { LOG_WRN("Can't change to a different socket, request ongoing."); ret = -EALREADY; - goto out; + goto release; } memcpy(&client->address, addr, sizeof(*addr)); @@ -384,7 +388,7 @@ int coap_client_req(struct coap_client *client, int sock, const struct sockaddr if (has_ongoing_request(client)) { LOG_WRN("Can't change to a different socket, request ongoing."); ret = -EALREADY; - goto out; + goto release; } memset(&client->address, 0, sizeof(client->address)); @@ -397,7 +401,7 @@ int coap_client_req(struct coap_client *client, int sock, const struct sockaddr ret = coap_client_init_request(client, req, internal_req, false); if (ret < 0) { LOG_ERR("Failed to initialize coap request"); - goto out; + goto release; } if (client->send_echo) { @@ -405,7 +409,7 @@ int coap_client_req(struct coap_client *client, int sock, const struct sockaddr client->echo_option.value, client->echo_option.len); if (ret < 0) { LOG_ERR("Failed to append echo option"); - goto out; + goto release; } client->send_echo = false; } @@ -413,28 +417,36 @@ int coap_client_req(struct coap_client *client, int sock, const struct sockaddr ret = coap_client_schedule_poll(client, sock, req, internal_req); if (ret < 0) { LOG_ERR("Failed to schedule polling"); - goto out; + goto release; } - /* only TYPE_CON messages need pending tracking */ - if (coap_header_get_type(&internal_req->request) == COAP_TYPE_CON) { - ret = coap_pending_init(&internal_req->pending, &internal_req->request, - &client->address, params); + ret = coap_pending_init(&internal_req->pending, &internal_req->request, + &client->address, params); - if (ret < 0) { - LOG_ERR("Failed to initialize pending struct"); - goto out; - } + if (ret < 0) { + LOG_ERR("Failed to initialize pending struct"); + goto release; + } - coap_pending_cycle(&internal_req->pending); - internal_req->is_observe = coap_request_is_observe(&internal_req->request); - LOG_DBG("Request is_observe %d", internal_req->is_observe); + /* Non-Confirmable messages are not retried, but we still track the lifetime as + * replies are acceptable. + */ + if (coap_header_get_type(&internal_req->request) == COAP_TYPE_NON_CON) { + internal_req->pending.retries = 0; } + coap_pending_cycle(&internal_req->pending); + internal_req->is_observe = coap_request_is_observe(&internal_req->request); + LOG_DBG("Request is_observe %d", internal_req->is_observe); ret = send_request(sock, internal_req->request.data, internal_req->request.offset, 0, &client->address, client->socklen); if (ret < 0) { - LOG_ERR("Transmission failed: %d", errno); + ret = -errno; + } + +release: + if (ret < 0) { + LOG_ERR("Failed to send request: %d", ret); reset_internal_request(internal_req); } else { /* Do not return the number of bytes sent */ @@ -521,6 +533,11 @@ static void coap_client_resend_handler(struct coap_client *client) for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) { if (timeout_expired(&client->requests[i])) { + if (!client->requests[i].coap_request.confirmable) { + release_internal_request(&client->requests[i]); + continue; + } + ret = resend_request(client, &client->requests[i]); if (ret < 0) { report_callback_error(&client->requests[i], ret); From 4148a010038b70d0a19abfe19848f62084d413ce Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Mon, 18 Nov 2024 15:53:21 +0200 Subject: [PATCH 29/29] [nrf fromtree] net: lib: coap_client: Const pointers in request CoAP client does not modify any of the members, so change all pointers to const. Signed-off-by: Seppo Takalo (cherry picked from commit bc4f026ea946c5bf587edd91f043672fad617cdc) --- include/zephyr/net/coap_client.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/include/zephyr/net/coap_client.h b/include/zephyr/net/coap_client.h index bbeebb2d26c..ce09c56dd2e 100644 --- a/include/zephyr/net/coap_client.h +++ b/include/zephyr/net/coap_client.h @@ -53,16 +53,16 @@ typedef void (*coap_client_response_cb_t)(int16_t result_code, * @brief Representation of a CoAP client request. */ struct coap_client_request { - enum coap_method method; /**< Method of the request */ - bool confirmable; /**< CoAP Confirmable/Non-confirmable message */ - const char *path; /**< Path of the requested resource */ - enum coap_content_format fmt; /**< Content format to be used */ - uint8_t *payload; /**< User allocated buffer for send request */ - size_t len; /**< Length of the payload */ - coap_client_response_cb_t cb; /**< Callback when response received */ - struct coap_client_option *options; /**< Extra options to be added to request */ - uint8_t num_options; /**< Number of extra options */ - void *user_data; /**< User provided context */ + enum coap_method method; /**< Method of the request */ + bool confirmable; /**< CoAP Confirmable/Non-confirmable message */ + const char *path; /**< Path of the requested resource */ + enum coap_content_format fmt; /**< Content format to be used */ + const uint8_t *payload; /**< User allocated buffer for send request */ + size_t len; /**< Length of the payload */ + coap_client_response_cb_t cb; /**< Callback when response received */ + const struct coap_client_option *options; /**< Extra options to be added to request */ + uint8_t num_options; /**< Number of extra options */ + void *user_data; /**< User provided context */ }; /**