On Tue, Dec 16, 2025 at 9:31 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. > > Differences between v1 and v2 of this series: > * In this version, the struct argument version of lflow_table_add_lflow > takes a pointer to struct lflow_table_add_args instead of a bare > struct. This should save some unnecessary copying of arguments. > * In patch 2, we define ovn_lflow_add_default_drop() in terms of > ovn_lflow_add(). This means we do not need to repeat struct > initialization, which means we do not need special START and END > macros for struct initialization. > > Mark Michelson (9): > lflow-mgr: Use struct argument for lflow addition. > lflows: Create new ovn_lflow_add_default_drop(). > 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_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 | 47 +- > northd/lflow-mgr.h | 117 ++- > northd/northd.c | 1812 +++++++++++++++++++++++--------------------- > 3 files changed, 1045 insertions(+), 931 deletions(-)
Thanks for the improvements. I had one comment in patch 1, to swap the functions lflow_table_add_lflow() and lflow_table_add_lflow__(). But after going through the entire series and the last patch, it made sense why you did that way. LGTM. For the entire series: Acked-by: Numan Siddique <[email protected]> Numan > > -- > 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
