LwM2M 1.10 changes: LwM2M library use net_app API internally #1473
Conversation
The LwM2M library does not use net_app APIs internally. To help this effort let's establish a user facing structure "lwm2m_ctx" (similar to http_client_ctx and mqtt_ctx) and start it off by wrappering the net_context structure. Future patches will add user setup options to this structure and eventually remove the net_context structure in favor of a net_app_ctx. Signed-off-by: Michael Scott <[email protected]>
Signed-off-by: Michael Scott <[email protected]>
ghost
added
the
In progress
|
This is patchset #2 of 7 for 1.10 development window:
|
|
@rbtchc I can't add you as an official reviewer but please review and add comments / approval |
subsys/net/lib/lwm2m/lwm2m_engine.c
Outdated
| client_ctx = CONTAINER_OF(obs->net_ctx, | ||
| struct lwm2m_ctx, net_ctx); | ||
| client_ctx = CONTAINER_OF(obs->net_app_ctx, | ||
| struct lwm2m_ctx, net_app_ctx); | ||
| if (!client_ctx) { |
There was a problem hiding this comment.
I think checking the client_ctx returned from CONTAINER_OF() macro is meaningless.
It always returns w/ the memory address deducted by the offset and is unlikely to be NULL.
If this makes sense to you, then we could remove the checking here and at other places.
| if (memcmp(addr, &clients[i].reg_server, | ||
| app_ctx = &clients[i].ctx->net_app_ctx; | ||
| if (clients[i].ctx) { | ||
| if (memcmp(addr, &app_ctx->default_ctx->remote, | ||
| sizeof(addr)) == 0) { |
There was a problem hiding this comment.
sizeof(addr) -> sizeof(*addr)
There was a problem hiding this comment.
Good catch! Fixing.
mike-scott
force-pushed
the
lwm2m-net-app2
branch
from
e4d7722
to
4e93896
Compare
|
Fixed comment from @rbtchc diff: https://hastebin.com/vacogusewa.cpp |
subsys/net/lib/lwm2m/lwm2m_engine.c
Outdated
| struct zoap_pending *pending; | ||
| int r; | ||
|
|
||
| pending = zoap_pending_next_to_expire(pendings, NUM_PENDINGS); | ||
| client_ctx = CONTAINER_OF(work, struct lwm2m_ctx, retransmit_work); | ||
| if (!client_ctx) { |
There was a problem hiding this comment.
Hi @mike-scott ,
I left two comments yesterday.
One probably folded because I left the comment on the outdated part of the code.
I check the macro "CONTAINER_OF", it's a simple offset deduction based on the given memory address. I suggest you can remove the check here for it's unlikely the address will be NULL.
if (!client_ctx) {
...
}
Signed-off-by: Michael Scott <[email protected]>
This allows use to associate easily the replies / pending operations with a specific network connection. Signed-off-by: Michael Scott <[email protected]>
This is part of lwm2m_ctx and is not needed. Signed-off-by: Michael Scott <[email protected]>
In preparation for the move to net_app APIs, we will need to pass net_app_ctx structures around to the following functions: lwm2m_udp_sendto() udp_request_handler() Let's add the parameter as net_context for now so the transition will be smoother later. Signed-off-by: Michael Scott <[email protected]>
This is the final stage of moving the LwM2M library internals to the net_app APIs. This means we can support DTLS and other built-in features in the future. All of the logic for establishing the network connection is removed from the sample app. Signed-off-by: Michael Scott <[email protected]>
Replace with a check of the state machine's state instead. Signed-off-by: Michael Scott <[email protected]>
mike-scott
force-pushed
the
lwm2m-net-app2
branch
from
4e93896
to
5d5b5b6
Compare
|
Removed client_ctx check per @rbtchc recommendation. Diff: https://hastebin.com/ledubaxara.cpp |
|
Added @pfalcon purely for code review if he has time. No pressure. |
| app_ctx = &clients[i].ctx->net_app_ctx; | ||
| if (clients[i].ctx) { | ||
| if (memcmp(addr, &app_ctx->default_ctx->remote, | ||
| sizeof(*addr)) == 0) { |
There was a problem hiding this comment.
I just realize that in IPv6 case, the sockaddr in the app_ctx doesn't have the "sin6_scope_id" setup while "addr" does.
There's no match even if the packet is coming from the same peer.
There was a problem hiding this comment.
Yep, I am going to update this patch to use net_ipv4_addr_cmp() / net_ipv6_addr_cmp().
mike-scott
force-pushed
the
lwm2m-net-app2
branch
from
5d5b5b6
to
ef460b5
Compare
|
As discussed, updated patch to use net_ipv6_addr_cmp() and net_ipv4_addr_cmp() in find_clients_index(). Diff: https://hastebin.com/eyihiliver.php |
|
Do we want to compare the port number as well? |
|
Bleh. You're right. These functions dont check port. I'll fix that tomorrow. Also, adding an initial check against sa_family != and remove the useless continue block at the end by making eack IP version check a stand alone if block. |
All throughout the LwM2M library we use sockaddr values which are basically the same as the net_app_ctx's remote addr. There's no reason to keep these extra sockaddr values around. The net_app framework client won't accept incoming requests on sockaddr other than the one we're connected to. Signed-off-by: Michael Scott <[email protected]>
This is the first part of a large refactoring of LwM2M library message functions and will simplify observer handling later. Signed-off-by: Michael Scott <[email protected]>
mike-scott
force-pushed
the
lwm2m-net-app2
branch
from
ef460b5
to
644e557
Compare
|
Updated IP address checks as described (included port). diff: https://hastebin.com/ixatulital.php |
|
LGTM |
ghost
removed
the
In progress
…oject-rtos#1473) Signed-off-by: Geoff Gustafson <[email protected]>
This patchset moves the internals of the LwM2M library to the net_app API. To facilitate this we establish an lwm2m_context structure and use this to pass around various objects needed for app control. Also remove / cleanup fields which are now unused as a result.