> 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.
Will this patch leave ongoing traffic not disturbed while upgrading OVS?
I am asking since I notice that the existing ports, which keep packet flowing,
in the fast datapath (i.e., kernel) will be garbage collected in
open_dpif_backer.
Best,Jing
>
> Bug #16086.
> Signed-off-by: Gurucharan Shetty <gshe...@nicira.com>
> ---
> ofproto/ofproto-dpif.c | 55
> +++++++++++++++++++++++++++++++++++++++++++++---
> vswitchd/bridge.c | 11 ++++++++++
> vswitchd/vswitch.xml | 17 +++++++++++++++
> 3 files changed, 80 insertions(+), 3 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index c4f7d25..31f04f3 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -72,6 +72,8 @@ COVERAGE_DEFINE(facet_suppress);
> * flow translation. */
> #define MAX_RESUBMIT_RECURSION 64
>
> +extern bool flow_restore_wait;
> +
> /* Number of implemented OpenFlow tables. */
> enum { N_TABLES = 255 };
> enum { TBL_INTERNAL = N_TABLES - 1 }; /* Used for internal hidden rules.
> */
> @@ -671,6 +673,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. */
> @@ -947,6 +950,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 && !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;
> @@ -1040,7 +1058,10 @@ type_run(const char *type)
> }
> }
>
> - if (timer_expired(&backer->next_expiration)) {
> + if (!backer->recv_set_enable) {
> + /* A minimum of 1000 ms delay. */
> + 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);
> }
> @@ -1106,6 +1127,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.
> *
> @@ -1305,9 +1331,16 @@ 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);
> + if (flow_restore_wait) {
> + backer->recv_set_enable = false;
> + } else {
> + backer->recv_set_enable = true;
> + }
> *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. */
> @@ -1331,7 +1364,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));
> @@ -1569,6 +1602,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 (flow_restore_wait) {
> + return 0;
> + }
> +
> HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
> port_run_fast(ofport);
> }
> @@ -1584,6 +1623,12 @@ run(struct ofproto *ofproto_)
> struct ofbundle *bundle;
> int error;
>
> + /* Do not perform any periodic activity required by 'ofproto' while
> + * waiting for flow restore to complete. */
> + if (flow_restore_wait) {
> + return 0;
> + }
> +
> if (!clogged) {
> complete_operations(ofproto);
> }
> @@ -1658,6 +1703,10 @@ wait(struct ofproto *ofproto_)
> struct ofport_dpif *ofport;
> struct ofbundle *bundle;
>
> + if (flow_restore_wait) {
> + return;
> + }
> +
> if (!clogged && !list_is_empty(&ofproto->completions)) {
> poll_immediate_wake();
> }
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index e10036c..db70b77 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -183,6 +183,9 @@ static long long int iface_stats_timer = LLONG_MIN;
> #define OFP_PORT_ACTION_WINDOW 10
> static bool reconfiguring = false;
>
> +/* The default value of true waits for flow restore. */
> +bool flow_restore_wait = true;
> +
> static void add_del_bridges(const struct ovsrec_open_vswitch *);
> static void bridge_update_ofprotos(void);
> static void bridge_create(const struct ovsrec_bridge *);
> @@ -2321,6 +2324,14 @@ bridge_run(void)
> * returns immediately. */
> bridge_init_ofproto(cfg);
>
> + /* bridge_run(), when called for the first time will always have
> + * flow_restore_wait as true. Once the value is false, we no longer
> + * need to check its value. */
> + if (flow_restore_wait) {
> + 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 4396779..3896946 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -104,6 +104,23 @@
> column or to <code>false</code> to explicitly disable it.
> </column>
>
> + <column name="other_config" key="flow-restore-wait"
> + type='{"type": "boolean"}'>
> + <p>
> + When <code>ovs-vswitchd</code> is started with this value set as
> + <code>true</code>, 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 <code>false</code>,
> + <code>ovs-vswitchd</code> will start receiving packets from the
> + datapath and re-setup the flows.
> + </p>
> + <p>
> + This option is only useful when used during Open vSwitch upgrades
> or
> + restarts and flow restoration is needed before processing any new
> + packets.
> + </p>
> + </column>
> +
> <column name="statistics" key="cpu"
> type='{"type": "integer", "minInteger": 1}'>
> <p>
> --
> 1.7.9.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
