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 > 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
