[ovs-dev] [PATCH v3 ovn 1/3] northd: optimize build_lb_rules routine
Mark Gray
mark.d.gray at redhat.com
Fri Aug 27 16:09:17 UTC 2021
On 26/08/2021 17:50, Lorenzo Bianconi wrote:
> Introduce ovn_lflow_add_at_with_hash to use a precomputed hash if not
> dependent on the logical flow.
> Introduce ovn_dp_group_add_with_reference routine to update the
> dapath_group with the datapath pointer without run a lflow lookup.
> Optimize build_lb_rules routine relying on ovn_lflow_add_at_with_hash
> and ovn_dp_group_add_with_reference since they do not depend on the
> datapath if meter is NULL.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> ---
> northd/ovn-northd.c | 71 ++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 58 insertions(+), 13 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index af413aba4..5efa54ee5 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -4362,7 +4362,7 @@ static struct hashrow_locks lflow_locks;
> /* Adds a row with the specified contents to the Logical_Flow table.
> * Version to use when locking is required.
> */
> -static void
> +static struct ovn_lflow *
> do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od,
> uint32_t hash, enum ovn_stage stage, uint16_t priority,
> const char *match, const char *actions, const char *io_port,
> @@ -4378,7 +4378,7 @@ do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od,
> actions, ctrl_meter, hash);
> if (old_lflow) {
> hmapx_add(&old_lflow->od_group, od);
> - return;
> + return old_lflow;
> }
> }
>
> @@ -4397,6 +4397,31 @@ do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od,
> } else {
> hmap_insert_fast(lflow_map, &lflow->hmap_node, hash);
> }
> + return lflow;
> +}
> +
> +static struct ovn_lflow *
> +ovn_lflow_add_at_with_hash(struct hmap *lflow_map, struct ovn_datapath *od,
> + enum ovn_stage stage, uint16_t priority,
> + const char *match, const char *actions,
> + const char *io_port, const char *ctrl_meter,
> + const struct ovsdb_idl_row *stage_hint,
> + const char *where, uint32_t hash)
> +{
> + struct ovn_lflow *lflow;
> +
> + ovs_assert(ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od));
> + if (use_logical_dp_groups && use_parallel_build) {
> + lock_hash_row(&lflow_locks, hash);
> + lflow = do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match,
> + actions, io_port, stage_hint, where,
> + ctrl_meter);
> + unlock_hash_row(&lflow_locks, hash);
> + } else {
> + lflow = do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match,
> + actions, io_port, stage_hint, where, ctrl_meter);
> + }
> + return lflow;
> }
>
> /* Adds a row with the specified contents to the Logical_Flow table. */
> @@ -4407,24 +4432,34 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od,
> const char *ctrl_meter,
> const struct ovsdb_idl_row *stage_hint, const char *where)
> {
> - ovs_assert(ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od));
> -
> uint32_t hash;
>
> hash = ovn_logical_flow_hash(ovn_stage_get_table(stage),
> ovn_stage_get_pipeline_name(stage),
> priority, match,
> actions);
> + ovn_lflow_add_at_with_hash(lflow_map, od, stage, priority, match, actions,
> + io_port, ctrl_meter, stage_hint, where, hash);
> +}
>
> - if (use_logical_dp_groups && use_parallel_build) {
> +static bool
> +ovn_dp_group_add_with_reference(struct ovn_lflow *lflow_ref,
> + struct ovn_datapath *od,
> + uint32_t hash)
> +{
> + if (!use_logical_dp_groups || !lflow_ref) {
> + return false;
> + }
> +
> + if (use_parallel_build) {
> lock_hash_row(&lflow_locks, hash);
> - do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match,
> - actions, io_port, stage_hint, where, ctrl_meter);
> + hmapx_add(&lflow_ref->od_group, od);
> unlock_hash_row(&lflow_locks, hash);
> } else {
> - do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match,
> - actions, io_port, stage_hint, where, ctrl_meter);
> + hmapx_add(&lflow_ref->od_group, od);
> }
> +
> + return true;
> }
>
> /* Adds a row with the specified contents to the Logical_Flow table. */
> @@ -6372,17 +6407,27 @@ build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb,
> ds_put_format(match, " && %s.dst == %d", proto, lb_vip->vip_port);
> priority = 120;
> }
> +
> + struct ovn_lflow *lflow_ref = NULL;
> + uint32_t hash = ovn_logical_flow_hash(
> + ovn_stage_get_table(S_SWITCH_IN_STATEFUL),
> + ovn_stage_get_pipeline_name(S_SWITCH_IN_STATEFUL), priority,
> + ds_cstr(match), ds_cstr(action));
> +
> for (size_t j = 0; j < lb->n_nb_ls; j++) {
> struct ovn_datapath *od = lb->nb_ls[j];
>
> if (reject) {
> meter = copp_meter_get(COPP_REJECT, od->nbs->copp,
> meter_groups);
> + } else if (ovn_dp_group_add_with_reference(lflow_ref, od, hash)) {
> + continue;
> }
Should this be another if statement instead of else if?
if (reject) {
meter = copp_meter_get(COPP_REJECT, od->nbs->copp,
meter_groups);
}
if (ovn_dp_group_add_with_reference(lflow_ref, od, hash)) {
continue;
}
> - ovn_lflow_add_with_hint__(lflows, od, S_SWITCH_IN_STATEFUL,
> - priority, ds_cstr(match),
> - ds_cstr(action), NULL, meter,
> - &lb->nlb->header_);
> + lflow_ref = ovn_lflow_add_at_with_hash(lflows, od,
> + S_SWITCH_IN_STATEFUL, priority,
> + ds_cstr(match), ds_cstr(action),
> + NULL, meter, &lb->nlb->header_,
> + OVS_SOURCE_LOCATOR, hash);
> }
> }
> }
>
More information about the dev
mailing list