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

gcoap: add lazy init mode #12529

Closed
wants to merge 2 commits into from
Closed

Conversation

pokgak
Copy link
Contributor

@pokgak pokgak commented Oct 21, 2019

Contribution description

This PR introduces lazy init mode for gcoap. lazy init mode is for applications that want to first do something before gcoap starts its server thread (e.g. loading credential into credman for (D)TLS). It can be enabled by adding USEMODULE += gcoap_lazy_init to the application Makefile

An alternative to this approach would be to just take gcoap_init() out of autoinit and calls it manually from the application but this may break existing applications, so I prefer not to do that if there is an alternative.

How it works

In lazy init mode, the call to gcoap_init() only initializes the _coap_state but does not start the coap server thread. It will be started at the first gcoap_register_listener() call if there is no server that is started already.

As long as there is no server running, NON requests can be sent as usual but the responses will be ignored. Sending a CON request, on the other hand, will fail because a CON request expects to receive a response. After the server is started (gcoap_register_listener() is called), the application should behave like a normal gcoap application.

Testing procedure

For now, I only tested sending NON and CON requests from a lazy init client to a gcoap normal server.

Set BUILD_IN_DOCKER=0 if you are using Docker to build because the env values are not carried into the container for building.

  1. From RIOT base directory: cd examples/gcoap
  2. Start a normal gcoap instance: PORT=tap0 make all term
  3. Start a gcoap in lazy init mode: PORT=tap1 USEMODULE=gcoap_lazy_init make all term
  4. Send a NON from lazy init instance to the normal: coap put <server ip> 5683 /cli/stats 100
  5. Check the output of coap info on the server, cli stats must be 100 now
  6. Send a CON from lazy init instance to the normal: coap put -c <server ip> 5683 /cli/stats 200
  7. Step 5. should fail and the value of /cli/stats at the server should stay 100.

Issues/PRs references

This is part of the work needed to add DTLS support to gcoap. See #12104 (comment).

gcoap lazy init mode is for applications that wants to first do
something before gcoap starts its server thread (e.g. loading credential
into credman for (D)TLS). It can be enabled by adding `gcoap_lazy_init`
to `USEMODULE`.

In lazy init mode, the call to gcoap_init() only initializes relevant
_coap_state but does not starts the coap server thread. The server
will be started at the first gcoap_register_listener() call, if no
server is started already.

As long as there is no server running, NON requests can be sent as usual
but the responses will be ignored. Sending a CON request on the other
hand will fail because a CON request expects to receive a response.
@pokgak
Copy link
Contributor Author

pokgak commented Oct 21, 2019

@kb2ma this is based on your idea at the last summit.
@leandrolanzieri @smlng might be interested in this.

I hope to get feedback on this and if I could proceed with it :)

@benpicco benpicco added Area: network Area: Networking Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 21, 2019
@pokgak pokgak mentioned this pull request Oct 22, 2019
8 tasks
@leandrolanzieri leandrolanzieri added the Area: CoAP Area: Constrained Application Protocol implementations label Oct 24, 2019
@pokgak pokgak force-pushed the pr/gcoap/lazy_init_mode branch from f6967fc to 49d1b9e Compare November 6, 2019 09:18
@kb2ma
Copy link
Member

kb2ma commented Nov 12, 2019

Thanks for adding me as a reviewer. I plan to look at this after feedback on #11036, hopefully within a week. FWIW, I've been in contact with @pokgak offine. ;-)

Copy link
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

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

Some initial comments

makefiles/pseudomodules.inc.mk Outdated Show resolved Hide resolved
sys/net/application_layer/gcoap/gcoap.c Outdated Show resolved Hide resolved
sys/net/application_layer/gcoap/gcoap.c Show resolved Hide resolved
@@ -633,6 +651,15 @@ kernel_pid_t gcoap_init(void)

void gcoap_register_listener(gcoap_listener_t *listener)
{
#ifdef MODULE_GCOAP_LAZY_INIT
Copy link
Contributor

Choose a reason for hiding this comment

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

If a node is not registering additional resources (I know, maybe not the most common of cases), and acts only as a client, then it won't be able to perform CON requests right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup. This is checked in gcoap_req_send(), if a CON request is made, it will fail to send:

#ifdef MODULE_GCOAP_LAZY_INIT
if (msg_type == COAP_TYPE_CON && _pid == KERNEL_PID_UNDEF) {
DEBUG("gcoap: cannot send CON request in lazy init mode\n");
return 0;
}
#endif

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I saw that. Is that a wanted behaviour?: Every node must register at least one listener to be able to send CON requests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could find another way to trigger the server to start

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively, instead of returning an error, the server can be started from gcoap_req_send() when the client wants to send a CON request. The main goal of lazy init is the delayed start of the server until it is really needed. So starting the server from gcoap_req_send() still fits in the requirement.

Copy link
Member

Choose a reason for hiding this comment

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

@kb2ma
Copy link
Member

kb2ma commented Nov 24, 2019

Thanks @pokgak for taking a stab at implementation. It's pretty clear this idea was not my best. :-/ The amount of code and the issues raised show it would be simpler to be more explicit about startup.

I just published an alternative PR, #12798 to accomplish this goal. An application can explicitly start gcoap, but by default it still starts at auto-init time.

@pokgak
Copy link
Contributor Author

pokgak commented Nov 28, 2019

Closed in favor of #12798.

@pokgak pokgak closed this Nov 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants