diff mbox

[ovs-dev,v1] ovn-northd: allow DHCPv6 respond multiple IA Address Options

Message ID 1472451352-2293-1-git-send-email-zealokii@gmail.com
State Accepted
Headers show

Commit Message

Zong Kai LI Aug. 29, 2016, 6:15 a.m. UTC 
From: Zongkai LI <zealokii@gmail.com>

This patch tries to allow OVN native DHCPv6 responder to respond multiple
IA Address Options in a DHCPv6 reply message, this will help a lswitch port
to configure all IPv6 addresses belongs to it in DHCPv6 processing.

This fixes issue lswitch port can only get one IPv6 address, when it has
addresses looks like:
addresses : ["fa:16:3e:66:31:ac 10.0.0.6 fd55:cb39:f835:0:f816:3eff:fe66:31ac
              fdad:1234:5678:0:f816:3eff:fe66:31ac"]
and dhcpv6_options it refers has cidr "fd55:cb39:f835::/64". The lswitch port
will get only IPv6 address "fd55:cb39:f835:0:f816:3eff:fe66:31ac" in DHCPv6
processing routine.

Signed-off-by: Zongkai LI <zealokii@gmail.com>
---
 ovn/northd/ovn-northd.c | 79 +++++++++++++++++++++++++++----------------------
 1 file changed, 43 insertions(+), 36 deletions(-)

Comments

Ben Pfaff Sept. 14, 2016, 11:16 p.m. UTC | #1 
On Mon, Aug 29, 2016 at 02:15:52PM +0800, Zong Kai LI wrote:
> From: Zongkai LI <zealokii@gmail.com>
> 
> This patch tries to allow OVN native DHCPv6 responder to respond multiple
> IA Address Options in a DHCPv6 reply message, this will help a lswitch port
> to configure all IPv6 addresses belongs to it in DHCPv6 processing.
> 
> This fixes issue lswitch port can only get one IPv6 address, when it has
> addresses looks like:
> addresses : ["fa:16:3e:66:31:ac 10.0.0.6 fd55:cb39:f835:0:f816:3eff:fe66:31ac
>               fdad:1234:5678:0:f816:3eff:fe66:31ac"]
> and dhcpv6_options it refers has cidr "fd55:cb39:f835::/64". The lswitch port
> will get only IPv6 address "fd55:cb39:f835:0:f816:3eff:fe66:31ac" in DHCPv6
> processing routine.
> 
> Signed-off-by: Zongkai LI <zealokii@gmail.com>

Hi, thanks for working on this.

Can you add a test (or adjust an existing test) to verify that multiple
IPv6 addresses work properly?

Thanks,

Ben.
diff mbox

Patch

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 19f3fb1..d8ca91e 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1852,7 +1852,7 @@  build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip,
 }
 
 static bool
-build_dhcpv6_action(struct ovn_port *op, struct in6_addr *offer_ip,
+build_dhcpv6_action(struct ovn_port *op, struct lport_addresses *offer_ips,
                     struct ds *options_action, struct ds *response_action)
 {
     if (!op->nbsp->dhcpv6_options) {
@@ -1868,11 +1868,20 @@  build_dhcpv6_action(struct ovn_port *op, struct in6_addr *offer_ip,
         free(error);
         return false;
     }
-    struct in6_addr ip6_mask = ipv6_addr_bitxor(offer_ip, &host_ip);
-    ip6_mask = ipv6_addr_bitand(&ip6_mask, &mask);
-    if (!ipv6_mask_is_any(&ip6_mask)) {
-        /* offer_ip doesn't belongs to the cidr defined in lport's DHCPv6
-         * options.*/
+
+    bool dhcpv6_option_match = false;
+    for (size_t i = 0; i != offer_ips->n_ipv6_addrs; i++) {
+        struct in6_addr ip6_mask = ipv6_addr_bitxor(
+            &offer_ips->ipv6_addrs[i].addr, &host_ip);
+        ip6_mask = ipv6_addr_bitand(&ip6_mask, &mask);
+        if (ipv6_mask_is_any(&ip6_mask)) {
+            dhcpv6_option_match = true;
+            break;
+        }
+    }
+    if (!dhcpv6_option_match) {
+        /* None of lswitch port ipv6 addresses belongs to the cidr defined in
+         * lswitch port's DHCPv6 options.*/
         return false;
     }
 
@@ -1896,11 +1905,12 @@  build_dhcpv6_action(struct ovn_port *op, struct in6_addr *offer_ip,
     ipv6_string_mapped(server_ip, &lla);
 
     char ia_addr[INET6_ADDRSTRLEN + 1];
-    ipv6_string_mapped(ia_addr, offer_ip);
+    ds_put_cstr(options_action, REGBIT_DHCP_OPTS_RESULT" = put_dhcpv6_opts(");
+    for (size_t i = 0; i != offer_ips->n_ipv6_addrs; i++) {
+        ipv6_string_mapped(ia_addr, &offer_ips->ipv6_addrs[i].addr);
+        ds_put_format(options_action, "ia_addr = %s, ", ia_addr);
+    }
 
-    ds_put_format(options_action,
-                  REGBIT_DHCP_OPTS_RESULT" = put_dhcpv6_opts(ia_addr = %s, ",
-                  ia_addr);
     struct smap_node *node;
     SMAP_FOR_EACH (node, &op->nbsp->dhcpv6_options->options) {
         ds_put_format(options_action, "%s = %s, ", node->key, node->value);
@@ -2700,33 +2710,30 @@  build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
                 }
             }
 
-            for (size_t j = 0; j < op->lsp_addrs[i].n_ipv6_addrs; j++) {
-                struct ds options_action = DS_EMPTY_INITIALIZER;
-                struct ds response_action = DS_EMPTY_INITIALIZER;
-                if (build_dhcpv6_action(
-                        op, &op->lsp_addrs[i].ipv6_addrs[j].addr,
-                        &options_action, &response_action)) {
-                    struct ds match = DS_EMPTY_INITIALIZER;
-                    ds_put_format(
-                        &match, "inport == %s && eth.src == %s"
-                        " && ip6.dst == ff02::1:2 && udp.src == 546 &&"
-                        " udp.dst == 547", op->json_key,
-                        op->lsp_addrs[i].ea_s);
-
-                    ovn_lflow_add(lflows, op->od, S_SWITCH_IN_DHCP_OPTIONS, 100,
-                                  ds_cstr(&match), ds_cstr(&options_action));
-
-                    /* If REGBIT_DHCP_OPTS_RESULT is set to 1, it means the
-                     * put_dhcpv6_opts action is successful */
-                    ds_put_cstr(&match, " && "REGBIT_DHCP_OPTS_RESULT);
-                    ovn_lflow_add(lflows, op->od, S_SWITCH_IN_DHCP_RESPONSE, 100,
-                                  ds_cstr(&match), ds_cstr(&response_action));
-                    ds_destroy(&match);
-                    ds_destroy(&options_action);
-                    ds_destroy(&response_action);
-                    break;
-                }
+            struct ds options_action = DS_EMPTY_INITIALIZER;
+            struct ds response_action = DS_EMPTY_INITIALIZER;
+            if (build_dhcpv6_action(
+                    op, &op->lsp_addrs[i],
+                    &options_action, &response_action)) {
+                struct ds match = DS_EMPTY_INITIALIZER;
+                ds_put_format(
+                    &match, "inport == %s && eth.src == %s"
+                    " && ip6.dst == ff02::1:2 && udp.src == 546 &&"
+                    " udp.dst == 547", op->json_key,
+                    op->lsp_addrs[i].ea_s);
+
+                ovn_lflow_add(lflows, op->od, S_SWITCH_IN_DHCP_OPTIONS, 100,
+                              ds_cstr(&match), ds_cstr(&options_action));
+
+                /* If REGBIT_DHCP_OPTS_RESULT is set to 1, it means the
+                 * put_dhcpv6_opts action is successful */
+                ds_put_cstr(&match, " && "REGBIT_DHCP_OPTS_RESULT);
+                ovn_lflow_add(lflows, op->od, S_SWITCH_IN_DHCP_RESPONSE, 100,
+                              ds_cstr(&match), ds_cstr(&response_action));
+                ds_destroy(&match);
             }
+            ds_destroy(&options_action);
+            ds_destroy(&response_action);
         }
     }