On 12/5/25 4:34 AM, Mark Michelson via dev wrote: > This creates an alternative to lflow_table_add_lflow(). This alternative, > lflow_table_add_lflow_(), takes its arguments in a structure, rather > than as individual function parameters.
Hi, Mark. I didn't read the whole thing, but this looks interesting! Thanks! The '#define DEFAULT_DROP' in the patch 9 feels a little icky as it's a macro to replace a few consequent required arguments, which may be easy to misuse. It makes the argument order harder to track. Not sure if we maybe need to keep one function variant for this one. > > This provides the basis for redefining the many ovn_lflow_add() macro > variations. In this commit, we define ovn_lflow_add() as a VFUNC. The 7 > argument version of ovn_lflow_add() is defined as ovn_flow_add_7(). This > macro uses lflow_table_add_lflow_(). Looking at the structure of these macros, I wonder if we even need the VFUNC here. It feels like we could just take the ovn_flow_add_8 macro from the next patch, replace ARG8 in the prototype with ... and in the body with the __VA_ARGS__, then it could just accept any number of extra arguments, so we would not need to have 4 variants of the same macro. > > We do not touch northd.c in this commit, but this implicitly converts > all pre-existing calls to ovn_lflow_add() to the new version which uses > the struct argument. > > Upcoming commits will convert the other ovn_lflow_add()-related macros > to use ovn_lflow_add() with different argument numbers. > > Signed-off-by: Mark Michelson <[email protected]> > --- > northd/lflow-mgr.c | 10 ++++++++++ > northd/lflow-mgr.h | 36 +++++++++++++++++++++++++++++++----- > 2 files changed, 41 insertions(+), 5 deletions(-) > > diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c > index 43dd1d947..dfa3c39e9 100644 > --- a/northd/lflow-mgr.c > +++ b/northd/lflow-mgr.c > @@ -773,6 +773,16 @@ lflow_table_add_lflow(struct lflow_table *lflow_table, > lflow_hash_unlock(hash_lock); > } > > +void > +lflow_table_add_lflow_(struct lflow_table_add_args args, const char *where) nit: Somehow this coding style rule is violated in many places in OVN, but the coding style document prescribes to use double underscore as a suffix for "internal only" things. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
