[ovs-dev] [PATCH ovn 2/2] ovn-controller: Access OVS Datapath table only if supported.

Dumitru Ceara dceara at redhat.com
Wed Sep 1 19:13:09 UTC 2021


Use the newly added IDL API, ovsdb_idl_server_has_table(), through the
auto generated ovsrec_server_has_datapath_table() to determine if it's
OK to read and potentially create a Datapath record in the local OVS DB.

In order to do that we also need to bump the OVS submodule to include
7502849e9581 ("ovsdb-idl: Add APIs to query if a table and a column is
present.").

Fixes: 56e2cd3a2f06 ("ovn-controller: Detect OVS datapath capabilities.")
Reported-at: https://bugzilla.redhat.com/1992705
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
 controller/ovn-controller.c |   32 +++++++++++++++++++-------------
 lib/features.c              |    5 +++++
 ovs                         |    2 +-
 3 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 739048cf8..0031a1035 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -439,12 +439,11 @@ process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
                const struct ovsrec_bridge_table *bridge_table,
                const struct ovsrec_open_vswitch_table *ovs_table,
                const struct ovsrec_bridge **br_int_,
-               const struct ovsrec_datapath **br_int_dp_)
+               const struct ovsrec_datapath **br_int_dp)
 {
     const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table);
-    const struct ovsrec_datapath *br_int_dp = NULL;
 
-    ovs_assert(br_int_ && br_int_dp_);
+    ovs_assert(br_int_);
     if (ovs_idl_txn) {
         if (!br_int) {
             br_int = create_br_int(ovs_idl_txn, ovs_table);
@@ -476,15 +475,16 @@ process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
                 ovsrec_bridge_set_fail_mode(br_int, "secure");
                 VLOG_WARN("Integration bridge fail-mode changed to 'secure'.");
             }
-            br_int_dp = get_br_datapath(cfg, datapath_type);
-            if (!br_int_dp) {
-                br_int_dp = create_br_datapath(ovs_idl_txn, cfg,
-                                               datapath_type);
+            if (br_int_dp) {
+                *br_int_dp = get_br_datapath(cfg, datapath_type);
+                if (!(*br_int_dp)) {
+                    *br_int_dp = create_br_datapath(ovs_idl_txn, cfg,
+                                                    datapath_type);
+                }
             }
         }
     }
     *br_int_ = br_int;
-    *br_int_dp_ = br_int_dp;
 }
 
 static const char *
@@ -3519,8 +3519,10 @@ main(int argc, char *argv[])
             ovsrec_open_vswitch_table_get(ovs_idl_loop.idl);
         const struct ovsrec_bridge *br_int = NULL;
         const struct ovsrec_datapath *br_int_dp = NULL;
-        process_br_int(ovs_idl_txn, bridge_table, ovs_table,
-                       &br_int, &br_int_dp);
+        process_br_int(ovs_idl_txn, bridge_table, ovs_table, &br_int,
+                       ovsrec_server_has_datapath_table(ovs_idl_loop.idl)
+                       ? &br_int_dp
+                       : NULL);
 
         /* Enable ACL matching for double tagged traffic. */
         if (ovs_idl_txn) {
@@ -3563,9 +3565,13 @@ main(int argc, char *argv[])
                                       &chassis_private);
             }
 
-            /* If any OVS feature support changed, force a full recompute. */
-            if (br_int_dp
-                    && ovs_feature_support_update(&br_int_dp->capabilities)) {
+            /* If any OVS feature support changed, force a full recompute.
+             * 'br_int_dp' is valid only if an OVS transaction is possible.
+             */
+            if (ovs_idl_txn
+                && ovs_feature_support_update(br_int_dp
+                                              ? &br_int_dp->capabilities
+                                              : NULL)) {
                 VLOG_INFO("OVS feature set changed, force recompute.");
                 engine_set_force_recompute(true);
             }
diff --git a/lib/features.c b/lib/features.c
index 87d04ee3f..fddf4c450 100644
--- a/lib/features.c
+++ b/lib/features.c
@@ -62,8 +62,13 @@ ovs_feature_is_supported(enum ovs_feature_value feature)
 bool
 ovs_feature_support_update(const struct smap *ovs_capabilities)
 {
+    static struct smap empty_caps = SMAP_INITIALIZER(&empty_caps);
     bool updated = false;
 
+    if (!ovs_capabilities) {
+        ovs_capabilities = &empty_caps;
+    }
+
     for (size_t i = 0; i < ARRAY_SIZE(all_ovs_features); i++) {
         enum ovs_feature_value value = all_ovs_features[i].value;
         const char *name = all_ovs_features[i].name;
diff --git a/ovs b/ovs
index daf627f45..7502849e9 160000
--- a/ovs
+++ b/ovs
@@ -1 +1 @@
-Subproject commit daf627f459ffbc7171d42a2c01f80754bfd54edc
+Subproject commit 7502849e9581a1dabe53eac51fd34521126c7b3f



More information about the dev mailing list