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() and ovn_lflow_add_default_drop(). We
accomplish this by changing ovn_lflow_add() into a variadic macro.
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 any number of additional
arguments 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 mainly due to the potential difficulties
this could cause with backports in the future. This series does not add
any new functionality to the code, so it could be argued that this
change is not worth it.

A secondary reason this is marked as RFC is that, despite the
simplification, it could be argued this removes some type-safety that
was present in the old macros. For instance,
ovn_lflow_add_with_dp_group() did not allow an ovn_datapath argument to
be passed in. With the new setup, the ovn_datapath argument is required,
and users have to set it NULL to get the desired effect. Similarly,
ovn_lflow_add_drop_with_desc() was the only macro that allowed a flow
description to be passed in. This implies that a flow description is
only relevant for "drop" flows. However, the new set of macros allows
flow descriptions to be passed in for any type of flow.

Changes from v1:
* The "where" argument is now part of the lflow_table_add_args
  structure.
* I figured out a way to not have to repeat the struct initialization
  over and over for multiple macros. This is accomplished with the START
  and END macros that are added in this version.
* This version no longer uses VFUNC. Instead, ovn_lflow_add() takes 7
  named arguments and "...". The way the helper macros are defined, we
  can simply sub __VA_ARGS__ into the macro definition and it works.
* This version keeps the ovn_lflow_add_default_drop() macro.
* The ovn_lflow_add_default_drop() change has been moved to patch 2. This
  way the series consists of two patches that add macros, followed by 6
  patches that remove macros.
* This version uses double underscores for private functions.
* This version changes the reasons for the patch being marked RFC in the
  cover letter.

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 |   39 +-
 northd/lflow-mgr.h |  129 ++--
 northd/northd.c    | 1811 +++++++++++++++++++++++---------------------
 3 files changed, 1047 insertions(+), 932 deletions(-)

-- 
2.51.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to