Hi Ben,

I have verified the modified code works as expected when run with/without 
--dpdk on the vswitchd command line.

I'm wondering why there is a call to 'discover_types' in two places?

I think it might be to do with something that I could not figure out when I was 
trying to ensure discover_types was not called more often that necessary. 

I had initially tried to align the call to 'discover_types' with the 
'initial_config_done' flag in 'bridge_run'. However on the first invocation 
(the first 3 in fact) 'cfg' was NULL (i.e. there was no record in Open_vSwitch 
table). Then on the 4th invocation the idl sequence number is bumped and 
'bridge_reconfigure' is handed an empty ' ovsrec_open_vswitch' which is then 
commited to the db before vswitchd daemonizes.

However, and this is what I could not get, was that handing the empty ' 
ovsrec_open_vswitch' ('null_cfg') to discover_types when doing the initial 
config did not update the Open_vSwitch table as desired  - I was expecting 
discover_types to update the idl record and the the commit on 'daemonize_txn' 
to do the needful.  ( I had removed the separate txn create/commit from 
discover_types at this point).

Thanks,
Billy.



-----Original Message-----
From: O Mahony, Billy 
Sent: Tuesday, March 24, 2015 5:22 PM
To: 'Ben Pfaff'
Cc: dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH v2] vswitch.ovsschema: Add datapath_types and 
port_types.

Thanks Ben, our Open Stack guys will be happy.

/Billy.

-----Original Message-----
From: Ben Pfaff [mailto:b...@nicira.com]
Sent: Tuesday, March 24, 2015 4:13 PM
To: O Mahony, Billy
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v2] vswitch.ovsschema: Add datapath_types and 
port_types.

On Tue, Mar 24, 2015 at 10:39:11AM +0000, billy.o.mah...@intel.com wrote:
> From: Billy O'Mahony <billy.o.mah...@intel.com>
> 
> At startup enumerate datapath and port types and add this information 
> to the datapath_types and port_types columns in the ovsdb.
> 
> This allows an ovsdb client to query the datapath in order to 
> determine if certain datapath and port types exist. For example, by 
> querying the port_types column, an ovsdb client will be able to 
> determine if this instance of ovs-vswitchd was compiled with DPDK support.
> 
> Signed-off-by: Mark D. Gray <mark.d.g...@intel.com>
> Signed-off-by: Billy O'Mahony <billy.o.mah...@intel.com>

Thanks for the patch.

I changed the name of port_types to iface_types to match the name of the 
Interface table.

I added some more documentation.

I changed the bridge.c code to refresh the types whenever we reconfigure.  This 
is just in case the admin switches out databases underneath us so that it needs 
refreshing.  (It's not necessary to do this on every trip through the main loop 
as the original patch does
though.)

I simplified discover_types() by adding a new function sset_array().

I made some style fixes.

I dropped some unrelated changes.

Applied to master as follows:

--8<--------------------------cut here-------------------------->8--

From: "Mark D. Gray" <mark.d.g...@intel.com>
Date: Tue, 24 Mar 2015 10:39:11 +0000
Subject: [PATCH] vswitch.ovsschema: Add datapath_types and port_types.

At startup enumerate datapath and port types and add this information to the 
datapath_types and port_types columns in the ovsdb.

This allows an ovsdb client to query the datapath in order to determine if 
certain datapath and port types exist. For example, by querying the port_types 
column, an ovsdb client will be able to determine if this instance of 
ovs-vswitchd was compiled with DPDK support.

Signed-off-by: Mark D. Gray <mark.d.g...@intel.com>
Signed-off-by: Billy O'Mahony <billy.o.mah...@intel.com> [b...@nicira.com made 
several changes]
Signed-off-by: Ben Pfaff <b...@nicira.com>
---
 lib/sset.c                 | 38 ++++++++++++++++++++++++--------------
 lib/sset.h                 |  3 ++-
 vswitchd/bridge.c          | 40 ++++++++++++++++++++++++++++++++++++++++
 vswitchd/vswitch.ovsschema | 12 +++++++++---
 vswitchd/vswitch.xml       | 36 ++++++++++++++++++++++++++++++++----
 5 files changed, 107 insertions(+), 22 deletions(-)

diff --git a/lib/sset.c b/lib/sset.c
index 443538d..33c4298 100644
--- a/lib/sset.c
+++ b/lib/sset.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2011, 2012, 2013, 2015 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -269,21 +269,12 @@ sset_at_position(const struct sset *set, uint32_t 
*bucketp, uint32_t *offsetp)
     return SSET_NODE_FROM_HMAP_NODE(hmap_node);
 }
 
-static int
-compare_string_pointers(const void *a_, const void *b_) -{
-    const char *const *a = a_;
-    const char *const *b = b_;
-
-    return strcmp(*a, *b);
-}
-
-/* Returns a null-terminated array of pointers to the strings in 'set', sorted
- * alphabetically.  The caller must free the returned array when it is no
+/* Returns a null-terminated array of pointers to the strings in 'set', 
+in no
+ * particular order.  The caller must free the returned array when it 
+is no
  * longer needed, but the strings in the array belong to 'set' and thus must
  * not be modified or freed. */
 const char **
-sset_sort(const struct sset *set)
+sset_array(const struct sset *set)
 {
     size_t n = sset_count(set);
     const char **array;
@@ -298,7 +289,26 @@ sset_sort(const struct sset *set)
     ovs_assert(i == n);
     array[n] = NULL;
 
-    qsort(array, n, sizeof *array, compare_string_pointers);
+    return array;
+}
+
+static int
+compare_string_pointers(const void *a_, const void *b_) {
+    const char *const *a = a_;
+    const char *const *b = b_;
+
+    return strcmp(*a, *b);
+}
 
+/* Returns a null-terminated array of pointers to the strings in 'set', 
+sorted
+ * alphabetically.  The caller must free the returned array when it is 
+no
+ * longer needed, but the strings in the array belong to 'set' and thus 
+must
+ * not be modified or freed. */
+const char **
+sset_sort(const struct sset *set)
+{
+    const char **array = sset_array(set);
+    qsort(array, sset_count(set), sizeof *array, 
+compare_string_pointers);
     return array;
 }
diff --git a/lib/sset.h b/lib/sset.h
index 1e864ef..35bf463 100644
--- a/lib/sset.h
+++ b/lib/sset.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2011, 2012, 2013, 2015 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -80,6 +80,7 @@ struct sset_node *sset_at_position(const struct sset *,
           : false);                                 \
          (NAME) = (NEXT))
 
+const char **sset_array(const struct sset *);
 const char **sset_sort(const struct sset *);
 

 /* Implementation helper macros. */
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 4213a79..2e90ea2 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -18,6 +18,7 @@
 #include <errno.h>
 #include <inttypes.h>
 #include <stdlib.h>
+
 #include "async-append.h"
 #include "bfd.h"
 #include "bitmap.h"
@@ -26,6 +27,7 @@
 #include "coverage.h"
 #include "daemon.h"
 #include "dirs.h"
+#include "dpif.h"
 #include "dynamic-string.h"
 #include "hash.h"
 #include "hmap.h"
@@ -317,6 +319,7 @@ static ofp_port_t iface_get_requested_ofp_port(
     const struct ovsrec_interface *);
 static ofp_port_t iface_pick_ofport(const struct ovsrec_interface *);
 
+
 /* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)
  *
  * This is deprecated.  It is only for compatibility with broken device 
drivers @@ -335,6 +338,8 @@ static void add_vlan_splinter_ports(struct bridge *,
                                     const unsigned long int *splinter_vlans,
                                     struct shash *ports);
 
+static void discover_types(const struct ovsrec_open_vswitch *cfg);
+
 static void
 bridge_init_ofproto(const struct ovsrec_open_vswitch *cfg)  { @@ -394,6 +399,8 
@@ bridge_init(const char *remote)
 
     ovsdb_idl_omit_alert(idl, &ovsrec_open_vswitch_col_cur_cfg);
     ovsdb_idl_omit_alert(idl, &ovsrec_open_vswitch_col_statistics);
+    ovsdb_idl_omit_alert(idl, &ovsrec_open_vswitch_col_datapath_types);
+    ovsdb_idl_omit_alert(idl, &ovsrec_open_vswitch_col_iface_types);
     ovsdb_idl_omit(idl, &ovsrec_open_vswitch_col_external_ids);
     ovsdb_idl_omit(idl, &ovsrec_open_vswitch_col_ovs_version);
     ovsdb_idl_omit(idl, &ovsrec_open_vswitch_col_db_version);
@@ -571,6 +578,10 @@ bridge_reconfigure(const struct ovsrec_open_vswitch 
*ovs_cfg)
         smap_get_int(&ovs_cfg->other_config, "n-handler-threads", 0),
         smap_get_int(&ovs_cfg->other_config, "n-revalidator-threads", 0));
 
+    if (ovs_cfg) {
+        discover_types(ovs_cfg);
+    }
+
     /* Destroy "struct bridge"s, "struct port"s, and "struct iface"s according
      * to 'ovs_cfg', with only very minimal configuration otherwise.
      *
@@ -2940,6 +2951,7 @@ bridge_run(void)
 
         if (cfg) {
             ovsrec_open_vswitch_set_cur_cfg(cfg, cfg->next_cfg);
+            discover_types(cfg);
         }
 
         /* If we are completing our initial configuration for this run @@ 
-5023,3 +5035,31 @@ mirror_refresh_stats(struct mirror *m)
 
     ovsrec_mirror_set_statistics(m->cfg, keys, values, stat_cnt);  }
+
+/*
+ * Add registered netdev and dpif types to ovsdb to allow external
+ * applications to query the capabilities of the Open vSwitch instance
+ * running on the node.
+ */
+static void
+discover_types(const struct ovsrec_open_vswitch *cfg) {
+    struct sset types;
+
+    /* Datapath types. */
+    sset_init(&types);
+    dp_enumerate_types(&types);
+    const char **datapath_types = sset_array(&types);
+    ovsrec_open_vswitch_set_datapath_types(cfg, datapath_types,
+                                           sset_count(&types));
+    free(datapath_types);
+    sset_destroy(&types);
+
+    /* Port types. */
+    sset_init(&types);
+    netdev_enumerate_types(&types);
+    const char **iface_types = sset_array(&types);
+    ovsrec_open_vswitch_set_iface_types(cfg, iface_types, sset_count(&types));
+    free(iface_types);
+    sset_destroy(&types);
+}
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema index 
4898b16..35f145f 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
 {"name": "Open_vSwitch",
- "version": "7.11.2",
- "cksum": "320332148 22294",
+ "version": "7.12.1",
+ "cksum": "2211824403 22535",
  "tables": {
    "Open_vSwitch": {
      "columns": {
@@ -39,7 +39,13 @@
                   "min": 0, "max": 1}},
        "system_version": {
          "type": {"key": {"type": "string"},
-                  "min": 0, "max": 1}}},
+                  "min": 0, "max": 1}},
+       "datapath_types": {
+         "type": {"key": {"type": "string"},
+                  "min": 0, "max": "unlimited"}},
+       "iface_types": {
+         "type": {"key": {"type": "string"},
+                  "min": 0, "max": "unlimited"}}},
      "isRoot": true,
      "maxRows": 1},
    "Bridge": {
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index 07f3bea..e04aefc 
100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -423,6 +423,28 @@
 
     </group>
 
+    <group title="Capabilities">
+      <p>
+        These columns report capabilities of the Open vSwitch instance.
+      </p>
+      <column name="datapath_types">
+        <p>
+          This column reports the different dpifs registered with the system.
+          These are the values that this instance supports in the <ref
+          column="datapath_type" table="Bridge"/> column of the <ref
+          table="Bridge"/> table.
+        </p>
+      </column>
+      <column name="iface_types">
+        <p>
+          This column reports the different netdevs registered with the system.
+          These are the values that this instance supports in the <ref
+          column="type" table="Interface"/> column of the <ref
+          table="Interface"/> table.
+        </p>
+      </column>
+    </group>
+
     <group title="Database Configuration">
       <p>
         These columns primarily configure the Open vSwitch database @@ -928,9 
+950,12 @@
 
     <group title="Other Features">
       <column name="datapath_type">
-        Name of datapath provider.  The kernel datapath has
-        type <code>system</code>.  The userspace datapath has
-        type <code>netdev</code>.
+        Name of datapath provider.  The kernel datapath has type
+        <code>system</code>.  The userspace datapath has type
+        <code>netdev</code>.  A manager may refer to the <ref
+        table="Open_vSwitch" column="datapath_types"/> column of the <ref
+        table="Open_vSwitch"/> table for a list of the types accepted by this
+        Open vSwitch instance.
       </column>
 
       <column name="external_ids" key="bridge-id"> @@ -1747,7 +1772,10 @@
     <group title="System-Specific Details">
       <column name="type">
         <p>
-          The interface type, one of:
+          The interface type.  The types supported by a particular instance of
+          Open vSwitch are listed in the <ref table="Open_vSwitch"
+          column="iface_types"/> column in the <ref table="Open_vSwitch"/>
+          table.  The following types are defined:
         </p>
 
         <dl>
--
2.1.3

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to