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

Jan Romann notifications at github.com
Sat Jul 3 17:27:07 CEST 2021


@JKRhb commented on this pull request.

Here are some more aspects that might require discussion

> @@ -119,7 +132,10 @@ static void *_thread(void *args)
     event_queue_t event_queue;
     event_queue_init(&event_queue);
     dhcpv6_client_init(&event_queue, SOCK_ADDR_ANY_NETIF);
-    /* TODO: add configuration for IA_NA here, when implemented */

Should the auto-initialization also cover IA_PD? 

> -    default 2
+    default 3
     help
         If you change this, please make sure that
         @ref CONFIG_GNRC_NETIF_IPV6_GROUPS_NUMOF is also large enough to fit the
         addresses' solicited nodes multicast addresses.
-        Default: 2 (1 link-local + 1 global address).
+        Default: 3 (1 link-local + 2 global addresses).

I guess these changes should be replaced with a the possibility to configure `GNRC_NETIF_IPV6_DHCPV6_IA_NA_ADDR`?

> @@ -101,6 +101,18 @@ extern "C" {
 #define GNRC_NETIF_IPV6_RTR_ADDR   (0)
 #endif
 
+/**
+ * @brief   Number of global unicast addresses needed for a
+ *          @ref net_dhcpv6_client "DHCPv6 Client" using IA_NA
+ *
+ * @note    Used for calculation of @ref CONFIG_GNRC_NETIF_IPV6_ADDRS_NUMOF
+ */
+#ifdef MODULE_GNRC_DHCPV6_CLIENT_IA_NA
+#define GNRC_NETIF_IPV6_DHCPV6_IA_NA_ADDR   (1)

Should a `NUMOF` suffix be added to this identifier?

> @@ -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_ADDR_LEASE_MAX
+#define CONFIG_DHCPV6_CLIENT_ADDR_LEASE_MAX (1U)

Should the number of possible leases depend on `GNRC_NETIF_IPV6_DHCPV6_IA_NA_ADDR` (or vice versa)?

> +                        char addr_str[IPV6_ADDR_MAX_STR_LEN];
+
+                        if (&lease->addr != NULL &&
+                            !ipv6_addr_equal(&lease->addr, addr)) {
+                            /* A different address has been leased to the client */
+                            netif_set_opt(netif, NETOPT_IPV6_ADDR_REMOVE, 64 << 8, &lease->addr, sizeof(lease->addr));
+                        }
+
+                        lease->leased = 1U;
+                        memcpy(&lease->addr, addr, sizeof(ipv6_addr_t));
+                        DEBUG("DHCPv6 client: ADD IP ADDRESS %s\n",
+                            ipv6_addr_to_str(addr_str, addr, sizeof(addr_str)));
+
+                        if (netif_set_opt(netif, NETOPT_IPV6_ADDR, 64 << 8, addr, sizeof(*addr)) > 0) {

This is the current logic I used for adding IP addresses obtained via IA_NA to the net interface. Should this rather be moved somewhere else as it is the case with IA_PD and `dhcpv6_client_conf_prefix`?

-- 
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-698600159
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210703/91a97ca5/attachment.htm>


More information about the notifications mailing list