Jakub Sitnicki demonstrated that repeatedly calculating row hashes is expensive, so this should improve ovn-northd performance.
Reported-by: Jakub Sitnicki <[email protected]> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-February/344404.html Signed-off-by: Ben Pfaff <[email protected]> --- ovn/lib/ovn-sb-idl.ann | 20 ++++++++++++++++++++ ovn/lib/ovn-util.c | 27 +++++++++++++++++++++++++++ ovn/lib/ovn-util.h | 7 +++++++ ovn/northd/ovn-northd.c | 28 +++++++++++++++++----------- 4 files changed, 71 insertions(+), 11 deletions(-) diff --git a/ovn/lib/ovn-sb-idl.ann b/ovn/lib/ovn-sb-idl.ann index 2dfc64e3c4a7..e51238b92e97 100644 --- a/ovn/lib/ovn-sb-idl.ann +++ b/ovn/lib/ovn-sb-idl.ann @@ -7,3 +7,23 @@ s["idlPrefix"] = "sbrec_" s["idlHeader"] = "\"ovn/lib/ovn-sb-idl.h\"" + +s["hDecls"] = '#include "ovn/lib/ovn-util.h"' + +# Adds an integer column named 'column' to 'table' in 's'. The column +# values is calculated with 'expression' based on the values of the columns +# named in the array 'dependencies'. +def synthesize_integer_column(s, table, column, dependencies, expression): + s["tables"][table]["columns"][column] = { + "type": "integer", + "extensions": { + "dependencies": dependencies, + "parse": "row->%s = %s;" % (column, expression), + "synthetic": True + } + } + +synthesize_integer_column(s, "Logical_Flow", "hash", + ["logical_datapath", "table_id", "pipeline", + "priority", "match", "actions"], + "sbrec_logical_flow_hash(row)") diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c index a554c23f5ae8..e9464e926d74 100644 --- a/ovn/lib/ovn-util.c +++ b/ovn/lib/ovn-util.c @@ -17,6 +17,7 @@ #include "dirs.h" #include "openvswitch/vlog.h" #include "ovn/lib/ovn-nb-idl.h" +#include "ovn/lib/ovn-sb-idl.h" VLOG_DEFINE_THIS_MODULE(ovn_util); @@ -329,3 +330,29 @@ ovn_is_known_nb_lsp_type(const char *type) return false; } + +uint32_t +sbrec_logical_flow_hash(const struct sbrec_logical_flow *lf) +{ + const struct sbrec_datapath_binding *ld = lf->logical_datapath; + if (!ld) { + return 0; + } + + return ovn_logical_flow_hash(&ld->header_.uuid, + lf->table_id, lf->pipeline, + lf->priority, lf->match, lf->actions); +} + +uint32_t +ovn_logical_flow_hash(const struct uuid *logical_datapath, + uint8_t table_id, const char *pipeline, + uint16_t priority, + const char *match, const char *actions) +{ + size_t hash = uuid_hash(logical_datapath); + hash = hash_2words((table_id << 16) | priority, hash); + hash = hash_string(pipeline, hash); + hash = hash_string(match, hash); + return hash_string(actions, hash); +} diff --git a/ovn/lib/ovn-util.h b/ovn/lib/ovn-util.h index 9b456426dc67..7ff9f9a1c73b 100644 --- a/ovn/lib/ovn-util.h +++ b/ovn/lib/ovn-util.h @@ -19,6 +19,7 @@ #include "lib/packets.h" struct nbrec_logical_router_port; +struct sbrec_logical_flow; struct uuid; struct ipv4_netaddr { @@ -69,4 +70,10 @@ const char *default_sb_db(void); bool ovn_is_known_nb_lsp_type(const char *type); +uint32_t sbrec_logical_flow_hash(const struct sbrec_logical_flow *); +uint32_t ovn_logical_flow_hash(const struct uuid *logical_datapath, + uint8_t table_id, const char *pipeline, + uint16_t priority, + const char *match, const char *actions); + #endif diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 4d95a3d9d40e..5d764f6e9404 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -194,6 +194,13 @@ ovn_stage_get_pipeline(enum ovn_stage stage) return (stage >> 8) & 1; } +/* Returns the pipeline name to which 'stage' belongs. */ +static const char * +ovn_stage_get_pipeline_name(enum ovn_stage stage) +{ + return ovn_stage_get_pipeline(stage) == P_IN ? "ingress" : "egress"; +} + /* Returns the table to which 'stage' belongs. */ static uint8_t ovn_stage_get_table(enum ovn_stage stage) @@ -2271,10 +2278,11 @@ struct ovn_lflow { static size_t ovn_lflow_hash(const struct ovn_lflow *lflow) { - size_t hash = uuid_hash(&lflow->od->key); - hash = hash_2words((lflow->stage << 16) | lflow->priority, hash); - hash = hash_string(lflow->match, hash); - return hash_string(lflow->actions, hash); + return ovn_logical_flow_hash(&lflow->od->key, + ovn_stage_get_table(lflow->stage), + ovn_stage_get_pipeline_name(lflow->stage), + lflow->priority, lflow->match, + lflow->actions); } static bool @@ -2331,7 +2339,7 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od, static struct ovn_lflow * ovn_lflow_find(struct hmap *lflows, struct ovn_datapath *od, enum ovn_stage stage, uint16_t priority, - const char *match, const char *actions) + const char *match, const char *actions, uint32_t hash) { struct ovn_lflow target; ovn_lflow_init(&target, od, stage, priority, @@ -2339,8 +2347,7 @@ ovn_lflow_find(struct hmap *lflows, struct ovn_datapath *od, NULL, NULL); struct ovn_lflow *lflow; - HMAP_FOR_EACH_WITH_HASH (lflow, hmap_node, ovn_lflow_hash(&target), - lflows) { + HMAP_FOR_EACH_WITH_HASH (lflow, hmap_node, hash, lflows) { if (ovn_lflow_equal(lflow, &target)) { return lflow; } @@ -6014,7 +6021,7 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, = !strcmp(sbflow->pipeline, "ingress") ? P_IN : P_OUT; struct ovn_lflow *lflow = ovn_lflow_find( &lflows, od, ovn_stage_build(dp_type, pipeline, sbflow->table_id), - sbflow->priority, sbflow->match, sbflow->actions); + sbflow->priority, sbflow->match, sbflow->actions, sbflow->hash); if (lflow) { ovn_lflow_destroy(&lflows, lflow); } else { @@ -6023,13 +6030,12 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, } struct ovn_lflow *lflow, *next_lflow; HMAP_FOR_EACH_SAFE (lflow, next_lflow, hmap_node, &lflows) { - enum ovn_pipeline pipeline = ovn_stage_get_pipeline(lflow->stage); + const char *pipeline = ovn_stage_get_pipeline_name(lflow->stage); uint8_t table = ovn_stage_get_table(lflow->stage); sbflow = sbrec_logical_flow_insert(ctx->ovnsb_txn); sbrec_logical_flow_set_logical_datapath(sbflow, lflow->od->sb); - sbrec_logical_flow_set_pipeline( - sbflow, pipeline == P_IN ? "ingress" : "egress"); + sbrec_logical_flow_set_pipeline(sbflow, pipeline); sbrec_logical_flow_set_table_id(sbflow, table); sbrec_logical_flow_set_priority(sbflow, lflow->priority); sbrec_logical_flow_set_match(sbflow, lflow->match); -- 2.16.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
