On 12/26/2017 10:08 AM, Justin Pettit wrote:
Thanks for trying it out, Greg.  I don't know why that would affect the kernel 
checks, and I don't see them on my system with this patch:

-=-=-=-=-=-=-=-
## ------------------------------ ##
## openvswitch 2.8.90 test suite. ##
## ------------------------------ ##

datapath-sanity

   1: datapath - ping between two ports               ok
   2: datapath - http between two ports               ok
   3: datapath - ping between two ports on vlan       ok
-=-=-=-=-=-=-=-

I had also gotten a clean run on our internal tester for the entire series.  Do 
you mind doing a bit more digging on your side to see if you can narrow down 
the cause?

--Justin

Hmmm... curious.  Yes, I'll dig a little deeper.

Thanks,

- Greg


On Dec 26, 2017, at 10:45 AM, Gregory Rose <[email protected]> wrote:

On 12/21/2017 2:25 PM, Justin Pettit wrote:
This will have callers in the future.

Signed-off-by: Justin Pettit <[email protected]>
---
  ofproto/ofproto-dpif-trace.c |  2 +-
  ofproto/ofproto-dpif.c       | 92 +++++++++++++++++++++++++++++++-------------
  ofproto/ofproto-dpif.h       | 13 +++++--
  3 files changed, 77 insertions(+), 30 deletions(-)
Justin,

I was reviewing and testing this patch series and I found that the 'check-kmod' 
tests are all failing
after applying the patch series.


datapath-sanity

   1: datapath - ping between two ports               FAILED 
(system-traffic.at:23)
   2: datapath - http between two ports               FAILED 
(system-traffic.at:43)
   3: datapath - ping between two ports on vlan       FAILED 
(system-traffic.at:69)
   4: datapath - ping between two ports on cvlan      FAILED 
(system-traffic.at:100)
   5: datapath - ping6 between two ports              FAILED 
(system-traffic.at:128)
   6: datapath - ping6 between two ports on vlan      FAILED 
(system-traffic.at:159)
   7: datapath - ping6 between two ports on cvlan     FAILED 
(system-traffic.at:190)
   8: datapath - ping over bond                       FAILED 
(system-traffic.at:215)
   9: datapath - ping over vxlan tunnel               FAILED 
(system-traffic.at:256)
  10: datapath - ping over vxlan6 tunnel              FAILED 
(system-traffic.at:299)
  11: datapath - ping over gre tunnel                 FAILED 
(system-traffic.at:339)
  12: datapath - ping over geneve tunnel              FAILED 
(system-traffic.at:380)
  13: datapath - ping over geneve6 tunnel             FAILED 
(system-traffic.at:423)
  14: datapath - clone action                         FAILED 
(system-traffic.at:455)

I've done no further investigation as to why yet but will if it would help out. 
 All the user space checks from 'make check' are passing just fine.

Thanks,

- Greg
diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index d5da48e326bb..4999d1d6f326 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -354,7 +354,7 @@ parse_flow_and_packet(int argc, const char *argv[],
              goto exit;
          }
  -        *ofprotop = ofproto_dpif_lookup(argv[1]);
+        *ofprotop = ofproto_dpif_lookup_by_name(argv[1]);
          if (!*ofprotop) {
              error = "Unknown bridge name";
              goto exit;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 43b7b89f26e4..7d628e11328a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -58,6 +58,7 @@
  #include "openvswitch/ofp-print.h"
  #include "openvswitch/ofp-util.h"
  #include "openvswitch/ofpbuf.h"
+#include "openvswitch/uuid.h"
  #include "openvswitch/vlog.h"
  #include "ovs-lldp.h"
  #include "ovs-rcu.h"
@@ -71,6 +72,7 @@
  #include "unaligned.h"
  #include "unixctl.h"
  #include "util.h"
+#include "uuid.h"
  #include "vlan-bitmap.h"
    VLOG_DEFINE_THIS_MODULE(ofproto_dpif);
@@ -187,7 +189,12 @@ COVERAGE_DEFINE(rev_mcast_snooping);
  struct shash all_dpif_backers = SHASH_INITIALIZER(&all_dpif_backers);
    /* All existing ofproto_dpif instances, indexed by ->up.name. */
-struct hmap all_ofproto_dpifs = HMAP_INITIALIZER(&all_ofproto_dpifs);
+struct hmap all_ofproto_dpifs_by_name =
+                          HMAP_INITIALIZER(&all_ofproto_dpifs_by_name);
+
+/* All existing ofproto_dpif instances, indexed by ->uuid. */
+struct hmap all_ofproto_dpifs_by_uuid =
+                          HMAP_INITIALIZER(&all_ofproto_dpifs_by_uuid);
    static bool ofproto_use_tnl_push_pop = true;
  static void ofproto_unixctl_init(void);
@@ -268,7 +275,8 @@ enumerate_names(const char *type, struct sset *names)
      struct ofproto_dpif *ofproto;
        sset_clear(names);
-    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node,
+                   &all_ofproto_dpifs_by_name) {
          if (strcmp(type, ofproto->up.type)) {
              continue;
          }
@@ -311,7 +319,8 @@ lookup_ofproto_dpif_by_port_name(const char *name)
  {
      struct ofproto_dpif *ofproto;
  -    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node,
+                   &all_ofproto_dpifs_by_name) {
          if (sset_contains(&ofproto->ports, name)) {
              return ofproto;
          }
@@ -368,7 +377,8 @@ type_run(const char *type)
          simap_init(&tmp_backers);
          simap_swap(&backer->tnl_backers, &tmp_backers);
  -        HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+        HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node,
+                       &all_ofproto_dpifs_by_name) {
              struct ofport_dpif *iter;
                if (backer != ofproto->backer) {
@@ -433,7 +443,8 @@ type_run(const char *type)
          backer->need_revalidate = 0;
            xlate_txn_start();
-        HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+        HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node,
+                       &all_ofproto_dpifs_by_name) {
              struct ofport_dpif *ofport;
              struct ofbundle *bundle;
  @@ -522,7 +533,8 @@ process_dpif_all_ports_changed(struct dpif_backer *backer)
      const char *devname;
        sset_init(&devnames);
-    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node,
+                   &all_ofproto_dpifs_by_name) {
          if (ofproto->backer == backer) {
              struct ofport *ofport;
  @@ -552,8 +564,8 @@ process_dpif_port_change(struct dpif_backer *backer, 
const char *devname)
          return;
      }
  -    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node,
-                   &all_ofproto_dpifs) {
+    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node,
+                   &all_ofproto_dpifs_by_name) {
          if (simap_contains(&ofproto->backer->tnl_backers, devname)) {
              return;
          }
@@ -604,7 +616,8 @@ process_dpif_port_error(struct dpif_backer *backer, int 
error)
  {
      struct ofproto_dpif *ofproto;
  -    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node,
+                   &all_ofproto_dpifs_by_name) {
          if (ofproto->backer == backer) {
              sset_clear(&ofproto->port_poll_set);
              ofproto->port_poll_errno = error;
@@ -1458,8 +1471,12 @@ construct(struct ofproto *ofproto_)
          }
      }
  -    hmap_insert(&all_ofproto_dpifs, &ofproto->all_ofproto_dpifs_node,
+    hmap_insert(&all_ofproto_dpifs_by_name,
+                &ofproto->all_ofproto_dpifs_by_name_node,
                  hash_string(ofproto->up.name, 0));
+    hmap_insert(&all_ofproto_dpifs_by_uuid,
+                &ofproto->all_ofproto_dpifs_by_uuid_node,
+                uuid_hash(&ofproto->uuid));
      memset(&ofproto->stats, 0, sizeof ofproto->stats);
        ofproto_init_tables(ofproto_, N_TABLES);
@@ -1558,7 +1575,10 @@ destruct(struct ofproto *ofproto_, bool del)
       * to the ofproto or anything in it. */
      udpif_synchronize(ofproto->backer->udpif);
  -    hmap_remove(&all_ofproto_dpifs, &ofproto->all_ofproto_dpifs_node);
+    hmap_remove(&all_ofproto_dpifs_by_name,
+                &ofproto->all_ofproto_dpifs_by_name_node);
+    hmap_remove(&all_ofproto_dpifs_by_uuid,
+                &ofproto->all_ofproto_dpifs_by_uuid_node);
        OFPROTO_FOR_EACH_TABLE (table, &ofproto->up) {
          CLS_FOR_EACH (rule, up.cr, &table->cls) {
@@ -2849,7 +2869,8 @@ bundle_flush_macs(struct ofbundle *bundle, bool 
all_ofprotos)
              if (all_ofprotos) {
                  struct ofproto_dpif *o;
  -                HMAP_FOR_EACH (o, all_ofproto_dpifs_node, 
&all_ofproto_dpifs) {
+                HMAP_FOR_EACH (o, all_ofproto_dpifs_by_name_node,
+                               &all_ofproto_dpifs_by_name) {
                      if (o != ofproto) {
                          struct mac_entry *e;
  @@ -3528,7 +3549,8 @@ ofport_update_peer(struct ofport_dpif *ofport)
          return;
      }
  -    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node,
+                   &all_ofproto_dpifs_by_name) {
          struct ofport *peer_ofport;
          struct ofport_dpif *peer;
          char *peer_peer;
@@ -4943,12 +4965,13 @@ get_netflow_ids(const struct ofproto *ofproto_,
  }
  
  struct ofproto_dpif *
-ofproto_dpif_lookup(const char *name)
+ofproto_dpif_lookup_by_name(const char *name)
  {
      struct ofproto_dpif *ofproto;
  -    HMAP_FOR_EACH_WITH_HASH (ofproto, all_ofproto_dpifs_node,
-                             hash_string(name, 0), &all_ofproto_dpifs) {
+    HMAP_FOR_EACH_WITH_HASH (ofproto, all_ofproto_dpifs_by_name_node,
+                             hash_string(name, 0),
+                             &all_ofproto_dpifs_by_name) {
          if (!strcmp(ofproto->up.name, name)) {
              return ofproto;
          }
@@ -4956,6 +4979,20 @@ ofproto_dpif_lookup(const char *name)
      return NULL;
  }
  +struct ofproto_dpif *
+ofproto_dpif_lookup_by_uuid(const struct uuid *uuid)
+{
+    struct ofproto_dpif *ofproto;
+
+    HMAP_FOR_EACH_WITH_HASH (ofproto, all_ofproto_dpifs_by_uuid_node,
+                             uuid_hash(uuid), &all_ofproto_dpifs_by_uuid) {
+        if (uuid_equals(&ofproto->uuid, uuid)) {
+            return ofproto;
+        }
+    }
+    return NULL;
+}
+
  static void
  ofproto_unixctl_fdb_flush(struct unixctl_conn *conn, int argc,
                            const char *argv[], void *aux OVS_UNUSED)
@@ -4963,7 +5000,7 @@ ofproto_unixctl_fdb_flush(struct unixctl_conn *conn, int 
argc,
      struct ofproto_dpif *ofproto;
        if (argc > 1) {
-        ofproto = ofproto_dpif_lookup(argv[1]);
+        ofproto = ofproto_dpif_lookup_by_name(argv[1]);
          if (!ofproto) {
              unixctl_command_reply_error(conn, "no such bridge");
              return;
@@ -4972,7 +5009,8 @@ ofproto_unixctl_fdb_flush(struct unixctl_conn *conn, int 
argc,
          mac_learning_flush(ofproto->ml);
          ovs_rwlock_unlock(&ofproto->ml->rwlock);
      } else {
-        HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+        HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node,
+                       &all_ofproto_dpifs_by_name) {
              ovs_rwlock_wrlock(&ofproto->ml->rwlock);
              mac_learning_flush(ofproto->ml);
              ovs_rwlock_unlock(&ofproto->ml->rwlock);
@@ -4989,7 +5027,7 @@ ofproto_unixctl_mcast_snooping_flush(struct unixctl_conn 
*conn, int argc,
      struct ofproto_dpif *ofproto;
        if (argc > 1) {
-        ofproto = ofproto_dpif_lookup(argv[1]);
+        ofproto = ofproto_dpif_lookup_by_name(argv[1]);
          if (!ofproto) {
              unixctl_command_reply_error(conn, "no such bridge");
              return;
@@ -5001,7 +5039,8 @@ ofproto_unixctl_mcast_snooping_flush(struct unixctl_conn 
*conn, int argc,
          }
          mcast_snooping_mdb_flush(ofproto->ms);
      } else {
-        HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+        HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node,
+                       &all_ofproto_dpifs_by_name) {
              if (!mcast_snooping_enabled(ofproto->ms)) {
                  continue;
              }
@@ -5027,7 +5066,7 @@ ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int 
argc OVS_UNUSED,
      const struct ofproto_dpif *ofproto;
      const struct mac_entry *e;
  -    ofproto = ofproto_dpif_lookup(argv[1]);
+    ofproto = ofproto_dpif_lookup_by_name(argv[1]);
      if (!ofproto) {
          unixctl_command_reply_error(conn, "no such bridge");
          return;
@@ -5063,7 +5102,7 @@ ofproto_unixctl_mcast_snooping_show(struct unixctl_conn 
*conn,
      struct mcast_group_bundle *b;
      struct mcast_mrouter_bundle *mrouter;
  -    ofproto = ofproto_dpif_lookup(argv[1]);
+    ofproto = ofproto_dpif_lookup_by_name(argv[1]);
      if (!ofproto) {
          unixctl_command_reply_error(conn, "no such bridge");
          return;
@@ -5114,7 +5153,8 @@ get_ofprotos(struct shash *ofproto_shash)
  {
      const struct ofproto_dpif *ofproto;
  -    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node,
+                   &all_ofproto_dpifs_by_name) {
          char *name = xasprintf("%s@%s", ofproto->up.type, ofproto->up.name);
          shash_add_nocopy(ofproto_shash, name, ofproto);
      }
@@ -5403,7 +5443,7 @@ ofproto_unixctl_dpif_dump_flows(struct unixctl_conn *conn,
      struct dpif_flow f;
      int error;
  -    ofproto = ofproto_dpif_lookup(argv[argc - 1]);
+    ofproto = ofproto_dpif_lookup_by_name(argv[argc - 1]);
      if (!ofproto) {
          unixctl_command_reply_error(conn, "no such bridge");
          return;
@@ -5488,7 +5528,7 @@ ofproto_unixctl_dpif_show_dp_features(struct unixctl_conn 
*conn,
  {
      struct ds ds = DS_EMPTY_INITIALIZER;
      const char *br = argv[argc -1];
-    struct ofproto_dpif *ofproto = ofproto_dpif_lookup(br);
+    struct ofproto_dpif *ofproto = ofproto_dpif_lookup_by_name(br);
        if (!ofproto) {
          unixctl_command_reply_error(conn, "no such bridge");
@@ -5507,7 +5547,7 @@ ofproto_unixctl_dpif_set_dp_features(struct unixctl_conn 
*conn,
      struct ds ds = DS_EMPTY_INITIALIZER;
      const char *br = argv[1];
      const char *name, *value;
-    struct ofproto_dpif *ofproto = ofproto_dpif_lookup(br);
+    struct ofproto_dpif *ofproto = ofproto_dpif_lookup_by_name(br);
      bool changed;
        if (!ofproto) {
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 0857c070c8ac..0bb07638c15e 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -60,6 +60,7 @@
  struct dpif_flow_stats;
  struct ofproto_async_msg;
  struct ofproto_dpif;
+struct uuid;
  struct xlate_cache;
    /* Number of implemented OpenFlow tables. */
@@ -251,7 +252,10 @@ struct ofport_dpif *odp_port_to_ofport(const struct 
dpif_backer *, odp_port_t);
  /* A bridge based on a "dpif" datapath. */
    struct ofproto_dpif {
-    struct hmap_node all_ofproto_dpifs_node; /* In 'all_ofproto_dpifs'. */
+    /* In 'all_ofproto_dpifs_by_name'. */
+    struct hmap_node all_ofproto_dpifs_by_name_node;
+    struct hmap_node all_ofproto_dpifs_by_uuid_node;
+
      struct ofproto up;
      struct dpif_backer *backer;
  @@ -305,9 +309,12 @@ struct ofproto_dpif {
  };
    /* All existing ofproto_dpif instances, indexed by ->up.name. */
-extern struct hmap all_ofproto_dpifs;
+extern struct hmap all_ofproto_dpifs_by_name;
+struct ofproto_dpif *ofproto_dpif_lookup_by_name(const char *name);
  -struct ofproto_dpif *ofproto_dpif_lookup(const char *name);
+/* All existing ofproto_dpif instances, indexed by ->uuid. */
+extern struct hmap all_ofproto_dpifs_by_uuid;
+struct ofproto_dpif *ofproto_dpif_lookup_by_uuid(const struct uuid *uuid);
    ovs_version_t ofproto_dpif_get_tables_version(struct ofproto_dpif *);

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to