[ovs-dev] [dp version v2] bridge: Store datapath version into ovsdb
Justin Pettit
jpettit at nicira.com
Thu Oct 23 01:42:49 UTC 2014
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 <azhou at nicira.com> 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.
> ---
> 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
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list