Did you see my feedback on v1? I don't think this addresses those issues. --Justin
> On Oct 22, 2014, at 6:29 PM, Andy Zhou <[email protected]> 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 <[email protected]> > > ---- > v1->v2: Get version string from dpif_class. > --- > lib/dpif-netdev.c | 7 +++++++ > lib/dpif-netlink.c | 30 ++++++++++++++++++++++++++++++ > lib/dpif-provider.h | 4 ++++ > lib/dpif.c | 16 ++++++++++++++++ > lib/dpif.h | 1 + > lib/util.c | 27 +++++++++++++++++++++++++++ > lib/util.h | 4 ++++ > ofproto/ofproto-dpif.c | 17 +++++++++++++++++ > ofproto/ofproto-provider.h | 13 +++++++++++++ > tests/ovs-vsctl.at | 2 ++ > vswitchd/bridge.c | 33 +++++++++++++++++++++++++++++++++ > vswitchd/vswitch.ovsschema | 6 ++++-- > vswitchd/vswitch.xml | 5 +++++ > 13 files changed, 163 insertions(+), 2 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 2009206..84035a3 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -2520,6 +2520,12 @@ dp_netdev_reset_pmd_threads(struct dp_netdev *dp) > } > } > > +static char * > +dpif_netdev_get_datapath_version(void) > +{ > + return ovs_version_dup(); > +} > + > static void > dp_netdev_flow_used(struct dp_netdev_flow *netdev_flow, > int cnt, int size, > @@ -3100,6 +3106,7 @@ const struct dpif_class dpif_netdev_class = { > dpif_netdev_register_upcall_cb, > dpif_netdev_enable_upcall, > dpif_netdev_disable_upcall, > + dpif_netdev_get_datapath_version, > }; > > static void > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c > index 67c2814..25d6bd0 100644 > --- a/lib/dpif-netlink.c > +++ b/lib/dpif-netlink.c > @@ -1903,6 +1903,35 @@ dpif_netlink_recv_purge(struct dpif *dpif_) > fat_rwlock_unlock(&dpif->upcall_lock); > } > > +static char * > +dpif_netlink_get_datapath_version(void) > +{ > + char *version_str = NULL; > + > +#ifdef __linux__ > + > +#define MAX_VERSION_STR_SIZE 80 > +#define LINUX_DATAPATH_VERSION_FILE "/sys/module/openvswitch/version" > + FILE *f; > + > + f = fopen(LINUX_DATAPATH_VERSION_FILE, "r"); > + if (f) { > + char *newline; > + char version[MAX_VERSION_STR_SIZE]; > + > + fgets(version, MAX_VERSION_STR_SIZE, f); > + newline = strchr(version, '\n'); > + if (newline) { > + *newline = '\0'; > + } > + fclose(f); > + version_str = xstrdup(version); > + } > +#endif > + > + return version_str; > +} > + > const struct dpif_class dpif_netlink_class = { > "system", > dpif_netlink_enumerate, > @@ -1940,6 +1969,7 @@ const struct dpif_class dpif_netlink_class = { > NULL, /* register_upcall_cb */ > NULL, /* enable_upcall */ > NULL, /* disable_upcall */ > + dpif_netlink_get_datapath_version, /* get_datapath_version */ > }; > > static int > diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h > index 65cf505..9d4c688 100644 > --- a/lib/dpif-provider.h > +++ b/lib/dpif-provider.h > @@ -363,6 +363,10 @@ struct dpif_class { > > /* Disables upcalls if 'dpif' directly executes upcall functions. */ > void (*disable_upcall)(struct dpif *); > + > + /* Get datapath version. Caller is responsible for free the string > + * returned. */ > + char *(*get_datapath_version)(void); > }; > > extern const struct dpif_class dpif_netlink_class; > diff --git a/lib/dpif.c b/lib/dpif.c > index 64e6a0e..deba2a8 100644 > --- a/lib/dpif.c > +++ b/lib/dpif.c > @@ -1379,6 +1379,22 @@ dpif_recv_wait(struct dpif *dpif, uint32_t handler_id) > } > } > > +/* > + * Return the datpath version. Caller is responsible for freeing > + * the string. > + */ > +char * > +dpif_get_dp_version(const struct dpif *dpif) > +{ > + char *version = NULL; > + > + if (dpif->dpif_class->get_datapath_version) { > + version = dpif->dpif_class->get_datapath_version(); > + } > + > + return version; > +} > + > /* Obtains the NetFlow engine type and engine ID for 'dpif' into > '*engine_type' > * and '*engine_id', respectively. */ > void > diff --git a/lib/dpif.h b/lib/dpif.h > index f88fa78..2bab4d0 100644 > --- a/lib/dpif.h > +++ b/lib/dpif.h > @@ -790,6 +790,7 @@ void dpif_get_netflow_ids(const struct dpif *, > int dpif_queue_to_priority(const struct dpif *, uint32_t queue_id, > uint32_t *priority); > > +char *dpif_get_dp_version(const struct dpif *); > #ifdef __cplusplus > } > #endif > diff --git a/lib/util.c b/lib/util.c > index fb2ff51..6f43a88 100644 > --- a/lib/util.c > +++ b/lib/util.c > @@ -52,6 +52,9 @@ DEFINE_PER_THREAD_MALLOCED_DATA(char *, subprogram_name); > /* --version option output. */ > static char *program_version; > > +/* ovs version stored in OVSDB */ > +static char *ovs_version = NULL; > + > /* Buffer used by ovs_strerror() and ovs_format_message(). */ > DEFINE_STATIC_PER_THREAD_DATA(struct { char s[128]; }, > strerror_buffer, > @@ -1842,3 +1845,27 @@ OVS_CONSTRUCTOR(winsock_start) { > } > } > #endif > + > +void > +ovs_version_init(const char *version) > +{ > + if (ovs_version) { > + free(ovs_version); > + ovs_version = NULL; > + } > + > + if (version) { > + ovs_version = xstrdup(version); > + } > +} > + > +char * > +ovs_version_dup(void) > +{ > + return ovs_version ? xstrdup(ovs_version) : NULL; > +} > + > +void ovs_version_uninit(void) > +{ > + free(ovs_version); > +} > diff --git a/lib/util.h b/lib/util.h > index f171dbf..4187366 100644 > --- a/lib/util.h > +++ b/lib/util.h > @@ -294,6 +294,10 @@ void free_cacheline(void *); > void ovs_strlcpy(char *dst, const char *src, size_t size); > void ovs_strzcpy(char *dst, const char *src, size_t size); > > +void ovs_version_init(const char *version); > +char *ovs_version_dup(void); > +void ovs_version_uninit(void); > + > NO_RETURN void ovs_abort(int err_no, const char *format, ...) > PRINTF_FORMAT(2, 3); > NO_RETURN void ovs_abort_valist(int err_no, const char *format, va_list) > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index d965d38..78e49be 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -280,6 +280,9 @@ struct dpif_backer { > /* Maximum number of MPLS label stack entries that the datapath supports > * in a match */ > size_t max_mpls_depth; > + > + /* Version string of the datapath */ > + char *dp_version_string; > }; > > /* All existing ofproto_backer instances, indexed by ofproto->up.type. */ > @@ -837,6 +840,7 @@ close_dpif_backer(struct dpif_backer *backer) > shash_find_and_delete(&all_dpif_backers, backer->type); > recirc_id_pool_destroy(backer->rid_pool); > free(backer->type); > + free(backer->dp_version_string); > dpif_close(backer->dpif); > free(backer); > } > @@ -965,6 +969,7 @@ open_dpif_backer(const char *type, struct dpif_backer > **backerp) > * as the kernel module checks that the 'pid' in userspace action > * is non-zero. */ > backer->variable_length_userdata = check_variable_length_userdata(backer); > + backer->dp_version_string = dpif_get_dp_version(backer->dpif); > > return error; > } > @@ -4084,6 +4089,17 @@ ofproto_dpif_send_packet(const struct ofport_dpif > *ofport, struct ofpbuf *packet > return error; > } > > +/* Return the version string of the datapath that backs up > + * this 'ofproto' > + */ > +static const char * > +get_datapath_version(const struct ofproto *ofproto_) > +{ > + struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); > + > + return ofproto->backer->dp_version_string; > +} > + > static bool > set_frag_handling(struct ofproto *ofproto_, > enum ofp_config_flags frag_handling) > @@ -5452,4 +5468,5 @@ const struct ofproto_class ofproto_dpif_class = { > group_dealloc, /* group_dealloc */ > group_modify, /* group_modify */ > group_get_stats, /* group_get_stats */ > + get_datapath_version, /* get_datapath_version */ > }; > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h > index 158f86e..f295f76 100644 > --- a/ofproto/ofproto-provider.h > +++ b/ofproto/ofproto-provider.h > @@ -1659,6 +1659,19 @@ struct ofproto_class { > > enum ofperr (*group_get_stats)(const struct ofgroup *, > struct ofputil_group_stats *); > + > +/* ## --------------------- ## */ > +/* ## Datapath information ## */ > +/* ## --------------------- ## */ > + /* Retrieve the version string of the datpath. The version > + * string can be NULL if it can not be determined. > + * > + * The version retuned is read only. The caller should not > + * free it. > + * > + * This function should be NULL if an implementation does not support it. > + */ > + const char *(*get_datapath_version)(const struct ofproto *); > }; > > extern const struct ofproto_class ofproto_dpif_class; > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at > index 2f83566..d02f993 100644 > --- a/tests/ovs-vsctl.at > +++ b/tests/ovs-vsctl.at > @@ -648,6 +648,7 @@ _uuid : <0> > controller : [] > datapath_id : [] > datapath_type : "" > +datapath_version : "" > external_ids : {} > fail_mode : [] > flood_vlans : [] > @@ -1146,6 +1147,7 @@ _uuid : <1> > controller : [] > datapath_id : [] > datapath_type : "" > +datapath_version : "" > external_ids : {} > fail_mode : [] > flood_vlans : [] > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > index 5f6000e..2b7c6be 100644 > --- a/vswitchd/bridge.c > +++ b/vswitchd/bridge.c > @@ -315,6 +315,7 @@ static void add_vlan_splinter_ports(struct bridge *, > const unsigned long int *splinter_vlans, > struct shash *ports); > > + > static void > bridge_init_ofproto(const struct ovsrec_open_vswitch *cfg) > { > @@ -381,6 +382,7 @@ bridge_init(const char *remote) > ovsdb_idl_omit(idl, &ovsrec_open_vswitch_col_system_version); > > ovsdb_idl_omit_alert(idl, &ovsrec_bridge_col_datapath_id); > + ovsdb_idl_omit_alert(idl, &ovsrec_bridge_col_datapath_version); > ovsdb_idl_omit_alert(idl, &ovsrec_bridge_col_status); > ovsdb_idl_omit_alert(idl, &ovsrec_bridge_col_rstp_status); > ovsdb_idl_omit_alert(idl, &ovsrec_bridge_col_stp_enable); > @@ -465,6 +467,7 @@ bridge_exit(void) > bridge_destroy(br); > } > ovsdb_idl_destroy(idl); > + ovs_version_uninit(); > } > > /* Looks at the list of managers in 'ovs_cfg' and extracts their remote IP > @@ -595,6 +598,9 @@ bridge_reconfigure(const struct ovsrec_open_vswitch > *ovs_cfg) > ovs_strerror(error)); > shash_destroy(&br->wanted_ports); > bridge_destroy(br); > + } else { > + /* Trigger storing datapath version. */ > + seq_change(connectivity_seq_get()); > } > } > } > @@ -2320,6 +2326,28 @@ iface_refresh_stats(struct iface *iface) > } > > static void > +br_refresh_datapath_info(struct bridge *br) > +{ > + const char *(*get_version)(const struct ofproto *); > + > + if (!br->ofproto) { > + return; > + } > + > + get_version = br->ofproto->ofproto_class->get_datapath_version; > + if (get_version){ > + const char *version; > + > + version = (*get_version)(br->ofproto); > + if (version) { > + ovsrec_bridge_set_datapath_version(br->cfg, version); > + } else { > + ovsrec_bridge_set_datapath_version(br->cfg, ""); > + } > + } > +} > + > +static void > br_refresh_stp_status(struct bridge *br) > { > struct smap smap = SMAP_INITIALIZER(&smap); > @@ -2695,6 +2723,7 @@ run_status_update(void) > > br_refresh_stp_status(br); > br_refresh_rstp_status(br); > + br_refresh_datapath_info(br); > HMAP_FOR_EACH (port, hmap_node, &br->ports) { > struct iface *iface; > > @@ -2808,6 +2837,10 @@ bridge_run(void) > } > cfg = ovsrec_open_vswitch_first(idl); > > + if (cfg) { > + ovs_version_init(cfg->ovs_version); > + } > + > /* Initialize the ofproto library. This only needs to run once, but > * it must be done after the configuration is set. If the > * initialization has already occurred, bridge_init_ofproto() > diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema > index 1817766..196c33c 100644 > --- a/vswitchd/vswitch.ovsschema > +++ b/vswitchd/vswitch.ovsschema > @@ -1,6 +1,6 @@ > {"name": "Open_vSwitch", > - "version": "7.10.1", > - "cksum": "2340049037 21461", > + "version": "7.11.1", > + "cksum": "1038213587 21518", > "tables": { > "Open_vSwitch": { > "columns": { > @@ -49,6 +49,8 @@ > "mutable": false}, > "datapath_type": { > "type": "string"}, > + "datapath_version": { > + "type": "string"}, > "datapath_id": { > "type": {"key": "string", "min": 0, "max": 1}, > "ephemeral": true}, > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index d90f221..0af0637 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -582,6 +582,11 @@ > column="other-config" key="datapath-id"/> instead.) > </column> > > + <column name="datapath_version"> > + Reports the OpenFlow datapath version in use. Can be empty if > + datapath version can not be determined. > + </column> > + > <column name="other_config" key="datapath-id"> > Exactly 16 hex digits to set the OpenFlow datapath ID to a specific > value. May not be all-zero. > -- > 1.9.1 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
