When a logical switch port was deleted, the Advertised_MAC_Binding row
referencing its Port_Binding could still be present at the time of
deletion, causing an OVSDB referential integrity violation.

Fix this by replacing engine_noop_handler with a proper incremental
handler.

Signed-off-by: Alexandra Rukomoinikova <[email protected]>
---
v1 --> v2: fixed tests
---
 northd/en-advertised-route-sync.c | 19 +++++++++++++++++++
 northd/en-advertised-route-sync.h |  3 +++
 northd/inc-proc-northd.c          |  5 +----
 tests/ovn-inc-proc-graph-dump.at  |  2 +-
 tests/ovn-northd.at               |  3 +++
 5 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/northd/en-advertised-route-sync.c 
b/northd/en-advertised-route-sync.c
index 4a8d13232..0ec86cedd 100644
--- a/northd/en-advertised-route-sync.c
+++ b/northd/en-advertised-route-sync.c
@@ -674,6 +674,25 @@ dynamic_routes_northd_change_handler(struct engine_node 
*node, void *data_)
     return EN_HANDLED_UNCHANGED;
 }
 
+enum engine_input_handler_result
+dynamic_routes_adv_mac_northd_change_handler(struct engine_node *node,
+                                             void *data_ OVS_UNUSED)
+{
+    struct northd_data *northd_data = engine_get_input_data("northd", node);
+    struct northd_tracked_data *trk_data = &northd_data->trk_data;
+    if (!northd_has_tracked_data(trk_data)) {
+        return EN_UNHANDLED;
+    }
+
+    if (!hmapx_is_empty(&northd_data->trk_data.trk_lsps.created) ||
+        !hmapx_is_empty(&northd_data->trk_data.trk_lsps.updated) ||
+        !hmapx_is_empty(&northd_data->trk_data.trk_lsps.deleted)) {
+        return EN_UNHANDLED;
+    }
+
+    return EN_HANDLED_UNCHANGED;
+}
+
 static bool
 should_advertise_route(const struct ovn_datapath *advertising_od,
                        const struct ovn_port *advertising_op,
diff --git a/northd/en-advertised-route-sync.h 
b/northd/en-advertised-route-sync.h
index 71cd417de..f90d50b99 100644
--- a/northd/en-advertised-route-sync.h
+++ b/northd/en-advertised-route-sync.h
@@ -43,6 +43,9 @@ dynamic_routes_northd_change_handler(struct engine_node 
*node, void *data_);
 enum engine_input_handler_result
 dynamic_routes_lr_stateful_change_handler(struct engine_node *node,
                                           void *data_);
+enum engine_input_handler_result
+dynamic_routes_adv_mac_northd_change_handler(struct engine_node *,
+                                             void * OVS_UNUSED);
 
 void *en_advertised_mac_binding_sync_init(struct engine_node *,
                                           struct engine_arg *);
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index a2b464411..a91a7011a 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -364,11 +364,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
                      NULL);
     engine_add_input(&en_advertised_mac_binding_sync,
                      &en_sb_advertised_mac_binding, NULL);
-    /* No need for an explicit handler for northd changes.
-     * We do need to access en_northd (input) data, i.e., to
-     * lookup OVN ports. */
     engine_add_input(&en_advertised_mac_binding_sync, &en_northd,
-                     engine_noop_handler);
+                     dynamic_routes_adv_mac_northd_change_handler);
 
     engine_add_input(&en_learned_route_sync, &en_sb_learned_route,
                      learned_route_sync_sb_learned_route_change_handler);
diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/ovn-inc-proc-graph-dump.at
index 3750339d0..25ee7da10 100644
--- a/tests/ovn-inc-proc-graph-dump.at
+++ b/tests/ovn-inc-proc-graph-dump.at
@@ -229,7 +229,7 @@ digraph "Incremental-Processing-Engine" {
        advertised_mac_binding_sync [[style=filled, shape=box, fillcolor=white, 
label="advertised_mac_binding_sync"]];
        SB_port_binding -> advertised_mac_binding_sync [[label=""]];
        SB_advertised_mac_binding -> advertised_mac_binding_sync [[label=""]];
-       northd -> advertised_mac_binding_sync [[label="engine_noop_handler"]];
+       northd -> advertised_mac_binding_sync 
[[label="dynamic_routes_adv_mac_northd_change_handler"]];
        northd_output [[style=filled, shape=box, fillcolor=white, 
label="northd_output"]];
        acl_id -> northd_output [[label="northd_output_acl_id_handler"]];
        sync_from_sb -> northd_output [[label=""]];
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index f87b14c9a..55e242995 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -20231,6 +20231,9 @@ check_row_count Advertised_MAC_Binding 1
 check ovn-nbctl --wait=sb clear logical_switch ls-evpn other_config
 check_row_count Advertised_MAC_Binding 0
 
+AT_CHECK([grep -c "referential integrity violation" northd/ovn-northd.log], 
[1], [0
+])
+
 OVN_CLEANUP_NORTHD
 AT_CLEANUP
 ])
-- 
2.48.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to