diff mbox series

[ovs-dev,branch-23.09] controller: Avoid quadratic complexity for multi-chassis ports.

Message ID 20240923073753.66155-1-amusil@redhat.com
State Accepted
Headers show
Series [ovs-dev,branch-23.09] controller: Avoid quadratic complexity for multi-chassis ports. | expand

Checks

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

Commit Message

Ales Musil Sept. 23, 2024, 7:37 a.m. UTC
The processing for ports had a quadratic complexity when MTU change
was involved. Avoid the unnecessary processing and do the lookup loop
only once and only in case it was actually needed. In addition to the
quadratic complexity the condition to do the flow recompute was wrong
and this resulted for the lookup in physical_handle_flows_for_lport()
to run for all ports.

The performance difference is very noticeable, see the number below
in a test with 800 LSPs:
Before:
physical_flow_output, handler for input if_status_mgr took 2072ms

After:
physical_flow_output, handler for input if_status_mgr took 4ms

Fixes: cdd8dea88b3d ("Track interface MTU in if-status-mgr")
Fixes: 7084cf437421 ("Always funnel multichassis port traffic through tunnels")
Co-authored-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Ales Musil <amusil@redhat.com>
Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit 16836c3796f7af68437f9f834b40d87c801dc27c)
---
 controller/ovn-controller.c | 16 +--------
 controller/physical.c       | 49 ++++++++++++---------------
 controller/physical.h       |  3 ++
 tests/ovn.at                | 67 +++++++++++++++++++++++++++++++++++++
 4 files changed, 93 insertions(+), 42 deletions(-)
diff mbox series

Patch

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 66b656044..c714278b2 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -4742,21 +4742,7 @@  pflow_output_if_status_mgr_handler(struct engine_node *node,
         }
         if (pb->n_additional_chassis) {
             /* Update flows for all ports in datapath. */
-            struct sbrec_port_binding *target =
-                sbrec_port_binding_index_init_row(
-                    p_ctx.sbrec_port_binding_by_datapath);
-            sbrec_port_binding_index_set_datapath(target, pb->datapath);
-
-            const struct sbrec_port_binding *binding;
-            SBREC_PORT_BINDING_FOR_EACH_EQUAL (
-                    binding, target, p_ctx.sbrec_port_binding_by_datapath) {
-                bool removed = sbrec_port_binding_is_deleted(binding);
-                if (!physical_handle_flows_for_lport(binding, removed, &p_ctx,
-                                                     &pfo->flow_table)) {
-                    return false;
-                }
-            }
-            sbrec_port_binding_index_destroy_row(target);
+            physical_multichassis_reprocess(pb, &p_ctx, &pfo->flow_table);
         } else {
             /* If any multichassis ports, update flows for the port. */
             bool removed = sbrec_port_binding_is_deleted(pb);
diff --git a/controller/physical.c b/controller/physical.c
index c93fe9aa6..ad439c71e 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -2245,33 +2245,9 @@  physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
         }
     }
 
-    if (ldp) {
-        bool multichassis_state_changed = (
-            !!pb->additional_chassis ==
-            !!shash_find(&ldp->multichassis_ports, pb->logical_port)
-        );
-        if (multichassis_state_changed) {
-            if (pb->additional_chassis) {
-                add_local_datapath_multichassis_port(
-                    ldp, pb->logical_port, pb);
-            } else {
-                remove_local_datapath_multichassis_port(
-                    ldp, pb->logical_port);
-            }
-
-            struct sbrec_port_binding *target =
-                sbrec_port_binding_index_init_row(
-                    p_ctx->sbrec_port_binding_by_datapath);
-            sbrec_port_binding_index_set_datapath(target, ldp->datapath);
-
-            const struct sbrec_port_binding *port;
-            SBREC_PORT_BINDING_FOR_EACH_EQUAL (
-                    port, target, p_ctx->sbrec_port_binding_by_datapath) {
-                ofctrl_remove_flows(flow_table, &port->header_.uuid);
-                physical_eval_port_binding(p_ctx, port, flow_table);
-            }
-            sbrec_port_binding_index_destroy_row(target);
-        }
+    if (sbrec_port_binding_is_updated(
+            pb, SBREC_PORT_BINDING_COL_ADDITIONAL_CHASSIS) || removed) {
+        physical_multichassis_reprocess(pb, p_ctx, flow_table);
     }
 
     if (!removed) {
@@ -2288,6 +2264,25 @@  physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
     return true;
 }
 
+void
+physical_multichassis_reprocess(const struct sbrec_port_binding *pb,
+                                struct physical_ctx *p_ctx,
+                                struct ovn_desired_flow_table *flow_table)
+{
+    struct sbrec_port_binding *target =
+            sbrec_port_binding_index_init_row(
+                    p_ctx->sbrec_port_binding_by_datapath);
+    sbrec_port_binding_index_set_datapath(target, pb->datapath);
+
+    const struct sbrec_port_binding *port;
+    SBREC_PORT_BINDING_FOR_EACH_EQUAL (port, target,
+                                       p_ctx->sbrec_port_binding_by_datapath) {
+        ofctrl_remove_flows(flow_table, &port->header_.uuid);
+        physical_eval_port_binding(p_ctx, port, flow_table);
+    }
+    sbrec_port_binding_index_destroy_row(target);
+}
+
 void
 physical_handle_mc_group_changes(struct physical_ctx *p_ctx,
                                  struct ovn_desired_flow_table *flow_table)
diff --git a/controller/physical.h b/controller/physical.h
index 1f1ed55ef..c81613ae2 100644
--- a/controller/physical.h
+++ b/controller/physical.h
@@ -78,4 +78,7 @@  bool physical_handle_flows_for_lport(const struct sbrec_port_binding *,
                                      bool removed,
                                      struct physical_ctx *,
                                      struct ovn_desired_flow_table *);
+void physical_multichassis_reprocess(const struct sbrec_port_binding *,
+                                     struct physical_ctx *,
+                                     struct ovn_desired_flow_table *);
 #endif /* controller/physical.h */
diff --git a/tests/ovn.at b/tests/ovn.at
index df4b9e22c..987975c1d 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -38063,3 +38063,70 @@  OVN_CHECK_PACKETS([hv/vif1-tx.pcap], [expected-vif1])
 
 AT_CLEANUP
 ])
+
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([Multichassis port I-P processing])
+ovn_start
+
+net_add n1
+
+sim_add hv1
+as hv1
+check ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.11
+check ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
+
+sim_add hv2
+as hv2
+check ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.12
+check ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
+
+check ovn-nbctl ls-add ls
+check ovn-nbctl lsp-add ls multi
+check ovn-nbctl lsp-set-options multi requested-chassis=hv1
+
+check ovn-nbctl lsp-add ls ln
+check ovn-nbctl lsp-set-type ln localnet
+check ovn-nbctl lsp-set-addresses ln unknown
+check ovn-nbctl lsp-set-options ln network_name=phys
+
+check ovn-nbctl lsp-add ls lsp1 \
+    -- lsp-set-options lsp1 requested-chassis=hv1
+as hv1 check ovs-vsctl -- add-port br-int lsp1 \
+    -- set Interface lsp1 external-ids:iface-id=lsp1
+
+for hv in hv1 hv2; do
+    as $hv check ovs-vsctl -- add-port br-int multi \
+        -- set Interface multi external-ids:iface-id=multi
+done
+
+wait_for_ports_up
+ovn-nbctl --wait=hv sync
+
+OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=37 | grep -c check_pkt_larger) -eq 0])
+
+check ovn-nbctl --wait=hv lsp-set-options multi requested-chassis=hv1,hv2
+OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=37 | grep -c check_pkt_larger) -eq 4])
+
+check ovn-nbctl --wait=hv lsp-set-options multi requested-chassis=hv1
+OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=37 | grep -c check_pkt_larger) -eq 0])
+
+check ovn-nbctl --wait=hv lsp-set-options multi requested-chassis=hv1,hv2
+OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=37 | grep -c check_pkt_larger) -eq 4])
+
+as hv2 check ovs-vsctl del-port multi
+OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=37 | grep -c check_pkt_larger) -eq 0])
+
+as hv2 check ovs-vsctl -- add-port br-int multi \
+    -- set Interface multi external-ids:iface-id=multi
+OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=37 | grep -c check_pkt_larger) -eq 4])
+
+check ovn-nbctl --wait=hv lsp-del multi
+OVS_WAIT_UNTIL([test $(as hv2 ovs-ofctl dump-flows br-int table=37 | grep -c check_pkt_larger) -eq 0])
+
+OVN_CLEANUP([hv1],[hv2])
+
+AT_CLEANUP
+])