On Tue, Aug 31, 2021 at 9:41 AM Dumitru Ceara <[email protected]> wrote:
>
> On 8/27/21 9:17 PM, Ilya Maximets wrote:
> > The '/* Push changes to the Logical_Flow table to database. */' loop
> > reads all the datapaths from the datapath group and checks them even
> > if it's not required.
> >
> > To check that flow has at least one valid datapath, we need to find
> > one valid datapath.  To find Sb logical flow we need a datapath type
> > and for this we only need one datapath again.  Technically, the only
> > place where we need to check all datapaths is when we're checking
> > certain Sb datapath group for the first time.  In all other cases
> > we can validate/discard the flow by other characteristics that we
> > already know.
> >
> > The change saves a fair amount of time in this loop.  Up to a few
> > seconds in case of a big database.
> >
> > More comments added to make the code easier to read.
> >
> > Signed-off-by: Ilya Maximets <[email protected]>
> > ---
>
> [...]
>
> > @@ -13307,24 +13299,46 @@ build_lflows(struct northd_context *ctx, struct 
> > hmap *datapaths,
> >               * updates. */
> >              bool update_dp_group = false;
> >
> > -            if (n_datapaths != hmapx_count(&lflow->od_group)) {
> > -                /* Groups are different. */
> > +            if ((!lflow->dpg && dp_group) || (lflow->dpg && !dp_group)) {
> > +                /* Need to add or delete datapath group. */
> >                  update_dp_group = true;
> > -            } else if (lflow->dpg && lflow->dpg->dp_group) {
> > +            } else if (!lflow->dpg && !dp_group) {
> > +                /* No datapath group and not needed. */
> > +            } else if (lflow->dpg->dp_group) {
> >                  /* We know the datapath group in Sb that should be used. */
> >                  if (lflow->dpg->dp_group != dp_group) {
> >                      /* Flow has different datapath group in the database.  
> > */
> >                      update_dp_group = true;
> >                  }
> >                  /* Datapath group is already up to date. */
> > -            } else if (n_datapaths) {
> > +            } else {
> > +                /* There is a datapath group and we need to perfrom
>
> Typo: perfrom
>
> Except for this, I think the rest is fine.  You might want to rebase
> though as I had to manually apply the second patch in this series on top
> of the latest code.
>
> Acked-by: Dumitru Ceara <[email protected]>

Thanks Ilya and Dumitru.  I corrected the typo and applied both the
patches to the main branch.

Numan

>
> Thanks!
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to