tests/gnrc_rpl_srh: fix test assumption#12440
Conversation
If the destination address or an address within the source route is multicast within a RPL source routing header, a receiving node is supposed to just discard the packets, but not to send an ICMPv6 error message, as the test assumes at the moment. Source: https://tools.ietf.org/html/rfc6554#section-4.2
858ce4e to
ee2126a
Compare
A node is not supposed to send an ICMPv6 error message when the destination or one of the addresses in the source route is multicast but is supposed to be silently discarded (see [RFC4443] and [RFC6554]). If we leave the `err_ptr` unset, the [node will not send an error message][err_ptr set]. [RFC 4443]: https://tools.ietf.org/html/rfc4443#section-2.4 [RFC 6554]: https://tools.ietf.org/html/rfc6554#section-4.2 [err_ptr set]: https://github.com/RIOT-OS/RIOT/blob/9bc600a/sys/net/gnrc/network_layer/ipv6/ext/rh/gnrc_ipv6_ext_rh.c#L100-L105
See #12442 for that. |
cgundogan
left a comment
There was a problem hiding this comment.
The change matches the spec [1] now 👍. However, what is the reasoning behind modifying the test to include UDP headers in the test packets? At first sight, the change to UDP appears unrelated to the fix, but there's probably something behind that? Though, I cannot find any hint neither in the code doc, nor in the commit message.
Mainly to check if something is forwarded (in the error case). The routing header and the IPv6 header both change so they are no guaranteed point of reference. The added UDP header provides that with the (randomly selected) destination port. |
cgundogan
left a comment
There was a problem hiding this comment.
Just some nitpicking and I do not have a strong opinion on this, but maybe you could use a number from the dynamic and/or private port range [1] instead of the randomly chosen 2606.
I leave the decision to you. ACK to the core change of this PR.
[1] https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml
|
Isn't using a User Port just as valid? The dynamic range is mainly for ephemeral ports. As this is a "fantasy application" using 2602 as specific port might be just as valid. Especially since I don't bind the port (neither on the node nor the host), there should be no worries about using that port. |
|
(fun fact: |
|
Or do you want me to change it because of that so the DNS "reply" is to an ephemeral port? |
If yes, I still disagree with changing since specifically for testing it is important to send unexpectedly formatted or valued packets ;-). |
|
Alright, was just a suggestion (: GO |
|
Backport provided in #12443 |
Contribution description
If the destination address or an address within the source route is multicast within a RPL source routing header, a receiving node is supposed to just discard the packets, but not to send an ICMPv6 error message, as the test assumes at the moment.
Source: https://tools.ietf.org/html/rfc6554#section-4.2
There is also a fix that prevents the implementation from sending such an error message: If we leave the
err_ptrunset in these cases, the node will not send an error message.Testing procedure
Read the algorithm provided above, especially the section regarding multicast:
The test cases that check multicast addresses in
tests/gnrc_rpl_srhshould now pass (the test in general does not pass completely (but will, when #12442 is merged).Issues/PRs references
Fixes one of the issues in #12436
Accompanies #12442