ofconn connection parameters, such as probe_interval and max_backoff,
are always set to their default values when vswitchd starts up even if
the user has configured these to be something different in ovsdb:

  $ ovs-vsctl set controller UUID inactivity_probe=9000

  $ journalctl -u ovs-vswitchd.service | grep "inactivity"
  ovs|10895|rconn|DBG|dp1<->tcp:127.0.0.1:6653: idle 9 seconds,
  sending inactivity probe

  $ systemctl restart openvswitch-switch.service

  $ journalctl -u ovs-vswitchd.service | grep "inactivity"
  ovs|00848|rconn|DBG|dp1<->tcp:127.0.0.1:6653: idle 5 seconds,
  sending inactivity probe

This bug was introduced by commit a0baa7df (connmgr: Make treatment of
active and passive connections more uniform.).

This happens because ofservice_reconfigure() loops over each
ofconn in ofservice->conns and calls ofconn_reconfigure() on it
to set the configuration parameters, however when ofservice_reconfigure()
is called from ofservice_create(), ofservice->conns hasn't been populated
yet so ofconn_reconfigure() is never called.

This commit moves the ofservice_reconfigure() call to ofconn_create()
where ofservice->conns is populated.

This commit also removes the hardcoded default values for
inactivity_probe (5s) and max_backoff (8s) on initial creation
of the ofservice, as these config values are available from the
ofproto_controller struct c.

Signed-off-by: Brad Cowie <b...@faucet.nz>
---
 ofproto/connmgr.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index b092e9e04..eafed8fe7 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1209,7 +1209,7 @@ ofconn_create(struct ofservice *ofservice, struct rconn 
*rconn,
     hmap_init(&ofconn->bundles);
     ofconn->next_bundle_expiry_check = time_msec() + BUNDLE_EXPIRY_INTERVAL;
 
-    ofconn_set_rate_limit(ofconn, settings->rate_limit, settings->burst_limit);
+    ofservice_reconfigure(ofservice, settings);
 
     ovs_mutex_unlock(&ofproto_mutex);
 }
@@ -1915,10 +1915,7 @@ connmgr_count_hidden_rules(const struct connmgr *mgr)
 }
 
 /* Creates a new ofservice for 'target' in 'mgr'.  Returns 0 if successful,
- * otherwise a positive errno value.
- *
- * ofservice_reconfigure() must be called to fully configure the new
- * ofservice. */
+ * otherwise a positive errno value. */
 static void
 ofservice_create(struct connmgr *mgr, const char *target,
                  const struct ofproto_controller *c)
@@ -1928,7 +1925,9 @@ ofservice_create(struct connmgr *mgr, const char *target,
     struct rconn *rconn = NULL;
     if (!vconn_verify_name(target)) {
         char *name = ofconn_make_name(mgr, target);
-        rconn = rconn_create(5, 8, c->dscp, c->allowed_versions);
+        rconn = rconn_create(
+            c->probe_interval, c->max_backoff,
+            c->dscp, c->allowed_versions);
         rconn_connect(rconn, target, name);
         free(name);
     } else if (!pvconn_verify_name(target)) {
@@ -1951,7 +1950,6 @@ ofservice_create(struct connmgr *mgr, const char *target,
     ofservice->rconn = rconn;
     ofservice->pvconn = pvconn;
     ofservice->s = *c;
-    ofservice_reconfigure(ofservice, c);
 
     VLOG_INFO("%s: added %s controller \"%s\"",
               mgr->name, ofconn_type_to_string(ofservice->type), target);
-- 
2.34.1

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

Reply via email to