From: Gurucharan Shetty Date: Tue, 28 May 2013 12:52:36 +0000 (+0000) Subject: ovs-vswitchd: An option to wait for userspace flow restore to complete. X-Git-Tag: sliver-openvswitch-1.10.90-3~6^2~183 X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;ds=sidebyside;h=40358701d5d7c8994b36260ed539a4d9c6d20cfe;p=sliver-openvswitch.git ovs-vswitchd: An option to wait for userspace flow restore to complete. While upgrading openvswitch, it helps to restore openflow flows before starting packet processing. Typically we want to restart openvswitch, add the openflow flows and then start packet processing. To do this, we look for the other_config:flow-restore-wait column in the Open_vSwitch table during startup. If set as true, we disable receiving packets from the datapath, expiring or flushing flows and running any periodic ofproto activities. This option does not prevent the addition and deletion of ports. Once this option is set to false, we return to normal processing. An upcoming commit will use this feature in Open vSwitch startup scripts. Bug #16086. Signed-off-by: Gurucharan Shetty --- diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index c47fd9014..c86620628 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -649,6 +649,7 @@ struct dpif_backer { struct tag_set revalidate_set; /* Revalidate only matching facets. */ struct hmap drop_keys; /* Set of dropped odp keys. */ + bool recv_set_enable; /* Enables or disables receiving packets. */ }; /* All existing ofproto_backer instances, indexed by ofproto->up.type. */ @@ -918,6 +919,21 @@ type_run(const char *type) push_all_stats(); } + /* If vswitchd started with other_config:flow_restore_wait set as "true", + * and the configuration has now changed to "false", enable receiving + * packets from the datapath. */ + if (!backer->recv_set_enable && !ofproto_get_flow_restore_wait()) { + backer->recv_set_enable = true; + + error = dpif_recv_set(backer->dpif, backer->recv_set_enable); + if (error) { + VLOG_ERR("Failed to enable receiving packets in dpif."); + return error; + } + dpif_flow_flush(backer->dpif); + backer->need_revalidate = REV_RECONFIGURE; + } + if (backer->need_revalidate || !tag_set_is_empty(&backer->revalidate_set)) { struct tag_set revalidate_set = backer->revalidate_set; @@ -1011,7 +1027,10 @@ type_run(const char *type) } } - if (timer_expired(&backer->next_expiration)) { + if (!backer->recv_set_enable) { + /* Wake up before a max of 1000ms. */ + timer_set_duration(&backer->next_expiration, 1000); + } else if (timer_expired(&backer->next_expiration)) { int delay = expire(backer); timer_set_duration(&backer->next_expiration, delay); } @@ -1077,6 +1096,11 @@ dpif_backer_run_fast(struct dpif_backer *backer, int max_batch) { unsigned int work; + /* If recv_set_enable is false, we should not handle upcalls. */ + if (!backer->recv_set_enable) { + return 0; + } + /* Handle one or more batches of upcalls, until there's nothing left to do * or until we do a fixed total amount of work. * @@ -1276,9 +1300,12 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp) backer->need_revalidate = 0; simap_init(&backer->tnl_backers); tag_set_init(&backer->revalidate_set); + backer->recv_set_enable = !ofproto_get_flow_restore_wait(); *backerp = backer; - dpif_flow_flush(backer->dpif); + if (backer->recv_set_enable) { + dpif_flow_flush(backer->dpif); + } /* Loop through the ports already on the datapath and remove any * that we don't need anymore. */ @@ -1302,7 +1329,7 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp) shash_add(&all_dpif_backers, type, backer); - error = dpif_recv_set(backer->dpif, true); + error = dpif_recv_set(backer->dpif, backer->recv_set_enable); if (error) { VLOG_ERR("failed to listen on datapath of type %s: %s", type, strerror(error)); @@ -1538,6 +1565,12 @@ run_fast(struct ofproto *ofproto_) struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); struct ofport_dpif *ofport; + /* Do not perform any periodic activity required by 'ofproto' while + * waiting for flow restore to complete. */ + if (ofproto_get_flow_restore_wait()) { + return 0; + } + HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) { port_run_fast(ofport); } @@ -1557,6 +1590,12 @@ run(struct ofproto *ofproto_) complete_operations(ofproto); } + /* Do not perform any periodic activity below required by 'ofproto' while + * waiting for flow restore to complete. */ + if (ofproto_get_flow_restore_wait()) { + return 0; + } + error = run_fast(ofproto_); if (error) { return error; @@ -1631,6 +1670,10 @@ wait(struct ofproto *ofproto_) poll_immediate_wake(); } + if (ofproto_get_flow_restore_wait()) { + return; + } + dpif_wait(ofproto->backer->dpif); dpif_recv_wait(ofproto->backer->dpif); if (ofproto->sflow) { diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 2f91d659c..3788c2f60 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -228,6 +228,9 @@ static struct shash init_ofp_ports = SHASH_INITIALIZER(&init_ofp_ports); static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); +/* The default value of true waits for flow restore. */ +static bool flow_restore_wait = true; + /* Must be called to initialize the ofproto library. * * The caller may pass in 'iface_hints', which contains an shash of @@ -655,6 +658,19 @@ ofproto_set_ipfix(struct ofproto *ofproto, return (bo || fo) ? EOPNOTSUPP : 0; } } + +void +ofproto_set_flow_restore_wait(bool flow_restore_wait_db) +{ + flow_restore_wait = flow_restore_wait_db; +} + +bool +ofproto_get_flow_restore_wait(void) +{ + return flow_restore_wait; +} + /* Spanning Tree Protocol (STP) configuration. */ diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h index acb1790c4..18241e767 100644 --- a/ofproto/ofproto.h +++ b/ofproto/ofproto.h @@ -249,6 +249,8 @@ int ofproto_set_ipfix(struct ofproto *, const struct ofproto_ipfix_bridge_exporter_options *, const struct ofproto_ipfix_flow_exporter_options *, size_t); +void ofproto_set_flow_restore_wait(bool flow_restore_wait_db); +bool ofproto_get_flow_restore_wait(void); int ofproto_set_stp(struct ofproto *, const struct ofproto_stp_settings *); int ofproto_get_stp_status(struct ofproto *, struct ofproto_stp_status *); diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index f067c9de4..3bdb5db53 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -2321,6 +2321,13 @@ bridge_run(void) * returns immediately. */ bridge_init_ofproto(cfg); + /* Once the value of flow-restore-wait is false, we no longer should + * check its value from the database. */ + if (cfg && ofproto_get_flow_restore_wait()) { + ofproto_set_flow_restore_wait(smap_get_bool(&cfg->other_config, + "flow-restore-wait", false)); + } + /* Let each datapath type do the work that it needs to do. */ sset_init(&types); ofproto_enumerate_types(&types); diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index 47b13d219..e3ea29182 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -71,6 +71,53 @@ The Citrix XenServer universally unique identifier for the physical host as displayed by xe host-list. + + +

+ When ovs-vswitchd starts up, it has an empty flow table + and therefore it handles all arriving packets in its default fashion + according to its configuration, by dropping them or sending them to + an OpenFlow controller or switching them as a standalone switch. + This behavior is ordinarily desirable. However, if + ovs-vswitchd is restarting as part of a ``hot-upgrade,'' + then this leads to a relatively long period during which packets are + mishandled. +

+

+ This option allows for improvement. When ovs-vswitchd + starts with this value set as true, it will neither + flush or expire previously set datapath flows nor will it send and + receive any packets to or from the datapath. When this value is + later set to false, ovs-vswitchd will + start receiving packets from the datapath and re-setup the flows. +

+

+ Thus, with this option, the procedure for a hot-upgrade of + ovs-vswitchd becomes roughly the following: +

+
    +
  1. + Stop ovs-vswitchd. +
    2. +
  2. + Set + to true. +
    3. +
  3. + Start ovs-vswitchd. +
    4. +
  4. + Use ovs-ofctl (or some other program, such as an + OpenFlow controller) to restore the OpenFlow flow table + to the desired state. +
    5. +
  5. + Set + to false (or remove it entirely from the database). +
    6. +
+