| Message ID | 20210824211520.1034341-1-i.maximets@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] ovn-northd: Hash pipeline as a digit. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
Simple enough, works for me. Acked-by: Mark Michelson <mmichels@redhat.com> On 8/24/21 5:15 PM, Ilya Maximets wrote: > Currently pipeline is hashed as a string ("egress" or "ingress"), > and this seems wasteful. Hashing it as a digit (P_IN or P_OUT) > to eliminate one call to hash_string() for every generated logical > flow. > > In my testing this saves 1.5 - 3 % of a processing time. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > lib/ovn-util.c | 14 ++++++++++---- > lib/ovn-util.h | 8 +++++++- > northd/ovn-northd.c | 8 +------- > 3 files changed, 18 insertions(+), 12 deletions(-) > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > index 3805923c8..683ca37d9 100644 > --- a/lib/ovn-util.c > +++ b/lib/ovn-util.c > @@ -523,11 +523,18 @@ ovn_is_known_nb_lsp_type(const char *type) > return false; > } > > +static enum ovn_pipeline > +ovn_pipeline_from_name(const char *pipeline) > +{ > + return pipeline[0] == 'i' ? P_IN : P_OUT; > +} > + > uint32_t > sbrec_logical_flow_hash(const struct sbrec_logical_flow *lf) > { > const struct sbrec_datapath_binding *ld = lf->logical_datapath; > - uint32_t hash = ovn_logical_flow_hash(lf->table_id, lf->pipeline, > + uint32_t hash = ovn_logical_flow_hash(lf->table_id, > + ovn_pipeline_from_name(lf->pipeline), > lf->priority, lf->match, > lf->actions); > > @@ -535,12 +542,11 @@ sbrec_logical_flow_hash(const struct sbrec_logical_flow *lf) > } > > uint32_t > -ovn_logical_flow_hash(uint8_t table_id, const char *pipeline, > +ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline pipeline, > uint16_t priority, > const char *match, const char *actions) > { > - size_t hash = hash_2words((table_id << 16) | priority, 0); > - hash = hash_string(pipeline, hash); > + size_t hash = hash_2words((table_id << 16) | priority, pipeline); > hash = hash_string(match, hash); > return hash_string(actions, hash); > } > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > index 9935cad34..b0bc70a16 100644 > --- a/lib/ovn-util.h > +++ b/lib/ovn-util.h > @@ -102,8 +102,14 @@ const char *db_table_usage(struct ds *tables, > > bool ovn_is_known_nb_lsp_type(const char *type); > > +/* The two pipelines in an OVN logical flow table. */ > +enum ovn_pipeline { > + P_IN, /* Ingress pipeline. */ > + P_OUT /* Egress pipeline. */ > +}; > + > uint32_t sbrec_logical_flow_hash(const struct sbrec_logical_flow *); > -uint32_t ovn_logical_flow_hash(uint8_t table_id, const char *pipeline, > +uint32_t ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline pipeline, > uint16_t priority, > const char *match, const char *actions); > uint32_t ovn_logical_flow_hash_datapath(const struct uuid *logical_datapath, > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index a9a3987b8..b22cf5388 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -120,12 +120,6 @@ static const char *ssl_ca_cert_file; > > /* Pipeline stages. */ > > -/* The two pipelines in an OVN logical flow table. */ > -enum ovn_pipeline { > - P_IN, /* Ingress pipeline. */ > - P_OUT /* Egress pipeline. */ > -}; > - > /* The two purposes for which ovn-northd uses OVN logical datapaths. */ > enum ovn_datapath_type { > DP_SWITCH, /* OVN logical switch. */ > @@ -4412,7 +4406,7 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od, > uint32_t hash; > > hash = ovn_logical_flow_hash(ovn_stage_get_table(stage), > - ovn_stage_get_pipeline_name(stage), > + ovn_stage_get_pipeline(stage), > priority, match, > actions); > >
On Fri, Aug 27, 2021 at 1:08 PM Mark Michelson <mmichels@redhat.com> wrote: > > Simple enough, works for me. > > Acked-by: Mark Michelson <mmichels@redhat.com> Thanks. I applied this patch to the main branch. Numan > > On 8/24/21 5:15 PM, Ilya Maximets wrote: > > Currently pipeline is hashed as a string ("egress" or "ingress"), > > and this seems wasteful. Hashing it as a digit (P_IN or P_OUT) > > to eliminate one call to hash_string() for every generated logical > > flow. > > > > In my testing this saves 1.5 - 3 % of a processing time. > > > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > > --- > > lib/ovn-util.c | 14 ++++++++++---- > > lib/ovn-util.h | 8 +++++++- > > northd/ovn-northd.c | 8 +------- > > 3 files changed, 18 insertions(+), 12 deletions(-) > > > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > > index 3805923c8..683ca37d9 100644 > > --- a/lib/ovn-util.c > > +++ b/lib/ovn-util.c > > @@ -523,11 +523,18 @@ ovn_is_known_nb_lsp_type(const char *type) > > return false; > > } > > > > +static enum ovn_pipeline > > +ovn_pipeline_from_name(const char *pipeline) > > +{ > > + return pipeline[0] == 'i' ? P_IN : P_OUT; > > +} > > + > > uint32_t > > sbrec_logical_flow_hash(const struct sbrec_logical_flow *lf) > > { > > const struct sbrec_datapath_binding *ld = lf->logical_datapath; > > - uint32_t hash = ovn_logical_flow_hash(lf->table_id, lf->pipeline, > > + uint32_t hash = ovn_logical_flow_hash(lf->table_id, > > + ovn_pipeline_from_name(lf->pipeline), > > lf->priority, lf->match, > > lf->actions); > > > > @@ -535,12 +542,11 @@ sbrec_logical_flow_hash(const struct sbrec_logical_flow *lf) > > } > > > > uint32_t > > -ovn_logical_flow_hash(uint8_t table_id, const char *pipeline, > > +ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline pipeline, > > uint16_t priority, > > const char *match, const char *actions) > > { > > - size_t hash = hash_2words((table_id << 16) | priority, 0); > > - hash = hash_string(pipeline, hash); > > + size_t hash = hash_2words((table_id << 16) | priority, pipeline); > > hash = hash_string(match, hash); > > return hash_string(actions, hash); > > } > > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > > index 9935cad34..b0bc70a16 100644 > > --- a/lib/ovn-util.h > > +++ b/lib/ovn-util.h > > @@ -102,8 +102,14 @@ const char *db_table_usage(struct ds *tables, > > > > bool ovn_is_known_nb_lsp_type(const char *type); > > > > +/* The two pipelines in an OVN logical flow table. */ > > +enum ovn_pipeline { > > + P_IN, /* Ingress pipeline. */ > > + P_OUT /* Egress pipeline. */ > > +}; > > + > > uint32_t sbrec_logical_flow_hash(const struct sbrec_logical_flow *); > > -uint32_t ovn_logical_flow_hash(uint8_t table_id, const char *pipeline, > > +uint32_t ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline pipeline, > > uint16_t priority, > > const char *match, const char *actions); > > uint32_t ovn_logical_flow_hash_datapath(const struct uuid *logical_datapath, > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index a9a3987b8..b22cf5388 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -120,12 +120,6 @@ static const char *ssl_ca_cert_file; > > > > /* Pipeline stages. */ > > > > -/* The two pipelines in an OVN logical flow table. */ > > -enum ovn_pipeline { > > - P_IN, /* Ingress pipeline. */ > > - P_OUT /* Egress pipeline. */ > > -}; > > - > > /* The two purposes for which ovn-northd uses OVN logical datapaths. */ > > enum ovn_datapath_type { > > DP_SWITCH, /* OVN logical switch. */ > > @@ -4412,7 +4406,7 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od, > > uint32_t hash; > > > > hash = ovn_logical_flow_hash(ovn_stage_get_table(stage), > > - ovn_stage_get_pipeline_name(stage), > > + ovn_stage_get_pipeline(stage), > > priority, match, > > actions); > > > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/lib/ovn-util.c b/lib/ovn-util.c index 3805923c8..683ca37d9 100644 --- a/lib/ovn-util.c +++ b/lib/ovn-util.c @@ -523,11 +523,18 @@ ovn_is_known_nb_lsp_type(const char *type) return false; } +static enum ovn_pipeline +ovn_pipeline_from_name(const char *pipeline) +{ + return pipeline[0] == 'i' ? P_IN : P_OUT; +} + uint32_t sbrec_logical_flow_hash(const struct sbrec_logical_flow *lf) { const struct sbrec_datapath_binding *ld = lf->logical_datapath; - uint32_t hash = ovn_logical_flow_hash(lf->table_id, lf->pipeline, + uint32_t hash = ovn_logical_flow_hash(lf->table_id, + ovn_pipeline_from_name(lf->pipeline), lf->priority, lf->match, lf->actions); @@ -535,12 +542,11 @@ sbrec_logical_flow_hash(const struct sbrec_logical_flow *lf) } uint32_t -ovn_logical_flow_hash(uint8_t table_id, const char *pipeline, +ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline pipeline, uint16_t priority, const char *match, const char *actions) { - size_t hash = hash_2words((table_id << 16) | priority, 0); - hash = hash_string(pipeline, hash); + size_t hash = hash_2words((table_id << 16) | priority, pipeline); hash = hash_string(match, hash); return hash_string(actions, hash); } diff --git a/lib/ovn-util.h b/lib/ovn-util.h index 9935cad34..b0bc70a16 100644 --- a/lib/ovn-util.h +++ b/lib/ovn-util.h @@ -102,8 +102,14 @@ const char *db_table_usage(struct ds *tables, bool ovn_is_known_nb_lsp_type(const char *type); +/* The two pipelines in an OVN logical flow table. */ +enum ovn_pipeline { + P_IN, /* Ingress pipeline. */ + P_OUT /* Egress pipeline. */ +}; + uint32_t sbrec_logical_flow_hash(const struct sbrec_logical_flow *); -uint32_t ovn_logical_flow_hash(uint8_t table_id, const char *pipeline, +uint32_t ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline pipeline, uint16_t priority, const char *match, const char *actions); uint32_t ovn_logical_flow_hash_datapath(const struct uuid *logical_datapath, diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index a9a3987b8..b22cf5388 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -120,12 +120,6 @@ static const char *ssl_ca_cert_file; /* Pipeline stages. */ -/* The two pipelines in an OVN logical flow table. */ -enum ovn_pipeline { - P_IN, /* Ingress pipeline. */ - P_OUT /* Egress pipeline. */ -}; - /* The two purposes for which ovn-northd uses OVN logical datapaths. */ enum ovn_datapath_type { DP_SWITCH, /* OVN logical switch. */ @@ -4412,7 +4406,7 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od, uint32_t hash; hash = ovn_logical_flow_hash(ovn_stage_get_table(stage), - ovn_stage_get_pipeline_name(stage), + ovn_stage_get_pipeline(stage), priority, match, actions);
Currently pipeline is hashed as a string ("egress" or "ingress"), and this seems wasteful. Hashing it as a digit (P_IN or P_OUT) to eliminate one call to hash_string() for every generated logical flow. In my testing this saves 1.5 - 3 % of a processing time. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- lib/ovn-util.c | 14 ++++++++++---- lib/ovn-util.h | 8 +++++++- northd/ovn-northd.c | 8 +------- 3 files changed, 18 insertions(+), 12 deletions(-)