ovn-controller: Stop dropping bind_vport requests immediately after h…

…andling.

ovn-controller immediately removes the vport_bindings requests that were
generated by VIFs after handling them locally, this approach is intended
to avoid binding the vport to one VIF only and allocate the vport
between the different VIFs that exist in the vport:virtual-parents.

Although the behavior mentioned above is correct, in some cases when the
SB Database is busy the transaction that binds this vport to the desired
VIF/chassis can fail and the controller will not re-try to bind the
vport again because we deleted the bind_vport request in the previous
loop/TXN.

This patch aims to change the above behavior by storing the bind_vport
requests for a bit longer time and this is done by the following:
    1. mark each new bind_vport request as new.

    2. loop0: ovn-controller will try to handle this bind_vport request
       for the first time as usual (no change).

    3. loop0: ovn-controller will try to delete the already handled bind_vport
       request as usual but first, it will check if this request is marked as new and
       if so the controller will mark this request as an old request and keep it,
       otherwise remove it.

    4. loop1: ovn-controller will try to commit the same change again for
       the old request, if the previous commit in loop0 succeeded the
       change will not have any effect on SB, otherwise we will try to
       commit the same vport_bind request again.

    5. loop1: delete the old bind_vport request.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1954659
Signed-off-by: Mohammad Heib <mheib@redhat.com>
Acked-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Mark Michelson <mmichels@redhat.com>

controller/pinctrl.c

@@ -6649,30 +6649,68 @@ struct put_vport_binding {
     uint32_t vport_key;
 
     uint32_t vport_parent_key;
+
+    /* This vport record Only relevant if "new_record" is true. */
+    bool new_record;
 };
 
 /* Contains "struct put_vport_binding"s. */
 static struct hmap put_vport_bindings;
 
+/*
+ * Validate if the vport_binding record that was added
+ * by the pinctrl thread is still relevant and needs
+ * to be updated in the SBDB or not.
+ *
+ * vport_binding record is only relevant and needs to be updated in SB if:
+ *   2. The put_vport_binding:new_record is true:
+ *       The new_record will be set to "true" when this vport record is created
+ *       by function "pinctrl_handle_bind_vport".
+ *
+ *       After the first attempt to bind this vport to the chassis and
+ *       virtual_parent by function "run_put_vport_bindings" we will set the
+ *       value of vpb:new_record to "false" and keep it in "put_vport_bindings"
+ *
+ *       After the second attempt of binding the vpb it will be removed by
+ *       this function.
+ *
+ *       The above guarantees that we will try to bind the vport twice in
+ *       a certain amount of time.
+ *
+*/
+static bool
+is_vport_binding_relevant(struct put_vport_binding *vpb)
+{
+
+    if (vpb->new_record) {
+        vpb->new_record = false;
+        return true;
+    }
+    return false;
+}
+
 static void
 init_put_vport_bindings(void)
 {
     hmap_init(&put_vport_bindings);
 }
 
 static void
-flush_put_vport_bindings(void)
+flush_put_vport_bindings(bool force_flush)
 {
     struct put_vport_binding *vport_b;
-    HMAP_FOR_EACH_POP (vport_b, hmap_node, &put_vport_bindings) {
-        free(vport_b);
+    HMAP_FOR_EACH_SAFE (vport_b, hmap_node, &put_vport_bindings) {
+        if (!is_vport_binding_relevant(vport_b) || force_flush) {
+            hmap_remove(&put_vport_bindings, &vport_b->hmap_node);
+            free(vport_b);
+        }
     }
 }
 
 static void
 destroy_put_vport_bindings(void)
 {
-    flush_put_vport_bindings();
+    flush_put_vport_bindings(true);
     hmap_destroy(&put_vport_bindings);
 }
 
@@ -6750,7 +6788,7 @@ run_put_vport_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn,
                               sbrec_port_binding_by_key, chassis, vpb);
     }
 
-    flush_put_vport_bindings();
+    flush_put_vport_bindings(false);
 }
 
 /* Called with in the pinctrl_handler thread context. */
@@ -6788,7 +6826,7 @@ pinctrl_handle_bind_vport(
     vpb->dp_key = dp_key;
     vpb->vport_key = vport_key;
     vpb->vport_parent_key = vport_parent_key;
-
+    vpb->new_record = true;
     notify_pinctrl_main();
 }