[riot-notifications] [RIOT-OS/RIOT] Add MUD URL option to DHCPv6 client (#15508)

Jan Romann notifications at github.com
Thu Nov 26 17:06:02 CET 2020


@JKRhb commented on this pull request.

In the review below you can find some aspects that might need some improvement.

> @@ -69,6 +69,7 @@ extern "C" {
                                              *   delegation (IA_PD) option */
 #define DHCPV6_OPT_IAPFX            (26U)   /**< IA prefix option */
 #define DHCPV6_OPT_SMR              (82U)   /**< SOL_MAX_RT option */
+#define DHCPV6_OPT_MUD_URL          (112U)  /**< MUD URL option */

As the option code is not included in RFC 8415, should RFC 8520 be added in the comment or under `@see`?

> @@ -707,6 +718,23 @@ static void _solicit_servers(event_t *event)
     msg_len += _compose_elapsed_time_opt(time);
     msg_len += _compose_oro_opt((dhcpv6_opt_oro_t *)&send_buf[msg_len], oro_opts,
                                 ARRAY_SIZE(oro_opts));
+
+    #ifdef MUD_URL
+    char mud_url[] = MUD_URL;
+    if (strlen(mud_url) <= 253 && strlen(mud_url) > 0) {

253 is used as the max value as [RFC 8520](https://tools.ietf.org/html/rfc8520#section-10) states 

> [t]he entire option MUST NOT exceed 255 octets.

> +ifdef MUD_URL
+  QUOTATION = "\""
+  CFLAGS += -DMUD_URL=$(QUOTATION)$(MUD_URL)$(QUOTATION)
+endif

Is this a good way to include the MUD URL as a CFLAG or is there a more elegant solution?

> +    assert DHCP6OptUnknown in pkt
+    mud_packet = pkt[DHCP6OptUnknown]
+    assert mud_packet.optcode == 112
+    assert mud_packet.optlen == len(MUD_TEST_URL)
+    assert mud_packet.data == MUD_TEST_URL

This is the section where the newly defined `DHCP6OptMudUrl` class can be used instead of `DHCP6OptUnknown` once it is included in a new version of `scapy`.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/15508#pullrequestreview-539424824
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20201126/a2f145e4/attachment.htm>


More information about the notifications mailing list