[ovs-dev] [PATCH 4/4] ovn-northd: Reduce amount of flow hashing.
Numan Siddique
nusiddiq at redhat.com
Thu Feb 22 04:29:19 UTC 2018
On Thu, Feb 22, 2018 at 4:49 AM, Mark Michelson <mmichels at redhat.com> wrote:
> Hi,
>
> While testing an unrelated change in OVS master (using make sandbox
> SANDBOXFLAGS="--ovn"), I noticed that my laptop was making way more noise
> than normal. Looking at `top` output, an ovsdb-server process was running
> at 100% CPU, ovn-controller was running at 75% CPU, and ovn-northd was
> running at about 38% CPU. If I start the ovs sandbox, the problem does not
> present itself until after I run the "ovn-setup.sh" script. As soon as I
> run the script, the CPU usage goes soaring.
>
> If I revert to the patch just before this one ("implement synthetic
> columns"), the problem does not occur. At least in a sandbox environment,
> this patch appears to be causing some sort of busy loops that drive the
> CPU% of processes up. Looking at the change, it's not immediately obvious
> why this would be happening.
>
>
I can confirm that I am seeing the same with my sandbox environment. OVN
North db ovsdb-server is almost 100% and ovn-controller at around 90% in
my case.
Thanks
Numan
> On 02/14/2018 03:54 PM, Ben Pfaff wrote:
>
>> Jakub Sitnicki demonstrated that repeatedly calculating row hashes is
>> expensive, so this should improve ovn-northd performance.
>>
>> Reported-by: Jakub Sitnicki <jkbs at redhat.com>
>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-February
>> /344404.html
>> Signed-off-by: Ben Pfaff <blp at ovn.org>
>> ---
>> 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(l
>> flow->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(lf
>> low->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);
>>
>>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list