[ovs-dev] [PATCH ovn v8 1/4] lib: Add infrastructure for VIF plug providers.

Han Zhou hzhou at ovn.org
Fri Nov 5 07:39:47 UTC 2021


On Tue, Nov 2, 2021 at 4:29 AM Frode Nordahl <frode.nordahl at canonical.com>
wrote:
>
> New lib/vif-plug-provider module contains the infrastructure for
> registering VIF plug provider classes which may be hosted inside
> or outside the core OVN repository.
>
> New controller/vif-plug module adds internal interface for
> interacting with the VIF plug providers.
>
> Extend build system to allow building of built-in VIF plug
> providers and linking an externally built VIF plug provider.
>
> Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
> ---
>  Documentation/automake.mk                     |   2 +
>  Documentation/topics/index.rst                |   1 +
>  .../topics/vif-plug-providers/index.rst       |  32 +
>  .../vif-plug-providers/vif-plug-providers.rst | 209 ++++++
>  acinclude.m4                                  |  49 ++
>  configure.ac                                  |   2 +
>  controller/automake.mk                        |   4 +-
>  controller/test-vif-plug.c                    |  72 ++
>  controller/vif-plug.c                         | 632 ++++++++++++++++++
>  controller/vif-plug.h                         |  79 +++
>  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 +-
>  tests/automake.mk                             |  13 +-
>  tests/ovn-vif-plug.at                         |   8 +
>  17 files changed, 1619 insertions(+), 16 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
>
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index b3fd3d62b..704c80671 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -28,6 +28,8 @@ DOC_SOURCE = \
>         Documentation/topics/ovn-news-2.8.rst \
>         Documentation/topics/role-based-access-control.rst \
>         Documentation/topics/debugging-ddlog.rst \
> +       Documentation/topics/vif-plug-providers/index.rst \
> +       Documentation/topics/vif-plug-providers/vif-plug-providers.rst \
>         Documentation/howto/index.rst \
>         Documentation/howto/docker.rst \
>         Documentation/howto/firewalld.rst \
> diff --git a/Documentation/topics/index.rst
b/Documentation/topics/index.rst
> index d58d5618b..e9e49c742 100644
> --- a/Documentation/topics/index.rst
> +++ b/Documentation/topics/index.rst
> @@ -41,6 +41,7 @@ OVN
>     high-availability
>     role-based-access-control
>     ovn-news-2.8
> +   vif-plug-providers/index
>     testing
>
>  .. list-table::
> diff --git a/Documentation/topics/vif-plug-providers/index.rst
b/Documentation/topics/vif-plug-providers/index.rst
> new file mode 100644
> index 000000000..b7552ac4c
> --- /dev/null
> +++ b/Documentation/topics/vif-plug-providers/index.rst
> @@ -0,0 +1,32 @@
> +..
> +      Licensed under the Apache License, Version 2.0 (the "License");
you may
> +      not use this file except in compliance with the License. You may
obtain
> +      a copy of the License at
> +
> +          http://www.apache.org/licenses/LICENSE-2.0
> +
> +      Unless required by applicable law or agreed to in writing, software
> +      distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT
> +      WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the
> +      License for the specific language governing permissions and
limitations
> +      under the License.
> +
> +      Convention for heading levels in OVN documentation:
> +
> +      =======  Heading 0 (reserved for the title in a document)
> +      -------  Heading 1
> +      ~~~~~~~  Heading 2
> +      +++++++  Heading 3
> +      '''''''  Heading 4
> +
> +      Avoid deeper levels because they do not render well.
> +
> +==================
> +VIF Plug Providers
> +==================
> +
> +
> +.. toctree::
> +   :maxdepth: 2
> +
> +   vif-plug-providers
> diff --git
a/Documentation/topics/vif-plug-providers/vif-plug-providers.rst
b/Documentation/topics/vif-plug-providers/vif-plug-providers.rst
> new file mode 100644
> index 000000000..77ecf7e0f
> --- /dev/null
> +++ b/Documentation/topics/vif-plug-providers/vif-plug-providers.rst
> @@ -0,0 +1,209 @@
> +..
> +      Licensed under the Apache License, Version 2.0 (the "License");
you may
> +      not use this file except in compliance with the License. You may
obtain
> +      a copy of the License at
> +
> +          http://www.apache.org/licenses/LICENSE-2.0
> +
> +      Unless required by applicable law or agreed to in writing, software
> +      distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT
> +      WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the
> +      License for the specific language governing permissions and
limitations
> +      under the License.
> +
> +      Convention for heading levels in OVN documentation:
> +
> +      =======  Heading 0 (reserved for the title in a document)
> +      -------  Heading 1
> +      ~~~~~~~  Heading 2
> +      +++++++  Heading 3
> +      '''''''  Heading 4
> +
> +      Avoid deeper levels because they do not render well.
> +
> +==================
> +VIF Plug Providers
> +==================
> +
> +Traditionally it has been the CMSes responsibility to create VIFs as
part of
> +instance life cycle, and subsequently manage plug/unplug operations on
the
> +integration bridge following the conventions described in the
> +`Open vSwitch Integration Guide`_ for mapping of VIFs to OVN logical
port.
> +
> +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.
> +
> +Instead of every CMS having to develop their own version of an agent to
do
> +the plugging, we provide a pluggable infrastructure in OVN that allows
the
> +`ovn-controller` to perform the plugging on CMS direction.
> +
> +Hardware or platform specific details for initialization and lookup of
> +representor ports is provided by an plugging provider library hosted
inside or
> +outside the core OVN repository, and linked at OVN build time.
> +
> +Life Cycle of an OVN plugged VIF
> +--------------------------------
> +
> +1. CMS creates a record in the OVN Northbound Logical_Switch_Port table
with
> +   the options column containing the `vif-plug-type` key with a value
> +   corresponding to the `const char *type` provided by the VIF plug
provider
> +   implementation as well as a `requested-chassis` key with a value
pointing at
> +   the name or hostname of the chassis it wants the VIF plugged on.
Additional
> +   VIF plug provider specific key/value pairs must be provided for
successful
> +   lookup.
> +
> +2. `ovn-northd` looks up the name or hostname provided in the
> +   `requested-chassis` option and fills the OVN Southbound Port_Binding
> +   requested_chassis column, it also copies relevant options over to the
> +   Port_Binding record.
> +
> +3. `ovn-controller` monitors Southbound Port_Binding entries with a
> +   requested_chassis column pointing at its chassis UUID.  When it
encounters
> +   an entry with option `vif-plug-type` and it has registered a VIF plug
> +   provider matching that type, it will act on it even if no local
binding
> +   exists yet.
> +
> +4. It will fill the `struct vif_plug_port_ctx_in` as defined in
> +   `lib/vif-plug.h` with `op_type` set to 'PLUG_OP_CREATE' and make a
call to
> +   the VIF plug providers `vif_plug_port_prepare` callback function.
VIF plug
> +   provider performs lookup and fills the `struct vif_plug_port_ctx_out`
as
> +   defined in `lib/vif-plug.h`.
> +
> +5. `ovn-controller` creates a port and interface record in the local
OVSDB
> +   using the details provided by the VIF plug provider and also adds
> +   `external-ids:iface-id` with value matching the logical port name and
> +   `external-ids:ovn-plugged` with value matching the logical port
> +   `vif-plug-type`.  When the port creation is done a call will first be
made
> +   to the VIF plug providers `vif_plug_port_finish` function and then to
the
> +   `vif_plug_port_ctx_destroy` function to free any memory allocated by
the VIF
> +   plug implementation.
> +
> +6. The Open vSwitch vswitchd will assign a ofport to the newly created
> +   interface and on the next `ovn-controller` main loop iteration flows
will be
> +   installed.
> +
> +7. On each main loop iteration the `ovn-controller` will in addition to
normal
> +   flow processing make a call to the VIF plug provider again similar to
the
> +   first creation in case anything needs updating for the interface
record.
> +
> +8. The port will be unplugged when an event occurs which would make the
> +   `ovn-controller` release a logical port, for example the
Logical_Switch_Port
> +   and Port_Binding entry disappearing from the database or its
> +   `requested_chassis` column being pointed to a different chassis.
> +
> +
> +The VIF plug provider interface
> +-------------------------------
> +
> +The interface between internals of OVN and a VIF plug provider is a set
of
> +callbacks as defined by the `struct vif_plug_class` in
> +`lib/vif-plug-provider.h`.
> +
> +It is important to note that these callbacks will be called in the
critical
> +path of the `ovn-controller` processing loop, so care must be taken to
make the
> +implementation as efficient as possible, and under no circumstance can
any of
> +the callback functions make calls that block.
> +
> +On `ovn-controller` startup, VIF plug providers made available at build
time
> +will be registered by the identifier provided in the `const char *type`
> +pointer, at this time the `init` function pointer will be called if it is
> +non-NULL.
> +
> +> **Note**: apart from the `const char *type` pointer, no attempt will
be made
> +            to access VIF plug provider data or functions before the
call to
> +            the `init` has been made.
> +
> +On `ovn-controller` exit, the VIF plug providers registered in the above
> +mentioned procedure will have their `destroy` function pointer called if
it is
> +non-NULL.
> +
> +If the VIF plug provider has internal lookup tables that need to be
maintained
> +they can define a `run` function which will be called as part of the
> +`ovn-controller` main loop.  If there are any changes encountered the
function
> +should return 'true' to signal that further processing is necessary,
'false'
> +otherwise.
> +
> +On update of Interface records the `ovn-controller` will pass on a `sset`
> +to the `ovsport_update_iface` function containing options the plug
> +implementation finds pertinent to maintain for successful operation.
This
> +`sset` is retrieved by making a call to the plug implementation
> +`vif_plug_get_maintained_iface_options` function pointer if it is
non-NULL.
> +This allows presence of other users of the OVSDB maintaining a different
set of
> +options on the same set of Interface records without wiping out their
changes.
> +
> +Before creating or updating an existing interface record the VIF plug
provider
> +`vif_plug_port_prepare` function pointer will be called with valid
pointers to
> +`struct vif_plug_port_ctx_in` and `struct vif_plug_port_ctx_out` data
> +structures.  If the VIF plug provider implementation is able to perform
lookup
> +it should fill the `struct vif_plug_port_ctx_out` data structure and
return
> +'true'.  The `ovn-controller` will then create or update the
port/interface
> +records and then call `vif_plug_port_finish` when the transactions
commits and
> +`vif_plug_port_ctx_destroy` to free any allocated memory.  If the VIF
plug
> +provider implementation is unable to perform lookup or prepare the
desired
> +resource at this time, it should return 'false' which will tell the
> +`ovn-controller` to not plug the port, in this case it will not call
> +`vif_plug_port_finish` nor `vif_plug_port_ctx_destroy`.
> +
> +> **Note**: The VIF plug provider implementation should exhaust all
> +            non-blocking options to succeed with lookup from within the
> +            `vif_plug_port_prepare` handler, including refreshing lookup
> +            tables if necessary.
> +
> +Before removing port and interface records previously plugged by the
> +`ovn-controller` as identified by presence of the Interface
> +`external-ids:ovn-plugged` key, the `ovn-controller` will look up the
> +`vif-plug-type` from `external-ids:ovn-plugged`, fill
> +`struct vif_plug_port_ctx_in` with `op_type` set to 'PLUG_OP_REMOVE' and
make a
> +call to `vif_plug_port_prepare`.  After the port and interface has been
removed
> +a call will be made to `vif_plug_port_finish`.  Both calls will be made
with
> +the pointer to `vif_plug_port_ctx_out` set to 'NULL', and no call will
be made
> +to `vif_plug_port_ctx_destroy`.
> +
> +Building with in-tree VIF plug providers
> +----------------------------------------
> +
> +VIF plug providers hosted in the OVN repository live under
> +`lib/vif-plug-providers`:
> +
> +To enable them, provide the `--enable-vif-plug-providers` command line
option
> +to the configure script when building OVN.
> +
> +Building with an externally provided VIF plug provider
> +------------------------------------------------------
> +
> +There is also infrastructure in place to support linking OVN with an
externally
> +built VIF plug provider.
> +
> +This external VIF plug provider must define a NULL-terminated array of
pointers
> +to `struct vif_plug_class` data structures named
`vif_plug_provider_classes`.
> +Example:
> +
> +.. code-block:: none
> +
> +   const struct vif_plug_class *vif_plug_provider_classes[] = {
> +       &vif_plug_foo,
> +       NULL,
> +   };
> +
> +The name of the repository for the external VIF plug provider should be
the
> +same as the name of the library it produces, and the built library
artifact
> +should be placed in lib/.libs.  Example:
> +
> +.. code-block:: none
> +
> +   ovn-vif-foo/
> +   ovn-vif-foo/lib/.libs/libovn-vif-foo.la
> +
> +To enable such a VIF plug provider provide the
> +`--with-vif-plug-provider=/path/to/ovn-vif-foo` command line option to
the
> +configure script when building OVN.
> +
> +.. LINKS
> +.. _Open vSwitch Integration Guide:
> +   https://docs.openvswitch.org/en/latest/topics/integration/
> diff --git a/acinclude.m4 b/acinclude.m4
> index e7f829520..3e4a54086 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -441,3 +441,52 @@ AC_DEFUN([OVN_CHECK_OVS], [
>    AC_MSG_CHECKING([OVS version])
>    AC_MSG_RESULT([$OVSVERSION])
>  ])
> +
> +dnl OVN_CHECK_VIF_PLUG_PROVIDER
> +dnl
> +dnl Check for external VIF plug provider
> +AC_DEFUN([OVN_CHECK_VIF_PLUG_PROVIDER], [
> +  AC_ARG_VAR([VIF_PLUG_PROVIDER])
> +  AC_ARG_WITH(
> +    [vif-plug-provider],
> +
 [AC_HELP_STRING([--with-vif-plug-provider=/path/to/provider/repository],
> +                    [Specify path to a configured and built VIF plug
provider repository])],
> +    [if test "$withval" = yes; then
> +       if test -z "$VIF_PLUG_PROVIDER"; then
> +         AC_MSG_ERROR([To build with external VIF plug provider, specify
the path to a configured and built plug provider repository
--with-vif-plug-provider or in \$VIF_PLUG_PROVIDER]),
> +       fi
> +       VIF_PLUG_PROVIDER="$(realpath $VIF_PLUG_PROVIDER)"
> +     else
> +       VIF_PLUG_PROVIDER="$(realpath $withval)"
> +     fi
> +     _vif_plug_provider_name="$(basename $VIF_PLUG_PROVIDER)"
> +     if test ! -f
"$VIF_PLUG_PROVIDER/lib/.libs/lib${_vif_plug_provider_name}.la"; then
> +       AC_MSG_ERROR([$withval is not a configured and built VIF plug
provider library repository])
> +     fi
> +     VIF_PLUG_PROVIDER_LDFLAGS="-L$VIF_PLUG_PROVIDER/lib/.libs
-l$_vif_plug_provider_name"
> +    ],
> +    [VIF_PLUG_PROVIDER=no])
> +  AC_MSG_CHECKING([for VIF plug provider])
> +  AC_MSG_RESULT([$VIF_PLUG_PROVIDER])
> +  AC_SUBST([VIF_PLUG_PROVIDER_LDFLAGS])
> +  AM_CONDITIONAL([HAVE_VIF_PLUG_PROVIDER], [test "$VIF_PLUG_PROVIDER" !=
no])
> +  if test "$VIF_PLUG_PROVIDER" != no; then
> +    AC_DEFINE([HAVE_VIF_PLUG_PROVIDER], [1],
> +              [Build and link with external VIF plug provider])
> +  fi
> +])
> +
> +dnl OVN_ENABLE_VIF_PLUG
> +dnl
> +dnl Enable built-in plug providers
> +AC_DEFUN([OVN_ENABLE_VIF_PLUG], [
> +    AC_ARG_ENABLE(
> +      [vif-plug-providers],
> +      [AC_HELP_STRING([--enable-vif-plug-providers], [Enable building of
built-in VIF plug providers])],
> +      [], [enable_vif_plug=no])
> +    AM_CONDITIONAL([ENABLE_VIF_PLUG], [test "$enable_vif_plug" != no])
> +    if test "$enable_vif_plug" != no; then
> +      AC_DEFINE([ENABLE_VIF_PLUG], [1],
> +                [Build built-in VIF plug providers])
> +    fi
> +])
> diff --git a/configure.ac b/configure.ac
> index d1b9b4d55..5a3a5987b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -172,6 +172,8 @@ OVS_ENABLE_SPARSE
>  OVS_CHECK_DDLOG([0.47])
>  OVS_CHECK_PRAGMA_MESSAGE
>  OVN_CHECK_OVS
> +OVN_CHECK_VIF_PLUG_PROVIDER
> +OVN_ENABLE_VIF_PLUG
>  OVS_CTAGS_IDENTIFIERS
>  AC_SUBST([OVS_CFLAGS])
>  AC_SUBST([OVS_LDFLAGS])
> diff --git a/controller/automake.mk b/controller/automake.mk
> index ad2d68af2..9f9b49fe0 100644
> --- a/controller/automake.mk
> +++ b/controller/automake.mk
> @@ -37,7 +37,9 @@ controller_ovn_controller_SOURCES = \
>         controller/local_data.c \
>         controller/local_data.h \
>         controller/ovsport.h \
> -       controller/ovsport.c
> +       controller/ovsport.c \
> +       controller/vif-plug.h \
> +       controller/vif-plug.c
>
>  controller_ovn_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/
libopenvswitch.la
>  man_MANS += controller/ovn-controller.8
> diff --git a/controller/test-vif-plug.c b/controller/test-vif-plug.c
> new file mode 100644
> index 000000000..01ff37d8f
> --- /dev/null
> +++ b/controller/test-vif-plug.c
> @@ -0,0 +1,72 @@
> +/* Copyright (c) 2021, Canonical
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +#include <errno.h>
> +
> +#include "vif-plug.h"
> +#include "vif-plug-provider.h"
> +#include "smap.h"
> +#include "sset.h"
> +#include "tests/ovstest.h"
> +
> +static void
> +test_vif_plug(struct ovs_cmdl_context *ctx OVS_UNUSED)
> +{
> +    const struct vif_plug_class *vif_plug_class;
> +
> +    ovs_assert(vif_plug_provider_unregister("dummy") == EINVAL);
> +
> +    ovs_assert(!vif_plug_provider_register(&vif_plug_dummy_class));
> +    vif_plug_class = vif_plug_provider_get("dummy");
> +    ovs_assert(vif_plug_provider_register(&vif_plug_dummy_class) ==
EEXIST);
> +
> +    ovs_assert(
> +        sset_contains(
> +            vif_plug_get_maintained_iface_options(vif_plug_class),
> +            "plug-dummy-option"));
> +
> +    struct vif_plug_port_ctx_in ctx_in = {
> +        .op_type = PLUG_OP_CREATE,
> +        .lport_name = "lsp1",
> +        .lport_options = SMAP_INITIALIZER(&ctx_in.lport_options),
> +    };
> +    struct vif_plug_port_ctx_out ctx_out;
> +    vif_plug_port_prepare(vif_plug_class, &ctx_in, &ctx_out);
> +    ovs_assert(!strcmp(ctx_out.name, "lsp1"));
> +    ovs_assert(!strcmp(ctx_out.type, "internal"));
> +    ovs_assert(!strcmp(smap_get(
> +            &ctx_out.iface_options, "vif-plug-dummy-option"), "value"));
> +
> +    vif_plug_port_finish(vif_plug_class, &ctx_in, &ctx_out);
> +    vif_plug_port_ctx_destroy(vif_plug_class, &ctx_in, &ctx_out);
> +    vif_plug_provider_destroy_all();
> +}
> +
> +static void
> +test_vif_plug_main(int argc, char *argv[])
> +{
> +    set_program_name(argv[0]);
> +    static const struct ovs_cmdl_command commands[] = {
> +        {"run", NULL, 0, 0, test_vif_plug, OVS_RO},
> +        {NULL, NULL, 0, 0, NULL, OVS_RO},
> +    };
> +    struct ovs_cmdl_context ctx;
> +    ctx.argc = argc - 1;
> +    ctx.argv = argv + 1;
> +    ovs_cmdl_run_command(&ctx, commands);
> +}
> +
> +OVSTEST_REGISTER("test-vif-plug", test_vif_plug_main);
> diff --git a/controller/vif-plug.c b/controller/vif-plug.c
> new file mode 100644
> index 000000000..2cd7cd09a
> --- /dev/null
> +++ b/controller/vif-plug.c
> @@ -0,0 +1,632 @@
> +/*
> + * Copyright (c) 2021 Canonical
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +
> +/* OVS includes */
> +#include "lib/vswitch-idl.h"
> +#include "openvswitch/shash.h"
> +#include "openvswitch/vlog.h"
> +
> +/* OVN includes */
> +#include "binding.h"
> +#include "lib/ovn-sb-idl.h"
> +#include "lport.h"
> +#include "ovsport.h"
> +#include "vif-plug.h"
> +#include "vif-plug-provider.h"
> +
> +VLOG_DEFINE_THIS_MODULE(vif_plug);
> +
> +#define OVN_PLUGGED_EXT_ID "ovn-plugged"
> +#define VIF_PLUG_OPTION_TYPE "vif-plug-type"
> +#define VIF_PLUG_OPTION_MTU_REQUEST "vif-plug-mtu-request"
> +
> +void
> +vif_plug_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> +{
> +    ovsdb_idl_track_add_column(ovs_idl,
&ovsrec_interface_col_mtu_request);
> +}
> +
> +/* Get the class level 'maintained_iface_options' set. */
> +const struct sset *
> +vif_plug_get_maintained_iface_options(
> +        const struct vif_plug_class *vif_plug_class)
> +{
> +    return vif_plug_class->vif_plug_get_maintained_iface_options ?
> +           vif_plug_class->vif_plug_get_maintained_iface_options() :
NULL;
> +}
> +
> +/* Prepare the logical port as identified by 'ctx_in' for port creation,
update
> + * or removal as specified by 'ctx_in->op_type'.
> + *
> + * When 'ctx_in->op_type' is PLUG_OP_CREATE the plug implementation must
fill
> + * 'ctx_out' with data to apply to the interface record maintained by
OVN on
> + * its behalf.
> + *
> + * When 'ctx_in_op_type' is PLUG_OP_REMOVE 'ctx_out' should be set to
NULL and
> + * the plug implementation must not attempt to use 'ctx_out'.
> + *
> + * The data in 'ctx_out' is owned by the plug implementation, and a call
must
> + * be made to vif_plug_port_ctx_destroy when done with it. */
> +bool
> +vif_plug_port_prepare(const struct vif_plug_class *vif_plug_class,
> +                      const struct vif_plug_port_ctx_in *ctx_in,
> +                      struct vif_plug_port_ctx_out *ctx_out)
> +{
> +    return vif_plug_class->vif_plug_port_prepare(ctx_in, ctx_out);
> +}
> +
> +/* Notify the VIF plug implementation that a port creation, update or
removal
> + * has been committed to the database. */
> +void
> +vif_plug_port_finish(const struct vif_plug_class *vif_plug_class,
> +                     const struct vif_plug_port_ctx_in *ctx_in,
> +                     struct vif_plug_port_ctx_out *ctx_out)
> +{
> +    vif_plug_class->vif_plug_port_finish(ctx_in, ctx_out);
> +}
> +
> +/* Free any data allocated to 'ctx_out' in a prevous call to
> + * vif_plug_port_prepare. */
> +void
> +vif_plug_port_ctx_destroy(const struct vif_plug_class *vif_plug_class,
> +                          const struct vif_plug_port_ctx_in *ctx_in,
> +                          struct vif_plug_port_ctx_out *ctx_out)
> +{
> +    vif_plug_class->vif_plug_port_ctx_destroy(ctx_in, ctx_out);
> +}
> +
> +static struct vif_plug_port_ctx *
> +build_port_ctx(const struct vif_plug_class *vif_plug,
> +                  const enum vif_plug_op_type op_type,
> +                  const struct vif_plug_ctx_in *vif_plug_ctx_in,
> +                  const struct sbrec_port_binding *pb,
> +                  const struct ovsrec_interface *iface,
> +                  const char *iface_id)
> +{
> +    struct vif_plug_port_ctx *new_ctx = xzalloc(
> +        sizeof *new_ctx);
> +
> +    new_ctx->vif_plug = vif_plug;
> +    new_ctx->vif_plug_port_ctx_in.op_type = op_type;
> +    new_ctx->vif_plug_port_ctx_in.ovs_table = vif_plug_ctx_in->ovs_table;
> +    new_ctx->vif_plug_port_ctx_in.br_int = vif_plug_ctx_in->br_int;
> +    new_ctx->vif_plug_port_ctx_in.lport_name = pb ?
> +        xstrdup(pb->logical_port) : iface_id ? xstrdup(iface_id) : NULL;
> +    /* Prepare vif_plug_port_ctx_in smaps for use.
> +     *
> +     * Note that smap_init does not allocate memory.  Any memory
allocated by
> +     * putting data into the vif_plug_port_ctx_in smaps will be
destroyed by
> +     * calls to smap_destroy in destroy_port_ctx */
> +    smap_init(&new_ctx->vif_plug_port_ctx_in.lport_options);
> +    smap_init(&new_ctx->vif_plug_port_ctx_in.iface_options);
> +
> +    if (pb) {
> +        smap_clone(&new_ctx->vif_plug_port_ctx_in.lport_options,
> +                   &pb->options);
> +    }
> +
> +    if (iface) {
> +        new_ctx->vif_plug_port_ctx_in.iface_name = xstrdup(iface->name);
> +        new_ctx->vif_plug_port_ctx_in.iface_type = xstrdup(iface->type);
> +        smap_clone(&new_ctx->vif_plug_port_ctx_in.iface_options,
> +                   &iface->options);
> +    }
> +
> +    /* Prepare vif_plug_port_ctx_out smaps for use.
> +     *
> +     * Note that smap_init does not allocate memory.  Any memory
allocated by
> +     * putting data into the vif_plug_port_ctx_out smaps is the
responsibility
> +     * of the VIF plug provider through a call to
vif_plug_port_ctx_destroy. */
> +    smap_init(&new_ctx->vif_plug_port_ctx_out.iface_options);
> +
> +    return new_ctx;
> +}
> +
> +static void
> +destroy_port_ctx(struct vif_plug_port_ctx *ctx)
> +{
> +    smap_destroy(&ctx->vif_plug_port_ctx_in.lport_options);
> +    smap_destroy(&ctx->vif_plug_port_ctx_in.iface_options);
> +    if (ctx->vif_plug_port_ctx_in.lport_name) {
> +        free((char *)ctx->vif_plug_port_ctx_in.lport_name);
> +    }
> +    if (ctx->vif_plug_port_ctx_in.iface_name) {
> +        free((char *)ctx->vif_plug_port_ctx_in.iface_name);
> +    }
> +    if (ctx->vif_plug_port_ctx_in.iface_type) {
> +        free((char *)ctx->vif_plug_port_ctx_in.iface_type);
> +    }
> +    /* Note that data associated with ctx->vif_plug_port_ctx_out must be
> +     * destroyed by the plug provider implementation with a call to
> +     * vif_plug_port_ctx_destroy prior to calling this function */
> +    free(ctx);
> +}
> +
> +/* When we add deletion of rows to the transaction, the data structures
> + * associated with the rows will immediately be freed from the IDL, and
as
> + * such we can no longer access them.
> + *
> + * Since IDL commits are handled asynchronously we can have a few engine
> + * iterations where the deleted data shows up when iterating over table
> + * contents, but the IDL *_is_deleted() call will not reliably
categorize the
> + * data as deleted.  This is in contrast to the IDL behaviour when some
other
> + * process deletes data from the database, so this may be an OVS IDL
bug, or it
> + * could be it's just expected that the program consuming the IDL will
know not
> + * to access rows it has deleted.
> + *

I have some concerns for this part. Firstly, let me clarify that
*_is_deleted() is not supposed to be used to check if a row is deleted in
current idl loop. It is instead used for telling the type of a change for a
*change-tracked* row - is it deleted or newly created, etc. So this is not
a bug (but probably documentation can be enhanced). However, I don't
understand why would it be possible to access deleted rows in the current
IDL iteration. The statement "we can have a few engine iterations where the
deleted data shows up when iterating over table contents" seems unexpected
behavior. It should never happen. If you encountered such behavior it may
indicate something more serious, such as, did we forget to clear any
references to rows somewhere? If that's the case, there can be dangling
pointer access somewhere else before calling this function. It is better to
make sure that doesn't happen. Did you see such problems?

> + * To deal with this, we keep a reference for ourself to avoid
attempting to
> + * remove the same data multiple times while waiting for the transaction
to
> + * commit.  The tracking data will be cleared after commit at the end of
the
> + * ovn-controller main loop.
> + */
> +static void
> +transact_delete_port(const struct vif_plug_ctx_in *vif_plug_ctx_in,
> +                     const struct vif_plug_ctx_out *vif_plug_ctx_out,
> +                     const struct vif_plug_port_ctx *vif_plug_port_ctx,
> +                     const struct ovsrec_port *port)
> +{
> +    shash_add(vif_plug_ctx_out->deleted_iface_ids,
> +              vif_plug_port_ctx->vif_plug_port_ctx_in.lport_name,
> +              vif_plug_port_ctx);
> +    ovsport_remove(vif_plug_ctx_in->br_int, port);
> +}
> +
> +static void
> +transact_create_port(const struct vif_plug_ctx_in *vif_plug_ctx_in,
> +                     const struct vif_plug_ctx_out *vif_plug_ctx_out,
> +                     const struct vif_plug_port_ctx *vif_plug_port_ctx,
> +                     const struct smap *iface_external_ids,
> +                     const int64_t mtu_request)
> +{
> +    shash_add(vif_plug_ctx_out->changed_iface_ids,
> +              vif_plug_port_ctx->vif_plug_port_ctx_in.lport_name,
> +              vif_plug_port_ctx);
> +    ovsport_create(vif_plug_ctx_in->ovs_idl_txn, vif_plug_ctx_in->br_int,
> +                   vif_plug_port_ctx->vif_plug_port_ctx_out.name,
> +                   vif_plug_port_ctx->vif_plug_port_ctx_out.type,
> +                   NULL, iface_external_ids,
> +
&vif_plug_port_ctx->vif_plug_port_ctx_out.iface_options,
> +                   mtu_request);
> +}
> +
> +static void
> +transact_update_port(const struct ovsrec_interface *iface_rec,
> +                     const struct vif_plug_ctx_in *vif_plug_ctx_in
OVS_UNUSED,
> +                     const struct vif_plug_ctx_out *vif_plug_ctx_out,
> +                     const struct vif_plug_port_ctx *vif_plug_port_ctx,
> +                     const struct smap *iface_external_ids,
> +                     const int64_t mtu_request)
> +{
> +    shash_add(vif_plug_ctx_out->changed_iface_ids,
> +              vif_plug_port_ctx->vif_plug_port_ctx_in.lport_name,
> +              vif_plug_port_ctx);
> +    ovsport_update_iface(
> +        iface_rec,
> +        vif_plug_port_ctx->vif_plug_port_ctx_out.type,
> +        iface_external_ids,
> +        NULL,
> +        &vif_plug_port_ctx->vif_plug_port_ctx_out.iface_options,
> +        vif_plug_get_maintained_iface_options(
> +            vif_plug_port_ctx->vif_plug),
> +        mtu_request);
> +}
> +
> +
> +static bool
> +consider_unplug_iface(const struct ovsrec_interface *iface,
> +                      const struct sbrec_port_binding *pb,
> +                      struct vif_plug_ctx_in *vif_plug_ctx_in,
> +                      struct vif_plug_ctx_out *vif_plug_ctx_out)
> +{
> +    const char *vif_plug_type = smap_get(&iface->external_ids,
> +                                         OVN_PLUGGED_EXT_ID);
> +    const char *iface_id = smap_get(&iface->external_ids, "iface-id");
> +    const struct ovsrec_port *port = ovsport_lookup_by_interface(
> +        vif_plug_ctx_in->ovsrec_port_by_interfaces,
> +        (struct ovsrec_interface *) iface);
> +
> +    if (vif_plug_type && iface_id && port) {
> +        const struct vif_plug_class *vif_plug;
> +        if (!(vif_plug = vif_plug_provider_get(vif_plug_type))) {
> +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
1);
> +            VLOG_WARN_RL(&rl,
> +                         "Unable to open VIF plug provider for "
> +                         "%s %s iface-id %s",
> +                         VIF_PLUG_OPTION_TYPE, vif_plug_type, iface_id);
> +            /* While we are unable to handle this, asking for a recompute
> +             * will not change that fact. */
> +            return true;
> +        }
> +        if (!vif_plug_ctx_in->chassis_rec || !vif_plug_ctx_in->br_int
> +            || !vif_plug_ctx_in->ovs_idl_txn)
> +        {
> +            /* Some of our prerequisites are not available, ask for a
> +             * recompute. */
> +            return false;
> +        }
> +
> +        /* Our contract with the VIF plug provider is that
vif_plug_port_finish
> +         * will be called with a vif_plug_port_ctx_in object once the
> +         * transaction commits.
> +         *
> +         * Since this happens asynchronously we need to allocate memory
for
> +         * and duplicate any database references so that they stay valid.
> +         *
> +         * The data is freed with a call to destroy_port_ctx after the
> +         * transaction completes at the end of the ovn-controller main
> +         * loop. */
> +        struct vif_plug_port_ctx *vif_plug_port_ctx = build_port_ctx(
> +            vif_plug, PLUG_OP_REMOVE, vif_plug_ctx_in, pb, iface,
iface_id);
> +
> +        if (!vif_plug_port_prepare(vif_plug,
> +
&vif_plug_port_ctx->vif_plug_port_ctx_in,
> +                                   NULL)) {
> +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
1);
> +            VLOG_INFO_RL(&rl,
> +                         "Not unplugging iface %s (iface-id %s) on
direction "
> +                         "from VIF plug provider.",
> +                         iface->name, iface_id);
> +            destroy_port_ctx(vif_plug_port_ctx);
> +            return true;
> +        }
> +        VLOG_INFO("Unplugging port %s from %s for iface-id %s on this "
> +                  "chassis.",
> +                  port->name,
> +                  vif_plug_ctx_in->br_int->name,
> +                  iface_id);
> +
> +        /* Add and track delete operation to the transaction */
> +        transact_delete_port(vif_plug_ctx_in, vif_plug_ctx_out,
> +                             vif_plug_port_ctx, port);
> +        return true;
> +    }
> +    return true;
> +}
> +
> +static int64_t
> +get_plug_mtu_request(const struct smap *lport_options)
> +{
> +    return smap_get_int(lport_options, VIF_PLUG_OPTION_MTU_REQUEST, 0);
> +}
> +
> +static bool
> +consider_plug_lport_create__(const struct vif_plug_class *vif_plug,
> +                             const struct smap *iface_external_ids,
> +                             const struct sbrec_port_binding *pb,
> +                             struct vif_plug_ctx_in *vif_plug_ctx_in,
> +                             struct vif_plug_ctx_out *vif_plug_ctx_out)
> +{
> +    if (!vif_plug_ctx_in->chassis_rec || !vif_plug_ctx_in->br_int
> +        || !vif_plug_ctx_in->ovs_idl_txn) {
> +        /* Some of our prerequisites are not available, ask for a
recompute. */
> +        return false;
> +    }
> +
> +    /* Our contract with the VIF plug provider is that
vif_plug_port_finish
> +     * will be called with vif_plug_port_ctx_in and vif_plug_port_ctx_out
> +     * objects once the transaction commits.
> +     *
> +     * Since this happens asynchronously we need to allocate memory for
> +     * and duplicate any database references so that they stay valid.
> +     *
> +     * The data is freed with a call to destroy_port_ctx after the
> +     * transaction completes at the end of the ovn-controller main
> +     * loop. */
> +    struct vif_plug_port_ctx *vif_plug_port_ctx = build_port_ctx(
> +        vif_plug, PLUG_OP_CREATE, vif_plug_ctx_in, pb, NULL, NULL);
> +
> +    if (!vif_plug_port_prepare(vif_plug,
> +                           &vif_plug_port_ctx->vif_plug_port_ctx_in,
> +                           &vif_plug_port_ctx->vif_plug_port_ctx_out)) {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +        VLOG_INFO_RL(&rl,
> +                     "Not plugging lport %s on direction from VIF plug "
> +                     "provider.",
> +                     pb->logical_port);
> +        destroy_port_ctx(vif_plug_port_ctx);
> +        return true;
> +    }
> +
> +    VLOG_INFO("Plugging port %s into %s for lport %s on this "
> +              "chassis.",
> +              vif_plug_port_ctx->vif_plug_port_ctx_out.name,
> +              vif_plug_ctx_in->br_int->name,
> +              pb->logical_port);
> +    transact_create_port(vif_plug_ctx_in, vif_plug_ctx_out,
> +                         vif_plug_port_ctx,
> +                         iface_external_ids,
> +                         get_plug_mtu_request(&pb->options));
> +    return true;
> +}
> +
> +static bool
> +consider_plug_lport_update__(const struct vif_plug_class *vif_plug,
> +                             const struct smap *iface_external_ids,
> +                             const struct sbrec_port_binding *pb,
> +                             struct local_binding *lbinding,
> +                             struct vif_plug_ctx_in *vif_plug_ctx_in,
> +                             struct vif_plug_ctx_out *vif_plug_ctx_out)
> +{
> +    if (!vif_plug_ctx_in->chassis_rec || !vif_plug_ctx_in->br_int
> +        || !vif_plug_ctx_in->ovs_idl_txn) {
> +        /* Some of our prerequisites are not available, ask for a
recompute. */
> +        return false;
> +    }
> +    /* Our contract with the VIF plug provider is that
vif_plug_port_finish
> +     * will be called with vif_plug_port_ctx_in and vif_plug_port_ctx_out
> +     * objects once the transaction commits.
> +     *
> +     * Since this happens asynchronously we need to allocate memory for
> +     * and duplicate any database references so that they stay valid.
> +     *
> +     * The data is freed with a call to destroy_port_ctx after the
> +     * transaction completes at the end of the ovn-controller main
> +     * loop. */
> +    struct vif_plug_port_ctx *vif_plug_port_ctx = build_port_ctx(
> +        vif_plug, PLUG_OP_CREATE, vif_plug_ctx_in, pb, NULL, NULL);
> +
> +    if (!vif_plug_port_prepare(vif_plug,
> +                               &vif_plug_port_ctx->vif_plug_port_ctx_in,
> +
&vif_plug_port_ctx->vif_plug_port_ctx_out)) {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +        VLOG_INFO_RL(&rl,
> +                     "Not updating lport %s on direction from VIF plug "
> +                     "provider.",
> +                     pb->logical_port);
> +        destroy_port_ctx(vif_plug_port_ctx);
> +        return true;
> +    }
> +
> +    if (strcmp(lbinding->iface->name,
> +               vif_plug_port_ctx->vif_plug_port_ctx_out.name)) {
> +        VLOG_WARN("Attempt of incompatible change to existing "
> +                  "port detected, please recreate port: %s",
> +                   pb->logical_port);
> +        vif_plug_port_ctx_destroy(vif_plug,
> +
 &vif_plug_port_ctx->vif_plug_port_ctx_in,
> +
 &vif_plug_port_ctx->vif_plug_port_ctx_out);
> +        destroy_port_ctx(vif_plug_port_ctx);
> +        return false;
> +    }
> +    VLOG_DBG("updating iface for: %s", pb->logical_port);
> +    transact_update_port(lbinding->iface, vif_plug_ctx_in,
vif_plug_ctx_out,
> +                         vif_plug_port_ctx, iface_external_ids,
> +                         get_plug_mtu_request(&pb->options));
> +
> +    return true;
> +}
> +
> +static bool
> +consider_plug_lport(const struct sbrec_port_binding *pb,
> +                    struct local_binding *lbinding,
> +                    struct vif_plug_ctx_in *vif_plug_ctx_in,
> +                    struct vif_plug_ctx_out *vif_plug_ctx_out)
> +{
> +    bool ret = true;
> +    if (lport_can_bind_on_this_chassis(vif_plug_ctx_in->chassis_rec, pb)
> +        && pb->requested_chassis == vif_plug_ctx_in->chassis_rec) {
> +        const char *vif_plug_type = smap_get(&pb->options,
> +                                             VIF_PLUG_OPTION_TYPE);
> +        if (!vif_plug_type) {
> +            /* Nothing for us to do and we don't need a recompute. */
> +            return true;
> +        }
> +
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +        const struct vif_plug_class *vif_plug;
> +        if (!(vif_plug = vif_plug_provider_get(vif_plug_type))) {
> +            VLOG_WARN_RL(&rl,
> +                         "Unable to open VIF plug provider for %s: '%s' "
> +                         "lport %s",
> +                         VIF_PLUG_OPTION_TYPE,
> +                         vif_plug_type,
> +                         pb->logical_port);
> +            /* While we are unable to handle this, asking for a
recompute will
> +             * not change that fact. */
> +            return true;
> +        }
> +        const struct smap iface_external_ids = SMAP_CONST2(
> +                &iface_external_ids,
> +                OVN_PLUGGED_EXT_ID, vif_plug_type,
> +                "iface-id", pb->logical_port);
> +        if (lbinding && lbinding->iface) {
> +            if (!smap_get(&lbinding->iface->external_ids,
> +                          OVN_PLUGGED_EXT_ID))
> +            {
> +                VLOG_WARN_RL(&rl,
> +                             "CMS requested plugging of lport %s, but a
port "
> +                             "that is not maintained by OVN already
exsist "
> +                             "in local vSwitch: "UUID_FMT,
> +                             pb->logical_port,
> +                             UUID_ARGS(&lbinding->iface->header_.uuid));
> +                return false;
> +            }
> +            ret = consider_plug_lport_update__(vif_plug,
&iface_external_ids,
> +                                               pb, lbinding,
vif_plug_ctx_in,
> +                                               vif_plug_ctx_out);
> +        } else {
> +            ret = consider_plug_lport_create__(vif_plug,
&iface_external_ids,
> +                                               pb, vif_plug_ctx_in,
> +                                               vif_plug_ctx_out);
> +        }
> +    }
> +
> +    return ret;
> +}
> +
> +static bool
> +vif_plug_iface_touched_this_txn(
> +        const struct vif_plug_ctx_out *vif_plug_ctx_out,
> +        const char *iface_id)
> +{
> +    return shash_find(vif_plug_ctx_out->changed_iface_ids, iface_id)
> +           || shash_find(vif_plug_ctx_out->deleted_iface_ids, iface_id);
> +}
> +
> +static bool
> +vif_plug_handle_lport_vif(const struct sbrec_port_binding *pb,
> +                          struct vif_plug_ctx_in *vif_plug_ctx_in,
> +                          struct vif_plug_ctx_out *vif_plug_ctx_out)
> +{
> +    if (vif_plug_iface_touched_this_txn(vif_plug_ctx_out,
pb->logical_port)) {
> +        return true;
> +    }
> +    bool handled = true;
> +    struct local_binding *lbinding = local_binding_find(
> +        vif_plug_ctx_in->local_bindings, pb->logical_port);
> +
> +    if (lport_can_bind_on_this_chassis(vif_plug_ctx_in->chassis_rec,
pb)) {
> +        handled &= consider_plug_lport(pb, lbinding,
> +                                       vif_plug_ctx_in,
vif_plug_ctx_out);
> +    } else if (lbinding && lbinding->iface) {
> +        handled &= consider_unplug_iface(lbinding->iface, pb,
> +                                         vif_plug_ctx_in,
vif_plug_ctx_out);
> +    }
> +    return handled;
> +}
> +
> +static bool
> +vif_plug_handle_iface(const struct ovsrec_interface *iface_rec,
> +                      struct vif_plug_ctx_in *vif_plug_ctx_in,
> +                      struct vif_plug_ctx_out *vif_plug_ctx_out)
> +{
> +    bool handled = true;
> +    const char *vif_plug_type = smap_get(&iface_rec->external_ids,
> +                                         OVN_PLUGGED_EXT_ID);
> +    const char *iface_id = smap_get(&iface_rec->external_ids,
"iface-id");
> +    if (!vif_plug_type || !iface_id
> +        || vif_plug_iface_touched_this_txn(vif_plug_ctx_out, iface_id)) {
> +        return true;
> +    }
> +    struct local_binding *lbinding = local_binding_find(
> +        vif_plug_ctx_in->local_bindings, iface_id);
> +    const struct sbrec_port_binding *pb = lport_lookup_by_name(
> +        vif_plug_ctx_in->sbrec_port_binding_by_name, iface_id);
> +    if (pb && lbinding
> +        && lport_can_bind_on_this_chassis(vif_plug_ctx_in->chassis_rec,
pb)) {
> +        /* Something changed on a interface we have previously plugged,
> +         * consider updating it */
> +        handled &= consider_plug_lport(pb, lbinding,
> +                                       vif_plug_ctx_in,
vif_plug_ctx_out);
> +    } else if (!pb
> +               || !lport_can_bind_on_this_chassis(
> +                       vif_plug_ctx_in->chassis_rec, pb)) {
> +        /* No lport for this interface or it is destined for different
chassis,
> +         * consuder unplugging it */
> +        handled &= consider_unplug_iface(iface_rec, pb,
> +                                         vif_plug_ctx_in,
vif_plug_ctx_out);
> +    }
> +    return handled;
> +}
> +
> +void
> +vif_plug_run(struct vif_plug_ctx_in *vif_plug_ctx_in,
> +             struct vif_plug_ctx_out *vif_plug_ctx_out)
> +{
> +    /* 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 naive check so that we do not erronously unplug ports
> +     * because the data is just not loaded yet.
> +     */

It seems this approach still has some problems.

When the process starts, it is good that vif_plug_run() is called only if
both br_int and chassis are not NULL, which means at least both OVS and SB
DBs are connected. And then with "count" number of iterations we expect all
the initial data downloading has completed, so the "count" need to be big
enough. Assume that 3 is big enough, but in fact data downloading completed
at the first iteration, and then we created a new PB in SB DB which
triggers the second iteration. However, depends on the
inactivity-probe-interval (it is configurable for SB DB connection, and in
large scale environment this is usually configured to be very long, e.g. 3
min), it has to wait for ~3 min before the newly created PB can be handled
by the VIF plug. If we found out that in some cases 3 is not enough and
then we increase it to 10 for the worse case scenario, it means in best
case scenarios - say, the downloading completes in 3 iterations, then we
will have to wait for 7 * 3 = ~20min before our newly created PB during
this 20 min to take effect. So I would use the counter only for deleting
VIFs, but for creating VIFs I think it is better to handle it asap without
checking this counter.

Another problem is when connection is lost and then reconnected. In most
cases the IDL content can be persisted and only download the delta when
reconnected, but when it is bad luck, the IDL data would be cleared and
re-download everything. So there is a chance the VIFs will be deleted since
this counter here is 0 already. I think the count need to be reset to N
whenever IDL data is cleared.

Thanks,
Han

> +    static int prime_idl_count = 3;
> +    if (prime_idl_count && --prime_idl_count > 0) {
> +        return;
> +    }
> +
> +    if (!vif_plug_ctx_in->chassis_rec) {
> +        return;
> +    }
> +    const struct ovsrec_interface *iface_rec;
> +    OVSREC_INTERFACE_TABLE_FOR_EACH (iface_rec,
> +                                     vif_plug_ctx_in->iface_table) {
> +        vif_plug_handle_iface(iface_rec, vif_plug_ctx_in,
vif_plug_ctx_out);
> +    }
> +
> +    struct sbrec_port_binding *target =
> +        sbrec_port_binding_index_init_row(
> +            vif_plug_ctx_in->sbrec_port_binding_by_requested_chassis);
> +    sbrec_port_binding_index_set_requested_chassis(
> +        target,
> +        vif_plug_ctx_in->chassis_rec);
> +    const struct sbrec_port_binding *pb;
> +    SBREC_PORT_BINDING_FOR_EACH_EQUAL (
> +            pb, target,
> +            vif_plug_ctx_in->sbrec_port_binding_by_requested_chassis) {
> +        enum en_lport_type lport_type = get_lport_type(pb);
> +        if (lport_type == LP_VIF) {
> +            vif_plug_handle_lport_vif(pb, vif_plug_ctx_in,
vif_plug_ctx_out);
> +        }
> +    }
> +    sbrec_port_binding_index_destroy_row(target);
> +}
> +
> +static void
> +vif_plug_finish_deleted__(struct shash *deleted_iface_ids, bool
txn_success)
> +{
> +    struct shash_node *node, *next;
> +    SHASH_FOR_EACH_SAFE (node, next, deleted_iface_ids) {
> +        struct vif_plug_port_ctx *vif_plug_port_ctx = node->data;
> +        if (txn_success) {
> +            vif_plug_port_finish(vif_plug_port_ctx->vif_plug,
> +                             &vif_plug_port_ctx->vif_plug_port_ctx_in,
> +                             NULL);
> +        }
> +        shash_delete(deleted_iface_ids, node);
> +        destroy_port_ctx(vif_plug_port_ctx);
> +    }
> +}
> +
> +void
> +vif_plug_clear_deleted(struct shash *deleted_iface_ids) {
> +    vif_plug_finish_deleted__(deleted_iface_ids, false);
> +}
> +
> +void
> +vif_plug_finish_deleted(struct shash *deleted_iface_ids) {
> +    vif_plug_finish_deleted__(deleted_iface_ids, true);
> +}
> +
> +static void
> +vif_plug_finish_changed__(struct shash *changed_iface_ids, bool
txn_success)
> +{
> +    struct shash_node *node, *next;
> +    SHASH_FOR_EACH_SAFE (node, next, changed_iface_ids) {
> +        struct vif_plug_port_ctx *vif_plug_port_ctx = node->data;
> +        if (txn_success) {
> +            vif_plug_port_finish(vif_plug_port_ctx->vif_plug,
> +
&vif_plug_port_ctx->vif_plug_port_ctx_in,
> +
&vif_plug_port_ctx->vif_plug_port_ctx_out);
> +        }
> +        vif_plug_port_ctx_destroy(vif_plug_port_ctx->vif_plug,
> +
 &vif_plug_port_ctx->vif_plug_port_ctx_in,
> +
 &vif_plug_port_ctx->vif_plug_port_ctx_out);
> +        shash_delete(changed_iface_ids, node);
> +        destroy_port_ctx(vif_plug_port_ctx);
> +    }
> +}
> +
> +void
> +vif_plug_clear_changed(struct shash *deleted_iface_ids) {
> +    vif_plug_finish_changed__(deleted_iface_ids, false);
> +}
> +
> +void
> +vif_plug_finish_changed(struct shash *deleted_iface_ids) {
> +    vif_plug_finish_changed__(deleted_iface_ids, true);
> +}
> diff --git a/controller/vif-plug.h b/controller/vif-plug.h
> new file mode 100644
> index 000000000..7a1978e38
> --- /dev/null
> +++ b/controller/vif-plug.h
> @@ -0,0 +1,79 @@
> +/*
> + * Copyright (c) 2021 Canonical
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef VIF_PLUG_H
> +#define VIF_PLUG_H 1
> +
> +/*
> + * VIF Plug, the controller internal interface to the VIF plug provider
> + * infrastructure.
> + */
> +
> +#include "openvswitch/shash.h"
> +#include "smap.h"
> +
> +#ifdef  __cplusplus
> +extern "C" {
> +#endif
> +
> +struct vif_plug_ctx_in {
> +    struct ovsdb_idl_txn *ovs_idl_txn;
> +    struct ovsdb_idl_index *sbrec_port_binding_by_name;
> +    struct ovsdb_idl_index *sbrec_port_binding_by_requested_chassis;
> +    struct ovsdb_idl_index *ovsrec_port_by_interfaces;
> +    const struct ovsrec_open_vswitch_table *ovs_table;
> +    const struct ovsrec_bridge *br_int;
> +    const struct ovsrec_interface_table *iface_table;
> +    const struct sbrec_chassis *chassis_rec;
> +    const struct shash *local_bindings;
> +};
> +
> +struct vif_plug_ctx_out {
> +    struct shash *deleted_iface_ids;
> +    struct shash *changed_iface_ids;
> +};
> +
> +struct vif_plug_class;
> +struct vif_plug_port_ctx_out;
> +struct vif_plug_port_ctx_in;
> +
> +const struct sset * vif_plug_get_maintained_iface_options(
> +    const struct vif_plug_class *);
> +
> +bool vif_plug_port_prepare(const struct vif_plug_class *,
> +                           const struct vif_plug_port_ctx_in *,
> +                           struct vif_plug_port_ctx_out *);
> +void vif_plug_port_finish(const struct vif_plug_class *,
> +                          const struct vif_plug_port_ctx_in *,
> +                          struct vif_plug_port_ctx_out *);
> +void vif_plug_port_ctx_destroy(const struct vif_plug_class *,
> +                           const struct vif_plug_port_ctx_in *,
> +                           struct vif_plug_port_ctx_out *);
> +
> +struct ovsdb_idl;
> +
> +void vif_plug_register_ovs_idl(struct ovsdb_idl *ovs_idl);
> +void vif_plug_run(struct vif_plug_ctx_in *, struct vif_plug_ctx_out *);
> +void vif_plug_clear_changed(struct shash *deleted_iface_ids);
> +void vif_plug_finish_changed(struct shash *changed_iface_ids);
> +void vif_plug_clear_deleted(struct shash *deleted_iface_ids);
> +void vif_plug_finish_deleted(struct shash *changed_iface_ids);
> +
> +#ifdef  __cplusplus
> +}
> +#endif
> +
> +#endif /* vif-plug.h */
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 9f9f447d5..829aedfc5 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -4,6 +4,11 @@ lib_libovn_la_LDFLAGS = \
>          -Wl,--version-script=$(top_builddir)/lib/libovn.sym \
>          $(OVS_LIBDIR)/libopenvswitch.la \
>          $(AM_LDFLAGS)
> +
> +if HAVE_VIF_PLUG_PROVIDER
> +lib_libovn_la_LDFLAGS += $(VIF_PLUG_PROVIDER_LDFLAGS)
> +endif
> +
>  lib_libovn_la_SOURCES = \
>         lib/acl-log.c \
>         lib/acl-log.h \
> @@ -33,7 +38,10 @@ lib_libovn_la_SOURCES = \
>         lib/inc-proc-eng.h \
>         lib/lb.c \
>         lib/lb.h \
> -       lib/stopwatch-names.h
> +       lib/stopwatch-names.h \
> +       lib/vif-plug-provider.h \
> +       lib/vif-plug-provider.c \
> +       lib/vif-plug-providers/dummy/vif-plug-dummy.c
>  nodist_lib_libovn_la_SOURCES = \
>         lib/ovn-dirs.c \
>         lib/ovn-nb-idl.c \
> diff --git a/lib/vif-plug-provider.c b/lib/vif-plug-provider.c
> new file mode 100644
> index 000000000..798e90e26
> --- /dev/null
> +++ b/lib/vif-plug-provider.c
> @@ -0,0 +1,204 @@
> +/*
> + * Copyright (c) 2021 Canonical
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +#include "vif-plug-provider.h"
> +
> +#include <errno.h>
> +#include <stdint.h>
> +#include <string.h>
> +
> +#include "openvswitch/vlog.h"
> +#include "openvswitch/shash.h"
> +#include "smap.h"
> +#include "sset.h"
> +#include "lib/inc-proc-eng.h"
> +
> +VLOG_DEFINE_THIS_MODULE(vif_plug_provider);
> +
> +#ifdef ENABLE_VIF_PLUG
> +static const struct vif_plug_class *base_vif_plug_classes[] = {
> +};
> +#endif
> +
> +static struct shash vif_plug_classes =
SHASH_INITIALIZER(&vif_plug_classes);
> +
> +/* Protects the 'vif_plug_classes' shash. */
> +static struct ovs_mutex vif_plug_classes_mutex = OVS_MUTEX_INITIALIZER;
> +
> +/* Initialize the the VIF plug infrastructure by registering known
classes */
> +void
> +vif_plug_provider_initialize(void)
> +{
> +    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> +
> +    if (ovsthread_once_start(&once)) {
> +#ifdef ENABLE_VIF_PLUG
> +        /* Register built-in VIF plug provider classes */
> +        for (int i = 0; i < ARRAY_SIZE(base_vif_plug_classes); i++) {
> +            vif_plug_provider_register(base_vif_plug_classes[i]);
> +        }
> +#endif
> +#ifdef HAVE_VIF_PLUG_PROVIDER
> +        /* Register external VIF plug provider classes.
> +         *
> +         * Note that we cannot use the ARRAY_SIZE macro here as
> +         * vif_plug_provider_classes is defined in external code which
is not
> +         * available at compile time.  The convention is to use a
> +         * NULL-terminated array instead. */
> +        for (const struct vif_plug_class **pp =
vif_plug_provider_classes;
> +             pp && *pp;
> +             pp++)
> +        {
> +            vif_plug_provider_register(*pp);
> +        }
> +#endif
> +        ovsthread_once_done(&once);
> +    }
> +}
> +
> +static int
> +vif_plug_provider_register__(const struct vif_plug_class *new_class)
> +{
> +    struct vif_plug_class *vif_plug_class;
> +    int error;
> +
> +    if (shash_find(&vif_plug_classes, new_class->type)) {
> +        VLOG_WARN("attempted to register duplicate VIF plug provider:
%s",
> +                  new_class->type);
> +        return EEXIST;
> +    }
> +
> +    error = new_class->init ? new_class->init() : 0;
> +    if (error) {
> +        VLOG_WARN("failed to initialize %s VIF plug provider class: %s",
> +                  new_class->type, ovs_strerror(error));
> +        return error;
> +    }
> +
> +    vif_plug_class = xmalloc(sizeof *vif_plug_class);
> +    memcpy(vif_plug_class, new_class, sizeof *vif_plug_class);
> +
> +    shash_add(&vif_plug_classes, new_class->type, vif_plug_class);
> +
> +    return 0;
> +}
> +
> +/* Register the new VIF plug provider referred to in 'new_class' and
perform
> + * any class level initialization as specified in its vif_plug_class. */
> +int
> +vif_plug_provider_register(const struct vif_plug_class *new_class)
> +{
> +    int error;
> +
> +    ovs_mutex_lock(&vif_plug_classes_mutex);
> +    error = vif_plug_provider_register__(new_class);
> +    ovs_mutex_unlock(&vif_plug_classes_mutex);
> +
> +    return error;
> +}
> +
> +static int
> +vif_plug_provider_unregister__(const char *type)
> +{
> +    int error;
> +    struct shash_node *node;
> +    struct vif_plug_class *vif_plug_class;
> +
> +    node = shash_find(&vif_plug_classes, type);
> +    if (!node) {
> +        return EINVAL;
> +    }
> +
> +    vif_plug_class = node->data;
> +    error = vif_plug_class->destroy ? vif_plug_class->destroy() : 0;
> +    if (error) {
> +        VLOG_WARN("failed to destroy %s VIF plug class: %s",
> +                  vif_plug_class->type, ovs_strerror(error));
> +        return error;
> +    }
> +
> +    shash_delete(&vif_plug_classes, node);
> +    free(vif_plug_class);
> +
> +    return 0;
> +}
> +
> +/* Unregister the VIF plug provider identified by 'type' and perform any
class
> + * level de-initialization as specified in its vif_plug_class. */
> +int
> +vif_plug_provider_unregister(const char *type)
> +{
> +    int error;
> +
> +    ovs_mutex_lock(&vif_plug_classes_mutex);
> +    error = vif_plug_provider_unregister__(type);
> +    ovs_mutex_unlock(&vif_plug_classes_mutex);
> +
> +    return error;
> +}
> +
> +/* Check whether there are any VIF plug providers registered */
> +bool
> +vif_plug_provider_has_providers(void)
> +{
> +    return !shash_is_empty(&vif_plug_classes);
> +}
> +
> +const struct vif_plug_class *
> +vif_plug_provider_get(const char *type)
> +{
> +    struct vif_plug_class *vif_plug_class;
> +
> +    ovs_mutex_lock(&vif_plug_classes_mutex);
> +    vif_plug_class = shash_find_data(&vif_plug_classes, type);
> +    ovs_mutex_unlock(&vif_plug_classes_mutex);
> +
> +    return vif_plug_class;
> +}
> +
> +/* Iterate over VIF plug providers and call their run function.
> + *
> + * Returns 'true' if any of the providers run functions return 'true',
'false'
> + * otherwise.
> + *
> + * A return value of 'true' means that data has changed. */
> +bool
> +vif_plug_provider_run_all(void)
> +{
> +    struct shash_node *node, *next;
> +    bool changed = false;
> +
> +    SHASH_FOR_EACH_SAFE (node, next, &vif_plug_classes) {
> +        struct vif_plug_class *vif_plug_class = node->data;
> +        if (vif_plug_class->run && vif_plug_class->run(vif_plug_class)) {
> +            changed = true;
> +        }
> +    }
> +    return changed;
> +}
> +
> +/* De-initialize and unregister the VIF plug provider classes. */
> +void
> +vif_plug_provider_destroy_all(void)
> +{
> +    struct shash_node *node, *next;
> +
> +    SHASH_FOR_EACH_SAFE (node, next, &vif_plug_classes) {
> +        struct vif_plug_class *vif_plug_class = node->data;
> +        vif_plug_provider_unregister(vif_plug_class->type);
> +    }
> +}
> diff --git a/lib/vif-plug-provider.h b/lib/vif-plug-provider.h
> new file mode 100644
> index 000000000..c0c74b073
> --- /dev/null
> +++ b/lib/vif-plug-provider.h
> @@ -0,0 +1,163 @@
> +/*
> + * Copyright (c) 2021 Canonical
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef VIF_PLUG_PROVIDER_H
> +#define VIF_PLUG_PROVIDER_H 1
> +
> +/* Interface for VIF plug providers.
> + *
> + * A VIF plug provider implementation performs lookup and/or
initialization of
> + * ports, typically representor ports, using generic non-blocking
hardware
> + * interfaces.  This allows the ovn-controller to, upon the CMS's
request,
> + * create ports and interfaces in the chassis's Open vSwitch instances
(also
> + * known as vif plugging).
> + *
> + * This module contains the infrastructure for registering VIF plug
providers
> + * which may be hosted inside or outside the core OVN repository.
> + */
> +
> +#include <stdbool.h>
> +
> +#include "smap.h"
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +struct vif_plug_class;
> +struct ovsdb_idl_txn;
> +struct ovsrec_bridge;
> +
> +enum vif_plug_op_type {
> +    PLUG_OP_CREATE = 1, /* Port is created or updated */
> +    PLUG_OP_REMOVE,     /* Port is removed from this chassis */
> +};
> +
> +struct vif_plug_port_ctx_in {
> +    /* Operation being performed */
> +    enum vif_plug_op_type op_type;
> +
> +    /* These are provided so that the plug implementation may make
decisions
> +     * based on environmental factors such as settings in the
open-vswitch
> +     * table and datapath type settings on the integration bridge. */
> +    const struct ovsrec_open_vswitch_table *ovs_table;
> +    const struct ovsrec_bridge *br_int;
> +
> +    /* Name of logical port, can be useful for plugging library to track
any
> +     * per port resource initialization. */
> +    const char *lport_name;
> +
> +    /* Logical port options, while OVN will forward the contents
verbatim from
> +     * the Southbound database, the convention is for the plugging
library to
> +     * only make decisions based on the vif-plug-* options. */
> +    struct smap lport_options;
> +
> +    /* When OVN knows about an existing interface record associated with
this
> +     * lport, these will be filled in with information about it. */
> +    const char *iface_name;
> +    const char *iface_type;
> +    struct smap iface_options;
> +};
> +
> +struct vif_plug_port_ctx_out {
> +    /* The name to use for port and interface record. */
> +    char *name;
> +
> +    /* Type of interface to create. */
> +    char *type;
> +
> +    /* Options to set on the interface record. */
> +    struct smap iface_options;
> +};
> +
> +struct vif_plug_port_ctx {
> +    const struct vif_plug_class *vif_plug;
> +    struct vif_plug_port_ctx_in vif_plug_port_ctx_in;
> +    struct vif_plug_port_ctx_out vif_plug_port_ctx_out;
> +};
> +
> +struct vif_plug_class {
> +    /* Type of VIF plug provider in this class. */
> +    const char *type;
> +
> +    /* Called when the VIF plug provider is registered, typically at
program
> +     * startup.
> +     *
> +     * This function may be set to null if a VIF plug class needs no
> +     * initialization at registration time. */
> +    int (*init)(void);
> +
> +    /* Called when the VIF plug provider is unregistered, typically at
program
> +     * exit.
> +     *
> +     * This function may be set to null if a plug class needs no
> +     * de-initialization at unregister time.*/
> +    int (*destroy)(void);
> +
> +    /* Performs periodic work needed by VIF plug provider, if any is
necessary.
> +     * Returns 'true; if anything (i.e. lookup tables) changed, 'false'
> +     * otherwise. */
> +    bool (*run)(struct vif_plug_class *);
> +
> +    /* Retrieve Interface options this VIF plug provider will maintain.
This
> +     * sset is used to know which items to remove when maintaining the
database
> +     * record. */
> +    const struct sset * (*vif_plug_get_maintained_iface_options)(void);
> +
> +    /* Pass vif_plug_port_ctx_in to VIF plug provider implementation to
prepare
> +     * for port creation/update.
> +     *
> +     * The VIF plug provider implemantation can perform lookup or any
per port
> +     * initialization and should fill vif_plug_port_ctx_out with data
required
> +     * for port/interface creation.  The VIF plug implementation should
return
> +     * 'true' if it wants the caller to create/update a port/interface,
'false'
> +     * otherwise.
> +     *
> +     * Data in the vif_plug_port_ctx_out struct is owned by the VIF plug
> +     * provider, and a call must be made to the vif_plug_port_ctx_destroy
> +     * callback to free up any allocations when done with port
creation/update.
> +     */
> +    bool (*vif_plug_port_prepare)(const struct vif_plug_port_ctx_in *,
> +                                  struct vif_plug_port_ctx_out *);
> +
> +    /* Notify VIF plug provider that port update is committed to OVSDB.
*/
> +    void (*vif_plug_port_finish)(const struct vif_plug_port_ctx_in *,
> +                                 struct vif_plug_port_ctx_out *);
> +
> +    /* Free any allocations made by the vif_plug_port_prepare callback.
*/
> +    void (*vif_plug_port_ctx_destroy)(const struct vif_plug_port_ctx_in
*,
> +                                      struct vif_plug_port_ctx_out *);
> +};
> +
> +extern const struct vif_plug_class vif_plug_dummy_class;
> +#ifdef HAVE_VIF_PLUG_PROVIDER
> +extern const struct vif_plug_class *vif_plug_provider_classes[];
> +#endif
> +
> +void vif_plug_provider_initialize(void);
> +int vif_plug_provider_register(const struct vif_plug_class *);
> +int vif_plug_provider_unregister(const char *type);
> +bool vif_plug_provider_has_providers(void);
> +const struct vif_plug_class * vif_plug_provider_get(const char *);
> +bool vif_plug_provider_run_all(void);
> +void vif_plug_provider_destroy_all(void);
> +void vif_plug_dummy_enable(void);
> +
> +#ifdef  __cplusplus
> +}
> +#endif
> +
> +#endif /* vif-plug-provider.h */
> diff --git a/lib/vif-plug-providers/dummy/vif-plug-dummy.c
b/lib/vif-plug-providers/dummy/vif-plug-dummy.c
> new file mode 100644
> index 000000000..bc8953e98
> --- /dev/null
> +++ b/lib/vif-plug-providers/dummy/vif-plug-dummy.c
> @@ -0,0 +1,120 @@
> +/*
> + * Copyright (c) 2021 Canonical
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +#include "lib/vif-plug-provider.h"
> +
> +#include <stdint.h>
> +
> +#include "openvswitch/vlog.h"
> +#include "smap.h"
> +#include "sset.h"
> +
> +#ifndef IFNAMSIZ
> +#define IFNAMSIZ 16
> +#endif
> +
> +VLOG_DEFINE_THIS_MODULE(vif_plug_dummy);
> +
> +static struct sset vif_plug_dummy_maintained_iface_options;
> +
> +static int
> +vif_plug_dummy_init(void)
> +{
> +    sset_init(&vif_plug_dummy_maintained_iface_options);
> +    sset_add(&vif_plug_dummy_maintained_iface_options,
> +             "vif-plug-dummy-option");
> +
> +    return 0;
> +}
> +
> +static int
> +vif_plug_dummy_destroy(void)
> +{
> +    sset_destroy(&vif_plug_dummy_maintained_iface_options);
> +
> +    return 0;
> +}
> +
> +static const struct sset*
> +vif_plug_dummy_get_maintained_iface_options(void)
> +{
> +    return &vif_plug_dummy_maintained_iface_options;
> +}
> +
> +static bool
> +vif_plug_dummy_run(struct vif_plug_class *plug)
> +{
> +    VLOG_DBG("vif_plug_dummy_run(%p)", plug);
> +
> +    return false;
> +}
> +
> +static bool
> +vif_plug_dummy_port_prepare(const struct vif_plug_port_ctx_in *ctx_in,
> +                            struct vif_plug_port_ctx_out *ctx_out)
> +{
> +    VLOG_DBG("vif_plug_dummy_port_prepare: %s", ctx_in->lport_name);
> +
> +    if (ctx_in->op_type == PLUG_OP_CREATE) {
> +        size_t lport_name_len = strlen(ctx_in->lport_name);
> +        ctx_out->name = xzalloc(IFNAMSIZ);
> +        memcpy(ctx_out->name, ctx_in->lport_name,
> +               (lport_name_len < IFNAMSIZ) ? lport_name_len : IFNAMSIZ -
1);
> +        ctx_out->type = xstrdup("internal");
> +        smap_init(&ctx_out->iface_options);
> +        smap_add(&ctx_out->iface_options, "vif-plug-dummy-option",
"value");
> +    }
> +
> +    return true;
> +}
> +
> +static void
> +vif_plug_dummy_port_finish(const struct vif_plug_port_ctx_in *ctx_in,
> +                           struct vif_plug_port_ctx_out *ctx_out
OVS_UNUSED)
> +{
> +    VLOG_DBG("vif_plug_dummy_port_finish: %s", ctx_in->lport_name);
> +}
> +
> +static void
> +vif_plug_dummy_port_ctx_destroy(const struct vif_plug_port_ctx_in
*ctx_in,
> +                                struct vif_plug_port_ctx_out *ctx_out)
> +{
> +    VLOG_DBG("vif_plug_dummy_port_ctx_destroy: %s", ctx_in->lport_name);
> +    ovs_assert(ctx_in->op_type == PLUG_OP_CREATE);
> +    free(ctx_out->name);
> +    free(ctx_out->type);
> +    smap_destroy(&ctx_out->iface_options);
> +}
> +
> +const struct vif_plug_class vif_plug_dummy_class = {
> +    .type = "dummy",
> +    .init = vif_plug_dummy_init,
> +    .destroy = vif_plug_dummy_destroy,
> +    .vif_plug_get_maintained_iface_options =
> +        vif_plug_dummy_get_maintained_iface_options,
> +    .run = vif_plug_dummy_run,
> +    .vif_plug_port_prepare = vif_plug_dummy_port_prepare,
> +    .vif_plug_port_finish = vif_plug_dummy_port_finish,
> +    .vif_plug_port_ctx_destroy = vif_plug_dummy_port_ctx_destroy,
> +};
> +
> +void
> +vif_plug_dummy_enable(void)
> +{
> +    vif_plug_provider_register(&vif_plug_dummy_class);
> +}
> +
> diff --git a/ovn-architecture.7.xml b/ovn-architecture.7.xml
> index a71798f68..027b3e858 100644
> --- a/ovn-architecture.7.xml
> +++ b/ovn-architecture.7.xml
> @@ -67,8 +67,9 @@
>      <li>
>        One or more (usually many) <dfn>hypervisors</dfn>.  Hypervisors
must run
>        Open vSwitch and implement the interface described in
> -      <code>Documentation/topics/integration.rst</code> in the OVN
source tree.
> -      Any hypervisor platform supported by Open vSwitch is acceptable.
> +      <code>Documentation/topics/integration.rst</code> in the Open
vSwitch
> +      source tree.  Any hypervisor platform supported by Open vSwitch is
> +      acceptable.
>      </li>
>
>      <li>
> @@ -318,11 +319,19 @@
>
>      <li>
>        On a hypervisor, any VIFs that are to be attached to logical
networks.
> -      The hypervisor itself, or the integration between Open vSwitch and
the
> -      hypervisor (described in
> -      <code>Documentation/topics/integration.rst</code>) takes care of
this.
> -      (This is not part of OVN or new to OVN; this is pre-existing
integration
> -      work that has already been done on hypervisors that support OVS.)
> +      For instances connected through software emulated ports such as
TUN/TAP
> +      or VETH pairs, the hypervisor itself will normally create ports
and plug
> +      them into the integration bridge.  For instances connected through
> +      representor ports, typically used with hardware offload, the
> +      <code>ovn-controller</code> may on CMS direction consult a plugging
> +      provider library for representor port lookup and plug them into the
> +      integration bridge (please refer to
> +      <code>Documentation/topics/plugging-providers.rst</code> for more
> +      information).  In both cases the conventions described in
> +      <code>Documentation/topics/integration.rst</code> in the Open
vSwitch
> +      source tree is followed to ensure mapping between OVN logical port
and
> +      VIF.  (This is pre-existing integration work that has already been
done
> +      on hypervisors that support OVS.)
>      </li>
>
>      <li>
> @@ -921,12 +930,12 @@
>        Eventually, a user powers on the VM that owns the VIF.  On the
hypervisor
>        where the VM is powered on, the integration between the hypervisor
and
>        Open vSwitch (described in
> -      <code>Documentation/topics/integration.rst</code>) adds the VIF to
the OVN
> -      integration bridge and stores <var>vif-id</var> in
> -      <code>external_ids</code>:<code>iface-id</code> to indicate that
the
> -      interface is an instantiation of the new VIF.  (None of this code
is new
> -      in OVN; this is pre-existing integration work that has already
been done
> -      on hypervisors that support OVS.)
> +      <code>Documentation/topics/integration.rst</code> in the Open
vSwitch
> +      source tree) adds the VIF to the OVN integration bridge and stores
> +      <var>vif-id</var> in
<code>external_ids</code>:<code>iface-id</code> to
> +      indicate that the interface is an instantiation of the new VIF.
 (None of
> +      this code is new in OVN; this is pre-existing integration work
that has
> +      already been done on hypervisors that support OVS.)
>      </li>
>
>      <li>
> diff --git a/tests/automake.mk b/tests/automake.mk
> index c4a7c0a5b..685d78c5b 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -38,7 +38,8 @@ TESTSUITE_AT = \
>         tests/ovn-ipam.at \
>         tests/ovn-features.at \
>         tests/ovn-lflow-cache.at \
> -       tests/ovn-ipsec.at
> +       tests/ovn-ipsec.at \
> +       tests/ovn-vif-plug.at
>
>  SYSTEM_KMOD_TESTSUITE_AT = \
>         tests/system-common-macros.at \
> @@ -243,13 +244,23 @@ tests_ovstest_SOURCES = \
>         tests/test-ovn.c \
>         controller/test-lflow-cache.c \
>         controller/test-ofctrl-seqno.c \
> +       controller/test-vif-plug.c \
>         lib/test-ovn-features.c \
>         northd/test-ipam.c
>
>  tests_ovstest_LDADD = $(OVS_LIBDIR)/daemon.lo \
>      $(OVS_LIBDIR)/libopenvswitch.la lib/libovn.la \
> +       controller/binding.$(OBJEXT) \
> +       controller/encaps.$(OBJEXT) \
> +       controller/ha-chassis.$(OBJEXT) \
> +       controller/if-status.$(OBJEXT) \
>         controller/lflow-cache.$(OBJEXT) \
> +       controller/local_data.$(OBJEXT) \
> +       controller/lport.$(OBJEXT) \
>         controller/ofctrl-seqno.$(OBJEXT) \
> +       controller/ovsport.$(OBJEXT) \
> +       controller/patch.$(OBJEXT) \
> +       controller/vif-plug.$(OBJEXT) \
>         northd/ipam.$(OBJEXT)
>
>  # Python tests.
> diff --git a/tests/ovn-vif-plug.at b/tests/ovn-vif-plug.at
> new file mode 100644
> index 000000000..86b0b4b84
> --- /dev/null
> +++ b/tests/ovn-vif-plug.at
> @@ -0,0 +1,8 @@
> +#
> +# Unit tests for the lib/vif-plug-provider.c and controller/vif-plug.c
modules.
> +#
> +AT_BANNER([OVN unit tests - vif-plug])
> +
> +AT_SETUP([unit test -- plugging infrastructure tests])
> +AT_CHECK([ovstest test-plug run], [0], [])
> +AT_CLEANUP
> --
> 2.32.0
>


More information about the dev mailing list