controller: Add random delay during fdb learning.

This change reduces the probability of conflicts when multiple nodes tries to add the same FDB entry to SB at the same time. When conflict occurs, OVN controller does full recompute which is heavy weight on the scale setup. Signed-off-by: Naveen Yerramneni <naveen.yerramneni@nutanix.com> Suggested-by: Dumitru Ceara <dceara@redhat.com> Acked-by: Ales Musil <amusil@redhat.com> Signed-off-by: Dumitru Ceara <dceara@redhat.com>
ovn-org · Jun 28, 2024 · 02d143f · 02d143f
1 parent 7e99500
commit 02d143f
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 15 deletions.
diff --git a/controller/mac-cache.c b/controller/mac-cache.c
@@ -245,7 +245,8 @@ mac_binding_lookup(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
 
 /* FDB. */
 struct fdb *
-fdb_add(struct hmap *map, struct fdb_data fdb_data) {
+fdb_add(struct hmap *map, struct fdb_data fdb_data, long long timestamp)
+{
     struct fdb *fdb = fdb_find(map, &fdb_data);
 
     if (!fdb) {
@@ -255,6 +256,7 @@ fdb_add(struct hmap *map, struct fdb_data fdb_data) {
     }
 
     fdb->data = fdb_data;
+    fdb->timestamp = timestamp;
 
     return fdb;
 }

diff --git a/controller/mac-cache.h b/controller/mac-cache.h
@@ -83,6 +83,7 @@ struct fdb {
     struct fdb_data data;
     /* Reference to the SB FDB record. */
     const struct sbrec_fdb *sbrec_fdb;
+    long long timestamp;
 };
 
 struct bp_packet_data {
@@ -149,7 +150,8 @@ mac_binding_lookup(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
                    const char *logical_port, const char *ip);
 
 /* FDB. */
-struct fdb *fdb_add(struct hmap *map, struct fdb_data fdb_data);
+struct fdb *fdb_add(struct hmap *map, struct fdb_data fdb_data,
+                    long long timestamp);
 
 void fdb_remove(struct hmap *map, struct fdb *fdb);
 

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
@@ -3323,7 +3323,7 @@ fdb_add_sb(struct mac_cache_data *data, const struct sbrec_fdb *sfdb)
         return;
     }
 
-    struct fdb *fdb = fdb_add(&data->fdbs, fdb_data);
+    struct fdb *fdb = fdb_add(&data->fdbs, fdb_data, 0);
 
     fdb->sbrec_fdb = sfdb;
 }

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
@@ -4737,6 +4737,7 @@ pinctrl_destroy(void)
 /* Buffered "put_mac_binding" operation. */
 
 #define MAX_MAC_BINDING_DELAY_MSEC 50
+#define MAX_FDB_DELAY_MSEC         50
 #define MAX_MAC_BINDINGS           1000
 
 /* Contains "struct mac_binding"s. */
@@ -8958,21 +8959,30 @@ run_put_fdbs(struct ovsdb_idl_txn *ovnsb_idl_txn,
         return;
     }
 
-    const struct fdb *fdb;
-    HMAP_FOR_EACH (fdb, hmap_node, &put_fdbs) {
-        run_put_fdb(ovnsb_idl_txn, sbrec_fdb_by_dp_key_mac,
-                    sbrec_port_binding_by_key,
-                    sbrec_datapath_binding_by_key, fdb);
+    long long now = time_msec();
+    struct fdb *fdb;
+    HMAP_FOR_EACH_SAFE (fdb, hmap_node, &put_fdbs) {
+        if (now >= fdb->timestamp) {
+            run_put_fdb(ovnsb_idl_txn, sbrec_fdb_by_dp_key_mac,
+                        sbrec_port_binding_by_key,
+                        sbrec_datapath_binding_by_key, fdb);
+            fdb_remove(&put_fdbs, fdb);
+        }
     }
-    fdbs_clear(&put_fdbs);
 }
 
 
 static void
 wait_put_fdbs(struct ovsdb_idl_txn *ovnsb_idl_txn)
+    OVS_REQUIRES(pinctrl_mutex)
 {
-    if (ovnsb_idl_txn && !hmap_is_empty(&put_fdbs)) {
-        poll_immediate_wake();
+    if (!ovnsb_idl_txn) {
+        return;
+    }
+
+    struct fdb *fdb;
+    HMAP_FOR_EACH (fdb, hmap_node, &put_fdbs) {
+        poll_timer_wait_until(fdb->timestamp);
     }
 }
 
@@ -8992,6 +9002,8 @@ pinctrl_handle_put_fdb(const struct flow *md, const struct flow *headers)
             .mac = headers->dl_src,
     };
 
-    fdb_add(&put_fdbs, fdb_data);
+    uint32_t delay = random_range(MAX_FDB_DELAY_MSEC) + 1;
+    long long timestamp = time_msec() + delay;
+    fdb_add(&put_fdbs, fdb_data, timestamp);
     notify_pinctrl_main();
 }
diff --git a/tests/ovn.at b/tests/ovn.at
@@ -34395,21 +34395,21 @@ AT_CHECK([ovn-nbctl --wait=hv sync])
 # Learning is disabled, the table should be empty
 send_packet 20
 AT_CHECK([ovn-nbctl --wait=hv sync])
-AT_CHECK([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:20") = 0])
+OVS_WAIT_UNTIL([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:20") = 0])
 
 # Enable learning on localnet port
 AT_CHECK([ovn-nbctl set logical_switch_port ln_port options:localnet_learn_fdb=true])
 AT_CHECK([ovn-nbctl --wait=hv sync])
 send_packet 20
 AT_CHECK([ovn-nbctl --wait=hv sync])
-AT_CHECK([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:20") = 1])
+OVS_WAIT_UNTIL([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:20") = 1])
 
 # Disable learning on localnet port
 AT_CHECK([ovn-nbctl set logical_switch_port ln_port options:localnet_learn_fdb=false])
 AT_CHECK([ovn-nbctl --wait=hv sync])
 send_packet 30
 AT_CHECK([ovn-nbctl --wait=hv sync])
-AT_CHECK([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:30") = 0])
+OVS_WAIT_UNTIL([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:30") = 0])
 
 OVN_CLEANUP([hv1])
 AT_CLEANUP