[riot-notifications] [RIOT-OS/RIOT] sys/net/dhcpv6: Add IA_NA support to the DHCPv6 client (#16228)

Martine Lenders notifications at github.com
Mon Jun 28 14:05:24 CEST 2021


@miri64 requested changes on this pull request.

Some initial review.

> @@ -59,6 +59,13 @@ extern "C" {
 #define CONFIG_DHCPV6_CLIENT_PFX_LEASE_MAX (1U)
 #endif
 
+/**
+ * @brief   Maximum number of leases to be stored
+ */
+#ifndef CONFIG_DHCPV6_CLIENT_LEASE_MAX
+#define CONFIG_DHCPV6_CLIENT_LEASE_MAX (1U)

```suggestion
#define CONFIG_DHCPV6_CLIENT_ADDR_LEASE_MAX (1U)
```
?

> -#define CONFIG_GNRC_NETIF_IPV6_ADDRS_NUMOF    (2)
+#define CONFIG_GNRC_NETIF_IPV6_ADDRS_NUMOF    (3)

Maybe make this dependent on the inclusion of the DHCPv6 IA_NA module?

> -        /* then => tgt_netif->aac_mode = GNRC_NETIF_AAC_DHCP; */
-        DEBUG("nib: would set interface %i to DHCPv6, "
-              "but is not implemented yet", netif->pid);
+
+        /* TODO: How will this flag be used? */
+        netif->ipv6.aac_mode = GNRC_NETIF_AAC_DHCP;

The comment says `tgt_netif`, so that should be used. As to your question: only interfaces with the DHCP flag should use DHCP IA_NA to fetch their address ;-). `aac_mode` means "_A_uto-_A_ddress _C_onfiguration Mode"

> @@ -14,7 +14,7 @@
 
 from scapy.all import AsyncSniffer, sendp, Ether, IPv6, UDP
 from scapy.all import DHCP6_Solicit, DHCP6_Advertise, DHCP6_Request, DHCP6_Reply
-from scapy.all import DHCP6OptClientId, DHCP6OptServerId, DHCP6OptIA_PD, DHCP6OptUnknown
+from scapy.all import DHCP6OptClientId, DHCP6OptServerId, DHCP6OptIA_PD, DHCP6OptMudUrl

This should go into its own commit at least, but since it is _completely_ unrelated, I would even prefer its own PR. This might also be very dependent on the scapy version.

> @@ -161,11 +158,11 @@ def testfunc(child):
     # and is still asking for a prefix delegation
     assert DHCP6OptIA_PD in pkt
 
-    assert DHCP6OptMUD in pkt
-    mud_option = pkt[DHCP6OptMUD]
+    assert DHCP6OptMudUrl in pkt
+    mud_option = pkt[DHCP6OptMudUrl]
     assert mud_option.optcode == 112

For the new PR: The check of the `optcode` is now probably now redunant, as the `DHCP6OptMudUrl` object gets created due to this `optcode`

> +# `gnrc_dhcpv6_client_auto_init` test
+
+This test utilizes [scapy] to test the DHCPv6 client configuration for a
+device which uses auto initialization for requesting a non-temporary address
+from a DHCPv6 server (`IA_NA`).

The name seems a bit weird.. prefix delegation and MUD also use auto initialization. How about calling the test `gnrc_dhcpv6_client_ia_na`?

> -#define CONFIG_GNRC_NETIF_IPV6_ADDRS_NUMOF    (2)
+#define CONFIG_GNRC_NETIF_IPV6_ADDRS_NUMOF    (3)

And maybe make the new functionality an optional module (we can, in a follow-up also make the other features optional, now that more are coming in).

-- 
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/16228#pullrequestreview-693863424
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210628/b3511c10/attachment.htm>


More information about the notifications mailing list