DO NOT MERGE: LwM2M holding PR for 1.10 #1255

mike-scott · 2017-08-24T17:07:11Z

Currently, the LwM2M uses net_context structures and the LwM2M sample has code to establish network communications.

This should be changed to use the net_app APIs so that the sample can remove most of the network
setup code and the library itself can benefit from other net_app features such as DTLS, etc.

mike-scott · 2017-08-24T17:08:10Z

@rbtchc I realize this code will conflict with your PR: #1049 We'll need to work out which will be merged first and how. :/ Please take a look.

mike-scott · 2017-08-24T17:08:25Z

@pfalcon @rsalveti Please take a look as well.

jukkar

Lot of changes, and I probably missed something. Anyway, this is a good start.

jukkar · 2017-08-24T19:22:31Z

include/net/lwm2m.h

+ * @details Context structure for the LwM2M high-level API.
+ */
+struct lwm2m_ctx {
+	/** Net app context structure */


Comment talks about "net app context" but the parameter is the net_context one.

Woops.. by the end of the patch set this is a net_app_ctx.. I probably didn't adjust the comment mid-patch series. I'll fix.

jukkar · 2017-08-24T19:29:56Z

samples/net/lwm2m_client/src/lwm2m-client.c

@@ -57,10 +57,12 @@ static struct device *led_dev;
 static u32_t led_state;

 #if defined(CONFIG_NET_IPV6)
-static struct net_app_ctx udp6;
+static struct net_app_ctx app_udp6;


net_app_ctx support both AF_INET and AF_INET6 at the same time, one just need to specify AF_UNSPEC when specifying local endpoint. So in this respect we could just have one net_app_ctx instead of two. Or perhaps this was removed in later patches, there was lot of changes so I might have missed it.

Hrm. At the end of the patch series, I end up with 2 lwm2m_ctx structures (each with it's own net_app_ctx). They handle IPv4 and IPv6 individually. Do we have any samples which illustrate dual use of a net_app_ctx?

Hmm, I seem to have talked about server side but the lwm2m is about client. So in client side this is not possible, sorry for confusion.

jukkar · 2017-08-24T19:53:49Z

subsys/net/lib/lwm2m/lwm2m_engine.c

 {
 	return net_context_sendto(pkt, dst_addr, NET_SOCKADDR_MAX_SIZE,
 				  NULL, K_NO_WAIT, NULL, NULL);
 }

 void lwm2m_udp_receive(struct lwm2m_ctx *client_ctx, struct net_pkt *pkt,
 		       bool handle_separate_response,
-		       int (*udp_request_handler)(struct zoap_packet *,
+		       int (*udp_request_handler)(struct net_context *net_ctx,


Please create a typedef for the function pointer, they are much more convinient to use and the code is easier to read after that.

Will do. I think early on I was heeding the checkpatch warning about no new typedefs :P

Woops, missed this in the changes.

mike-scott · 2017-08-24T21:08:45Z

Grr.. Github still hiding comments as "outdated".. annoying.

rbtchc · 2017-08-25T03:46:12Z

subsys/net/lib/lwm2m/lwm2m_engine.c

-						int num_zpendings)
+struct zoap_pending *lwm2m_init_message_pending(struct lwm2m_ctx *client_ctx,
+						struct zoap_packet *zpkt,
+						struct sockaddr *addr)


I suppose "addr" can be removed. Peer sockaddr can be obtained from client_ctx->net_app_ctx.default_ctx->remote

Eventually the addr here will be resolved from the security objects (see page 121 of the LwM2M spec example URL: coaps://bootstrap.example.com). If the 0/0/0 resource is a bootstrap server it will then point us to another server which would need to be resolved, etc.

@rbtchc Your original comment here now applies. Going forward the rd_client will start engine with peer_addr information on a per server basis. So net_app_ctx.default_ctx->remote will be the valid destination address as far as the engine is concerned.

I will add a commit to V3 addressing this.

I also retract my earlier agreement. As with lwm2m_udp_sendto(), this function is used by the observer notify logic. And there shouldn't be an inherent limitation as to who can subscribe to notifications except where access controls (to be added later) allow.

rbtchc · 2017-08-25T03:48:13Z

subsys/net/lib/lwm2m/lwm2m_engine.c

@@ -2181,17 +2175,17 @@ static int handle_request(struct zoap_packet *request,
 	return r;
 }

-int lwm2m_udp_sendto(struct net_pkt *pkt, const struct sockaddr *dst_addr)
+int lwm2m_udp_sendto(struct net_app_ctx *app_ctx, struct net_pkt *pkt,
+		     const struct sockaddr *dst_addr)


and this one, too

Same as above.

And the same here.

Actually, I take it back. lwm2m_udp_sendto() is used to send to any address which may have sent a message to the IoT device (not just the peer registered in the net_app_ctx). For instance, if you make a query via Copper plug-in, that IP address needs to be used in lwm2m_udp_sendto().

That being said, this brings up a security question. And there will be more patches in the future to add the access controls object. This should stop production devices from responding to anyone not allowed.

Agree. But I suppose it doesn't work when DTLS is enabled.

rbtchc · 2017-08-25T06:04:28Z

I check the connections on the shell console by executing command

select net
conn
And it prompts

 Context   	        Iface                Flags Local           	                Remote
 [ 1] 0x00405cc0	0x00405220    4DU   AF_UNSPEC	
 [ 2] 0x00405d1c	0x00405220    6DU   [2001:db8::1]:34161	[2001:db8::2]:5683
 [ 3] 0x00405d78	0x00405220    4DU   192.0.2.1:37773	        192.0.2.2:5683
 [ 4] 0x00405dd4	0x00405220    6DU   AF_UNSPEC

I'm not sure why there are four connections existing on the list.
I suppose there should only be two of them, one for IPv4 and one for IPv6 clients.

I, too, alter the interfaces to net_app while I was working on the bootstrap interfaces.
But I don't have the connection w/ Local equals to AF_UNSPEC.
I will try to figure out why.

mike-scott · 2017-08-25T06:17:21Z

@rbtchc Please don't test this too much atm. I have 2 net_app_ctx and I'm going to remove 1. That's the reason we have 2 extra AF_UNSPEC in the conn list.

I'll submit a new PR tomorrow during my daytime.

mike-scott · 2017-08-28T21:54:52Z

V2 posted. Diff: https://hastebin.com/seyogamofe.cs

Summary of changes:

Only 1 lwm2m_ctx in the sample since it will use IPv6/IPv4 based on the PEER IP address.
Removed peer-address, port and endpoint name from the LwM2M context. They were single use information items. (A lot of code was removed as a result)
The sample client no longer calls lwm2m_engine_start(). That's moved into the registration client. This simplifies the sample code even more.
Endpoint name was simplified to CONFIG_BOARD. This change will stop multiple samples on the same HW from running against 1 Leshan, however, for testing DTLS (an upcoming change) it's very hard to guess at random endpoint names when you need to add security information.
Addressed comments

mike-scott · 2017-08-28T22:04:22Z

I also have a pending DTLS support patch. I'm not sure if I should tack that on here as well. Or keep it for a separate PR.

mike-scott · 2017-08-28T22:51:51Z

Added a few more commits for the LwM2M cleanup:

Remove the needless "registered" field.
Implement timeout checks for the registration client
Set a default range for the client lifetime setting in Kconfig

mike-scott · 2017-08-28T22:54:00Z

Added the last 2 (hopefully) commits to this PR:

Adding DTLS support for LwM2M client (in the engine and in the sample)

NOTE: This currently is generating errors due to corruption in the decrypted IP header. This is being addressed in this PR: #1265

mike-scott · 2017-08-28T22:54:48Z

@rbtchc This is pretty much everything I have in my local tree. Feel free to test everything but the DTLS changes.

rbtchc · 2017-08-29T06:51:29Z

I've applied firmware patches on top of the modifications.
So far so good.

mike-scott · 2017-08-29T15:11:51Z

Fixed checkpatch error referencing spaces instead of tabs. No functional changes.

mike-scott · 2017-09-01T00:04:47Z

I've totally re-worked the timeout processing patch "net: lwm2m: implement client state timeout checks" and will probably remove that patch and everything that followed it from this PR. Then setup a new PR which implements the timeout handling and then another PR with the rest.

mike-scott · 2017-09-01T09:09:55Z

Hello @jukkar and @rbtchc,

I've posted V5, however it's a fairly large diff starting with "net: lwm2m: remove registration client "registered" field". I decided that I was unhappy with the old implementation of timeout handling. So, I went and refactored a lot of code so that the internal message sending APIs all use an "lwm2m_message" structure which keeps track of things a lot more cleanly than before.

I also rebased @rbtchc firmware support patches on top. And I dropped the DTLS patches which will come back once I migrate to the new COAP APIs.

mike-scott · 2017-09-01T09:10:24Z

I also rebased this all on top of latest master.

rbtchc · 2017-09-07T03:34:22Z

subsys/net/lib/lwm2m/lwm2m_engine.c

+	}
+
+	pkt = net_app_get_net_pkt(&msg->ctx->net_app_ctx, AF_UNSPEC,
+				       BUF_ALLOC_TIMEOUT);


should be aligned to &msg

rbtchc · 2017-09-07T03:45:36Z

subsys/net/lib/lwm2m/lwm2m_engine.c

+		zoap_header_set_token(&msg->zpkt, zoap_next_token(), 8);
+	}
+
+	if (msg->type == ZOAP_TYPE_CON) {


how about change to following to save an extra indent

if (msg->type != ZOAP_TYPE_CON) { return 0; } // Init pending

rbtchc · 2017-09-07T04:18:12Z

subsys/net/lib/lwm2m/lwm2m_obj_firmware_pull.c

+	off = parsed_uri.field_data[UF_HOST].off;
+	len = parsed_uri.field_data[UF_HOST].len;
+
+	/* IPv6 is wrapped by brackets */


Although these are changes made by me for firmware pull.

Line 274~301 & 307 ~ 310 can be removed.
Instead, we can pass the host string to net_app_init_udp_client() directly.
Check the patch: https://hastebin.com/ejonuvurit.diff

ZOAP library has certain limitations in parsing and preparation of coap messages. It can handle only on single network fragment. If network packet is split between multiple fragments it fails. This patch is just copy and rename of 'zoap' to 'coap'. Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

Current coap library fails to parse or prepare if packet is more than one fragment. Added support to handle multi fragment packet. Also well-known/core api used to prepare coap packet and send it through net context api immediately. This is goind to be problematic if user doesn't enable net context. Also user can not encrypt coap packets. Now api will return prepared coap packet to application. Application will send it to peer. Jira: ZEP-2210 Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

ZoAP tests are modified to use new CoAP API. Also modified tests name from 'zoap' to 'coap'. Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

Added deprecated statements to ZOAP library api, structs and enums. Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

Instead of generating an EINVAL error when there's no payload let's check for an offset == 0 and exit gracefully. Signed-off-by: Michael Scott <michael.scott@linaro.org>

This patch moves from the ZoAP API in subsys/net/lib/zoap to the CoAP API in subsys/net/lib/coap which handles multiple fragments for sending / receiving data. NOTE: This patch moves the LwM2M library over to the CoAP APIs but there will be a follow-up patch which re-writes the content formatter reader / writers to use net_pkt APIs for parsing across multiple net buffers. The current implementation assumes all of the data will land in 1 buffer. Samples using the library still need a fairly large NET_BUF_DATA_SIZE setting. (Example: CONFIG_NET_BUF_DATA_SIZE=384) Signed-off-by: Michael Scott <michael.scott@linaro.org>

Signed-off-by: Michael Scott <michael.scott@linaro.org>

This config is used with samples/net/lwm2m_client. It varies slightly from the config-coap.h file used with the COAP samples in that it has a larger SSL request size. Signed-off-by: Michael Scott <michael.scott@linaro.org>

For QEMU testing use CONF_FILE=prj_qemu_x86_dtls.conf For K64F testing use CONF_FILE=prj_frdm_k64f_dtls.conf Signed-off-by: Michael Scott <michael.scott@linaro.org>

This calculation reads the length portion of the COAP header to determine the length of the coap packet. However, when encrypted via DTLS this value seems to be getting corrupted. Let's change this calculation so that it will work for when DTLS is both enabled and disabled. Use the total length of the fragment data and substract back out the headers to get a correct value. Signed-off-by: Michael Scott <michael.scott@linaro.org>

The default net_context remote address is scrambled when using a connection via DTLS. Instead let's use the dtls context remote. Signed-off-by: Michael Scott <michael.scott@linaro.org>

Let's use conservative defaults for the LwM2M library to enable hardware with constrained resources. Users can increase where necessary. Signed-off-by: Michael Scott <michael.scott@linaro.org>

Support multiple HW with only 2 configs: 1) default operation w/o DTLS support 2) enable DTLS support NOTE: This also adjusts README.rst Signed-off-by: Michael Scott <michael.scott@linaro.org>

LwM2M is intended for constrained devices. The default samples settings are quite large by that standard and can be reduced to reflect actual usage. Signed-off-by: Michael Scott <michael.scott@linaro.org>

mike-scott · 2017-10-02T21:03:00Z

Rebased on master branch and added WIP DTLS support using new CoAP APIs.

This PR is mostly for @rbtchc to keep track of what's in my tree for upstream patches. But for others who are curious you're welcome to chime in.

NOTE! Still missing from this:

Some work to clean up lwm2m_msg in the case of an error response back to server in f6fcbc8.
Update the content formatter reader/writer APIs to use net_pkt parsing to drop the contiguous buffer requirement.

mike-scott · 2017-10-02T21:03:25Z

@dbkinder no need to review this for documentation changes as these patches will flow into normal PRs.

mike-scott · 2017-10-20T17:41:18Z

Closing. These were all merged via different PRs.

mike-scott · 2017-10-20T17:41:48Z

I should say.. the rest will be merged in different PRs :)

The OCF was not able to send packets over BLE 6lowpan consistently and segfaulting, this patch increase the data size to help improve stability. Fixes zephyrproject-rtos#1255, zephyrproject-rtos#1408, zephyrproject-rtos#1415 Signed-off-by: Jimmy Huang <jimmy.huang@intel.com>

mike-scott requested review from jukkar and tbursztyka as code owners August 24, 2017 17:07

jukkar added the net label Aug 24, 2017

jukkar reviewed Aug 24, 2017

View reviewed changes

rbtchc reviewed Aug 25, 2017

View reviewed changes

nashif added this to the v1.10 milestone Aug 26, 2017

mike-scott force-pushed the lwm2m-net-app branch from b8d676b to dfab969 Compare August 28, 2017 21:48

mike-scott force-pushed the lwm2m-net-app branch from edfcc14 to 76d37b9 Compare August 28, 2017 23:28

mike-scott force-pushed the lwm2m-net-app branch from c57db4a to 48a201a Compare August 29, 2017 15:11

This was referenced Aug 29, 2017

LWM2M: fix handle_reqest() errors #1273

Merged

Misc net fixes #1265

Merged

Add LwM2M firmware push/pull function #1049

Closed

mike-scott force-pushed the lwm2m-net-app branch from 48a201a to 4dd007f Compare September 1, 2017 09:03

rbtchc suggested changes Sep 7, 2017

View reviewed changes

rveerama1 and others added 20 commits October 2, 2017 13:47

net: tests: Update CoAP tests to use new API

6b27b03

ZoAP tests are modified to use new CoAP API. Also modified tests name from 'zoap' to 'coap'. Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

net: samples: Modify leds_demo to use new CoAP api's

def0d45

Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

net: samples: Modify zoap_server to new CoAP api's

6b6077e

Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

net: samples: Modify zoap_client to new CoAP api's

6a1ea81

Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

net: samples: Modify coaps_server to new CoAP api's

e099275

Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

net: samples: Modify coaps_client to new CoAP api's

00a441a

Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

net: samples: Add leds demo app over RPL and CoAP

e38a1dd

Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

net: Deprecate ZOAP library

54f9b22

Added deprecated statements to ZOAP library api, structs and enums. Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

net: coap: parse_option() exit gracefully when no payload

f56faa5

Instead of generating an EINVAL error when there's no payload let's check for an offset == 0 and exit gracefully. Signed-off-by: Michael Scott <michael.scott@linaro.org>

net: lwm2m: add support for DTLS

9ebb3eb

Signed-off-by: Michael Scott <michael.scott@linaro.org>

mbedtls: add lwm2m config for DTLS support

757d93b

This config is used with samples/net/lwm2m_client. It varies slightly from the config-coap.h file used with the COAP samples in that it has a larger SSL request size. Signed-off-by: Michael Scott <michael.scott@linaro.org>

samples: lwm2m: add support for DTLS

36faa44

For QEMU testing use CONF_FILE=prj_qemu_x86_dtls.conf For K64F testing use CONF_FILE=prj_frdm_k64f_dtls.conf Signed-off-by: Michael Scott <michael.scott@linaro.org>

net: lib: lwm2m: use correct remote address when DTLS is enabled

c51cc0a

The default net_context remote address is scrambled when using a connection via DTLS. Instead let's use the dtls context remote. Signed-off-by: Michael Scott <michael.scott@linaro.org>

net: lwm2m: reduce LwM2M footprint via lower defaults in Kconfig

57a82d6

Let's use conservative defaults for the LwM2M library to enable hardware with constrained resources. Users can increase where necessary. Signed-off-by: Michael Scott <michael.scott@linaro.org>

samples: lwm2m: cleanup prj_*.conf files

ac01204

Support multiple HW with only 2 configs: 1) default operation w/o DTLS support 2) enable DTLS support NOTE: This also adjusts README.rst Signed-off-by: Michael Scott <michael.scott@linaro.org>

samples: lwm2m: reduce footprint of lwm2m configs

c7969eb

LwM2M is intended for constrained devices. The default samples settings are quite large by that standard and can be reduced to reflect actual usage. Signed-off-by: Michael Scott <michael.scott@linaro.org>

mike-scott force-pushed the lwm2m-net-app branch from 9c34a28 to bb4cdc3 Compare October 2, 2017 20:54

nashif modified the milestones: v1.10, v1.10.0 Oct 3, 2017

mike-scott closed this Oct 20, 2017

mike-scott deleted the lwm2m-net-app branch October 20, 2017 18:02

DO NOT MERGE: LwM2M holding PR for 1.10 #1255

DO NOT MERGE: LwM2M holding PR for 1.10 #1255

Conversation

mike-scott commented Aug 24, 2017

mike-scott commented Aug 24, 2017

mike-scott commented Aug 24, 2017

jukkar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mike-scott commented Aug 24, 2017

rbtchc Aug 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbtchc commented Aug 25, 2017 • edited Loading

mike-scott commented Aug 25, 2017

mike-scott commented Aug 28, 2017 • edited Loading

mike-scott commented Aug 28, 2017

mike-scott commented Aug 28, 2017

mike-scott commented Aug 28, 2017

mike-scott commented Aug 28, 2017

rbtchc commented Aug 29, 2017

mike-scott commented Aug 29, 2017

mike-scott commented Sep 1, 2017

mike-scott commented Sep 1, 2017

mike-scott commented Sep 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbtchc Sep 7, 2017 • edited Loading

Choose a reason for hiding this comment

mike-scott commented Oct 2, 2017

mike-scott commented Oct 2, 2017

mike-scott commented Oct 20, 2017

mike-scott commented Oct 20, 2017

rbtchc Aug 25, 2017 •

edited

Loading

rbtchc commented Aug 25, 2017 •

edited

Loading

mike-scott commented Aug 28, 2017 •

edited

Loading

rbtchc Sep 7, 2017 •

edited

Loading