[ovs-dev] [PATCH v2] ofproto: Allow bundle idle timeout to be configured.

Flavio Leitner fbl at sysclose.org
Thu Apr 19 17:09:38 UTC 2018


In some cases 10 seconds might be too much time and in
other cases it might be too little.

The OpenFlow spec mandates that it should wait at least one
second, so enforce that as the minimum acceptable value.

Signed-off-by: Flavio Leitner <fbl at sysclose.org>
---
 ofproto/connmgr.c          |  24 +++++++--
 ofproto/connmgr.h          |   2 +
 ofproto/ofproto.c          |   7 +++
 ofproto/ofproto.h          |   1 +
 tests/ofproto.at           | 129 +++++++++++++++++++++++++++++++++++++++++++++
 vswitchd/bridge.c          |   3 +-
 vswitchd/ovs-vswitchd.8.in |   5 +-
 vswitchd/vswitch.xml       |  12 +++++
 8 files changed, 176 insertions(+), 7 deletions(-)

* V2
  - Included Ben's fix to use default if the config is removed.
  - Added addditional test to cover the above situation.
  - Made the timeout setting more clear.

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 964b8c8d8..f78b4c5ff 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -37,6 +37,7 @@
 #include "openvswitch/poll-loop.h"
 #include "openvswitch/rconn.h"
 #include "openvswitch/shash.h"
+#include "sat-math.h"
 #include "simap.h"
 #include "stream.h"
 #include "timeval.h"
@@ -138,10 +139,11 @@ struct ofconn {
 
 /* vswitchd/ovs-vswitchd.8.in documents the value of BUNDLE_IDLE_LIFETIME in
  * seconds.  That documentation must be kept in sync with the value below. */
-enum {
-    BUNDLE_EXPIRY_INTERVAL = 1000,  /* Check bundle expiry every 1 sec. */
-    BUNDLE_IDLE_TIMEOUT = 10000,    /* Expire idle bundles after 10 seconds. */
-};
+#define BUNDLE_EXPIRY_INTERVAL 1000     /* Check bundle expiry every 1 sec. */
+#define BUNDLE_IDLE_TIMEOUT_DEFAULT 10000   /* Expire idle bundles after
+                                             * 10 seconds. */
+
+static unsigned int bundle_idle_timeout = BUNDLE_IDLE_TIMEOUT_DEFAULT;
 
 static struct ofconn *ofconn_create(struct connmgr *, struct rconn *,
                                     enum ofconn_type, bool enable_async_msgs)
@@ -469,6 +471,18 @@ ofconn_get_ofproto(const struct ofconn *ofconn)
 {
     return ofconn->connmgr->ofproto;
 }
+
+/* Sets the bundle idle timeout to 'timeout' seconds, interpreting 0 as
+ * requesting the default timeout.
+ *
+ * The OpenFlow spec mandates the timeout to be at least one second; . */
+void
+connmgr_set_bundle_idle_timeout(unsigned timeout)
+{
+    bundle_idle_timeout = (timeout
+                           ? sat_mul(timeout, 1000)
+                           : BUNDLE_IDLE_TIMEOUT_DEFAULT);
+}
 
 /* OpenFlow configuration. */
 
@@ -1247,7 +1261,7 @@ static void
 bundle_remove_expired(struct ofconn *ofconn, long long int now)
 {
     struct ofp_bundle *b, *next;
-    long long int limit = now - BUNDLE_IDLE_TIMEOUT;
+    long long int limit = now - bundle_idle_timeout;
 
     HMAP_FOR_EACH_SAFE (b, next, node, &ofconn->bundles) {
         if (b->used <= limit) {
diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
index 2405fbd79..eb3be1668 100644
--- a/ofproto/connmgr.h
+++ b/ofproto/connmgr.h
@@ -86,6 +86,8 @@ void connmgr_get_memory_usage(const struct connmgr *, struct simap *usage);
 
 struct ofproto *ofconn_get_ofproto(const struct ofconn *);
 
+void connmgr_set_bundle_idle_timeout(unsigned timeout);
+
 void connmgr_retry(struct connmgr *);
 
 /* OpenFlow configuration. */
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 36f4c0b16..4af7e6486 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -680,6 +680,13 @@ ofproto_set_in_band_queue(struct ofproto *ofproto, int queue_id)
     connmgr_set_in_band_queue(ofproto->connmgr, queue_id);
 }
 
+/* Sets the bundle max idle timeout */
+void
+ofproto_set_bundle_idle_timeout(unsigned timeout)
+{
+    connmgr_set_bundle_idle_timeout(timeout);
+}
+
 /* Sets the number of flows at which eviction from the kernel flow table
  * will occur. */
 void
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 8c85bbf7f..3ca88baf0 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -312,6 +312,7 @@ void ofproto_reconnect_controllers(struct ofproto *);
 void ofproto_set_extra_in_band_remotes(struct ofproto *,
                                        const struct sockaddr_in *, size_t n);
 void ofproto_set_in_band_queue(struct ofproto *, int queue_id);
+void ofproto_set_bundle_idle_timeout(unsigned timeout);
 void ofproto_set_flow_limit(unsigned limit);
 void ofproto_set_max_idle(unsigned max_idle);
 void ofproto_set_forward_bpdu(struct ofproto *, bool forward_bpdu);
diff --git a/tests/ofproto.at b/tests/ofproto.at
index c1beea7ae..bf4166aea 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -5456,6 +5456,135 @@ OFPT_BARRIER_REPLY (OF1.4):
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto - bundle custom timeout (OpenFlow 1.4)])
+AT_KEYWORDS([monitor])
+OVS_VSWITCHD_START
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:bundle-idle-timeout=4])
+
+# Start a monitor, use the required protocol version
+ovs-ofctl -O OpenFlow14 monitor br0 --detach --no-chdir --pidfile >monitor.log 2>&1
+AT_CAPTURE_FILE([monitor.log])
+
+ovs-appctl time/stop
+
+# Send an OpenFlow14 message (05), OFPT_BUNDLE_CONTROL (21), length (10), xid (01)
+ovs-appctl -t ovs-ofctl ofctl/send "05 21 00 10 00 00 00 01 00 00 00 01 00 00 00 03"
+ovs-appctl time/warp 2000
+# Send a bundle flow mod, it should keep the bundle alive.
+ovs-appctl -t ovs-ofctl ofctl/send "05 22 00 a0 00 00 00 02 00 00 00 01 00 00 00 03 \
+05 0e 00 90 00 00 00 02 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 01 00 00 00 00 00 ff ff \
+ff ff ff ff ff ff ff ff ff ff ff ff 00 00 00 00 \
+00 01 00 42 80 00 00 04 00 00 00 01 80 00 08 06 \
+50 54 00 00 00 06 80 00 06 06 50 54 00 00 00 05 \
+80 00 0a 02 08 06 80 00 0c 02 00 00 80 00 2a 02 \
+00 02 80 00 2c 04 c0 a8 00 02 80 00 2e 04 c0 a8 \
+00 01 00 00 00 00 00 00 00 04 00 18 00 00 00 00 \
+00 00 00 10 00 00 00 03 00 00 00 00 00 00 00 00 \
+"
+ovs-appctl time/warp 2000
+# Send a bundle close, it should keep the bundle alive.
+ovs-appctl -t ovs-ofctl ofctl/send "05 21 00 10 00 00 00 03 00 00 00 01 00 02 00 03"
+ovs-appctl time/warp 4000
+# Make sure that timeouts are processed after the expiry, but still before the
+# current timeout of 4s.
+ovs-appctl time/warp 1000
+# Send a Commit, but too late.
+ovs-appctl -t ovs-ofctl ofctl/send "05 21 00 10 00 00 00 04 00 00 00 01 00 04 00 03"
+ovs-appctl -t ovs-ofctl ofctl/barrier
+OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
+
+AT_CHECK([ofctl_strip < monitor.log], [], [dnl
+send: OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=OPEN_REQUEST flags=atomic ordered
+OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=OPEN_REPLY flags=0
+send: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
+ bundle_id=0x1 flags=atomic ordered
+OFPT_FLOW_MOD (OF1.4): ADD table:1 priority=65535,arp,in_port=1,vlan_tci=0x0000/0x1fff,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,arp_spa=192.168.0.2,arp_tpa=192.168.0.1,arp_op=2 actions=output:3
+send: OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=CLOSE_REQUEST flags=atomic ordered
+OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=CLOSE_REPLY flags=0
+OFPT_ERROR (OF1.4): OFPBFC_TIMEOUT
+OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=OPEN_REQUEST flags=atomic ordered
+send: OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=COMMIT_REQUEST flags=atomic ordered
+OFPT_ERROR (OF1.4): OFPBFC_BAD_ID
+OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=COMMIT_REQUEST flags=atomic ordered
+OFPT_BARRIER_REPLY (OF1.4):
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto - bundle reset timeout to default (OpenFlow 1.4)])
+AT_KEYWORDS([monitor])
+OVS_VSWITCHD_START
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:bundle-idle-timeout=15])
+AT_CHECK([ovs-vsctl remove Open_vSwitch . other_config bundle-idle-timeout])
+
+# Start a monitor, use the required protocol version
+ovs-ofctl -O OpenFlow14 monitor br0 --detach --no-chdir --pidfile >monitor.log 2>&1
+AT_CAPTURE_FILE([monitor.log])
+
+ovs-appctl time/stop
+
+# Send an OpenFlow14 message (05), OFPT_BUNDLE_CONTROL (21), length (10), xid (01)
+ovs-appctl -t ovs-ofctl ofctl/send "05 21 00 10 00 00 00 01 00 00 00 01 00 00 00 03"
+ovs-appctl time/warp 8000
+# Send a bundle flow mod, it should keep the bundle alive.
+ovs-appctl -t ovs-ofctl ofctl/send "05 22 00 a0 00 00 00 02 00 00 00 01 00 00 00 03 \
+05 0e 00 90 00 00 00 02 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 01 00 00 00 00 00 ff ff \
+ff ff ff ff ff ff ff ff ff ff ff ff 00 00 00 00 \
+00 01 00 42 80 00 00 04 00 00 00 01 80 00 08 06 \
+50 54 00 00 00 06 80 00 06 06 50 54 00 00 00 05 \
+80 00 0a 02 08 06 80 00 0c 02 00 00 80 00 2a 02 \
+00 02 80 00 2c 04 c0 a8 00 02 80 00 2e 04 c0 a8 \
+00 01 00 00 00 00 00 00 00 04 00 18 00 00 00 00 \
+00 00 00 10 00 00 00 03 00 00 00 00 00 00 00 00 \
+"
+ovs-appctl time/warp 8000
+# Send a bundle close, it should keep the bundle alive.
+ovs-appctl -t ovs-ofctl ofctl/send "05 21 00 10 00 00 00 03 00 00 00 01 00 02 00 03"
+ovs-appctl time/warp 11000
+# Make sure that timeouts are processed after the expiry
+ovs-appctl time/warp 1000
+# Send a Commit, but too late.
+ovs-appctl -t ovs-ofctl ofctl/send "05 21 00 10 00 00 00 04 00 00 00 01 00 04 00 03"
+ovs-appctl -t ovs-ofctl ofctl/barrier
+OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
+
+AT_CHECK([ofctl_strip < monitor.log], [], [dnl
+send: OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=OPEN_REQUEST flags=atomic ordered
+OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=OPEN_REPLY flags=0
+send: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
+ bundle_id=0x1 flags=atomic ordered
+OFPT_FLOW_MOD (OF1.4): ADD table:1 priority=65535,arp,in_port=1,vlan_tci=0x0000/0x1fff,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,arp_spa=192.168.0.2,arp_tpa=192.168.0.1,arp_op=2 actions=output:3
+send: OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=CLOSE_REQUEST flags=atomic ordered
+OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=CLOSE_REPLY flags=0
+OFPT_ERROR (OF1.4): OFPBFC_TIMEOUT
+OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=OPEN_REQUEST flags=atomic ordered
+send: OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=COMMIT_REQUEST flags=atomic ordered
+OFPT_ERROR (OF1.4): OFPBFC_BAD_ID
+OFPT_BUNDLE_CONTROL (OF1.4):
+ bundle_id=0x1 type=COMMIT_REQUEST flags=atomic ordered
+OFPT_BARRIER_REPLY (OF1.4):
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
 
 AT_SETUP([ofproto - bundle open (OpenFlow 1.3)])
 AT_KEYWORDS([monitor])
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index d90997e3a..44aecbb64 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -598,7 +598,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
                                       OFPROTO_MAX_IDLE_DEFAULT));
     ofproto_set_vlan_limit(smap_get_int(&ovs_cfg->other_config, "vlan-limit",
                                        LEGACY_MAX_VLAN_HEADERS));
-
+    ofproto_set_bundle_idle_timeout(smap_get_int(&ovs_cfg->other_config,
+                                                 "bundle-idle-timeout", 0));
     ofproto_set_threads(
         smap_get_int(&ovs_cfg->other_config, "n-handler-threads", 0),
         smap_get_int(&ovs_cfg->other_config, "n-revalidator-threads", 0));
diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
index b6f192990..da0f0d370 100644
--- a/vswitchd/ovs-vswitchd.8.in
+++ b/vswitchd/ovs-vswitchd.8.in
@@ -326,7 +326,10 @@ replies for a switch defined time greater than 1s, it may send an
 ofp_error_msg with OFPET_BUNDLE_FAILED type and OFPBFC_TIMEOUT code.
 .
 .PP
-Open vSwitch implements idle bundle lifetime of 10 seconds.
+Open vSwitch implements default idle bundle lifetime of 10 seconds.
+(This is configurable via \fBother-config:bundle-idle-timeout\fR in
+the \fBOpen_vSwitch\fR table. See \fBovs-vswitchd.conf.db\fR(5)
+for details.)
 .
 .SH "LIMITS"
 .
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 9c2a8263e..9e03169b8 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -450,6 +450,18 @@
           VLAN.
         </p>
       </column>
+      <column name="other_config" key="bundle-idle-timeout"
+              type='{"type": "integer", "minInteger": 1}'>
+        <p>
+          The maximum time (in seconds) that idle bundles will wait
+          to be expired since it was either opened, modified or closed.
+        </p>
+        <p>
+          OpenFlow specification mandates the timeout to be at least one
+          second. The default is 10 seconds.
+        </p>
+    </column>
+
     </group>
 
     <group title="Status">
-- 
2.14.3




More information about the dev mailing list