Skip to content

Commit 8787683

Browse files
SeppoTakalogithub-actions[bot]
SeppoTakalo
authored and
github-actions[bot]
committed
[nrf fromtree] net: lib: coap_client: check poll() condition before retrying CoAP msg
Refactor the CoAP retry handling into the handle_poll() function, so that we only try to send retries if the socket reports POLLOUT. Also move the receiving into same loop, so when poll() reports POLLIN we recv() the message and handle it before proceeding to other sockets. Also fix tests to handle POLLOUT flag and add support for testing multiple clients. Signed-off-by: Seppo Takalo <[email protected]> (cherry picked from commit 4c6dd4c) (cherry picked from commit a10fdc4)
1 parent 8659fca commit 8787683

File tree

6 files changed

+194
-147
lines changed

6 files changed

+194
-147
lines changed

‎include/zephyr/net/coap_client.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,14 +108,12 @@ struct coap_client {
108108
int fd;
109109
struct sockaddr address;
110110
socklen_t socklen;
111-
bool response_ready;
112111
struct k_mutex lock;
113112
uint8_t send_buf[MAX_COAP_MSG_LEN];
114113
uint8_t recv_buf[MAX_COAP_MSG_LEN];
115114
struct coap_client_internal_request requests[CONFIG_COAP_CLIENT_MAX_REQUESTS];
116115
struct coap_option echo_option;
117116
bool send_echo;
118-
int socket_error;
119117
};
120118
/** @endcond */
121119

‎subsys/net/lib/coap/coap_client.c‎

Lines changed: 83 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,13 @@ static int num_clients;
2626
static K_SEM_DEFINE(coap_client_recv_sem, 0, 1);
2727
static atomic_t coap_client_recv_active;
2828

29+
static bool timeout_expired(struct coap_client_internal_request *internal_req);
30+
static void cancel_requests_with(struct coap_client *client, int error);
31+
static int recv_response(struct coap_client *client, struct coap_packet *response, bool *truncated);
32+
static int handle_response(struct coap_client *client, const struct coap_packet *response,
33+
bool response_truncated);
34+
35+
2936
static int send_request(int sock, const void *buf, size_t len, int flags,
3037
const struct sockaddr *dest_addr, socklen_t addrlen)
3138
{
@@ -127,27 +134,26 @@ static bool has_ongoing_exchange(struct coap_client *client)
127134
return false;
128135
}
129136

130-
static struct coap_client_internal_request *get_free_request(struct coap_client *client)
137+
static bool has_timeout_expired(struct coap_client *client)
131138
{
132139
for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) {
133-
if (client->requests[i].request_ongoing == false &&
134-
exchange_lifetime_exceeded(&client->requests[i])) {
135-
return &client->requests[i];
140+
if (timeout_expired(&client->requests[i])) {
141+
return true;
136142
}
137143
}
138-
139-
return NULL;
144+
return false;
140145
}
141146

142-
static bool has_ongoing_requests(void)
147+
static struct coap_client_internal_request *get_free_request(struct coap_client *client)
143148
{
144-
for (int i = 0; i < num_clients; i++) {
145-
if (has_ongoing_request(clients[i])) {
146-
return true;
149+
for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) {
150+
if (client->requests[i].request_ongoing == false &&
151+
exchange_lifetime_exceeded(&client->requests[i])) {
152+
return &client->requests[i];
147153
}
148154
}
149155

150-
return false;
156+
return NULL;
151157
}
152158

153159
static bool has_ongoing_exchanges(void)
@@ -498,86 +504,91 @@ static int resend_request(struct coap_client *client,
498504
return ret;
499505
}
500506

501-
static int coap_client_resend_handler(void)
507+
static void coap_client_resend_handler(struct coap_client *client)
502508
{
503509
int ret = 0;
504510

505-
for (int i = 0; i < num_clients; i++) {
506-
k_mutex_lock(&clients[i]->lock, K_FOREVER);
511+
k_mutex_lock(&client->lock, K_FOREVER);
507512

508-
for (int j = 0; j < CONFIG_COAP_CLIENT_MAX_REQUESTS; j++) {
509-
if (timeout_expired(&clients[i]->requests[j])) {
510-
ret = resend_request(clients[i], &clients[i]->requests[j]);
513+
for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) {
514+
if (timeout_expired(&client->requests[i])) {
515+
ret = resend_request(client, &client->requests[i]);
516+
if (ret < 0) {
517+
report_callback_error(&client->requests[i], ret);
518+
reset_internal_request(&client->requests[i]);
511519
}
512520
}
513-
514-
k_mutex_unlock(&clients[i]->lock);
515521
}
516522

517-
return ret;
523+
k_mutex_unlock(&client->lock);
518524
}
519525

520526
static int handle_poll(void)
521527
{
522528
int ret = 0;
523529

524-
while (1) {
525-
struct zsock_pollfd fds[CONFIG_COAP_CLIENT_MAX_INSTANCES] = {0};
526-
int nfds = 0;
527-
528-
/* Use periodic timeouts */
529-
for (int i = 0; i < num_clients; i++) {
530-
fds[i].fd = clients[i]->fd;
531-
fds[i].events = ZSOCK_POLLIN;
532-
fds[i].revents = 0;
533-
nfds++;
534-
}
530+
struct zsock_pollfd fds[CONFIG_COAP_CLIENT_MAX_INSTANCES] = {0};
531+
int nfds = 0;
535532

536-
ret = zsock_poll(fds, nfds, COAP_PERIODIC_TIMEOUT);
533+
/* Use periodic timeouts */
534+
for (int i = 0; i < num_clients; i++) {
535+
fds[i].fd = clients[i]->fd;
536+
fds[i].events = (has_ongoing_exchange(clients[i]) ? ZSOCK_POLLIN : 0) |
537+
(has_timeout_expired(clients[i]) ? ZSOCK_POLLOUT : 0);
538+
fds[i].revents = 0;
539+
nfds++;
540+
}
537541

538-
if (ret < 0) {
539-
LOG_ERR("Error in poll:%d", errno);
540-
errno = 0;
541-
return ret;
542-
} else if (ret == 0) {
543-
/* Resend all the expired pending messages */
544-
ret = coap_client_resend_handler();
542+
ret = zsock_poll(fds, nfds, COAP_PERIODIC_TIMEOUT);
545543

546-
if (ret < 0) {
547-
LOG_ERR("Error resending request: %d", ret);
548-
}
544+
if (ret < 0) {
545+
ret = -errno;
546+
LOG_ERR("Error in poll:%d", ret);
547+
return ret;
548+
} else if (ret == 0) {
549+
return 0;
550+
}
549551

550-
if (!has_ongoing_requests()) {
551-
return ret;
552-
}
552+
for (int i = 0; i < nfds; i++) {
553+
if (fds[i].revents & ZSOCK_POLLOUT) {
554+
coap_client_resend_handler(clients[i]);
555+
}
556+
if (fds[i].revents & ZSOCK_POLLIN) {
557+
struct coap_packet response;
558+
bool response_truncated = false;
553559

554-
} else {
555-
for (int i = 0; i < nfds; i++) {
560+
k_mutex_lock(&clients[i]->lock, K_FOREVER);
556561

557-
if (fds[i].revents & ZSOCK_POLLERR) {
558-
LOG_ERR("Error in poll for socket %d", fds[i].fd);
559-
clients[i]->socket_error = -EIO;
560-
}
561-
if (fds[i].revents & ZSOCK_POLLHUP) {
562-
LOG_ERR("Error in poll: POLLHUP for socket %d", fds[i].fd);
563-
clients[i]->socket_error = -ENOTCONN;
564-
}
565-
if (fds[i].revents & ZSOCK_POLLNVAL) {
566-
LOG_ERR("Error in poll: POLLNVAL - fd %d not open",
567-
fds[i].fd);
568-
clients[i]->socket_error = -EINVAL;
569-
}
570-
if (fds[i].revents & ZSOCK_POLLIN) {
571-
clients[i]->response_ready = true;
572-
}
562+
ret = recv_response(clients[i], &response, &response_truncated);
563+
if (ret < 0) {
564+
LOG_ERR("Error receiving response");
565+
cancel_requests_with(clients[i], -EIO);
566+
k_mutex_unlock(&clients[i]->lock);
567+
continue;
568+
}
573569

570+
ret = handle_response(clients[i], &response, response_truncated);
571+
if (ret < 0) {
572+
LOG_ERR("Error handling response");
574573
}
575574

576-
return 0;
575+
k_mutex_unlock(&clients[i]->lock);
576+
}
577+
if (fds[i].revents & ZSOCK_POLLERR) {
578+
LOG_ERR("Error in poll for socket %d", fds[i].fd);
579+
cancel_requests_with(clients[i], -EIO);
580+
}
581+
if (fds[i].revents & ZSOCK_POLLHUP) {
582+
LOG_ERR("Error in poll: POLLHUP for socket %d", fds[i].fd);
583+
cancel_requests_with(clients[i], -EIO);
584+
}
585+
if (fds[i].revents & ZSOCK_POLLNVAL) {
586+
LOG_ERR("Error in poll: POLLNVAL - fd %d not open", fds[i].fd);
587+
cancel_requests_with(clients[i], -EIO);
577588
}
578589
}
579590

580-
return ret;
591+
return 0;
581592
}
582593

583594
static bool token_compare(struct coap_client_internal_request *internal_req,
@@ -895,14 +906,13 @@ static int handle_response(struct coap_client *client, const struct coap_packet
895906
}
896907
}
897908
fail:
898-
client->response_ready = false;
899909
if (ret < 0 || !internal_req->is_observe) {
900910
internal_req->request_ongoing = false;
901911
}
902912
return ret;
903913
}
904914

905-
void coap_client_cancel_requests(struct coap_client *client)
915+
static void cancel_requests_with(struct coap_client *client, int error)
906916
{
907917
k_mutex_lock(&client->lock, K_FOREVER);
908918

@@ -914,33 +924,20 @@ void coap_client_cancel_requests(struct coap_client *client)
914924
* do not reenter it. In that case, the user knows their
915925
* request was cancelled anyway.
916926
*/
917-
report_callback_error(&client->requests[i], -ECANCELED);
918-
client->requests[i].request_ongoing = false;
919-
client->requests[i].is_observe = false;
927+
report_callback_error(&client->requests[i], error);
928+
reset_internal_request(&client->requests[i]);
920929
}
921930
}
922931
atomic_clear(&coap_client_recv_active);
923932
k_mutex_unlock(&client->lock);
924933

925-
/* Wait until after zsock_poll() can time out and return. */
926-
k_sleep(K_MSEC(COAP_PERIODIC_TIMEOUT));
927934
}
928935

929-
static void signal_socket_error(struct coap_client *cli)
936+
void coap_client_cancel_requests(struct coap_client *client)
930937
{
931-
for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) {
932-
struct coap_client_internal_request *req = &cli->requests[i];
933-
934-
if (!req->request_ongoing) {
935-
continue;
936-
}
937-
938-
req->request_ongoing = false;
939-
if (req->coap_request.cb) {
940-
req->coap_request.cb(cli->socket_error, 0, NULL, 0,
941-
true, req->coap_request.user_data);
942-
}
943-
}
938+
cancel_requests_with(client, -ECANCELED);
939+
/* Wait until after zsock_poll() can time out and return. */
940+
k_sleep(K_MSEC(COAP_PERIODIC_TIMEOUT));
944941
}
945942

946943
void coap_client_recv(void *coap_cl, void *a, void *b)
@@ -957,41 +954,6 @@ void coap_client_recv(void *coap_cl, void *a, void *b)
957954
goto idle;
958955
}
959956

960-
for (int i = 0; i < num_clients; i++) {
961-
if (clients[i]->response_ready) {
962-
struct coap_packet response;
963-
bool response_truncated = false;
964-
965-
k_mutex_lock(&clients[i]->lock, K_FOREVER);
966-
967-
ret = recv_response(clients[i], &response, &response_truncated);
968-
if (ret < 0) {
969-
LOG_ERR("Error receiving response");
970-
clients[i]->response_ready = false;
971-
k_mutex_unlock(&clients[i]->lock);
972-
if (ret == -EOPNOTSUPP) {
973-
LOG_ERR("Socket misconfigured.");
974-
goto idle;
975-
}
976-
continue;
977-
}
978-
979-
ret = handle_response(clients[i], &response, response_truncated);
980-
if (ret < 0) {
981-
LOG_ERR("Error handling response");
982-
}
983-
984-
clients[i]->response_ready = false;
985-
k_mutex_unlock(&clients[i]->lock);
986-
}
987-
988-
if (clients[i]->socket_error) {
989-
signal_socket_error(clients[i]);
990-
clients[i]->socket_error = 0;
991-
}
992-
993-
}
994-
995957
/* There are more messages coming */
996958
if (has_ongoing_exchanges()) {
997959
continue;

‎tests/net/lib/coap_client/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,4 @@ add_compile_definitions(CONFIG_COAP_CLIENT_MAX_REQUESTS=2)
3131
add_compile_definitions(CONFIG_COAP_CLIENT_MAX_INSTANCES=2)
3232
add_compile_definitions(CONFIG_COAP_MAX_RETRANSMIT=4)
3333
add_compile_definitions(CONFIG_COAP_BACKOFF_PERCENT=200)
34+
add_compile_definitions(CONFIG_COAP_LOG_LEVEL=4)

0 commit comments

Comments
 (0)