net: lib: coap_client: wait for all acknowledgements

This commit makes sure we continue to wait for extra confirmations even
after the request is done so we can handle duplicate confirmations if any.

Detailed description:

rfc7252#section-4.5 specifies that:

"The recipient SHOULD acknowledge each duplicate copy of a
 Confirmable message".

So if, for example, the client sends to a multicast destination address,
the server will get multiple requests and will confirm all of them.

Without this commit, the client will set the request to done after
receiving the first answer.
From here the request object will be marked as free and the duplicate
acknowledgements will stay buffered in the network stack.
Once the client tries to send a new request, it will unbuffer those
duplicate acknowledgements but now the request object is unallocated
so the client won't be able to handle those acknowledgements as duplicates.
It will instead treat it as an unexpected ACK.

To work around this issue, rfc7252#section-4.8.2 states that:

"EXCHANGE_LIFETIME is the time from starting to send a Confirmable
 message to the time when an acknowledgement is no longer expected,
 i.e., message-layer information about the message exchange can be
 purged."

Keeping the request object allocated for EXCHANGE_LIFETIME ensures that
duplicate acknowledgements can be handled accordingly.

This commit adds a basic implementation of what is stated in the RFC.

EXCHANGE_LIFETIME has been arbitrarily set to 3 * ACK_TIMEOUT which
seems more reasonable than the 247 seconds stated in the RFC.

Signed-off-by: Francois Gervais <[email protected]>

Author: fgervais

subsys/net/lib/coap/coap_client.c

@@ -16,6 +16,7 @@ LOG_MODULE_DECLARE(net_coap, CONFIG_COAP_LOG_LEVEL);
 #define COAP_VERSION 1
 #define COAP_SEPARATE_TIMEOUT 6000
 #define COAP_PERIODIC_TIMEOUT 500
+#define COAP_EXCHANGE_LIFETIME_FACTOR 3
 #define BLOCK1_OPTION_SIZE 4
 #define PAYLOAD_MARKER_SIZE 1
 
@@ -86,6 +87,21 @@ static int coap_client_schedule_poll(struct coap_client *client, int sock,
 	return 0;
 }
 
+static bool exchange_lifetime_exceeded(struct coap_client_internal_request *internal_req)
+{
+	int64_t time_since_t0, exchange_lifetime;
+
+	if (coap_header_get_type(&internal_req->request) == COAP_TYPE_NON_CON) {
+		return true;
+	}
+
+	time_since_t0 = k_uptime_get() - internal_req->pending.t0;
+	exchange_lifetime =
+		(internal_req->pending.params.ack_timeout * COAP_EXCHANGE_LIFETIME_FACTOR);
+
+	return time_since_t0 > exchange_lifetime;
+}
+
 static bool has_ongoing_request(struct coap_client *client)
 {
 	for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) {
@@ -97,10 +113,23 @@ static bool has_ongoing_request(struct coap_client *client)
 	return false;
 }
 
+static bool has_ongoing_exchange(struct coap_client *client)
+{
+	for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) {
+		if (client->requests[i].request_ongoing == true ||
+		    !exchange_lifetime_exceeded(&client->requests[i])) {
+			return true;
+		}
+	}
+
+	return false;
+}
+
 static struct coap_client_internal_request *get_free_request(struct coap_client *client)
 {
 	for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) {
-		if (client->requests[i].request_ongoing == false) {
+		if (client->requests[i].request_ongoing == false &&
+		    exchange_lifetime_exceeded(&client->requests[i])) {
 			return &client->requests[i];
 		}
 	}
@@ -110,13 +139,24 @@ static struct coap_client_internal_request *get_free_request(struct coap_client
 
 static bool has_ongoing_requests(void)
 {
-	bool has_requests = false;
+	for (int i = 0; i < num_clients; i++) {
+		if (has_ongoing_request(clients[i])) {
+			return true;
+		}
+	}
 
+	return false;
+}
+
+static bool has_ongoing_exchanges(void)
+{
 	for (int i = 0; i < num_clients; i++) {
-		has_requests |= has_ongoing_request(clients[i]);
+		if (has_ongoing_exchange(clients[i])) {
+			return true;
+		}
 	}
 
-	return has_requests;
+	return false;
 }
 
 static enum coap_block_size coap_client_default_block_size(void)
@@ -611,7 +651,8 @@ static struct coap_client_internal_request *get_request_with_token(
 	response_tkl = coap_header_get_token(resp, response_token);
 
 	for (int i = 0; i < CONFIG_COAP_CLIENT_MAX_REQUESTS; i++) {
-		if (client->requests[i].request_ongoing) {
+		if (client->requests[i].request_ongoing ||
+		    !exchange_lifetime_exceeded(&client->requests[i])) {
 			if (client->requests[i].request_tkl != response_tkl) {
 				continue;
 			}
@@ -910,7 +951,7 @@ static void coap_client_recv(void *coap_cl, void *a, void *b)
 		}
 
 		/* There are more messages coming */
-		if (has_ongoing_requests()) {
+		if (has_ongoing_exchanges()) {
 			continue;
 		} else {
 idle: