Thanks for having a look, Numan. I posted a non-RFC version of the
patch series today.

On Mon, Dec 15, 2025 at 2:54 PM Numan Siddique <[email protected]> wrote:
>
> On Thu, Dec 4, 2025 at 10:35 PM Mark Michelson via dev
> <[email protected]> wrote:
> >
> > The lflow_table_add_lflow() function takes 13 arguments. Many helper
> > macros exist to facilitate filling in those 13 arguments without having
> > to spell them all out every time. Unfortunately, the amount of different
> > macros has gotten a bit out of hand. Some macros, like
> > ovn_lflow_add_with_hint__() have misleading names. Some macros, like
> > ovn_lflow_add_drop_with_lport_hint_and_desc() have horribly unwieldy
> > names. Trying to add a new macro to the list is a minefield since you
> > need to cherry-pick exactly which arguments the new macro should require
> > and which ones the macro should fill in automatically.
> >
> > This series seeks to simplify things by getting rid of all helper macros
> > except for ovn_lflow_add(). We accomplish this by using the VFUNC()
> > macro to create different versions of ovn_lflow_add() that take
> > different numbers of arguments. All invocations of ovn_lflow_add()
> > require a minimum of 7 arguments:
> >  * lflow table
> >  * ovn_datapath
> >  * stage
> >  * priority
> >  * match
> >  * actions
> >  * lflow_ref
> >
> > Additional fields can be specified by adding an 8th, 9th, or 10th
> > argument using a series of helper macros to add specific field types.
> > The idea behind this is that any argument beyond the 7th can be any of
> > the available fields. This gives the flexibility to add lflows in any
> > number of combinations without having to define hyper-specific macros
> > for lflow addition.
> >
> > This series is marked as an RFC for a couple of reasons.
> >
>
> I had a quick look at the patches..  +1 from if you want to send formal 
> patches.
>
>
> > First, I have no idea whether people will look at this and think of
> > this as "simplified". For callers of ovn_lflow_add(), it is much
> > simpler. If you look at the macros in lflow-mgr.h, they're not simple
> > at all.
> >
> > Second, this patch is quite invasive. It touches many lines in northd.c.
> > This could lead to difficult backport situations in the future. This
> > doesn't add any new functionality to the existing code, so it may not be
> > worth justifying this potential future pain.
>
> I think it should be ok to handle backports since we mostly backport fixes.
> It would also discourage us from backporting features :)
>
> Numan
>
>
>
> >
> > Mark Michelson (9):
> >   lflow-mgr: Use struct argument for lflow addition.
> >   lflows: Remove ovn_lflow_add_with_hint().
> >   lflows: Remove ovn_lflow_add_with_lport_and_hint().
> >   lflows: Remove ovn_lflow_metered() and ovn_lflow_add_with_hint__().
> >   lflows: Remove ovn_lflow_add_with_dp_group().
> >   lflows: Remove ovn_lflow_add_default_drop().
> >   lflows: Remove ovn_lflow_add_drop_with_desc().
> >   lflows: Remove ovn_lflow_add_drop_with_lport_hint_and_desc().
> >   lflows: Make non-struct version of lflow_table_add_lflow() private.
> >
> >  northd/lflow-mgr.c |   39 +-
> >  northd/lflow-mgr.h |  161 ++--
> >  northd/northd.c    | 1840 +++++++++++++++++++++++---------------------
> >  3 files changed, 1092 insertions(+), 948 deletions(-)
> >
> > --
> > 2.51.1
> >
> > _______________________________________________
> > 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