[ovs-dev] [PATCH 1/2] ovs-ctl.in: Ability to save flows and kernel datapath config.

Ben Pfaff blp at nicira.com
Fri Oct 19 22:42:28 UTC 2012


On Fri, Oct 19, 2012 at 02:03:59PM -0700, Gurucharan Shetty wrote:
> On Fri, Oct 19, 2012 at 1:19 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > On Fri, Oct 19, 2012 at 11:06:27AM -0700, Gurucharan Shetty wrote:
> > > Add a new command - "restart" to ovs-ctl. Calling this command
> > > will save and restore the Openflow flows on each bridge while
> > > stopping and starting the userspace daemons respectively.
> > >
> > > Also, during a force-reload-kmod, save the flows and kernel datapath
> > > configuration. Use the saved datapath configuration while readding
> > > the kernel module and the flows while starting the userspace daemons.
> > >
> > > Feature #13555.
> > > Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>
> >
> > Thanks for doing this work.  Working on the startup scripts is
> > difficult work.
> >
> > I think that "ovs-save save-flows" creates additional temporary files,
> > with mktemp, that never get removed.  If so, one way to avoid that
> > would be to write to a single script commands of the form:
> >
> >         ovs-ofctl add-flows <bridge> - <<EOF
> >         ...flows...
> >         EOF
> >
> > (In that case, logging the entire script for restoring the flows is no
> > longer a good idea.)
> >
> The reason I chose to go ahead with the undeleted tmp files model
> was that the tmp files could be used to debug the issue
> later if restoring flows fail for some reason. But I guess, it is bad
> etiquette to leave them undeleted.

I think one could argue for saving a copy if restoring flows fails.
However: the most likely source of problems is "dump-flows" output
that "add-flows" fails to accept.  In such a case, I would hope that
ovs-ofctl prints the flow that is not accepted, which would usually be
enough to see the problem.  If it does, then it would be really
helpful to log ovs-ofctl's stdout and stderr, at least in failure
cases.  If it doesn't, then I guess we should work on that.

> > The logging seems more elaborate than I would expect in
> > restore_datapath and in stop_forwarding.  These functions now log the
> > commands that they are running and their success or failure.  Although
> > that's reasonable to do, if we are going to do it for these particular
> > actions then I'd think we'd want to do that for pretty much every
> > action.  (If you think that's a good idea then I'd be OK with it.)
> 
> I looked at the model we have for "Restoring interface configuration"
> action and I was confused about what to do for the 2 new additions
> that I am making. I will get rid of the logging for the 2 new additions.
> Should I keep the old logging intact?

Oh gosh, I didn't realize there was precedent for this.  Hmm.  Well,
never mind, let's do it this way.  (But watch out for logging the
whole file, for restoring the flow table, since the flow table can be
many thousands of lines long, or even longer.  I think we'd be better
off making sure that we can log the actual error, as discussed above,
instead of logging the whole thing.)

It'd be nice if you could factor out a shell function for this.

> > In force_reload_kmod, this change moves stopping ovs-vswitchd from
> > after starting and stopping the database to before starting and
> > stopping the database.  That will extend the period of time when new
> > flows cannot be set up.  Is it really necessary to do it?
> >
> In stop_forwarding, I use 'ovs-vsctl list-br' command which needs the
> database. According to the comment in the code, starting a large database
> may take time. So, I wanted to save the flows first and then stop the
> database.

The time that we're most worried about is the time during which
ovs-vswitchd is not running.  stop_forwarding kills ovs-vswitchd, so
doing the slow part (restarting ovsdb-server) after we kill
ovs-vswitchd and before we restart it is not a good idea.

I think you could use "ovs-appctl ofproto/list" or "ovs-dpctl
dump-dps" as a substitute for "ovs-vsctl list-br" here that does not
require the database to be running.

> > For the same reason, in the "restart" case, is it possible to change
> > the order to stop/start ovsdb, then stop/start ovs-vswitchd?
> >
> Out here too, I will need the bridges before hand. Wouldn't it delay things
> if I start the database first? I can get the bridges first, and then stop
> the database.

The total time isn't as important as the amount of time that
ovs-vswitchd is not running.  It's fine to stop and start the
database, then to stop and start vswitchd.

> > It's not obvious to me why saving datapaths and saving interfaces are
> > separate steps.  Can we combine these steps?
> >
> Are you saying, make a single function call to ovs-save? Or make 2 function
> calls, but use the same script to store the results?

The former.  There may well be some rationale behind the two steps, I
just didn't see it off-hand, so I was hoping you could help me out.

Thanks,

Ben.



More information about the dev mailing list