On Wed, May 29, 2013 at 4:46 PM, Ben Pfaff <b...@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 <gshe...@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.
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
