[ovs-dev] [no-slow 1/6] ofproto-dpif: Add ability to look up an ofproto by UUID.
Justin Pettit
jpettit at ovn.org
Tue Dec 26 18:08:34 UTC 2017
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 <gvrose8192 at gmail.com> wrote:
>
> On 12/21/2017 2:25 PM, Justin Pettit wrote:
>> This will have callers in the future.
>>
>> Signed-off-by: Justin Pettit <jpettit at ovn.org>
>> ---
>> 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 *);
>>
>
More information about the dev
mailing list