[ovs-dev] [PATCH ovn v10 0/4] Introduce infrastructure for VIF plug providers.

Han Zhou hzhou at ovn.org
Fri Nov 5 19:43:25 UTC 2021


On Fri, Nov 5, 2021 at 7:00 AM Frode Nordahl <frode.nordahl at canonical.com>
wrote:
>
> Introduce infrastructure for VIF plug providers and add feature to
> ovn-controller to add and remove ports on the integration bridge as
> directed by CMS through Logical_Switch_Port options.
>
> Traditionally it has been the CMSs responsibility to create Virtual
> Interfaces (VIFs) as part of instance (Container, Pod, Virtual
> Machine etc.) life cycle, and subsequently manage plug/unplug
> operations on the Open vSwitch integration bridge.
>
> With the advent of NICs connected to multiple distinct CPUs we can
> have a topology where the instance runs on one host and Open
> vSwitch and OVN runs on a different host, the smartnic control plane
> CPU.  The host facing interfaces will be visible to Open vSwitch
> and OVN as representor ports.
>
> The actions necessary for plugging and unplugging the representor
> port in Open vSwitch running on the smartnic control plane CPU would
> be the same for every CMS.
>
> Hardware or platform specific details for initialization and lookup
> of representor ports is provided by an plugging provider hosted
> inside or outside the core OVN repository, and linked at OVN build
> time.
>
> RFC1 -> RFC2:
>  - Introduce the plug-provider interface, remove hardware/platform
>    dependent code.
>  - Add ovsport module.
>  - Integrate with binding module.
>  - Split into multiple patches.
>
> RFC2 -> v1:
>  - Extend build system, `--with-plug-provider`.
>  - Check for required data in b_ctx_in and lbinding data structures.
>  - Split consider_plug_lport into update and create processing
>    functions.
>  - Consider unplug on release where relevant.
>  - Add ovn-controller `--enable-dummy-plug` option for testing.
>  - Consistent function naming and move ovsport module to controller/.
>  - Rename plug-test.c -> plug-dummy.c.
>  - Add functional- and unit- tests.
>
> v1 -> v2:
>  - Move update to controller/binding.h from patch 6 -> patch 5.
>  - Fix lint problems reported by 0-day Robot.
>
> v2 -> v3:
>  - Fix build system extension for plug provider.
>  - Implement DDlog version of northd change.
>  - Rebase on tip.
>
> v3 -> v4:
>  - sb:Port_Binding:plugged_by -> sb:Port_Binding:requested_chassis.
>  - Move documentation of plugin specific options to plugin
>    implementation.
>  - ovn-northd-ddlog: squash changes into same patch as C version, rework
>    the DDlog implementation.
>  - controller: Use requested_chassis column instead of parsing options.
>  - plug-provider: Remove the extra class instantiation layer and
>    refcounting.  Add documentation and various tweaks.
>  - controller: Add engine node for plug provider so that a plugin run
>    function can run at an appropriate time and also allow feeding back
>    information about changes that can not be handled incrementally.
>  - binding: Fix return values for plug functions so they adhere to the
>    incremental processing engine contract.
>  - Add support for building in-tree plug providers.
>
> v4 -> v5:
>  - binding: Make some data structures and functions public to allow
>    other modules to make use of its local binding tracking data
>    structures.
>  - Add separate incremental processing engine nodes for the plug
>    provider.
>  - Do change handling in the controller/plug module rather than piggy
>    backing on the binding module.
>  - Deal with asynchronous notification to plug provider class and
>    subsequent freeing of data structures when the transaction commits.
>  - Drop the representor plugin as in-tree plugin to address concerns about
>    it being Linux-only and having netlink/kernel dependencies.  Will
>    upstream to ovn-org/ovn-vif instead.
>
> v5 -> v6:
>  - Fix 0-day Robot documentation lexer warning treated as error:
>
https://mail.openvswitch.org/pipermail/ovs-build/2021-September/017538.html
>
> v6 -> v7:
>  - lport: Fall back to strcmp when requested-chassis option is set
>           but Port_Binding->requested_chassis is not filled yet.
>  - physical: Make use of common lport_can_bind_on_this_chassis helper
>              instead of duplicating the inverse check in the code.
>  - tests: Add test cases to cover currently uncovered requested-chassis
>           functionality.
>  - plug: Make smaps struct members instead of pointers.
>  - plug: Don't use is_deleted function when not iterating over tracked
>          records.  Avoid adding multiple delete or update iface requests
>          to the same transaction.
>  - plug: For full recompute, don't process until northd has populated
>          Port_Binding->requested_chassis to avoid thrashing on startup.
>          Also fix order of processing.
>  - plug: Handle failed transactions appropriately.
>  - tests: Clean up and extend plug test case.
>
> v7 -> v8:
>  - Patches 1 through 9 from v7 was merged.
>  - Use Port_Binding:requested_chassis index when iterating over PBs.
>  - Drop separate I-P engine nodes for the VIF plug providers at this
>    time, we can revisit the need for it in the event of VIF plug
>    providers gaining support for Scalable Functions and the increased
>    density that entails.
>  - Rename module, objects and folders from 'plug-' to 'vif-plug-*' to
>    avoid using too generic names.
>  - Don't call vif_plug_port_ctx_destroy on vif_plug_port_prepare
>    failure, make the convention that plug provider should clean up after
>    itself in that case.
>  - Simplify the unplug/plug thrashing avoidance on startup with a rather
>    naive counter to ensure IDL has data prior to attempting any
>    processing.  Once we identify the root of the issue this can be
removed.
>
> v8 -> v9:
>  - Amend comment with inaccurate statements about behavior of tracked
>    data.
>  - Increase the IDL prime counter and reset it whenever the OVNSB DB
>    reconnects.
>  - Don't add indexes not used by I-P as inputs to the I-P engine.
>  - Update NEWS statement.
>  - Fix Documentation path reference in ovn-architecture.7.xml
>
> v9 -> v10:
>  - Move creation of indexes from patch 2 -> patch 3
>
> Previous discussion:
>  - RFC1:
https://patchwork.ozlabs.org/project/ovn/patch/20210509140305.1910796-1-frode.nordahl@canonical.com/
>  - RFC2:
https://patchwork.ozlabs.org/project/ovn/cover/20210805145013.3033919-1-frode.nordahl@gmail.com/
>
> Frode Nordahl (4):
>   lib: Add infrastructure for VIF plug providers.
>   ovn-controller: Prepare VIF plug provider infrastructure.
>   controller: Consider plugging VIF on CMS request.
>   NEWS: Add note on infrastructure for VIF plug providers.
>
>  Documentation/automake.mk                     |   2 +
>  Documentation/topics/index.rst                |   1 +
>  .../topics/vif-plug-providers/index.rst       |  32 +
>  .../vif-plug-providers/vif-plug-providers.rst | 209 ++++++
>  NEWS                                          |   6 +
>  acinclude.m4                                  |  49 ++
>  configure.ac                                  |   2 +
>  controller/automake.mk                        |   4 +-
>  controller/ovn-controller.c                   |  81 ++-
>  controller/test-vif-plug.c                    |  72 ++
>  controller/vif-plug.c                         | 634 ++++++++++++++++++
>  controller/vif-plug.h                         |  80 +++
>  lib/automake.mk                               |  10 +-
>  lib/vif-plug-provider.c                       | 204 ++++++
>  lib/vif-plug-provider.h                       | 163 +++++
>  lib/vif-plug-providers/dummy/vif-plug-dummy.c | 120 ++++
>  ovn-architecture.7.xml                        |  35 +-
>  ovn-nb.xml                                    |  21 +
>  tests/automake.mk                             |  13 +-
>  tests/ovn-macros.at                           |   2 +-
>  tests/ovn-vif-plug.at                         |   8 +
>  tests/ovn.at                                  | 121 ++++
>  22 files changed, 1851 insertions(+), 18 deletions(-)
>  create mode 100644 Documentation/topics/vif-plug-providers/index.rst
>  create mode 100644
Documentation/topics/vif-plug-providers/vif-plug-providers.rst
>  create mode 100644 controller/test-vif-plug.c
>  create mode 100644 controller/vif-plug.c
>  create mode 100644 controller/vif-plug.h
>  create mode 100644 lib/vif-plug-provider.c
>  create mode 100644 lib/vif-plug-provider.h
>  create mode 100644 lib/vif-plug-providers/dummy/vif-plug-dummy.c
>  create mode 100644 tests/ovn-vif-plug.at
>
> --
> 2.32.0
>

Thanks Frode! I applied the series to the main branch with a minor comment
update in patch1, see below. I didn't want to bother you for another round
of patches for this, so hope it's fine.

Thanks for Numan's reviews for the previous versions as well, and I added
his acks except for the last patch which was added after his ack for the
series.

Regards,
Han

-----------------------------------------------------------------------------------------------------------------------------------
diff --git a/controller/vif-plug.c b/controller/vif-plug.c
index 1015a4c13..62b75263c 100644
--- a/controller/vif-plug.c
+++ b/controller/vif-plug.c
@@ -526,10 +526,11 @@ vif_plug_handle_iface(const struct ovsrec_interface
*iface_rec,
 }

 /* On initial startup or on IDL reconnect, several rounds of the main loop
may
- * run before data is actually loaded in the IDL.  This situation is
currently
- * not reflected in a call to ovsdb_idl_has_ever_connected().  Until we
find
- * the root of this issue we need this counter so that we do not erronously
- * unplug ports because the data is just not loaded yet.
+ * run before data is actually loaded in the IDL, primarily depending on
+ * conditional monitoring status and other events that could trigger main
loop
+ * runs during this period.  Until we find a reliable way to determine the
+ * completeness of the initial data downloading we need this counter so
that we
+ * do not erronously unplug ports because the data is just not loaded yet.
  */
 #define VIF_PLUG_PRIME_IDL_COUNT_SEEED 10
 static int vif_plug_prime_idl_count = VIF_PLUG_PRIME_IDL_COUNT_SEEED;


More information about the dev mailing list