There wasn't any check if SB is writable when we tried to commit
learned routes into SB, this would lead to assert and crash. In
order to prevent that track the SB state and add additional handler
that will trigger recompute of the route_exchange node if there are
SB changes pending and the SB is writable again.

Fixes: 866a5014ae45 ("controller: Support learning routes.")
Acked-by: Xavier Simonart <xsimo...@redhat.com>
Signed-off-by: Ales Musil <amu...@redhat.com>
---
v2: Remove the extra check pointed out by Xavier.
    Add ack from Xavier.
---
 controller/ovn-controller.c | 22 +++++++++++++++++++++-
 controller/route-exchange.c | 10 ++++++++--
 controller/route-exchange.h |  1 +
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index ebf4780f0..0da8a5800 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -5303,6 +5303,9 @@ route_sb_advertised_route_data_handler(struct engine_node 
*node, void *data)
 struct ed_type_route_exchange {
     /* We need the idl to check if the Learned_Route table exists. */
     struct ovsdb_idl *sb_idl;
+    /* Set to true when SB is readonly and we have routes that need
+     * to be inserted into SB. */
+    bool sb_changes_pending;
 };
 
 static enum engine_node_state
@@ -5336,7 +5339,9 @@ en_route_exchange_run(struct engine_node *node, void 
*data)
         .sbrec_port_binding_by_name = sbrec_port_binding_by_name,
         .announce_routes = &route_data->announce_routes,
     };
-    struct route_exchange_ctx_out r_ctx_out = {};
+    struct route_exchange_ctx_out r_ctx_out = {
+        .sb_changes_pending = false,
+    };
 
     hmap_init(&r_ctx_out.route_table_watches);
 
@@ -5346,9 +5351,22 @@ en_route_exchange_run(struct engine_node *node, void 
*data)
     route_table_watch_request_cleanup(&r_ctx_out.route_table_watches);
     hmap_destroy(&r_ctx_out.route_table_watches);
 
+    re->sb_changes_pending = r_ctx_out.sb_changes_pending;
+
     return EN_UPDATED;
 }
 
+static enum engine_input_handler_result
+route_exchange_sb_ro_handler(struct engine_node *node OVS_UNUSED, void *data)
+{
+    struct ed_type_route_exchange *re = data;
+    if (re->sb_changes_pending) {
+        return EN_UNHANDLED;
+    }
+
+    return EN_HANDLED_UNCHANGED;
+}
+
 
 static void *
 en_route_exchange_init(struct engine_node *node OVS_UNUSED,
@@ -5961,6 +5979,8 @@ main(int argc, char *argv[])
                      engine_noop_handler);
     engine_add_input(&en_route_exchange, &en_route_table_notify, NULL);
     engine_add_input(&en_route_exchange, &en_route_exchange_status, NULL);
+    engine_add_input(&en_route_exchange, &en_sb_ro,
+                     route_exchange_sb_ro_handler);
 
     engine_add_input(&en_addr_sets, &en_sb_address_set,
                      addr_sets_sb_address_set_handler);
diff --git a/controller/route-exchange.c b/controller/route-exchange.c
index 564d87b53..eef58e801 100644
--- a/controller/route-exchange.c
+++ b/controller/route-exchange.c
@@ -137,7 +137,8 @@ sb_sync_learned_routes(const struct vector *learned_routes,
                        const struct smap *bound_ports,
                        struct ovsdb_idl_txn *ovnsb_idl_txn,
                        struct ovsdb_idl_index *sbrec_port_binding_by_name,
-                       struct ovsdb_idl_index *sbrec_learned_route_by_datapath)
+                       struct ovsdb_idl_index *sbrec_learned_route_by_datapath,
+                       bool *sb_changes_pending)
 {
     struct hmap sync_routes = HMAP_INITIALIZER(&sync_routes);
     const struct sbrec_learned_route *sb_route;
@@ -186,6 +187,10 @@ sb_sync_learned_routes(const struct vector *learned_routes,
                 hmap_remove(&sync_routes, &route_e->hmap_node);
                 free(route_e);
             } else {
+                if (!ovnsb_idl_txn) {
+                    *sb_changes_pending = true;
+                    continue;
+                }
                 sb_route = sbrec_learned_route_insert(ovnsb_idl_txn);
                 sbrec_learned_route_set_datapath(sb_route, datapath);
                 sbrec_learned_route_set_logical_port(sb_route, logical_port);
@@ -268,7 +273,8 @@ route_exchange_run(const struct route_exchange_ctx_in 
*r_ctx_in,
         sb_sync_learned_routes(&received_routes, ad->db,
                                &ad->bound_ports, r_ctx_in->ovnsb_idl_txn,
                                r_ctx_in->sbrec_port_binding_by_name,
-                               r_ctx_in->sbrec_learned_route_by_datapath);
+                               r_ctx_in->sbrec_learned_route_by_datapath,
+                               &r_ctx_out->sb_changes_pending);
 
         route_table_add_watch_request(&r_ctx_out->route_table_watches,
                                       table_id);
diff --git a/controller/route-exchange.h b/controller/route-exchange.h
index 87cac5808..e3791c331 100644
--- a/controller/route-exchange.h
+++ b/controller/route-exchange.h
@@ -31,6 +31,7 @@ struct route_exchange_ctx_in {
 
 struct route_exchange_ctx_out {
     struct hmap route_table_watches;
+    bool sb_changes_pending;
 };
 
 void route_exchange_run(const struct route_exchange_ctx_in *,
-- 
2.49.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to