Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coap-engine,coap-transactions: Fix handling of separate response #953

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

akindestam
Copy link
Contributor

A response from a server might come as a "separate response" where first an
empty message ACK is sent to tell the client there is no need to retransmit, and
that it should expect a response message soon. (RFC 7252 §5.2.2)

To stop the client from retransmitting when our message has already been ACKed,
a field was added to coap_transaction_t that blocks the coap_sendto call in
coap_send_transaction, with the retrans_timer continuing to fire like normal so
that we can still time out if the response from the server never shows up.

The response message is sent with a new Message ID chosen by the server, and if
its type is CON it should be ACK:ed by the client to inform the server that the
response has been successfully received.

To be able to handle all of the above we also need to fix how transactions are
managed and matched; the response from the server won't be using the same
Message ID as in the request. According to RFC, request/response should be
matched using the token and endpoint information only (RFC 7252 §5.3), so
transactions were refactored to match on this information instead of matching on
Message ID (except in the case of an empty message ACK).

@akindestam akindestam force-pushed the coap-separate-response branch 2 times, most recently from ef14fde to 236a8e5 Compare May 8, 2019 10:41
@akindestam
Copy link
Contributor Author

akindestam commented Jun 20, 2019

Rebased on latest develop. Also added some missing includes

akindestam and others added 2 commits February 25, 2020 17:29
A response from a server might come as a "separate response" where first an
empty message ACK is sent to tell the client there is no need to retransmit, and
that it should expect a response message soon. (RFC 7252 §5.2.2)

To stop the client from retransmitting when our message has already been ACKed,
a field was added to coap_transaction_t that blocks the coap_sendto call in
coap_send_transaction, with the retrans_timer continuing to fire like normal so
that we can still time out if the response from the server never shows up.

The response message is sent with a new Message ID chosen by the server, and if
its type is CON it should be ACK:ed by the client to inform the server that the
response has been successfully received.

To be able to handle all of the above we also need to fix how transactions are
managed and matched; the response from the server won't be using the same
Message ID as in the request. According to RFC, request/response should be
matched using the token and endpoint information only (RFC 7252 §5.3), so
transactions were refactored to match on this information instead of matching on
Message ID (except in the case of an empty message ACK).
The confirmable type needs to be acknowledged. When DTLS is used the ack can no
longer be contained only in the header part of the coap message and the call to
coap_sendto() overwrites the payload of a message stored in uip_buf (calls
uip_udp_packet_sendto() from dtls_write with a data length > 0), hence the need
to store the payload before sending the ack is needed.
@g-oikonomou g-oikonomou added this to the Version 4.6 milestone Oct 22, 2020
@g-oikonomou g-oikonomou modified the milestones: Version 4.6, Version 4.7 Nov 25, 2020
@simonduq simonduq deleted the branch contiki-ng:develop August 8, 2023 14:26
@simonduq simonduq closed this Aug 8, 2023
@simonduq
Copy link
Member

simonduq commented Aug 8, 2023

Hi!

This was my mistake, very sorry about it, re-opening this PR now.

What happened is the following: I was switching the base branch for this repository https://github.com/wittra/contiki-ng from develop to wittra. But accidentally, I made the change on the wrong repo (this repo) and instead of switching the base branch I renamed it. And somehow github deleted develop and closed all PRs...

Many apologies for this mishap 🙏; I haven't contributed in a while.. but now at least everybody got some notification from me :p

@simonduq simonduq reopened this Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants