Message ID duplication not detected

[EduardPon]

It seems that duplicated message ID are also processed. In our case the LWM2M client callback is executed for all duplicated POST messages.

Copied from The Constrained Application Protocol (CoAP) June 2014:

A recipient might receive the same Confirmable message (as indicated by the Message ID and source endpoint) multiple times within the EXCHANGE_LIFETIME (Section 4.8.2), for example, when its Acknowledgement went missing or didn’t reach the original sender before the first timeout. The recipient SHOULD acknowledge each duplicate copy of a Confirmable message using the same Acknowledgement or Reset message but SHOULD process any request or response in the message only once. This rule MAY be relaxed in case the Confirmable message transports a request that is idempotent (see Section 5.1) or can be handled in an idempotent fashion.

openais-mbed-philips 0.1.0 | mbed-client 1.12.2 | | mbed-client-c 2.6.0 yottamodules\mbed-client-c | | nanostack-libservice 3.5.1 yottamodules\nanostack-libservice | | mbed-trace 1.1.3 yottamodules\mbed-trace | | mbed-client-mbed-os 3.0.0 yotta_modules\mbed-client-mbed-os

[ciarmcom]

ARM Internal Ref: IOTCLT-760

[anttiylitokola]

Hi, thanks for your report. We will look into this and get back to you when we have reproduced and investigated this issue.

[markus-becker-tridonic-com]

@EduardPon have you set SN_COAP_DUPLICATION_MAX_MSGS_COUNT? this looks to me, that it enables message de-duplication. see https://github.com/ARMmbed/mbed-client-c/blob/master/source/libCoap/src/sn_coap_protocol.c#L50

[EduardPon]

No I haven't. I was not aware about this definition.

Strange that this is disabled by default because it violates the CoAP standard. I would suggest to remove this switchable option and enable the duplication detection.

[yogpan01]

Hi, This is an option we can enable from configuration so that application can turn it on or off based on their requirement. I will add this to our backlog to provide configuration for enabling this flag. We have similar configuration for BLOCKWISE transfer.

[EduardPon]

Hello, @yogpan01, can you confirm this configuration option deletes duplicates and fixes the reason why I have opened this issue report.

What I don't understand is why it has a maximum count value? I would expect to enable or disable duplicate discarding.

Why would you allow to disable it and hereby violate the CoAP specification?

The recipient SHOULD acknowledge each duplicate copy of a Confirmable message using the same Acknowledgement or Reset message but SHOULD process any request or response in the message only once.

[yogpan01]

Hi, I checked and found that configuration is already present for enabling/disabling duplication logic. https://docs.mbed.com/docs/mbed-client-guide/en/latest/Introduction/ Check section "CoAP message deduplication" Defining non-zero value for this setting enables this feature, the count is mainly coap library implementation for keeping track of "n" number of duplicate messages.

[EduardPon]

Thanks for the info. But what I have understood, this relates to the duplication of messages to be transmitted, and not detecting duplicates at receiver side. These must only be acknowledged and don't be processed again. The current implementation processes every duplicate at the receiver side (CoAP spec violation).

[yogpan01]

Hi, So in case the duplicate message is detected , this is how it is currently handled https://github.com/ARMmbed/mbed-client-c/blob/master/source/libCoap/src/sn_coap_protocol.c#L664

/* Set returned status to User _/ returned_dst_coap_msg_ptr->coap_status = COAP_STATUS_PARSER_DUPLICATEDMSG; / Because duplicate message, return with coap_status set */ return returned_dst_coap_msg_ptr;

Perhaps , we should check this status in our end before invoking callback to application. Are you using mbed Client C++ API for your application ? We can add logic of checking this status code and avoid processing of these messages.

[EduardPon]

Hello,

Yes we are using the C++ API.

[yogpan01]

Thanks ! We will check now, how to handle this case safely on C++ side. I will keep you updated, hopefully we need not merge your PR if we are able to make the fix working from our end.

[yogpan01]

Hi @EduardPon , if you can try this PR https://github.com/ARMmbed/mbed-client/pull/251 to verify that duplication detection is working for you. We will merge and release the update.

[EduardPon]

Hello,

I tested the implementation change https://github.com/ARMmbed/mbed-client/pull/251, but not in the M2MNsdlInterface but in our NSDL CoAP resource_callback function. I did not get a COAP_STATUS_PARSER_DUPLICATED_MSG coap_status. So enabled the SN_COAP_DUPLICATION_MAX_MSGS_COUNT. Still the COAP_STATUS_PARSER_DUPLICATED_MSG coap_status is not returned, BUT the duplicates are discarded. So I would suggest to enable the COAP_STATUS_PARSER_DUPLICATED_MSG by default to be compliant with the CoAP specification. I know there is a resource penalty. I would suggest to add a remark about the CoAP non conformance when disabling the COAP_STATUS_PARSER_DUPLICATED_MSG.

Summary:

• The change https://github.com/ARMmbed/mbed-client/pull/251 is not needed.
• Enable the SN_COAP_DUPLICATION_MAX_MSGS_COUNT to discard duplicates.
• Add remark CoAP non conformance when SN_COAP_DUPLICATION_MAX_MSGS_COUNT is not enabled.

[yogpan01]

Hi, Thanks for the confirmation. mbed-client-c is internally handling the duplicated message so we will not need to handle it on resource level. We will check if there is any issue with enabling the duplication by default, hopefully it should not break anything but we would need to confirm this.