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

[DNM] RFC: LwM2M Bootstrap Feature Support #8570

Closed

Conversation

mike-scott
Copy link
Contributor

This series of patches adds LwM2M Bootstrap support to the Zephyr LwM2M client.

Let's rename lwm2m_engine_exec_cb_t to lwm2m_engine_user_cb_t so that
future user-code callbacks can make use of the same definition.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
LwM2M engine now supports optional resources that may need to be
setup or torn down in user-based code during object instance
creation / deletion.

Let's provide callbacks that can be used for this purpose.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
Don't use hard-coded value of 4 for passing the # of options to
coap_find_options() in handle_request().  This can easily get
out of sync.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
During transfer of object data via OMA TLV format, we can
encounter resources which are optional or not handled in base
LwM2M engine.  When these resources cannot be handled let's
read past them and continue on.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
This function will cleanup the engine context, even if it's
in various different states.  In general, the LwM2M subsys
should always call this function to perform the context close.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
Add client_identity buffer to security object for use with bootstrap
operation.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
Add server public and private key storage buffers to LwM2M security
object.  These will be used for bootstrap and future changes moving
the security information out of RD client.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
Add functions to the security obj for:
- finding a specific object instance by index
- finding a security index by object instance id

These will be needed to use the security data for connecting
to servers instead of passing in the data from the client.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
This config enables the bootstrap support logic in the RD client.

It also adjusts the default # of server and security instances to
reflect that the client will be connecting to a minimum of 2 servers.

NOTE: Related code will be added to the RD client over the next few
patches.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
MAX_RESOURCE_LEN can be used by sample applications when creating
storage buffers resource locations, so let's make it public.

Also, the previous value of 20 was incorrect, as resource instance
id is not included.  Instead correct the value to 16.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
During bootstrap mode both SERVER_SHORT_SERVER_ID and
SERVER_TRANSPORT_BINDING_ID can be overwritten with new information.

These need to be flagged as RW for now, otherwise bootstrap
process will error out.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
During BOOTSTRAP_WRITE we can safely ignore optional resources
(similar to OP_CREATE).  Let's add this handling to the OMA TLV
formatter.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
For object create and delete we normally will trigger the RD
client to send an updated list of object data.  However, we
don't want to do this during bootstrap mode.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
During BOOTSTRAP_WRITE, empty OPAQUE data can being written to
the secuity objects.  Instead of looking for more data which will
never land, let's check the length and exit early if there's no
data.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
During bootstrap a DISCOVERY OP needs to be slightly changed:
- No object ID is required
- Don't avoid security object
- Don't return resource information

NOTE: Several comments had their "TODO" items updated.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
A bootstrap delete operation removes the existing security information
from the client prior to writing new information.

Let's handle this in the engine.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
Once the LwM2M server has finished transferring the bootstrap
information, a "Bootstrap-Finish" operation will be sent to the
LwM2M client.

Per LwM2M Tech Specification 1.0.2 page 76, a Bootstrap-Finish
operation is a POST with the URI=/bs.

Add this handling to the LwM2M engine.

Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
@mike-scott mike-scott requested a review from rbtchc June 26, 2018 21:39
@mike-scott mike-scott self-assigned this Jun 26, 2018
@mike-scott mike-scott added Feature Request A request for a new feature area: Networking labels Jun 26, 2018
@mike-scott mike-scott added this to the v1.13.0 milestone Jun 26, 2018
@mike-scott
Copy link
Contributor Author

This is a rather large patch set that I need to probably clean up a bit more prior to merging. However, it also needs to be tested across several servers, so I'm posting it DNM / RFC for now.

@therealprof feel free to review / comment

NOTE:
Once this is merged, my focus will shift to:

  1. Addressing current issues w/ LwM2M client such as backoff timers during error paths, etc.
  2. Re-writing the current LwM2M library to use the sockets API

@mike-scott
Copy link
Contributor Author

Specifically, 07cb4ec commit needs to be broken down into smaller commits.

@mike-scott
Copy link
Contributor Author

@dbkinder No need to review for doc changes yet.

@mike-scott
Copy link
Contributor Author

@jukkar currently, I'm abusing a NET_APP config:
CONFIG_NET_APP_PEER_IPV6_ADDR with a value of "coap[s]://[2001:db8::2]:568[3|4]"

This is not the intended use for this config. I'd like a recommendation on options that handle the following data:

  1. IP address
  2. Protocol [coap|coaps]
  3. Conection Port

This could all be determined in the sample and then passed on as a single string to the LwM2M client.

@codecov-io
Copy link

Codecov Report

Merging #8570 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8570   +/-   ##
=======================================
  Coverage   63.95%   63.95%           
=======================================
  Files         429      429           
  Lines       41281    41281           
  Branches     6957     6957           
=======================================
  Hits        26403    26403           
  Misses      11720    11720           
  Partials     3158     3158

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85c1522...cb1fe67. Read the comment docs.

@therealprof
Copy link
Contributor

Looking forward to testing that tomorrow or so.

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with that compile fix it doesn't work for me since it is going into an endless loop:

[lib/lwm2m_engine] [DBG] lwm2m_engine_get: path:0/0/1, buf:0x20007597, buflen:1
[lib/lwm2m_engine] [DBG] lwm2m_engine_get: path:1/0/1, buf:0x20004160, buflen:4
[lib/lwm2m_rd_client] [DBG] sm_select_next_sec_inst: sec_obj_inst: 0
[lib/lwm2m_rd_client] [INF] sm_do_bootstrap_reg: Bootstrap started with endpoint 'frdm_k64f' with client lifetime 30
[lib/lwm2m_engine] [DBG] lwm2m_engine_start: URL: coap://212.18.24.70
[lib/lwm2m_rd_client] [DBG] sm_do_bootstrap_reg: Register ID with bootstrap server [212.18.24.70] as 'ep=frdm_k64f'
[lib/lwm2m_engine] [DBG] lwm2m_udp_receive: checking for reply from [212.18.24.70]
[lib/lwm2m_rd_client] [DBG] do_bootstrap_reply_cb: Bootstrap callback (code:4.4)
[lib/lwm2m_rd_client] [ERR] do_bootstrap_reply_cb: Failed: NOT_FOUND.  Not Retrying.
[lib/lwm2m_engine] [DBG] lwm2m_udp_receive: reply 0x200000bc handled and removed
[lib/lwm2m_engine] [DBG] lwm2m_engine_get: path:0/0/1, buf:0x20007597, buflen:1
[lib/lwm2m_engine] [DBG] lwm2m_engine_get: path:0/0/1, buf:0x20007597, buflen:1
[lib/lwm2m_engine] [DBG] lwm2m_engine_get: path:0/0/1, buf:0x20007597, buflen:1
...

Will need to have a closer look.

event = LWM2M_RD_CLIENT_EVENT_BOOTSTRAP_REG_COMPLETE;
} else if (sm_state == ENGINE_BOOTSTRAP_TRANS_DONE) {
event = LWM2M_RD_CLIENT_EVENT_BOOTSTRAP_TRANSFER_COMPLETE;
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing closing }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great find. I wonder if I pulled in the wrong patches some how.. this never would have compiled when I was testing.

@therealprof
Copy link
Contributor

Okay, the endless loop is caused by the client not using a bootstrap port and the server rejecting the /bs endpoint. Obviously that shouldn't ensue havoc.

@therealprof
Copy link
Contributor

The bootstrap also happily accepts being sent to a coaps registration server without DTLS enabled in the build, again causing an endless loop:

[lwm2m_obj_server] [DBG] server_create: Create LWM2M server instance: 1
[lib/lwm2m_engine] [DBG] lwm2m_udp_receive: checking for reply from [212.18.24.70]
[lib/lwm2m_rd_client] [INF] engine_bootstrap_finish: Bootstrap data transfer done!
[lwm2m-client] [DBG] rd_client_event: Bootstrap transfer complete
[lib/lwm2m_engine] [DBG] lwm2m_engine_get: path:0/1/1, buf:0x20007597, buflen:1
[lib/lwm2m_engine] [DBG] lwm2m_engine_get: path:1/1/1, buf:0x20004160, buflen:4
[lib/lwm2m_rd_client] [DBG] sm_select_next_sec_inst: sec_obj_inst: 1
[lib/lwm2m_rd_client] [INF] sm_do_registration: RD Client started with endpoint 'frdm_k64f' with client lifetime 60
[lib/lwm2m_rd_client] [ERR] sm_do_registration: Cannot init LWM2M engine (-43)
[lib/lwm2m_engine] [DBG] lwm2m_engine_get: path:0/1/1, buf:0x20007597, buflen:1
[lib/lwm2m_engine] [DBG] lwm2m_engine_get: path:1/1/1, buf:0x20004160, buflen:4
[lib/lwm2m_rd_client] [DBG] sm_select_next_sec_inst: sec_obj_inst: 1
...

@therealprof
Copy link
Contributor

... enabling DTLS in the build and bootstrapping without DTLS to a server with DTLS causes an exception:

[lwm2m-client] [DBG] rd_client_event: Bootstrap transfer complete
[lib/lwm2m_engine] [DBG] lwm2m_engine_context_close: Releasing DTLS context 0x20004814
***** USAGE FAULT *****
  Executing thread ID (thread): 0x20004e4c
  Faulting instruction address:  0x0
  Illegal use of the EPSR
Fatal fault in thread 0x20004e4c! Aborting.

Need to bring out the heavy gear I guess. 😉

@mike-scott
Copy link
Contributor Author

@therealprof Don't spend time debugging this yet, let me verify that this is the same patch set I've been testing locally.
NOTE: Your comments apply tho, as there is very little checking to make sure the user is doing something that "makes sense".

@therealprof
Copy link
Contributor

@mike-scott Any update? Maybe we can split up the patch and have some items merged as is like the create/delete callback support.

@mike-scott
Copy link
Contributor Author

@therealprof busy week and now holiday. I'll split the top 4 off into a separate PR. Then try and fix #5 in a better way in a new PR.

@nashif nashif added the DNM This PR should not be merged (Do Not Merge) label Jul 31, 2018
@nashif nashif modified the milestones: v1.13.0, v1.14.0 Aug 28, 2018
@nashif
Copy link
Member

nashif commented Sep 14, 2018

@mike-scott status of this PR?

@mike-scott
Copy link
Contributor Author

@nashif It's going to be reworked a bit once the socket support goes in.

I'm not sure if @therealprof ever found it useful. I put it up mostly for testing.

Maybe keep it open a bit longer. I've been context switching too much lately.

@therealprof
Copy link
Contributor

@mike-scott Very useful. I'm still hopeful for the split version of the patches. At the moment it's a whee bit buggy but that's probably because there's too much stuff at once.

@mike-scott
Copy link
Contributor Author

Closing stale PR. These changes are included in: #9832

@mike-scott mike-scott closed this Jan 30, 2019
@mike-scott mike-scott deleted the master-lwm2m-bootstrap branch February 11, 2019 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking DNM This PR should not be merged (Do Not Merge) Feature Request A request for a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants