On Mon, Mar 9, 2026 at 7:20 PM Mark Michelson <[email protected]> wrote:

> Thanks for v2 Jacob, it looks good to me!
>
> Acked-by: Mark Michelson <[email protected]>
>
> On Thu, Feb 26, 2026 at 3:09 PM Jacob Tanenbaum <[email protected]>
> wrote:
> >
> > If ovn_dp_groups ref_count is decremented when fetching old lflows
> > during a recalculation we can successfully determine if we can reuse the
> > flow or not.
> >
> > Reported-by: Mark Michelson <[email protected]>
> > Reported-at: https://issues.redhat.com/browse/FDP-2747
> > Signed-off-by: Jacob Tanenbaum <[email protected]>
> >
> >
> > ---
>

Thank you Jacob and Mark,
some small comments down below.


> > v1: initial version
> > v2: address Marks comments of only setting old_lflow->dpg = NULL if
> > old_lflow->dpg is set.
> >
> > diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
> > index 7e7d0b1bd..0d47ba226 100644
> > --- a/northd/lflow-mgr.c
> > +++ b/northd/lflow-mgr.c
> > @@ -1068,6 +1068,16 @@ do_ovn_lflow_add(struct lflow_table *lflow_table,
> size_t dp_bitmap_len,
> >      if (old_lflow) {
> >          dynamic_bitmap_realloc(&old_lflow->dpg_bitmap, dp_bitmap_len);
> >          if (old_lflow->sync_state != LFLOW_STALE) {
> > +
>

nit: No need for a newline.


> > +            if (old_lflow->dpg != NULL) {


nit: No need to check != NULL.


> > +                struct hmap *dp_groups;
> > +                enum ovn_datapath_type dp_type =
> > +                    ovn_stage_to_datapath_type(old_lflow->stage);
> > +                dp_groups = &lflow_table->dp_groups[dp_type];
>

We can probably add this directly to the release call
which makes the if less crowded IMO.


> > +                ovn_dp_group_release(dp_groups, old_lflow->dpg);
> > +                old_lflow->dpg = NULL;
> > +            }
> > +
> >              return old_lflow;
> >          }
> >          sbuuid = old_lflow->sb_uuid;
> > @@ -1268,7 +1278,6 @@ sync_lflow_to_sb(struct ovn_lflow *lflow,
> >
> >      if (pre_sync_dpg != lflow->dpg) {
> >          ovn_dp_group_use(lflow->dpg);
> > -        ovn_dp_group_release(dp_groups, pre_sync_dpg);
> >      }
> >
> >      lflow->sync_state = LFLOW_SYNCED;
> > --
> > 2.52.0
> >
>
>
I took care of that and applied it to main.

Regards,
Ales
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to