controller: Optimize postponed ports handling.

In some cases, when a port was going to be postponed, the same port (lsp)
was claimed four times:
- within runtime_data handling pb changes:
  - lsp is claimed as handling pb changes, and added to postponed list.
  - it's claimed again as handling postponed list.
- within runtime_data handling postponed list, the same two operations:
  - lsp is claimed as handling pb changes, and added to postponed list.
  - it's claimed again as handling postponed list.

With this patch, the lsp1 port would only only be claimed twice.
- within runtime_data handling pb changes:
  - lsp is claimed as handling pb changes, and added to postponed list.
- within runtime_data handling postponed list, the same two operations:
  - it's claimed again as handling postponed list.

This patch is only about CPU utilization; it should not result in any
flow changes.

Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
Signed-off-by: 0-day Robot <robot@bytheb.org>

controller/binding.c

@@ -3102,6 +3102,33 @@ handle_updated_port(struct binding_ctx_in *b_ctx_in,
     return handled;
 }
 
+bool
+binding_handle_postponed_ports(struct binding_ctx_in *b_ctx_in,
+                               struct binding_ctx_out *b_ctx_out)
+{
+    /* Also handle any postponed (throttled) ports. */
+    const char *port_name;
+    const struct sbrec_port_binding *pb;
+    bool handled = true;
+    struct sset postponed_ports = SSET_INITIALIZER(&postponed_ports);
+    sset_clone(&postponed_ports, b_ctx_out->postponed_ports);
+    SSET_FOR_EACH (port_name, &postponed_ports) {
+        pb = lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name,
+                                  port_name);
+        if (!pb) {
+            sset_find_and_delete(b_ctx_out->postponed_ports, port_name);
+            continue;
+        }
+        handled = handle_updated_port(b_ctx_in, b_ctx_out, pb);
+        if (!handled) {
+            break;
+        }
+    }
+    sset_destroy(&postponed_ports);
+    cleanup_claimed_port_timestamps();
+    return handled;
+}
+
 /* Returns true if the port binding changes resulted in local binding
  * updates, false otherwise.
  */
@@ -3248,25 +3275,6 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in,
         }
     }
 
-    /* Also handle any postponed (throttled) ports. */
-    const char *port_name;
-    struct sset postponed_ports = SSET_INITIALIZER(&postponed_ports);
-    sset_clone(&postponed_ports, b_ctx_out->postponed_ports);
-    SSET_FOR_EACH (port_name, &postponed_ports) {
-        pb = lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name,
-                                  port_name);
-        if (!pb) {
-            sset_find_and_delete(b_ctx_out->postponed_ports, port_name);
-            continue;
-        }
-        handled = handle_updated_port(b_ctx_in, b_ctx_out, pb);
-        if (!handled) {
-            break;
-        }
-    }
-    sset_destroy(&postponed_ports);
-    cleanup_claimed_port_timestamps();
-
     if (handled) {
         /* There may be new local datapaths added by the above handling, so go
          * through each port_binding of newly added local datapaths to update

controller/binding.h

@@ -196,6 +196,8 @@ bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
 
 bool binding_handle_ovs_interface_changes(struct binding_ctx_in *,
                                           struct binding_ctx_out *);
+bool binding_handle_postponed_ports(struct binding_ctx_in *,
+                                         struct binding_ctx_out *);
 bool binding_handle_port_binding_changes(struct binding_ctx_in *,
                                          struct binding_ctx_out *);
 void binding_tracked_dp_destroy(struct hmap *tracked_datapaths);

controller/ovn-controller.c

@@ -1540,6 +1540,38 @@ runtime_data_ovs_interface_shadow_handler(struct engine_node *node, void *data)
     return true;
 }
 
+static bool
+runtime_data_postponed_ports_handler(struct engine_node *node, void *data)
+{
+    struct ed_type_runtime_data *rt_data = data;
+    struct binding_ctx_in b_ctx_in;
+    struct binding_ctx_out b_ctx_out;
+    init_binding_ctx(node, rt_data, &b_ctx_in, &b_ctx_out);
+    if (!b_ctx_in.chassis_rec) {
+        return false;
+    }
+
+    rt_data->tracked = true;
+    b_ctx_out.tracked_dp_bindings = &rt_data->tracked_dp_bindings;
+
+    if (!binding_handle_postponed_ports(&b_ctx_in, &b_ctx_out)) {
+        return false;
+    }
+
+    rt_data->local_lports_changed = b_ctx_out.local_lports_changed;
+    rt_data->localnet_learn_fdb = b_ctx_out.localnet_learn_fdb;
+    rt_data->localnet_learn_fdb_changed = b_ctx_out.localnet_learn_fdb_changed;
+    if (b_ctx_out.related_lports_changed ||
+            b_ctx_out.non_vif_ports_changed ||
+            b_ctx_out.local_lports_changed ||
+            b_ctx_out.localnet_learn_fdb_changed ||
+            !hmap_is_empty(b_ctx_out.tracked_dp_bindings)) {
+        engine_set_node_state(node, EN_UPDATED);
+    }
+
+    return true;
+}
+
 static bool
 runtime_data_sb_port_binding_handler(struct engine_node *node, void *data)
 {
@@ -5262,9 +5294,10 @@ main(int argc, char *argv[])
                      runtime_data_sb_datapath_binding_handler);
     engine_add_input(&en_runtime_data, &en_sb_port_binding,
                      runtime_data_sb_port_binding_handler);
-    /* Reuse the same handler for any previously postponed ports. */
+    /* Run postponed_ports_handler after port_binding_handler in case port get
+     * deleted */
     engine_add_input(&en_runtime_data, &en_postponed_ports,
-                     runtime_data_sb_port_binding_handler);
+                     runtime_data_postponed_ports_handler);
     /* Run sb_ro_handler after port_binding_handler in case port get deleted */
     engine_add_input(&en_runtime_data, &en_sb_ro, runtime_data_sb_ro_handler);