[ovs-dev] [dp version V5] bridge: Store datapath version into ovsdb

[Ben Pfaff]

On Thu, Oct 23, 2014 at 03:52:33PM -0700, Andy Zhou wrote:
> OVS userspace are backward compatible with older Linux kernel modules.
> However, not having the most up-to-date datapath kernel modules can
> some times lead to user confusion. Storing the datapath version in
> OVSDB allows management software to check and optionally provide
> notifications to users.
> 
> Signed-off-by: Andy Zhou <azhou at nicira.com>
> 
> ----
> v1->v2:  Get version string from dpif_class.
> V2->V3:  Fix missspelling of 'datapath' in comments
> V3->v4:  Fix vswitch.xml description to referece Open vSwitch instead
>          of OpenFlow
> V4->V5:  Add handlinf for possible empty kernel module version file.
>          Update OVSDB with empty string when datapath version cannot
>          be determined.

Thanks for working on this.

I've been thinking about how dpif-netdev.c being built-in forces some
nasty contortions.  It's really not very nice how ovs-vswitchd has to
read its own version number out of the database to put into the
dpif-netdev version number (because the startup script is what puts it
there).  There might be better ways to handle that (maybe we could
move it out of the startup script), but I think that in the end it's
better to just use a "version" string that indicates that that the
datapath is built in.  That answers the real question that the
management software has (whether ovs-vswitchd and the datapath are the
same version) and it simplifies the code.  It also side-steps
questions of what happens if the version number in the database
changes at runtime (I think that currently it's cached and so the
datapath version won't get updated.)  So: I think it would be better
to return a string that says the datapath is built-in.  I suggest
"<built-in>".  (This should be documented.)

While we're at it, I think that it would be better to have a string
that represents an unknown version, rather than just the empty string,
because the latter makes it look like the database column simply
hasn't been updated yet.  I suggest "<unknown>".

br_refresh_datapath_info() looks more complicated than I think it
really is.  How about this:

static void
br_refresh_datapath_info(struct bridge *br)
{
    const char *version;

    version = (br->ofproto && br->ofproto->ofproto_class->get_datapath_version
               ? br->ofproto->ofproto_class->get_datapath_version(br->ofproto)
               : NULL);

    ovsrec_bridge_set_datapath_version(br->cfg,
                                       version ? version : "<unknown>");
}

Thanks,

Ben.