RFC7959: Move block handling logic down into libcoap #554

mrdeep1 · 2020-10-01T11:06:24Z

This PR is broken down into 2 parts - libcoap changes and example changes.

libcoap changes

RFC7959: Move block handling logic down into libcoap

This change removes the need for applications having to know about handling
requests for the next block etc. It also gives the application the ability
to pass a large body of data to be transmitted and be told when this body is
no longer required.

This logic is enabled in libcoap by setting COAP_BLOCK_USE_LIBCOAP using
coap_context_set_block_mode()

This change breaks down into 4 specific areas

Client doing a PUT/POST etc. of a large amount of data (BLOCK1).

coap_add_data_large_request() is used which provides a 'finished with data'
callback function as well as adding required options to the PDUs. This
function calls coap_add_data_large_internal().

Internal coap_handle_response_send_block() sends off the subsequent blocks
(using session->lg_xmit).

Server receiving PUT/POST etc. of a large amount of data (BLOCK1).

Internal coap_handle_request_get_block() acknowledges the appropriate blocks
(using session->lg_srcv) and either passes in the individual blocks or the body
depending on whether COAP_BLOCK_SINGLE_BODY is set or not using
coap_context_set_block_mode().

Server sending a large data response to GET / observe (BLOCK2)

coap_add_data_large_response() is used which provides a 'finished with data'
callback function. This function calls coap_add_data_large_internal() as well
as filling in the necessary options for each PDU.

Interal coap_handle_request_send_block() will then send the next requested block
(using session->lg_xmit).

Client receiving a large data response (BLOCK2)

coap_send_large() for the initial request sets this up.

Internal coap_handle_response_get_block() will then request the next block when
previous block received (using session->lg_crcv) and either passes the
individual blocks or the body depending on COAP_BLOCK_SINGLE_BODY being set in
coap_context_set_block_mode().

The following new internal functions have been added

coap_handle_request_get_block()
coap_handle_request_send_block()
coap_handle_response_get_block()
coap_handle_response_send_block()
coap_add_data_large_internal()

The following new functions have been added.

coap_add_data_large_request()
coap_add_data_large_response()
coap_context_set_block_mode()
coap_get_data_large()
coap_pdu_duplicate()
coap_send_large()
coap_session_init_token()
coap_session_new_token()

The coap_session_init_token() and coap_session_new_token() were added to make
the handling of unique tokens for each request/response easier.
https://datatracker.ietf.org/doc/html/draft-ietf-core-echo-request-tag-10#section-4

New files

include/coap2/coap_block_internal.h:
man/coap_block.txt.in:

New memory types

COAP_LG_XMIT
COAP_LG_SRCV
COAP_LG_CRCV

Example changes

RFC7959: Add in example support using updated libcoap handling of blocks

Remove block handling logic from coap-client and coap-server and make use of the
new libcoap functionality.

Add in new -L option to be able to define how libcoap is to work with block
handling. Update documentation accordingly.

obgm · 2020-10-14T14:34:17Z

Can you please rebase?

mrdeep1 · 2020-10-14T21:13:32Z

rebase code pushed

mrdeep1 · 2020-11-09T14:03:40Z

Code tidy up, minor fixes following testing just pushed.

mrdeep1 · 2020-11-12T13:56:25Z

rebased and code pushed

mrdeep1 · 2020-11-12T16:34:13Z

rebase and fix example error and pushed code

mrdeep1 · 2020-11-12T17:48:00Z

Fix bad rebase merge from #484 and code pushed

mrdeep1 · 2021-02-15T14:19:04Z

Rebased and updated after extensive testing.

obgm

Thanks again for this really awesome contribution. Here are some comments from a first review iteration.

examples/client.c

include/coap2/pdu.h

man/coap_block.txt.in

man/coap_pdu_setup.txt.in

src/block.c

obgm · 2021-02-19T11:51:29Z

src/block.c

+
+    /* Check if resource+query is already in use for large bodies (unlikely) */
+    lg_xmit = session->lg_xmit;
+    while (lg_xmit) {


I prefer to always use the utlist macros to improve readability and to avoid errors due to own linked-list implementations.

OK - will work on using LL_

mrdeep1 · 2021-02-19T13:36:13Z

Separate commits pushed - will get squashed at some point. The LL_ updates (last comment) still outstanding.

mrdeep1 · 2021-02-19T14:47:17Z

Previous 2 commits now squashed, new commit for latest changes. Working on LL_.

obgm · 2021-02-19T15:05:52Z

Uh, sorry, I just merged PR #583 and did not notice that this will result in a conflict with this PR.

mrdeep1 · 2021-02-22T13:51:26Z

My mistake, I was expecting Size2 which is not there. Working on a fix.

mrdeep1 · 2021-02-22T14:08:34Z

Fixed Size2 missing issue and code pushed.

mrdeep1 · 2021-02-22T14:23:40Z

..and fix another missing Size2 issue. Code pushed.

obgm · 2021-02-22T16:53:52Z

Thanks, this works better now. But it still seems to block when setting a block number larger than 0 with -b, e.g., examples/coap-client -v9 -b 1,512 coap://coap.me/.well-known/core or examples/coap-client -v9 -b 2001,512 coap://coap.me/.well-known/core.

mrdeep1 · 2021-02-22T17:01:05Z

I guess that if the client explicitly asks for a block.num != 0 that it should be responsible for all the other block handling for the body of data, rather than libcoap asking for all the subsequent blocks (because in effect this is random access).

I will fix it so that explicit block.num != 0 does not trigger additional block requests.

obgm · 2021-02-22T17:35:02Z

That makes sense to me.

mrdeep1 · 2021-02-23T09:56:02Z

Code (and docs) now correctly support random access block requests. Previous version of coap-client also hung on a single random access block request.

src/block.c

mrdeep1 · 2021-02-23T15:58:49Z

Updated code pushed.

This change removes the need for applications having to know about handling requests for the next block etc. It also gives the application the ability to pass a large body of data to be transmitted and be told when this body is no longer required. This logic is enabled in libcoap by setting COAP_BLOCK_USE_LIBCOAP using coap_context_set_block_mode() This change breaks down into 4 specific areas 1) Client doing a PUT/POST etc. of a large amount of data (BLOCK1). coap_add_data_large_request() is used which provides a 'finished with data' callback function as well as adding required options to the PDUs. This function calls coap_add_data_large_internal(). Internal coap_handle_response_send_block() sends off the subsequent blocks (using session->lg_xmit). 2) Server receiving PUT/POST etc. of a large amount of data (BLOCK1). Internal coap_handle_request_get_block() acknowledges the appropriate blocks (using session->lg_srcv) and either passes in the individual blocks or the body depending on whether COAP_BLOCK_SINGLE_BODY is set or not using coap_context_set_block_mode(). 3) Server sending a large data response to GET / observe (BLOCK2) coap_add_data_large_response() is used which provides a 'finished with data' callback function. This function calls coap_add_data_large_internal() as well as filling in the necessary options for each PDU. Interal coap_handle_request_send_block() will then send the next requested block (using session->lg_xmit). 4) Client receiving a large data response (BLOCK2) coap_send_large() for the initial request sets this up. Internal coap_handle_response_get_block() will then request the next block when previous block received (using session->lg_crcv) and either passes the individual blocks or the body depending on COAP_BLOCK_SINGLE_BODY being set in coap_context_set_block_mode(). ---- The following new internal functions have been added coap_handle_request_get_block() coap_handle_request_send_block() coap_handle_response_get_block() coap_handle_response_send_block() coap_add_data_large_internal() The following new functions have been added. coap_add_data_large_request() coap_add_data_large_response() coap_context_set_block_mode() coap_get_data_large() coap_pdu_duplicate() coap_send_large() coap_session_init_token() coap_session_new_token() The coap_session_init_token() and coap_session_new_token() were added to make the handling of unique tokens for each request/response easier. https://datatracker.ietf.org/doc/html/draft-ietf-core-echo-request-tag-10#section-4 New files include/coap2/coap_block_internal.h: man/coap_block.txt.in: New memory types COAP_LG_XMIT COAP_LG_SRCV COAP_LG_CRCV

Remove block handling logic from coap-client and coap-server and make use of the new libcoap functionality. Add in new -L option to be able to define how libcoap is to work with block handling. Update documentation accordingly.

mrdeep1 · 2021-02-23T18:21:44Z

rebased and pushed following #571 merge.

obgm · 2021-02-24T08:08:57Z

Thanks! Merging...

Merging obgm#522 and obgm#554 into develop caused Observe CON 2.05 initial block response not to get an ACK from the client and so the Observe response kept getting re-transmitted and caused other CON traffic to get queued up. Troubleshooting highlighted an issue when a PUT/POST was updating the contents of a resource and an Observe was active sending many payloads in a body. src/net.c: Make sure that an ACK is sent (if appropriate) when working with a block based response. examples/coap-server.c: Give dynamic data associated with a resource a reference count, and only free off the data when the reference count has dropped to 0.

Merging obgm#552 and obgm#554 into develop caused Observe CON 2.05 initial block response not to get an ACK from the client and so the Observe response kept getting re-transmitted and caused other CON traffic to get queued up. Troubleshooting highlighted an issue when a PUT/POST was updating the contents of a resource and an Observe was active sending many payloads in a body. src/net.c: Make sure that an ACK is sent (if appropriate) when working with a block based response. examples/coap-server.c: Give dynamic data associated with a resource a reference count, and only free off the data when the reference count has dropped to 0.

An empty response ACK is ignored if there was a request on the Send-Q. However if there was a subsequent duplicate ACK this would then fall through to the general handling logic in coap_dispatch(), and would trigger the ping handler (if set) and send a RST (if not mcast) which is not correct. Correct a change added in by obgm#554 so that all empty response ACKs are ignored.

mrdeep1 mentioned this pull request Oct 1, 2020

Lost sessions when utilizing Block-Wise transfers in CoAP #372

Closed

mrdeep1 mentioned this pull request Oct 1, 2020

Instructions on implementing block mode in server side #287

Closed

mrdeep1 force-pushed the add_data_large branch from 7288588 to 627872a Compare Oct 14, 2020

mrdeep1 force-pushed the add_data_large branch 5 times, most recently from 405382e to b9220b4 Compare Oct 22, 2020

mrdeep1 force-pushed the add_data_large branch from b9220b4 to 48cbd02 Compare Nov 9, 2020

mrdeep1 force-pushed the add_data_large branch from 48cbd02 to c9d5f16 Compare Nov 12, 2020

mrdeep1 force-pushed the add_data_large branch from c9d5f16 to 7875f59 Compare Nov 12, 2020

mrdeep1 force-pushed the add_data_large branch from 7875f59 to 184d406 Compare Nov 12, 2020

mrdeep1 mentioned this pull request Nov 27, 2020

Multi CoAP Client using libcoap in observer mode #573

Closed

mrdeep1 force-pushed the add_data_large branch from 184d406 to 15bc18a Compare Feb 15, 2021

mrdeep1 force-pushed the add_data_large branch 2 times, most recently from 5a738c2 to 67f72a2 Compare Feb 19, 2021

obgm reviewed Feb 19, 2021

View changes

mrdeep1 force-pushed the add_data_large branch from 1530d71 to 2730904 Compare Feb 19, 2021

mrdeep1 force-pushed the add_data_large branch 2 times, most recently from 175c70e to 937b7fc Compare Feb 19, 2021

mrdeep1 force-pushed the add_data_large branch from 0ef4fe7 to 97f2a6b Compare Feb 22, 2021

mrdeep1 force-pushed the add_data_large branch from 97f2a6b to 3511ef2 Compare Feb 22, 2021

mrdeep1 force-pushed the add_data_large branch from 3511ef2 to db28dcd Compare Feb 23, 2021

obgm reviewed Feb 23, 2021

View changes

src/block.c Outdated Show resolved Hide resolved

src/block.c Outdated Show resolved Hide resolved

src/block.c Outdated Show resolved Hide resolved

mrdeep1 force-pushed the add_data_large branch from db28dcd to acdbee9 Compare Feb 23, 2021

mrdeep1 force-pushed the add_data_large branch from acdbee9 to eb2635e Compare Feb 23, 2021

mrdeep1 added 2 commits Feb 23, 2021

mrdeep1 force-pushed the add_data_large branch from eb2635e to 4cd633e Compare Feb 23, 2021

obgm merged commit 7c54bd7 into obgm:develop Feb 24, 2021
10 checks passed

mrdeep1 deleted the add_data_large branch Feb 24, 2021

mrdeep1 mentioned this pull request Feb 25, 2021

Synchronous transmission of large payloads? #316

Closed

mrdeep1 changed the title ~~RFC7641: Move block handling logic down into libcoap~~ RFC7959: Move block handling logic down into libcoap Feb 25, 2021

mrdeep1 mentioned this pull request Feb 28, 2021

net.c: Fix side effect from merging 2 PRs #605

Merged

mrdeep1 mentioned this pull request Mar 31, 2021

net.c: Handle duplicate empty ACK #625

Merged

obgm / libcoap Public

RFC7959: Move block handling logic down into libcoap #554

RFC7959: Move block handling logic down into libcoap #554

mrdeep1 commented Oct 1, 2020 •

edited

obgm commented Oct 14, 2020

mrdeep1 commented Oct 14, 2020

mrdeep1 commented Nov 9, 2020

mrdeep1 commented Nov 12, 2020

mrdeep1 commented Nov 12, 2020

mrdeep1 commented Nov 12, 2020

mrdeep1 commented Feb 15, 2021

obgm left a comment

obgm Feb 19, 2021

mrdeep1 Feb 19, 2021

mrdeep1 commented Feb 19, 2021

mrdeep1 commented Feb 19, 2021

obgm commented Feb 19, 2021

mrdeep1 commented Feb 22, 2021

mrdeep1 commented Feb 22, 2021

mrdeep1 commented Feb 22, 2021

obgm commented Feb 22, 2021

mrdeep1 commented Feb 22, 2021

obgm commented Feb 22, 2021

mrdeep1 commented Feb 23, 2021

mrdeep1 commented Feb 23, 2021

mrdeep1 commented Feb 23, 2021

obgm commented Feb 24, 2021

obgm / libcoap Public

RFC7959: Move block handling logic down into libcoap #554

RFC7959: Move block handling logic down into libcoap #554

Conversation

mrdeep1 commented Oct 1, 2020 • edited

obgm commented Oct 14, 2020

mrdeep1 commented Oct 14, 2020

mrdeep1 commented Nov 9, 2020

mrdeep1 commented Nov 12, 2020

mrdeep1 commented Nov 12, 2020

mrdeep1 commented Nov 12, 2020

mrdeep1 commented Feb 15, 2021

obgm left a comment

obgm Feb 19, 2021

Choose a reason for hiding this comment

mrdeep1 Feb 19, 2021

Choose a reason for hiding this comment

mrdeep1 commented Feb 19, 2021

mrdeep1 commented Feb 19, 2021

obgm commented Feb 19, 2021

mrdeep1 commented Feb 22, 2021

mrdeep1 commented Feb 22, 2021

mrdeep1 commented Feb 22, 2021

obgm commented Feb 22, 2021

mrdeep1 commented Feb 22, 2021

obgm commented Feb 22, 2021

mrdeep1 commented Feb 23, 2021

mrdeep1 commented Feb 23, 2021

mrdeep1 commented Feb 23, 2021

obgm commented Feb 24, 2021

mrdeep1 commented Oct 1, 2020 •

edited