sys/net/nanocoap: Implement Observe (Server-Side) #21147

maribu · 2025-01-20T22:55:55Z

Contribution description

Clean up and extend the separate response feature of nanocoap
Add a bit of glue to use the separate response feature to implement observe

Testing procedure

Starting the Server

$ sudo ip tuntap add tap0 mode tap user $(whoami)
$ sudo ip link set tap0 up
$ make BOARD=native64 -C examples/nanocoap_server -j flash term

Running a Client

$ coap-client-notls -s 60 -m get 'coap://[fe80::9826:30ff:feb8:31f4%tap0]/time'
8326
9000
10001
11000
12001
13000
14000
15001
16000
17001
18000
19000
20001
21000
22000
[...]

Issues/PRs references

None

riot-ci · 2025-01-21T05:03:35Z

Murdock results

✔️ PASSED

b045d8c fixup! fixup! fixup! sys/net/nanocoap: implement observe

Success	Failures	Total	Runtime
10270	0	10271	08m:46s

Artifacts

Documentation preview

mguetschow

I just had a high-level look at it and have some nit's below. In general I don't feel confident enough in nanocoap_sock to tell whether this takes all nanocoap quirks into account, but the implementation looks good to me in general.

examples/nanocoap_server/coap_handler.c

mguetschow · 2025-01-21T10:22:17Z

examples/nanocoap_server/coap_handler.c

+        + 1 /* payload marker */
+        + 10 /* strlen("4294967295"), 4294967295 == UINT32_MAX */
+        + 1 /* '\n' */;
+    ssize_t hdr_len = coap_build_reply(pkt, COAP_CODE_CONTENT, buf, len, estimated_data_len);


Funny that payload_len includes the option length o.O

We should probably rename the variable. I doubt we are the first to be fooled by the misleading name.

sys/include/net/nanocoap_sock.h

mguetschow · 2025-01-21T10:34:40Z

sys/net/application_layer/nanocoap/nanocoap.c

+    if (IS_USED(MODULE_NANOCOAP_SERVER_OBSERVE) && (coap_get_type(pkt) == COAP_TYPE_RST)) {
+        nanocoap_unregister_observer_by_udp_ep(coap_request_ctx_get_remote_udp(ctx));


Hum, shouldn't that only apply when the pkt is a response to a notification on a given resource? https://datatracker.ietf.org/doc/html/rfc7641#section-3.5

Yes, but that would require state to track this. This behavior would be in spec, as a server MUST drop the client on a RST for a notification. It would overshoot in case the RST is unrelated. But that would be the same as the sever loosing state for other reasons (e.g. due to power cycling).

I'll add a comment.

On the other hand, it is 2 bytes of state to add per observation. I guess that is acceptable.

I added the message ID of the most recently sent notification to the state.

Keeping only the message ID of the last send notification can result in the client sending a few more RST messages (e.g. because the RST arrives after the next notification is already send out). But the client will just assume the RST got lost on the network and will keep sending RST until the registration is dropped.

On the other hand, it is 2 bytes of state to add per observation. I guess that is acceptable.

Probably not relevant to nanocoap, but keep in mind that CoAP over BP opens the can of worms that the message ID size might be more more than 2 bytes 😅. Larger distance (such as in interplanetary communication) means higher latency means more packets in the air means larger number space for messaging required.

mguetschow · 2025-01-21T10:36:22Z

sys/net/application_layer/nanocoap/sock.c

+     * is probably a good thing. */
+    (void)pkt;
+
+    return sock_udp_ep_equal(&ctx->remote, req->remote);


This simple check should probably be reflected in the API documentation for now.

This is actually the same check that has been used before:

RIOT/examples/nanocoap_server/coap_handler.c

Lines 207 to 210 in 297f7fe

if (event_timeout_is_pending(&event_timeout) && !sock_udp_ep_equal(context->remote, &_separate_ctx.remote)) {

puts("_separate_handler(): response already scheduled");

return coap_build_reply(pkt, COAP_CODE_SERVICE_UNAVAILABLE, buf, len, 0);

}

The new API would allow to also inspect the packet, but we have no real duplicate detection anywhere else in RIOT. Doing so correctly would require tracking some message IDs, and we don't have the code infrastructure for this. At least this would be a step in that direction by having the API prepared for this.

But given NSTART=1, I'm not sure if the overhead for tracking duplicates actually is worth it.

sys/net/application_layer/nanocoap/sock.c

MrKevinWeiss · 2025-01-21T13:50:42Z

Freeze is over so if anyone wants to pick this up...

This allows sending a separate response with CoAP Options and adds a helper to detect duplicate requests, so that resource handlers can repeat their empty ACK on duplicates.

This adds the new `nanocoap_server_observe` module that implements the server side of the CoAP Observe option. It does require cooperation from the resource handler to work, though. Co-Authored-By: mguetschow <[email protected]>

sys/include/net/nanocoap_sock.h

maribu added Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 20, 2025

github-actions bot added Area: network Area: Networking Area: CoAP Area: Constrained Application Protocol implementations Area: sys Area: System Area: examples Area: Example Applications labels Jan 20, 2025

maribu force-pushed the sys/net/nanocoap/observe branch 2 times, most recently from f0a998f to 124752f Compare January 21, 2025 08:06

maribu marked this pull request as ready for review January 21, 2025 08:06

maribu requested review from miri64 and PeterKietzmann as code owners January 21, 2025 08:06

maribu requested review from chrysn, benpicco and mguetschow January 21, 2025 08:07

maribu force-pushed the sys/net/nanocoap/observe branch 2 times, most recently from a3e9b1c to c4de045 Compare January 21, 2025 08:41

maribu added the State: waiting for end of feature-freeze label Jan 21, 2025

MrKevinWeiss added the Process: blocked by feature freeze Integration Process: The impact of this PR is too high for merging it during soft feature freeze. label Jan 21, 2025

maribu removed the State: waiting for end of feature-freeze label Jan 21, 2025

mguetschow reviewed Jan 21, 2025

View reviewed changes

MrKevinWeiss removed the Process: blocked by feature freeze Integration Process: The impact of this PR is too high for merging it during soft feature freeze. label Jan 21, 2025

maribu and others added 2 commits January 21, 2025 16:34

sys/net/nanocoap: improve separate response handling

f8872c1

This allows sending a separate response with CoAP Options and adds a helper to detect duplicate requests, so that resource handlers can repeat their empty ACK on duplicates.

sys/net/nanocoap: implement observe

26f4f06

This adds the new `nanocoap_server_observe` module that implements the server side of the CoAP Observe option. It does require cooperation from the resource handler to work, though. Co-Authored-By: mguetschow <[email protected]>

maribu force-pushed the sys/net/nanocoap/observe branch from c4de045 to 26f4f06 Compare January 21, 2025 15:38

maribu commented Jan 21, 2025

View reviewed changes

sys/include/net/nanocoap_sock.h Outdated Show resolved Hide resolved

maribu added 3 commits January 21, 2025 21:59

fixup! sys/net/nanocoap: implement observe

a42b672

fixup! fixup! sys/net/nanocoap: implement observe

91d3f3e

fixup! fixup! fixup! sys/net/nanocoap: implement observe

b045d8c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sys/net/nanocoap: Implement Observe (Server-Side) #21147

sys/net/nanocoap: Implement Observe (Server-Side) #21147

maribu commented Jan 20, 2025

riot-ci commented Jan 21, 2025 •

edited

Loading

mguetschow left a comment

mguetschow Jan 21, 2025

maribu Jan 21, 2025

mguetschow Jan 21, 2025

maribu Jan 21, 2025

maribu Jan 21, 2025

maribu Jan 21, 2025

miri64 Jan 21, 2025

mguetschow Jan 21, 2025

maribu Jan 21, 2025

MrKevinWeiss commented Jan 21, 2025

		if (IS_USED(MODULE_NANOCOAP_SERVER_OBSERVE) && (coap_get_type(pkt) == COAP_TYPE_RST)) {
		nanocoap_unregister_observer_by_udp_ep(coap_request_ctx_get_remote_udp(ctx));

	if (event_timeout_is_pending(&event_timeout) && !sock_udp_ep_equal(context->remote, &_separate_ctx.remote)) {
	puts("_separate_handler(): response already scheduled");
	return coap_build_reply(pkt, COAP_CODE_SERVICE_UNAVAILABLE, buf, len, 0);
	}

sys/net/nanocoap: Implement Observe (Server-Side) #21147

Are you sure you want to change the base?

sys/net/nanocoap: Implement Observe (Server-Side) #21147

Conversation

maribu commented Jan 20, 2025

Contribution description

Testing procedure

Starting the Server

Running a Client

Issues/PRs references

riot-ci commented Jan 21, 2025 • edited Loading

Murdock results

Artifacts

mguetschow 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

Choose a reason for hiding this comment

MrKevinWeiss commented Jan 21, 2025

riot-ci commented Jan 21, 2025 •

edited

Loading