On Mon, Dec 29, 2025 at 7:29 PM Numan Siddique <[email protected]> wrote:
> 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 Hi Mark, I think you might have missed my feedback on the RFC so putting it here too: 1) We should also get rid of ovn_lflow_add_default_drop() in favor of doing "ovn_lflow_add(..., debug_drop_action(), ..., WITH_DESC(...))". That was the plan anyway to actually have all drops with a comment, this will also force any future contributor to use only a single macro. 2) After the use of "ovn_lflow_add()" a lot of calls now spans on multiple lines we should re-align those if applicable. 3) If we want to make this happen it should be in 26.03, this makes it way easier for us to backport later on. Thanks, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
