[ovs-dev] [PATCH v2 1/3] ovs-vswitchd: An option to wait for userspace flow restore to complete.

Gurucharan Shetty shettyg at nicira.com
Thu May 30 18:12:50 UTC 2013


On Wed, May 29, 2013 at 4:46 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Tue, May 28, 2013 at 02:02:17PM +0000, Gurucharan Shetty wrote:
> > 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 <gshetty at nicira.com>
>
> The dependency here is the opposite of what we usually do.  That is,
> ordinarily, nothing in ofproto depends on anything in bridge; rather,
> bridge is a client of the ofproto library.  It would be best to maintain
> that client relationship here.  One natural way would be for ofproto to
> export a function to set this new flow-restore-wait value, instead of
> bridge to export a variable.  Another alternative would be to make
> ofproto export the variable instead of bridge, but hiding a global
> behind a function may be cleaner.
>
Okay. I will hide it inside a function of ofproto.


>
> "backer->recv_set_enable = !flow_restore_wait;" is simpler than this:
> > +    if (flow_restore_wait) {
> > +        backer->recv_set_enable = false;
> > +    } else {
> > +        backer->recv_set_enable = true;
> > +    }
>
Will do.

>
> The description in vswitch.xml is accurate, but I think that users would
> find it easier to understand if it stepped back a little and explained
> the larger issue.  How about this, instead:
>
>     When <code>ovs-vswitchd</code> 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
>     <code>ovs-vswitchd</code> 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 <code>ovs-vswitchd</code>
>     starts 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.
>
>     Thus, with this option, the procedure for a hot-upgrade of
>     <code>ovs-vswitchd</code> becomes roughly the following:
>
>     <ol>
>       <li>
>         Stop <code>ovs-vswitchd</code>.
>       </li>
>       <li>
>         Set <ref column="other_config" key="flow-restore-wait"/>
>         to <code>true</code>.
>       </li>
>       <li>
>         Start <code>ovs-vswitchd</code>.
>       </li>
>       <li>
>         Use <code>ovs-ofctl</code> (or some other program, such as an
>         OpenFlow controller</code>) to restore the OpenFlow flow table
>         to the desired state.
>       </li>
>       <li>
>         Set <ref column="other_config" key="flow-restore-wait"/>
>         to <code>false</code> (or remove it entirely from the database).
>       </li>
>     </ol>
>
I added this.


>
> In the later patch where it is true, it would be nice to also note that
> restart does all of the above.
>
Okay.

>
> It might also be nice to mention this in the top-level INSTALL, or at
> least provide a pointer.
>
I created a separate patch for this as I added details that are not limited
to
this patch.

>
> Thanks,
>
> Ben.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130530/6fcc863a/attachment-0003.html>


More information about the dev mailing list