sys/net/application_layer/gcoap: fix Observe notifications correlation

[fabian18]

Contribution description

When a CONfirmable CoAP request with an Observe option is sent, the sending client has to store the CoAP token to match multiple responses from the server sending Observe notifications.
The gcoap memo is not cleared when the 2.xx response for the client registration contains an Observe option.

RIOT/sys/net/application_layer/gcoap/gcoap.c

Lines 521 to 531 in 3f41494

	if (memo->send_limit >= 0) { /* if confirmable */
	*memo->msg.data.pdu_buf = 0; /* clear resend PDU buffer */
	}
	/* The memo must be kept if the response is an observe notification.
	* Non-2.xx notifications indicate that the associated observe entry
	* was removed on the server side. Then also free the memo here. */
	if (!observe_notification || (code_class != COAP_CLASS_SUCCESS)) {
	/* setting the state to unused frees (drops) the memo entry */
	memo->state = GCOAP_MEMO_UNUSED;
	}

For a confirmable request with a token length >0, the problem happens right before, where the resend buffer is released by actually overwriting the first byte of the request header in pdu_buf with 0. The first byte contains the token length which is required to match in:

RIOT/sys/net/application_layer/gcoap/gcoap.c

Lines 888 to 914 in 3f41494

	static gcoap_request_memo_t* _find_req_memo_by_token(const sock_udp_ep_t *remote,
	const uint8_t *token, size_t tkl)
	{
	/* no need to initialize struct; we only care about buffer contents below */
	coap_pkt_t memo_pdu_data;
	coap_pkt_t *memo_pdu = &memo_pdu_data;
	for (int i = 0; i < CONFIG_GCOAP_REQ_WAITING_MAX; i++) {
	if (_coap_state.open_reqs[i].state == GCOAP_MEMO_UNUSED) {
	continue;
	}
	gcoap_request_memo_t *memo = &_coap_state.open_reqs[i];
	memo_pdu->hdr = gcoap_request_memo_get_hdr(memo);
	if (coap_get_token_len(memo_pdu) == tkl) {
	if ((memcmp(token, coap_get_token(memo_pdu), tkl) == 0)
	&& (sock_udp_ep_equal(&memo->remote_ep, remote)
	/* Multicast addresses are not considered in matching responses */
	|| sock_udp_ep_is_multicast(&memo->remote_ep)
	)) {
	return memo;
	}
	}
	}
	return NULL;
	}

There, the overwritten request header is accessed in gcoap_request_memo_get_hdr().

RIOT/sys/include/net/gcoap.h

Lines 1182 to 1190 in 3f41494

	static inline coap_hdr_t *gcoap_request_memo_get_hdr(const gcoap_request_memo_t *memo)
	{
	if (memo->send_limit == GCOAP_SEND_LIMIT_NON) {
	return (coap_hdr_t *)&memo->msg.hdr_buf[0];
	}
	else {
	return (coap_hdr_t *)memo->msg.data.pdu_buf;
	}
	}

Solution:

When a response is received, the retransmission buffer is released, but before the header is copied to hdr_buf in:

RIOT/sys/include/net/gcoap.h

Lines 809 to 833 in 3f41494

	struct gcoap_request_memo {
	unsigned state; /**< State of this memo, a GCOAP_MEMO... */
	int send_limit; /**< Remaining resends, 0 if none;
	GCOAP_SEND_LIMIT_NON if non-confirmable */
	union {
	uint8_t hdr_buf[GCOAP_HEADER_MAXLEN];
	/**< Copy of PDU header, if no resends */
	gcoap_resend_t data; /**< Endpoint and PDU buffer, for resend */
	} msg; /**< Request message data; if confirmable,
	supports resending message */
	sock_udp_ep_t remote_ep; /**< Remote endpoint */
	gcoap_resp_handler_t resp_handler; /**< Callback for the response */
	void *context; /**< ptr to user defined context data */
	event_timeout_t resp_evt_tmout; /**< Limits wait for response */
	event_callback_t resp_tmout_cb; /**< Callback for response timeout */
	gcoap_socket_t socket; /**< Transport type to remote endpoint */
	#if IS_USED(MODULE_NANOCOAP_CACHE) || DOXYGEN
	/**
	* @brief Cache key for the request
	*
	* @note Only available with module ['nanocoap_cache'](@ref net_nanocoap_cache)
	*/
	uint8_t cache_key[CONFIG_NANOCOAP_CACHE_KEY_LENGTH];
	#endif
	};

Testing procedure

Internal project. CoAP Observe is not so well tested. I hope the explanation makes sense.
In examples/gcoap/client.c:

RIOT/examples/gcoap/client.c

Lines 397 to 408 in 3f41494

	if (_send(&buf[0], len, rem, NULL, tl) <= 0) {
	puts("gcoap_cli: msg send failed");
	}
	else {
	if (observe) {
	/* on successful observe request, store that this node is
	* observing / not observing anymore */
	observing = obs_value == COAP_OBS_REGISTER;
	}
	/* send Observe notification for /cli/stats */
	notify_observers();
	}

the notification is sent right after the registration was sent. I have not tried that.

Issues/PRs references

Depends on and includes #20711

[riot-ci]

Murdock results

✔️ PASSED

21a46dc examples/cord_lc: adapt Makefile.ci

Success	Failures	Total	Runtime
10193	0	10193	14m:07s

Artifacts

• Documentation preview

[fabian18]

Of course the Observe notification must be sent from the same address as well ...
I am at it to add it here.

RIOT/sys/net/application_layer/gcoap/gcoap.c

Lines 1824 to 1837 in 230df5f

	size_t gcoap_obs_send(const uint8_t *buf, size_t len,
	const coap_resource_t *resource)
	{
	gcoap_observe_memo_t *memo = NULL;
	_find_obs_memo_resource(&memo, resource);
	if (memo) {
	ssize_t bytes = _tl_send(&memo->socket, buf, len, memo->observer, NULL);
	return (size_t)((bytes > 0) ? bytes : 0);
	}
	else {
	return 0;
	}
	}

[fabian18]

Depends on #20711 now due to aux tx local supply in _handle_req()

[fabian18]

for better testing I added an /rtc resource to the gcoap example.
Use our tapsetup.sh and observe the notifications in wireshark for two native instances.

[benpicco]

A rebase would be good to only have the changes from this PR visible

[fabian18]

Only one observer per resource is supported:

 * A CoAP client may register for Observe notifications for any resource that
 * an application has registered with gcoap. An application does not need to
 * take any action to support Observe client registration. However, gcoap
 * limits registration for a given resource to a _single_ observer.

Because, given a request, identifying a resource we look for only one gcoap_observe_memo_t that has a pointer to that resource. See

RIOT/sys/net/application_layer/gcoap/gcoap.c

Line 670 in c2efa3e

_find_obs_memo_resource(&resource_memo, resource);

[fabian18]

Maybe there is a CoAP way to tell the client, that Observe cannot be honored for the reason that another client is observing the resource currently.

[benpicco]

Please squash directly

[fabian18]

Maybe there is a CoAP way to tell the client, that Observe cannot be honored for the reason that another client is observing the resource currently.

From RFC7641

A server that is unable or unwilling to add a new entry to the list
of observers of a resource MAY silently ignore the registration
request and process the GET request as usual. The resulting response
MUST NOT include an Observe Option, the absence of which signals to
the client that it will not be notified of changes to the resource
and, e.g., needs to poll the resource for its state instead.

[benpicco]

Looks like Makefile.ci needs an update - make generate-Makefile.ci should do

[fabian18]

I executed make -C examples/gcoap generate-Makefile.ci but actually it only purged boards from the list because I guess the builds are failing in the Docker container due to permission issue:
Example:

Launching build container using image "sha256:f5951bc41dfface6cac869181d703e62cbdd3b7976b0946130a38f2e658000b3".
docker run --rm --tty --user $(id -u) -v '/usr/share/zoneinfo/Europe/Berlin:/etc/localtime:ro' -v '/home/fabian.huessler@ml-pa.loc/RIOT:/data/riotbuild/riotbase:delegated' -v '/home/fabian.huessler@ml-pa.loc/.cargo/registry:/data/riotbuild/.cargo/registry:delegated' -v '/home/fabian.huessler@ml-pa.loc/.cargo/git:/data/riotbuild/.cargo/git:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -e 'BUILD_DIR=/data/riotbuild/riotbase/build' -e 'BUILD_IN_DOCKER=/data/riotbuild/riotbase/examples/gcoap/1' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles'      -e 'BOARD=stm32f469i-disco' -e 'RIOT_CI_BUILD=1' -e 'BOARD=stm32f469i-disco' -e 'RIOT_CI_BUILD=1' -e 'DISABLE_MODULE=' -e 'DEFAULT_MODULE=' -e 'FEATURES_REQUIRED=' -e 'FEATURES_BLACKLIST=' -e 'FEATURES_OPTIONAL=periph_rtc' -e 'USEMODULE=auto_init_gnrc_netif fmt gcoap gnrc_icmpv6_echo gnrc_ipv6_default netdev_default netutils od ps random shell shell_cmds_default uri_parser' -e 'USEPKG=' -e 'DISABLE_MODULE=' -e 'DEFAULT_MODULE=' -e 'FEATURES_REQUIRED=' -e 'FEATURES_BLACKLIST=' -e 'FEATURES_OPTIONAL=periph_rtc' -e 'USEMODULE=auto_init_gnrc_netif fmt gcoap gnrc_icmpv6_echo gnrc_ipv6_default netdev_default netutils od ps random shell shell_cmds_default uri_parser' -e 'USEPKG='  -w '/data/riotbuild/riotbase/examples/gcoap/' 'sha256:f5951bc41dfface6cac869181d703e62cbdd3b7976b0946130a38f2e658000b3' make 'BOARD=stm32f469i-disco' 'BOARD=stm32f469i-disco'   -j16  all
Building application "gcoap_example" for "stm32f469i-disco" with CPU "stm32".

Traceback (most recent call last):
  File "/data/riotbuild/riotbase/cpu/stm32/dist/irqs/gen_vectors.py", line 147, in <module>
    main(PARSER.parse_args())
  File "/data/riotbuild/riotbase/cpu/stm32/dist/irqs/gen_vectors.py", line 139, in main
    generate_vectors(context)
  File "/data/riotbuild/riotbase/cpu/stm32/dist/irqs/gen_vectors.py", line 132, in generate_vectors
    with open(dest_file, "w") as f_dest:
PermissionError: [Errno 13] Permission denied: '/data/riotbuild/riotbase/cpu/stm32/vectors/STM32F469xx.c'
make: *** [/data/riotbuild/riotbase/cpu/stm32/Makefile.include:109: /data/riotbuild/riotbase/cpu/stm32/vectors/STM32F469xx.c] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "/data/riotbuild/riotbase/cpu/stm32/dist/irqs/gen_irqs.py", line 155, in <module>
    main(PARSER.parse_args())
  File "/data/riotbuild/riotbase/cpu/stm32/dist/irqs/gen_irqs.py", line 147, in main
    generate_irqs(context)
  File "/data/riotbuild/riotbase/cpu/stm32/dist/irqs/gen_irqs.py", line 130, in generate_irqs
    with open(dest_file, "w") as f_dest:
PermissionError: [Errno 13] Permission denied: '/data/riotbuild/riotbase/cpu/stm32/include/irqs/f4/irqs.h'
make: *** [/data/riotbuild/riotbase/cpu/stm32/Makefile.include:123: /data/riotbuild/riotbase/cpu/stm32/include/irqs/f4/irqs.h] Error 1
make[1]: *** [/home/fabian.huessler@ml-pa.loc/RIOT/makefiles/docker.inc.mk:391: ..in-docker-container] Error 2
make[1]: Leaving directory '/home/fabian.huessler@ml-pa.loc/RIOT/examples/gcoap'
stm32f4discovery                        build failed

[fabian18]

I executed make -C examples/gcoap generate-Makefile.ci but actually it only purged boards from the list because I guess the builds are failing in the Docker container due to permission issue: Example:

Launching build container using image "sha256:f5951bc41dfface6cac869181d703e62cbdd3b7976b0946130a38f2e658000b3".
docker run --rm --tty --user $(id -u) -v '/usr/share/zoneinfo/Europe/Berlin:/etc/localtime:ro' -v '/home/fabian.huessler@ml-pa.loc/RIOT:/data/riotbuild/riotbase:delegated' -v '/home/fabian.huessler@ml-pa.loc/.cargo/registry:/data/riotbuild/.cargo/registry:delegated' -v '/home/fabian.huessler@ml-pa.loc/.cargo/git:/data/riotbuild/.cargo/git:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -e 'BUILD_DIR=/data/riotbuild/riotbase/build' -e 'BUILD_IN_DOCKER=/data/riotbuild/riotbase/examples/gcoap/1' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles'      -e 'BOARD=stm32f469i-disco' -e 'RIOT_CI_BUILD=1' -e 'BOARD=stm32f469i-disco' -e 'RIOT_CI_BUILD=1' -e 'DISABLE_MODULE=' -e 'DEFAULT_MODULE=' -e 'FEATURES_REQUIRED=' -e 'FEATURES_BLACKLIST=' -e 'FEATURES_OPTIONAL=periph_rtc' -e 'USEMODULE=auto_init_gnrc_netif fmt gcoap gnrc_icmpv6_echo gnrc_ipv6_default netdev_default netutils od ps random shell shell_cmds_default uri_parser' -e 'USEPKG=' -e 'DISABLE_MODULE=' -e 'DEFAULT_MODULE=' -e 'FEATURES_REQUIRED=' -e 'FEATURES_BLACKLIST=' -e 'FEATURES_OPTIONAL=periph_rtc' -e 'USEMODULE=auto_init_gnrc_netif fmt gcoap gnrc_icmpv6_echo gnrc_ipv6_default netdev_default netutils od ps random shell shell_cmds_default uri_parser' -e 'USEPKG='  -w '/data/riotbuild/riotbase/examples/gcoap/' 'sha256:f5951bc41dfface6cac869181d703e62cbdd3b7976b0946130a38f2e658000b3' make 'BOARD=stm32f469i-disco' 'BOARD=stm32f469i-disco'   -j16  all
Building application "gcoap_example" for "stm32f469i-disco" with CPU "stm32".

Traceback (most recent call last):
  File "/data/riotbuild/riotbase/cpu/stm32/dist/irqs/gen_vectors.py", line 147, in <module>
    main(PARSER.parse_args())
  File "/data/riotbuild/riotbase/cpu/stm32/dist/irqs/gen_vectors.py", line 139, in main
    generate_vectors(context)
  File "/data/riotbuild/riotbase/cpu/stm32/dist/irqs/gen_vectors.py", line 132, in generate_vectors
    with open(dest_file, "w") as f_dest:
PermissionError: [Errno 13] Permission denied: '/data/riotbuild/riotbase/cpu/stm32/vectors/STM32F469xx.c'
make: *** [/data/riotbuild/riotbase/cpu/stm32/Makefile.include:109: /data/riotbuild/riotbase/cpu/stm32/vectors/STM32F469xx.c] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "/data/riotbuild/riotbase/cpu/stm32/dist/irqs/gen_irqs.py", line 155, in <module>
    main(PARSER.parse_args())
  File "/data/riotbuild/riotbase/cpu/stm32/dist/irqs/gen_irqs.py", line 147, in main
    generate_irqs(context)
  File "/data/riotbuild/riotbase/cpu/stm32/dist/irqs/gen_irqs.py", line 130, in generate_irqs
    with open(dest_file, "w") as f_dest:
PermissionError: [Errno 13] Permission denied: '/data/riotbuild/riotbase/cpu/stm32/include/irqs/f4/irqs.h'
make: *** [/data/riotbuild/riotbase/cpu/stm32/Makefile.include:123: /data/riotbuild/riotbase/cpu/stm32/include/irqs/f4/irqs.h] Error 1
make[1]: *** [/home/fabian.huessler@ml-pa.loc/RIOT/makefiles/docker.inc.mk:391: ..in-docker-container] Error 2
make[1]: Leaving directory '/home/fabian.huessler@ml-pa.loc/RIOT/examples/gcoap'
stm32f4discovery                        build failed

Had to delete cpu/stm32/vectors/* and cpu/stm32/include/irqs/*