controller: disable OpenFlow inactivity probing

OpenFlow connection is established over unix socket, which is a reliable connection and doesn't require additional probing. With this patch openflow probing in ovn-controller -> ovs-vswitchd direction is disabled for all three connections: - OF flows management connection, - OF features negotiation connection, - pinctrl connection. ovn-controller external_ids:ovn-openflow-probe-interval is removed as not needed anymore. Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2023-May/404625.html Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> Acked-by: Han Zhou <hzhou@ovn.org> Acked-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Numan Siddique <numans@ovn.org>
ovn-org · Nov 2, 2023 · c16e5da · c16e5da
1 parent 4db5ab2
commit c16e5da
Show file tree

Hide file tree

Showing 7 changed files with 12 additions and 55 deletions.
diff --git a/NEWS b/NEWS
@@ -1,5 +1,10 @@
 Post v23.09.0
 -------------
+  - Disable OpenFlow inactivity probing between ovn-controller and OVS.
+    OF connection is established over unix socket, which is a reliable
+    connection method and doesn't require additional probing.
+    external_ids:ovn-openflow-probe-interval configuration option for
+    ovn-controller no longer matters and is ignored.
 
 OVN v23.09.0 - 15 Sep 2023
 --------------------------

diff --git a/controller/ofctrl.c b/controller/ofctrl.c
@@ -418,11 +418,9 @@ static void ofctrl_recv(const struct ofp_header *, enum ofptype);
 
 void
 ofctrl_init(struct ovn_extend_table *group_table,
-            struct ovn_extend_table *meter_table,
-            int inactivity_probe_interval)
+            struct ovn_extend_table *meter_table)
 {
-    swconn = rconn_create(inactivity_probe_interval, 0,
-                          DSCP_DEFAULT, 1 << OFP15_VERSION);
+    swconn = rconn_create(0, 0, DSCP_DEFAULT, 1 << OFP15_VERSION);
     tx_counter = rconn_packet_counter_create();
     hmap_init(&installed_lflows);
     hmap_init(&installed_pflows);
@@ -3077,14 +3075,6 @@ ofctrl_is_connected(void)
     return rconn_is_connected(swconn);
 }
 
-void
-ofctrl_set_probe_interval(int probe_interval)
-{
-    if (swconn) {
-        rconn_set_probe_interval(swconn, probe_interval);
-    }
-}
-
 void
 ofctrl_get_memory_usage(struct simap *usage)
 {

diff --git a/controller/ofctrl.h b/controller/ofctrl.h
@@ -49,8 +49,7 @@ struct ovn_desired_flow_table {
 
 /* Interface for OVN main loop. */
 void ofctrl_init(struct ovn_extend_table *group_table,
-                 struct ovn_extend_table *meter_table,
-                 int inactivity_probe_interval);
+                 struct ovn_extend_table *meter_table);
 bool ofctrl_run(const struct ovsrec_bridge *br_int,
                 const struct ovsrec_open_vswitch_table *,
                 struct shash *pending_ct_zones);
@@ -142,7 +141,6 @@ void ofctrl_check_and_add_flow_metered(struct ovn_desired_flow_table *,
 
 
 bool ofctrl_is_connected(void);
-void ofctrl_set_probe_interval(int probe_interval);
 void ofctrl_get_memory_usage(struct simap *usage);
 
 #endif /* controller/ofctrl.h */
diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
@@ -153,20 +153,6 @@
         </p>
       </dd>
 
-      <dt><code>external_ids:ovn-openflow-probe-interval</code></dt>
-      <dd>
-        <p>
-          The inactivity probe interval of the OpenFlow connection to the
-          OpenvSwitch integration bridge, in seconds.
-          If the value is zero, it disables the connection keepalive feature.
-        </p>
-
-        <p>
-          If the value is nonzero, then it will be forced to a value of
-          at least 5s.
-        </p>
-      </dd>
-
       <dt><code>external_ids:ovn-encap-type</code></dt>
       <dd>
         <p>

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
@@ -106,7 +106,6 @@ static unixctl_cb_func debug_ignore_startup_delay;
 
 #define DEFAULT_BRIDGE_NAME "br-int"
 #define DEFAULT_DATAPATH "system"
-#define OFCTRL_DEFAULT_PROBE_INTERVAL_SEC 0
 
 #define CONTROLLER_LOOP_STOPWATCH_NAME "flow-generation"
 #define OFCTRL_PUT_STOPWATCH_NAME "flow-installation"
@@ -587,22 +586,6 @@ update_ssl_config(const struct ovsrec_ssl_table *ssl_table)
     }
 }
 
-static int
-get_ofctrl_probe_interval(struct ovsdb_idl *ovs_idl)
-{
-    const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
-    if (!cfg) {
-        return OFCTRL_DEFAULT_PROBE_INTERVAL_SEC;
-    }
-
-    const struct ovsrec_open_vswitch_table *ovs_table =
-        ovsrec_open_vswitch_table_get(ovs_idl);
-    const char *chassis_id = get_ovs_chassis_id(ovs_table);
-    return get_chassis_external_id_value_int(
-        &cfg->external_ids, chassis_id,
-        "ovn-openflow-probe-interval", OFCTRL_DEFAULT_PROBE_INTERVAL_SEC);
-}
-
 /* Retrieves the pointer to the OVN Southbound database from 'ovs_idl' and
  * updates 'sbdb_idl' with that pointer. */
 static void
@@ -5434,8 +5417,7 @@ main(int argc, char *argv[])
             engine_get_internal_data(&en_mac_cache);
 
     ofctrl_init(&lflow_output_data->group_table,
-                &lflow_output_data->meter_table,
-                get_ofctrl_probe_interval(ovs_idl_loop.idl));
+                &lflow_output_data->meter_table);
     ofctrl_seqno_init();
 
     unixctl_command_register("group-table-list", "", 0, 0,
@@ -5561,7 +5543,6 @@ main(int argc, char *argv[])
                      &reset_ovnsb_idl_min_index,
                      &ctrl_engine_ctx, &ovnsb_expected_cond_seqno);
         update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl));
-        ofctrl_set_probe_interval(get_ofctrl_probe_interval(ovs_idl_loop.idl));
 
         struct ovsdb_idl_txn *ovnsb_idl_txn
             = ovsdb_idl_loop_run(&ovnsb_idl_loop);

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
@@ -3406,7 +3406,7 @@ pinctrl_handler(void *arg_)
     static long long int svc_monitors_next_run_time = LLONG_MAX;
     static long long int send_prefixd_time = LLONG_MAX;
 
-    swconn = rconn_create(5, 0, DSCP_DEFAULT, 1 << OFP15_VERSION);
+    swconn = rconn_create(0, 0, DSCP_DEFAULT, 1 << OFP15_VERSION);
 
     while (!latch_is_set(&pctrl->pinctrl_thread_exit)) {
         long long int bfd_time = LLONG_MAX;

diff --git a/lib/features.c b/lib/features.c
@@ -29,11 +29,10 @@
 #include "openvswitch/ofp-meter.h"
 #include "openvswitch/ofp-util.h"
 #include "ovn/features.h"
+#include "controller/ofctrl.h"
 
 VLOG_DEFINE_THIS_MODULE(features);
 
-#define FEATURES_DEFAULT_PROBE_INTERVAL_SEC 5
-
 /* Parses 'cap_name' from 'ovs_capabilities' and returns whether the
  * type of capability is supported or not. */
 typedef bool ovs_feature_parse_func(const struct smap *ovs_capabilities,
@@ -112,8 +111,7 @@ static void
 ovs_feature_rconn_setup(const char *br_name)
 {
     if (!swconn) {
-        swconn = rconn_create(FEATURES_DEFAULT_PROBE_INTERVAL_SEC, 0,
-                              DSCP_DEFAULT, 1 << OFP15_VERSION);
+        swconn = rconn_create(0, 0, DSCP_DEFAULT, 1 << OFP15_VERSION);
     }
 
     if (!rconn_is_connected(swconn)) {
@@ -124,7 +122,6 @@ ovs_feature_rconn_setup(const char *br_name)
         }
         free(target);
     }
-    rconn_set_probe_interval(swconn, FEATURES_DEFAULT_PROBE_INTERVAL_SEC);
 }
 
 static bool