[ovs-dev] [PATCH 07/14] vswitchd: Configuration of allowed OpenFlow versions

[Simon Horman]

On Thu, Nov 15, 2012 at 08:16:27PM -0800, Ben Pfaff wrote:
> On Fri, Nov 16, 2012 at 10:43:39AM +0900, Simon Horman wrote:
> > On Mon, Nov 12, 2012 at 10:23:00AM -0800, Ben Pfaff wrote:
> > > On Wed, Nov 07, 2012 at 05:03:05PM +0900, Simon Horman wrote:
> > > > Versions may be configured using a comma delimited list as
> > > > the value for 'openflow-versions' the 'other-config' column
> > > > of the Bridge table.
> > > > 
> > > > Currently list elements other than 'OpenFlow10' and 'OpenFlow12' will
> > > > be ignored.
> > > > 
> > > > If the list is empty, then OpenFlow10 is used.
> > > > This default is consistent with the behaviour of ovs-vswtichd
> > > > prior to this patch.
> > > > 
> > > > Signed-off-by: Simon Horman <horms at verge.net.au>
> > > 
> > > This seems to me like core enough functionality that we should add a new
> > > column for it.  I suggest "protocols" as the name and that it should be
> > > a set of strings, one protocol name per string, with a default used if
> > > the set is empty.
> > 
> > That sounds reasonable to me.
> 
> The implementation of ofputil_versions_from_string() in this patch calls
> ofputil_version_from_string() as a helper.  Reasonable enough on the
> face of it, but ofputil_version_from_string() exits with a fatal error
> if its input in invalid.  That's OK in ovs-ofctl, etc., but we don't
> want ovs-vswitchd to do that.

What is you preferred action when vswitchd encounters an unknown
version string? Ignore it? With a warning?

I think that the implication of such a situation is
that a value is allowed by the schema which is unknown
to the C code.

> There's a stray VLOG_WARN in here, too.

Thanks.