> On 26/08/2021 17:50, Lorenzo Bianconi wrote: > > Similar to build_lb_rules(), precompute hash and lflow pointer in > > build_lrouter_defrag_flows_for_lb routine since they do not depend on > > datapath updating datapath group. Using this approach we can reduce > > ovn-northd loop time of ~1s running an ovnnb_db from production > > environment: > > > > ovn-master: > > ----------- > > Statistics for 'ovnnb_db_run' > > Total samples: 37 > > Maximum: 12656 msec > > Minimum: 12276 msec > > 95th percentile: 12557.000000 msec > > Short term average: 12475.213654 msec > > Long term average: 12336.498446 msec > > > > build_lb_rules + build_lrouter_defrag_flows_for_lb: > > --------------------------------------------------- > > Statistics for 'ovnnb_db_run' > > Total samples: 37 > > Maximum: 11266 msec > > Minimum: 11039 msec > > 95th percentile: 11194.000000 msec > > Short term average: 11145.945629 msec > > Long term average: 11075.628051 msec > > > > ovn-nbctl lr-list | wc -l > > 121 > > ovn-nbctl ls-list | wc -l > > 241 > > ovn-nbctl lb-list | wc -l > > 47077 > > ovn-sbctl dump-flows |wc -l > > 9852935 > > > > Signed-off-by: Lorenzo Bianconi <[email protected]> > > --- > > northd/ovn-northd.c | 18 ++++++++++++++---- > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index 5efa54ee5..61fb5b159 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -9476,11 +9476,21 @@ build_lrouter_defrag_flows_for_lb(struct > > ovn_northd_lb *lb, > > > > ds_put_format(&defrag_actions, "ct_dnat;"); > > > > + struct ovn_lflow *lflow_ref = NULL; > > + uint32_t hash = ovn_logical_flow_hash( > > + ovn_stage_get_table(S_ROUTER_IN_DEFRAG), > > + ovn_stage_get_pipeline_name(S_ROUTER_IN_DEFRAG), prio, > > + ds_cstr(match), ds_cstr(&defrag_actions)); > > for (size_t j = 0; j < lb->n_nb_lr; j++) { > > - ovn_lflow_add_with_hint(lflows, lb->nb_lr[j], > > S_ROUTER_IN_DEFRAG, > > - prio, ds_cstr(match), > > - ds_cstr(&defrag_actions), > > - &lb->nlb->header_); > > + struct ovn_datapath *od = lb->nb_lr[j]; > > + if (ovn_dp_group_add_with_reference(lflow_ref, od, hash)) { > > + continue; > > + } > > For all of these patches, could something like the following change give > a further optimization? It would call ovn_lflow__xxxxx() only once and I > think it simplifies the code. Does it give a benefit? What do you think? > > /* Build datapath group */ > struct hmapx od_group; > hmapx_init(&od_group); > for (size_t j = 0; j < lb->n_nb_lr; j++) { > struct ovn_datapath *od = lb->nb_lr[j]; > if (ovn_dp_group_add_with_reference(&od_group, od)) { > continue; > } > } > > /* Add lfow specifying datapath group rather than datapath */ > ovn_lflow_add_at_with_dp_group(lflows, > S_SWITCH_IN_STATEFUL, priority, > ds_cstr(match), ds_cstr(action), > NULL, meter, &lb->nlb->header_);
Hi Mark, if I read correctly the code I guess we should pass od_group somewhere here, right? Anyway it seems to me this approach will not work with parrallel implementation, right? Regards, Lorenzo > > > + lflow_ref = ovn_lflow_add_at_with_hash(lflows, od, > > + S_ROUTER_IN_DEFRAG, prio, > > + ds_cstr(match), > > ds_cstr(&defrag_actions), > > + NULL, NULL, &lb->nlb->header_, > > + OVS_SOURCE_LOCATOR, hash); > > } > > } > > ds_destroy(&defrag_actions); > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
