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
