diff mbox series

[ovs-dev,v5,10/14] northd: Fix LSP incremental processing if dhcp options are set.

Message ID 20230809163742.1561608-1-numans@ovn.org
State Superseded
Headers show
Series northd: I-P for load balancer and lb groups | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Numan Siddique Aug. 9, 2023, 4:37 p.m. UTC
From: Numan Siddique <numans@ovn.org>

The flows to allow DHCP response from ovn-controller were missing
if a logical VIF port had dhcp v4/v6 options set and were handled
incrementally.

Fixes: 8bbd67891f68 ("northd: Incremental processing of VIF additions in 'lflow' node.")
Signed-off-by: Numan Siddique <numans@ovn.org>
---
 northd/northd.c     | 125 ++++++++++++++++++++++----------------------
 tests/ovn-northd.at |  25 +++++++++
 2 files changed, 87 insertions(+), 63 deletions(-)
diff mbox series

Patch

diff --git a/northd/northd.c b/northd/northd.c
index 7cc6189bc0..daf5d93430 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -8321,68 +8321,6 @@  build_acls(struct ovn_datapath *od, const struct chassis_features *features,
         }
     }
 
-    /* Add 34000 priority flow to allow DHCP reply from ovn-controller to all
-     * logical ports of the datapath if the CMS has configured DHCPv4 options.
-     * */
-    for (size_t i = 0; i < od->nbs->n_ports; i++) {
-        if (lsp_is_external(od->nbs->ports[i])) {
-            continue;
-        }
-
-        if (od->nbs->ports[i]->dhcpv4_options) {
-            const char *server_id = smap_get(
-                &od->nbs->ports[i]->dhcpv4_options->options, "server_id");
-            const char *server_mac = smap_get(
-                &od->nbs->ports[i]->dhcpv4_options->options, "server_mac");
-            const char *lease_time = smap_get(
-                &od->nbs->ports[i]->dhcpv4_options->options, "lease_time");
-            if (server_id && server_mac && lease_time) {
-                const char *dhcp_actions =
-                    has_stateful ? REGBIT_ACL_VERDICT_ALLOW" = 1; "
-                                   "ct_commit; next;"
-                                 : REGBIT_ACL_VERDICT_ALLOW" = 1; next;";
-                ds_clear(&match);
-                ds_put_format(&match, "outport == \"%s\" && eth.src == %s "
-                              "&& ip4.src == %s && udp && udp.src == 67 "
-                              "&& udp.dst == 68", od->nbs->ports[i]->name,
-                              server_mac, server_id);
-                ovn_lflow_add_with_lport_and_hint(
-                    lflows, od, S_SWITCH_OUT_ACL_EVAL, 34000, ds_cstr(&match),
-                    dhcp_actions, od->nbs->ports[i]->name,
-                    &od->nbs->ports[i]->dhcpv4_options->header_);
-            }
-        }
-
-        if (od->nbs->ports[i]->dhcpv6_options) {
-            const char *server_mac = smap_get(
-                &od->nbs->ports[i]->dhcpv6_options->options, "server_id");
-            struct eth_addr ea;
-            if (server_mac && eth_addr_from_string(server_mac, &ea)) {
-                /* Get the link local IP of the DHCPv6 server from the
-                 * server MAC. */
-                struct in6_addr lla;
-                in6_generate_lla(ea, &lla);
-
-                char server_ip[INET6_ADDRSTRLEN + 1];
-                ipv6_string_mapped(server_ip, &lla);
-
-                const char *dhcp6_actions =
-                    has_stateful ? REGBIT_ACL_VERDICT_ALLOW" = 1; "
-                                   "ct_commit; next;"
-                                 : REGBIT_ACL_VERDICT_ALLOW" = 1; next;";
-                ds_clear(&match);
-                ds_put_format(&match, "outport == \"%s\" && eth.src == %s "
-                              "&& ip6.src == %s && udp && udp.src == 547 "
-                              "&& udp.dst == 546", od->nbs->ports[i]->name,
-                              server_mac, server_ip);
-                ovn_lflow_add_with_lport_and_hint(
-                    lflows, od, S_SWITCH_OUT_ACL_EVAL, 34000, ds_cstr(&match),
-                    dhcp6_actions, od->nbs->ports[i]->name,
-                    &od->nbs->ports[i]->dhcpv6_options->header_);
-            }
-        }
-    }
-
     /* Add a 34000 priority flow to advance the DNS reply from ovn-controller,
      * if the CMS has configured DNS records for the datapath.
      */
@@ -9694,6 +9632,34 @@  build_dhcpv4_options_flows(struct ovn_port *op,
             ds_destroy(&options_action);
             ds_destroy(&response_action);
             ds_destroy(&ipv4_addr_match);
+
+            /* Add 34000 priority flow to allow DHCP reply from ovn-controller
+             * to the ogical port of the datapath if the CMS has configured
+             * DHCPv4 options.
+             * */
+            if (!is_external) {
+                const char *server_id = smap_get(
+                    &op->nbsp->dhcpv4_options->options, "server_id");
+                const char *server_mac = smap_get(
+                    &op->nbsp->dhcpv4_options->options, "server_mac");
+                const char *lease_time = smap_get(
+                    &op->nbsp->dhcpv4_options->options, "lease_time");
+                ovs_assert(server_id && server_mac && lease_time);
+                const char *dhcp_actions =
+                    (op->od->has_stateful_acl || op->od->has_lb_vip)
+                     ? REGBIT_ACL_VERDICT_ALLOW" = 1; ct_commit; next;"
+                     : REGBIT_ACL_VERDICT_ALLOW" = 1; next;";
+                ds_clear(&match);
+                ds_put_format(&match, "outport == %s && eth.src == %s "
+                              "&& ip4.src == %s && udp && udp.src == 67 "
+                              "&& udp.dst == 68",op->json_key,
+                              server_mac, server_id);
+                ovn_lflow_add_with_lport_lflow_ref(
+                    lflows, op->od, S_SWITCH_OUT_ACL_EVAL, 34000,
+                    ds_cstr(&match),dhcp_actions, op->key, NULL,
+                    &op->nbsp->dhcpv4_options->header_,
+                    lflow_dep_mgr, op->key, op->od);
+            }
             break;
         }
     }
@@ -9749,6 +9715,40 @@  build_dhcpv6_options_flows(struct ovn_port *op,
                 lflow_dep_mgr, op->key, op->od);
             ds_destroy(&options_action);
             ds_destroy(&response_action);
+
+            /* Add 34000 priority flow to allow DHCP reply from ovn-controller
+             * to the ogical port of the datapath if the CMS has configured
+             * DHCPv6 options.
+             * */
+            if (!is_external) {
+                const char *server_mac = smap_get(
+                    &op->nbsp->dhcpv6_options->options, "server_id");
+                struct eth_addr ea;
+                ovs_assert(server_mac &&
+                           eth_addr_from_string(server_mac, &ea));
+                /* Get the link local IP of the DHCPv6 server from the
+                * server MAC. */
+                struct in6_addr lla;
+                in6_generate_lla(ea, &lla);
+
+                char server_ip[INET6_ADDRSTRLEN + 1];
+                ipv6_string_mapped(server_ip, &lla);
+
+                const char *dhcp6_actions =
+                    (op->od->has_stateful_acl || op->od->has_lb_vip)
+                        ? REGBIT_ACL_VERDICT_ALLOW" = 1; ct_commit; next;"
+                        : REGBIT_ACL_VERDICT_ALLOW" = 1; next;";
+                ds_clear(&match);
+                ds_put_format(&match, "outport == %s && eth.src == %s "
+                              "&& ip6.src == %s && udp && udp.src == 547 "
+                              "&& udp.dst == 546", op->json_key,
+                              server_mac, server_ip);
+                ovn_lflow_add_with_lport_lflow_ref(
+                    lflows, op->od, S_SWITCH_OUT_ACL_EVAL, 34000,
+                    ds_cstr(&match), dhcp6_actions, op->key, NULL,
+                    &op->nbsp->dhcpv6_options->header_,
+                    lflow_dep_mgr, op->key, op->od);
+            }
             break;
         }
     }
@@ -16327,7 +16327,6 @@  build_lswitch_and_lrouter_iterate_by_lsp(struct ovn_port *op,
     build_lswitch_external_port(op, lflows, &op->lflow_dep_mgr);
     build_lswitch_ip_unicast_lookup(op, lflows, actions, match,
                                     &op->lflow_dep_mgr);
-
     /* Build Logical Router Flows. */
     build_ip_routing_flows_for_router_type_lsp(op, lr_ports, lflows);
     build_arp_resolve_flows_for_lsp(op, lflows, lr_ports, match, actions,
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 234d5a8bbd..a5f64ed5bd 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -9624,6 +9624,31 @@  check_recompute_counter 0 0
 
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
+# Associate DHCP for lsp0-2
+ovn-nbctl dhcp-options-create 192.168.0.0/24
+
+CIDR_UUID=$(ovn-nbctl --bare --columns=_uuid find dhcp_options cidr="192.168.0.0/24")
+ovn-nbctl dhcp-options-set-options $CIDR_UUID   lease_time=3600  router=192.168.0.1   server_id=192.168.0.1   server_mac=c0:ff:ee:00:00:01 hostname="\"foo\""
+
+check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
+ovn-nbctl --wait=sb lsp-set-dhcpv4-options lsp0-2 $CIDR_UUID
+check_recompute_counter 0 0
+
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+# Add IPv6 address and associate DHCPv6 for lsp0-2
+check ovn-nbctl lsp-set-addresses lsp0-2 "aa:aa:aa:00:00:01 192.168.0.11 aef0::4"
+d1="$(ovn-nbctl create DHCP_Options cidr="aef0\:\:/64" \
+options="\"server_id\"=\"00:00:00:10:00:01\"")"
+
+check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
+ovn-nbctl --wait=sb lsp-set-dhcpv6-options lsp0-2 ${d1}
+check_recompute_counter 0 0
+
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+check ovn-nbctl --wait=hv ls-del ls0
+
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])