On Thu, Oct 28, 2021 at 8:38 AM Han Zhou <[email protected]> wrote: > > > > On Wed, Oct 27, 2021 at 10:43 PM Han Zhou <[email protected]> wrote: > > > > > > On Tue, Oct 19, 2021 at 3:14 AM Frode Nordahl <[email protected]> > > wrote: > > > > > > New lib/plug-provider module contains the infrastructure for > > > registering plug provider classes which may be hosted inside or > > > outside the core OVN repository. > > > > > > New controller/plug module adds internal interface for interacting > > > with the plug providers. > > > > > > Extend build system to allow enabling building of built-in plugging > > > providers and linking an externally built plugging provider. > > > > > > Signed-off-by: Frode Nordahl <[email protected]> > > > > Hi Frode, > > > > Sorry for the slow review. Thanks for the detailed documentation which > > helps a lot for understanding, but I still have some questions and please > > see my comments below. > > Sorry that I forgot some more comments. Please see below.
Thank you for taking the time to review, much apprechiated. > > > > > --- > > > Documentation/automake.mk | 2 + > > > Documentation/topics/index.rst | 1 + > > > Documentation/topics/plug_providers/index.rst | 32 + > > > .../topics/plug_providers/plug-providers.rst | 196 +++++ > > > acinclude.m4 | 49 ++ > > > configure.ac | 2 + > > > controller/automake.mk | 4 +- > > > controller/plug.c | 670 ++++++++++++++++++ > > > controller/plug.h | 83 +++ > > > controller/test-plug.c | 70 ++ > > > lib/automake.mk | 10 +- > > > lib/plug-provider.c | 204 ++++++ > > > lib/plug-provider.h | 164 +++++ > > > lib/plug_providers/dummy/plug-dummy.c | 119 ++++ > > > ovn-architecture.7.xml | 35 +- > > > tests/automake.mk | 13 +- > > > tests/ovn-plug.at | 8 + > > > 17 files changed, 1646 insertions(+), 16 deletions(-) > > > create mode 100644 Documentation/topics/plug_providers/index.rst > > > create mode 100644 Documentation/topics/plug_providers/plug-providers.rst > > > create mode 100644 controller/plug.c > > > create mode 100644 controller/plug.h > > > create mode 100644 controller/test-plug.c > > > create mode 100644 lib/plug-provider.c > > > create mode 100644 lib/plug-provider.h > > > create mode 100644 lib/plug_providers/dummy/plug-dummy.c > > > create mode 100644 tests/ovn-plug.at > > > > > > diff --git a/Documentation/automake.mk b/Documentation/automake.mk > > > index b3fd3d62b..ff245d218 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/plug_providers/index.rst \ > > > + Documentation/topics/plug_providers/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..12bd113b7 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 > > > + plug_providers/index > > > testing > > > > > > .. list-table:: > > > diff --git a/Documentation/topics/plug_providers/index.rst > > > b/Documentation/topics/plug_providers/index.rst > > > new file mode 100644 > > > index 000000000..837eeae15 > > > --- /dev/null > > > +++ b/Documentation/topics/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. > > > + > > > +============== > > > +Plug Providers > > > +============== > > > + > > > + > > > +.. toctree:: > > > + :maxdepth: 2 > > > + > > > + plug-providers > > > diff --git a/Documentation/topics/plug_providers/plug-providers.rst > > > b/Documentation/topics/plug_providers/plug-providers.rst > > > new file mode 100644 > > > index 000000000..50fbd05f2 > > > --- /dev/null > > > +++ b/Documentation/topics/plug_providers/plug-providers.rst > > > @@ -0,0 +1,196 @@ > > > +.. > > > + 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. > > > + > > > +============== > > > +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 `plug-type` key with a value > > > corresponding > > > + to the `const char *type` provided by the 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 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 and when it > > > encounters > > > + a entry with option `plug-type` and it has registered a plug provider > > > + matching that type it will act on it even if no local binding exists > > > yet. > > > + > > > +4. It will fill the `struct plug_port_ctx_in` as defined in `lib/plug.h` > > > with > > > + `op_type` set to 'PLUG_OP_CREATE' and make a call to the plug > > > providers > > > + `plug_port_prepare` callback function. Plug provider performs lookup > > > and > > > + fills the `struct plug_port_ctx_out` as defined in `lib/plug.h`. > > > + > > > +5. `ovn-controller` creates a port and interface record in the local > > > OVSDB > > > + using the details provided by the 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 > > > `plug-type`. > > > + When the port creation is done a call will first be made to the plug > > > + providers `plug_port_finish` function and then to the > > > + `plug_port_ctx_destroy` function to free any memory allocated by the > > > plug > > > + implementation. > > > + > > > +6. The Open vSwitch vswitchd will assign a ofport to the newly created > > > + interface and on the next `ovn-controller` incremental engine loop > > > iteration > > > + flows will be installed. > > > + > > > +7. On any change to the Southbound Port_Binding record or full > > > recomputation > > > + the `ovn-controller` will in addition to normal flow processing make > > > a call > > > + to the 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 plug provider interface > > > +--------------------------- > > > + > > > +The interface between internals of OVN and a plug provider is a set of > > > +callbacks as defined by the `struct plug_class` in `lib/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, 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 plug provider data or functions before the call to the `init` > > > has > > > +been made. > > > + > > > +On `ovn-controller` exit, the plug providers registered in the above > > > mentioned > > > +procedure will have their `destroy` function pointer called if it is > > > non-NULL. > > > + > > > +If the 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` incremental processing engine 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 > > > +`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 plug > > > provider > > > +`plug_port_prepare` function pointer will be called with valid pointers > > > to > > > +`struct plug_port_ctx_in` and `struct plug_port_ctx_out` data > > > structures. If > > > +the plug provider implementation is able to perform lookup it should > > > fill the > > > +`struct plug_port_ctx_out` data structure and return 'true'. The > > > +`ovn-controller` will then create or update the port/interface records > > > and > > > +then call `plug_port_finish` and `plug_port_ctx_destroy`. If the 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 > > > +signal a full recomputation is necessary, in this case it will also not > > > call > > > +`plug_port_finish`, it will however make a call to > > > `plug_port_ctx_destroy`. > > > + > > > > Two questions here: > > 1. If plug_port_prepare() would return false, when would it allocate memory > > to plug_port_ctx_out? Would it be unnecessary to call > > plug_port_ctx_destroy()? My thought at the moment was that we would not know if the provider implementation had allocated memory before failing, and that a call to its destroy function would be pertinent. But I guess we could make the convention that the plug provider should not allocate memory or clean up after itself before returning false from the prepare function. > > 2. If plug provider is unable to prepare the desired resource at this time, > > how would recomputation help? Recompute is nothing but calling the engine > > node's run() function (instead of the change handler). Looking at the code > > in patch 12, the prepare function is provided by the provider and would be > > called the same way in run() and change handler, so if it fails in one > > situation how would it succeed in another with the same input? What I'm thinking here is that if a plug provider fails to look up / prepare something, chances are its lookup tables are out of sync or something has changed in an inconsistent manner elsewhere. So the thought is that a recompute could make the provider refresh its lookup tables and then the operation can succeed. > > In addition, the name "plug" may be too generic. Would vif-plug be better > > (same for module and folder names)? That suggestion is consistent with the naming of the ovn-org project hosting the plug provider implementations, ovn-vif, so that sounds good to me. > > Thanks, > > Han > > > > > +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 > > > +`plug-type` from `external-ids:ovn-plugged`, fill `struct > > > plug_port_ctx_in` > > > +with `op_type` set to 'PLUG_OP_REMOVE' and make a call to > > > `plug_port_prepare`. > > > +After the port and interface has been removed a call will be made to > > > +`plug_port_finish`. Both calls will be made with the pointer to > > > +`plug_port_ctx_out` set to 'NULL', and no call will be made to > > > +`plug_port_ctx_destroy`. > > > + > > > +Building with in-tree plug providers > > > +------------------------------------ > > > + > > > +Plug providers hosted in the OVN repository living under > > > `lib/plug_providers`: > > > + > > > +To enable them, provide the `--enable-plug-providers` command line > > > option to > > > +the configure script when building OVN. > > > + > > > +Building with an externally provided plug provider > > > +-------------------------------------------------- > > > + > > > +There is also infrastructure in place to support linking OVN with an > > > externally > > > +built plug provider. > > > + > > > +This external plug provider must define a NULL-terminated array of > > > pointers > > > +to `struct plug_class` data structures named `plug_provider_classes`. > > > Example: > > > + > > > +.. code-block:: none > > > + > > > + const struct plug_class *plug_provider_classes[] = { > > > + &plug_foo, > > > + NULL, > > > + }; > > > + > > > +The name of the repository for the external 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 plug provider provide the > > > +`--with-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..793a073d1 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_PLUG_PROVIDER > > > +dnl > > > +dnl Check for external plug provider > > > +AC_DEFUN([OVN_CHECK_PLUG_PROVIDER], [ > > > + AC_ARG_VAR([PLUG_PROVIDER]) > > > + AC_ARG_WITH( > > > + [plug-provider], > > > + [AC_HELP_STRING([--with-plug-provider=/path/to/provider/repository], > > > + [Specify path to a configured and built plug > > > provider repository])], > > > + [if test "$withval" = yes; then > > > + if test -z "$PLUG_PROVIDER"; then > > > + AC_MSG_ERROR([To build with external plug provider, specify the > > > path to a configured and built plug provider repository > > > --with-plug-provider or in \$PLUG_PROVIDER]), > > > + fi > > > + PLUG_PROVIDER="$(realpath $PLUG_PROVIDER)" > > > + else > > > + PLUG_PROVIDER="$(realpath $withval)" > > > + fi > > > + _plug_provider_name="$(basename $PLUG_PROVIDER)" > > > + if test ! -f > > > "$PLUG_PROVIDER/lib/.libs/lib${_plug_provider_name}.la"; then > > > + AC_MSG_ERROR([$withval is not a configured and built plug > > > provider library repository]) > > > + fi > > > + PLUG_PROVIDER_LDFLAGS="-L$PLUG_PROVIDER/lib/.libs > > > -l$_plug_provider_name" > > > + ], > > > + [PLUG_PROVIDER=no]) > > > + AC_MSG_CHECKING([for plug provider]) > > > + AC_MSG_RESULT([$PLUG_PROVIDER]) > > > + AC_SUBST([PLUG_PROVIDER_LDFLAGS]) > > > + AM_CONDITIONAL([HAVE_PLUG_PROVIDER], [test "$PLUG_PROVIDER" != no]) > > > + if test "$PLUG_PROVIDER" != no; then > > > + AC_DEFINE([HAVE_PLUG_PROVIDER], [1], > > > + [Build and link with external plug provider]) > > > + fi > > > +]) > > > + > > > +dnl OVN_ENABLE_PLUG > > > +dnl > > > +dnl Enable built-in plug providers > > > +AC_DEFUN([OVN_ENABLE_PLUG], [ > > > + AC_ARG_ENABLE( > > > + [plug-providers], > > > + [AC_HELP_STRING([--enable-plug-providers], [Enable building of > > > built-in plug providers])], > > > + [], [enable_plug=no]) > > > + AM_CONDITIONAL([ENABLE_PLUG], [test "$enable_plug" != no]) > > > + if test "$enable_plug" != no; then > > > + AC_DEFINE([ENABLE_PLUG], [1], > > > + [Build built-in plug providers]) > > > + fi > > > +]) > > > diff --git a/configure.ac b/configure.ac > > > index d1b9b4d55..715fe6740 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_PLUG_PROVIDER > > > +OVN_ENABLE_PLUG > > > OVS_CTAGS_IDENTIFIERS > > > AC_SUBST([OVS_CFLAGS]) > > > AC_SUBST([OVS_LDFLAGS]) > > > diff --git a/controller/automake.mk b/controller/automake.mk > > > index ad2d68af2..09fbbb1af 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/plug.h \ > > > + controller/plug.c > > > > > > controller_ovn_controller_LDADD = lib/libovn.la > > > $(OVS_LIBDIR)/libopenvswitch.la > > > man_MANS += controller/ovn-controller.8 > > > diff --git a/controller/plug.c b/controller/plug.c > > > new file mode 100644 > > > index 000000000..99c56b7e1 > > > --- /dev/null > > > +++ b/controller/plug.c > > > @@ -0,0 +1,670 @@ > > > +/* > > > + * 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 "plug.h" > > > +#include "plug-provider.h" > > > + > > > +VLOG_DEFINE_THIS_MODULE(plug); > > > + > > > +#define OVN_PLUGGED_EXT_ID "ovn-plugged" > > > + > > > +void > > > +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 * > > > +plug_get_maintained_iface_options(const struct plug_class *plug_class) > > > +{ > > > + return plug_class->plug_get_maintained_iface_options ? > > > + plug_class->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 plug_port_ctx_destroy when done with it. */ > > > +bool > > > +plug_port_prepare(const struct plug_class *plug_class, > > > + const struct plug_port_ctx_in *ctx_in, > > > + struct plug_port_ctx_out *ctx_out) > > > +{ > > > + return plug_class->plug_port_prepare(ctx_in, ctx_out); > > > +} > > > + > > > +/* Notify the plug implementation that a port creation, update or > > > removal has > > > + * been completed */ > > > +void > > > +plug_port_finish(const struct plug_class *plug_class, > > > + const struct plug_port_ctx_in *ctx_in, > > > + struct plug_port_ctx_out *ctx_out) > > > +{ > > > + plug_class->plug_port_finish(ctx_in, ctx_out); > > > +} > > > + > > > +/* Free any data allocated to 'ctx_out' in a prevous call to > > > + * plug_port_prepare. */ > > > +void > > > +plug_port_ctx_destroy(const struct plug_class *plug_class, > > > + const struct plug_port_ctx_in *ctx_in, > > > + struct plug_port_ctx_out *ctx_out) > > > +{ > > > + plug_class->plug_port_ctx_destroy(ctx_in, ctx_out); > > > +} > > > + > > > +static struct plug_port_ctx * > > > +build_port_ctx(const struct plug_class *plug, > > > + const enum plug_op_type op_type, > > > + const struct plug_ctx_in *plug_ctx_in, > > > + const struct sbrec_port_binding *pb, > > > + const struct ovsrec_interface *iface, > > > + const char *iface_id) > > > +{ > > > + struct plug_port_ctx *new_ctx = xzalloc( > > > + sizeof *new_ctx); > > > + > > > + new_ctx->plug = plug; > > > + new_ctx->plug_port_ctx_in.op_type = op_type; > > > + new_ctx->plug_port_ctx_in.ovs_table = plug_ctx_in->ovs_table; > > > + new_ctx->plug_port_ctx_in.br_int = plug_ctx_in->br_int; > > > + new_ctx->plug_port_ctx_in.lport_name = pb ? > > > + xstrdup(pb->logical_port) : iface_id ? xstrdup(iface_id) : NULL; > > > + /* Prepare plug_port_ctx_in smaps for use. > > > + * > > > + * Note that smap_init does not allocate memory. Any memory > > > allocated by > > > + * putting data into the plug_port_ctx_in smaps will be destroyed by > > > calls > > > + * to smap_destroy in destroy_port_ctx */ > > > + smap_init(&new_ctx->plug_port_ctx_in.lport_options); > > > + smap_init(&new_ctx->plug_port_ctx_in.iface_options); > > > + > > > + if (pb) { > > > + smap_clone(&new_ctx->plug_port_ctx_in.lport_options, > > > + &pb->options); > > > + } > > > + > > > + if (iface) { > > > + new_ctx->plug_port_ctx_in.iface_name = xstrdup(iface->name); > > > + new_ctx->plug_port_ctx_in.iface_type = xstrdup(iface->type); > > > + smap_clone(&new_ctx->plug_port_ctx_in.iface_options, > > > + &iface->options); > > > + } > > > + > > > + /* Prepare plug_port_ctx_out smaps for use. > > > + * > > > + * Note that smap_init does not allocate memory. Any memory > > > allocated by > > > + * putting data into the plug_port_ctx_out smaps is the > > > responsibility of > > > + * the plug provider through a call to plug_port_ctx_destroy. */ > > > + smap_init(&new_ctx->plug_port_ctx_out.iface_options); > > > + > > > + return new_ctx; > > > +} > > > + > > > +static void > > > +destroy_port_ctx(struct plug_port_ctx *ctx) > > > +{ > > > + smap_destroy(&ctx->plug_port_ctx_in.lport_options); > > > + smap_destroy(&ctx->plug_port_ctx_in.iface_options); > > > + if (ctx->plug_port_ctx_in.lport_name) { > > > + free((char *)ctx->plug_port_ctx_in.lport_name); > > > + } > > > + if (ctx->plug_port_ctx_in.iface_name) { > > > + free((char *)ctx->plug_port_ctx_in.iface_name); > > > + } > > > + if (ctx->plug_port_ctx_in.iface_type) { > > > + free((char *)ctx->plug_port_ctx_in.iface_type); > > > + } > > > + /* Note that data associated with ctx->plug_port_ctx_out must be > > > destroyed > > > + * by the plug provider implementation with a call to > > > 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. > > > + * > > > + * 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 plug_ctx_in *plug_ctx_in, > > > + const struct plug_ctx_out *plug_ctx_out, > > > + const struct plug_port_ctx *plug_port_ctx, > > > + const struct ovsrec_port *port) > > > +{ > > > + shash_add(plug_ctx_out->deleted_iface_ids, > > > + plug_port_ctx->plug_port_ctx_in.lport_name, > > > + plug_port_ctx); > > > + ovsport_remove(plug_ctx_in->br_int, port); > > > +} > > > + > > > +static void > > > +transact_create_port(const struct plug_ctx_in *plug_ctx_in, > > > + const struct plug_ctx_out *plug_ctx_out, > > > + const struct plug_port_ctx *plug_port_ctx, > > > + const struct smap *iface_external_ids, > > > + const int64_t mtu_request) > > > +{ > > > + shash_add(plug_ctx_out->changed_iface_ids, > > > + plug_port_ctx->plug_port_ctx_in.lport_name, > > > + plug_port_ctx); > > > + ovsport_create(plug_ctx_in->ovs_idl_txn, plug_ctx_in->br_int, > > > + plug_port_ctx->plug_port_ctx_out.name, > > > + plug_port_ctx->plug_port_ctx_out.type, > > > + NULL, iface_external_ids, > > > + &plug_port_ctx->plug_port_ctx_out.iface_options, > > > + mtu_request); > > > +} > > > + > > > +static void > > > +transact_update_port(const struct ovsrec_interface *iface_rec, > > > + const struct plug_ctx_in *plug_ctx_in OVS_UNUSED, > > > + const struct plug_ctx_out *plug_ctx_out, > > > + const struct plug_port_ctx *plug_port_ctx, > > > + const struct smap *iface_external_ids, > > > + const int64_t mtu_request) > > > +{ > > > + shash_add(plug_ctx_out->changed_iface_ids, > > > + plug_port_ctx->plug_port_ctx_in.lport_name, > > > + plug_port_ctx); > > > + ovsport_update_iface(iface_rec, > > > + plug_port_ctx->plug_port_ctx_out.type, > > > + iface_external_ids, > > > + NULL, > > > + &plug_port_ctx->plug_port_ctx_out.iface_options, > > > + plug_get_maintained_iface_options( > > > + plug_port_ctx->plug), > > > + mtu_request); > > > +} > > > + > > > + > > > +static bool > > > +consider_unplug_iface(const struct ovsrec_interface *iface, > > > + const struct sbrec_port_binding *pb, > > > + struct plug_ctx_in *plug_ctx_in, > > > + struct plug_ctx_out *plug_ctx_out) > > > +{ > > > + const char *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( > > > + plug_ctx_in->ovsrec_port_by_interfaces, > > > + (struct ovsrec_interface *) iface); > > > + > > > + if (plug_type && iface_id && port) { > > > + const struct plug_class *plug; > > > + if (!(plug = plug_provider_get(plug_type))) { > > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, > > > 1); > > > + VLOG_WARN_RL(&rl, > > > + "Unable to open plug provider for " > > > + "plug-type %s iface-id %s", > > > + plug_type, iface_id); > > > + /* While we are unable to handle this, asking for a recompute > > > + * will not change that fact. */ > > > + return true; > > > + } > > > + if (!plug_ctx_in->chassis_rec || !plug_ctx_in->br_int > > > + || !plug_ctx_in->ovs_idl_txn) > > > + { > > > + /* Some of our prerequisites are not available, ask for a > > > + * recompute. */ > > > + return false; > > > + } > > > + > > > + /* Our contract with the plug provider is that plug_port_finish > > > + * will be called with a plug_port_ctx_in object once the data > > > + * is actually deleted. > > > + * > > > + * 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 plug_port_ctx *plug_port_ctx = build_port_ctx( > > > + plug, PLUG_OP_REMOVE, plug_ctx_in, pb, iface, iface_id); > > > + > > > + if (!plug_port_prepare(plug, &plug_port_ctx->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 plugging library.", > > > + iface->name, iface_id); > > > + destroy_port_ctx(plug_port_ctx); > > > + return false; > > > + } > > > + VLOG_INFO("Unplugging port %s from %s for iface-id %s on this " > > > + "chassis.", > > > + port->name, > > > + plug_ctx_in->br_int->name, > > > + iface_id); > > > + > > > + /* Add and track delete operation to the transaction */ > > > + transact_delete_port(plug_ctx_in, plug_ctx_out, > > > + 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, "plug-mtu-request", 0); > > > +} > > > + > > > +static bool > > > +consider_plug_lport_create__(const struct plug_class *plug, > > > + const struct smap *iface_external_ids, > > > + const struct sbrec_port_binding *pb, > > > + struct plug_ctx_in *plug_ctx_in, > > > + struct plug_ctx_out *plug_ctx_out) > > > +{ > > > + if (!plug_ctx_in->chassis_rec || !plug_ctx_in->br_int > > > + || !plug_ctx_in->ovs_idl_txn) { > > > + /* Some of our prerequisites are not available, ask for a > > > recompute. */ > > > + return false; > > > + } > > > + > > > + /* Our contract with the plug provider is that plug_port_finish > > > + * will be called with plug_port_ctx_in and plug_port_ctx_out objects > > > + * once the port is actually created. > > > + * > > > + * 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 plug_port_ctx *plug_port_ctx = build_port_ctx( > > > + plug, PLUG_OP_CREATE, plug_ctx_in, pb, NULL, NULL); > > > + > > > + if (!plug_port_prepare(plug, > > > + &plug_port_ctx->plug_port_ctx_in, > > > + &plug_port_ctx->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 plugging " > > > + "library.", > > > + pb->logical_port); > > > + plug_port_ctx_destroy(plug, > > > + &plug_port_ctx->plug_port_ctx_in, > > > + &plug_port_ctx->plug_port_ctx_out); > > > + destroy_port_ctx(plug_port_ctx); > > > + return false; > > > + } > > > + > > > + VLOG_INFO("Plugging port %s into %s for lport %s on this " > > > + "chassis.", > > > + plug_port_ctx->plug_port_ctx_out.name, > > > plug_ctx_in->br_int->name, > > > + pb->logical_port); > > > + transact_create_port(plug_ctx_in, plug_ctx_out, > > > + plug_port_ctx, > > > + iface_external_ids, > > > + get_plug_mtu_request(&pb->options)); > > > + return true; > > > +} > > > + > > > +static bool > > > +consider_plug_lport_update__(const struct plug_class *plug, > > > + const struct smap *iface_external_ids, > > > + const struct sbrec_port_binding *pb, > > > + struct local_binding *lbinding, > > > + struct plug_ctx_in *plug_ctx_in, > > > + struct plug_ctx_out *plug_ctx_out) > > > +{ > > > + if (!plug_ctx_in->chassis_rec || !plug_ctx_in->br_int > > > + || !plug_ctx_in->ovs_idl_txn) { > > > + /* Some of our prerequisites are not available, ask for a > > > recompute. */ > > > + return false; > > > + } > > > + /* Our contract with the plug provider is that plug_port_finish > > > + * will be called with plug_port_ctx_in and plug_port_ctx_out objects > > > + * once the port is actually updated. > > > + * > > > + * 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 plug_port_ctx *plug_port_ctx = build_port_ctx( > > > + plug, PLUG_OP_CREATE, plug_ctx_in, pb, NULL, NULL); > > > + > > > + if (!plug_port_prepare(plug, > > > + &plug_port_ctx->plug_port_ctx_in, > > > + &plug_port_ctx->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 plugging " > > > + "library.", > > > + pb->logical_port); > > > + plug_port_ctx_destroy(plug, > > > + &plug_port_ctx->plug_port_ctx_in, > > > + &plug_port_ctx->plug_port_ctx_out); > > > + destroy_port_ctx(plug_port_ctx); > > > + return false; > > > + } > > > + > > > + if (strcmp(lbinding->iface->name, > > > plug_port_ctx->plug_port_ctx_out.name)) { > > > + VLOG_WARN("Attempt of incompatible change to existing " > > > + "port detected, please recreate port: %s", > > > + pb->logical_port); > > > + plug_port_ctx_destroy(plug, > > > + &plug_port_ctx->plug_port_ctx_in, > > > + &plug_port_ctx->plug_port_ctx_out); > > > + destroy_port_ctx(plug_port_ctx); > > > + return false; > > > + } > > > + VLOG_DBG("updating iface for: %s", pb->logical_port); > > > + transact_update_port(lbinding->iface, plug_ctx_in, plug_ctx_out, > > > + 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 plug_ctx_in *plug_ctx_in, > > > + struct plug_ctx_out *plug_ctx_out) > > > +{ > > > + bool ret = true; > > > + if (lport_can_bind_on_this_chassis(plug_ctx_in->chassis_rec, pb) > > > + && pb->requested_chassis == plug_ctx_in->chassis_rec) { > > > + const char *plug_type = smap_get(&pb->options, "plug-type"); > > > + if (!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 plug_class *plug; > > > + if (!(plug = plug_provider_get(plug_type))) { > > > + VLOG_WARN_RL(&rl, > > > + "Unable to open plug provider for plug-type: > > > '%s' " > > > + "lport %s", > > > + 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, 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__(plug, > > > &iface_external_ids, pb, > > > + lbinding, plug_ctx_in, > > > + plug_ctx_out); > > > + } else { > > > + ret = consider_plug_lport_create__(plug, > > > &iface_external_ids, pb, > > > + plug_ctx_in, > > > plug_ctx_out); > > > + } > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +static bool > > > +plug_iface_touched_this_txn(const struct plug_ctx_out *plug_ctx_out, > > > + const char *iface_id) > > > +{ > > > + return shash_find(plug_ctx_out->changed_iface_ids, iface_id) > > > + || shash_find(plug_ctx_out->deleted_iface_ids, iface_id); > > > +} > > > + > > > +static bool > > > +plug_handle_lport_vif(const struct sbrec_port_binding *pb, > > > + struct plug_ctx_in *plug_ctx_in, > > > + struct plug_ctx_out *plug_ctx_out) > > > +{ > > > + if (plug_iface_touched_this_txn(plug_ctx_out, pb->logical_port)) { > > > + return true; > > > + } > > > + bool handled = true; > > > + struct local_binding *lbinding = local_binding_find( > > > + plug_ctx_in->local_bindings, pb->logical_port); > > > + > > > + if (lport_can_bind_on_this_chassis(plug_ctx_in->chassis_rec, pb)) { > > > + handled &= consider_plug_lport(pb, lbinding, > > > + plug_ctx_in, plug_ctx_out); > > > + } else if (lbinding && lbinding->iface) { > > > + handled &= consider_unplug_iface(lbinding->iface, pb, > > > + plug_ctx_in, plug_ctx_out); > > > + } > > > + return handled; > > > +} > > > + > > > +static bool > > > +plug_handle_iface(const struct ovsrec_interface *iface_rec, > > > + struct plug_ctx_in *plug_ctx_in, > > > + struct plug_ctx_out *plug_ctx_out) > > > +{ > > > + bool handled = true; > > > + const char *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 (!plug_type || !iface_id > > > + || plug_iface_touched_this_txn(plug_ctx_out, iface_id)) { > > > + return true; > > > + } > > > + struct local_binding *lbinding = local_binding_find( > > > + plug_ctx_in->local_bindings, iface_id); > > > + const struct sbrec_port_binding *pb = lport_lookup_by_name( > > > + plug_ctx_in->sbrec_port_binding_by_name, iface_id); > > > + if (pb && lbinding > > > + && lport_can_bind_on_this_chassis(plug_ctx_in->chassis_rec, pb)) > > > { > > > + /* Something changed on a interface we have previously plugged, > > > + * consider updating it */ > > > + handled &= consider_plug_lport(pb, lbinding, > > > + plug_ctx_in, plug_ctx_out); > > > + } else if (!pb > > > + || !lport_can_bind_on_this_chassis( > > > + 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, > > > + plug_ctx_in, plug_ctx_out); > > > + } > > > + return handled; > > > +} > > > + > > > +void > > > +plug_run(struct plug_ctx_in *plug_ctx_in, > > > + struct plug_ctx_out *plug_ctx_out) > > > +{ > > > + if (!plug_ctx_in->pb_handler_has_run) { > > > + /* Avoid uneccessary unplug/plug thrashing. If we are starting > > > up > > > + * after having done a normal exit (unregistered our chassis > > > record), > > > + * we have to wait until northd has reinstated the > > > requested_chassis > > > + * pointer before table iterators and index lookups provide the > > > + * expected data */ > > I don't really understand this check. I understand that it is undesirable to > unplug all VIFs at startup before all the SB port-bindings are ready and > populated in the IDL, and later replug them. However, I don't see how this > flag would help. This flag only tells that there has been at least one > execution of the SB port binding change handler, but what would that > guarantee? The change handler itself could unplug the VIFs, too, right? It does resolve the unplug/plug thrashing on startup in my testing, but I agree with you that it is no guarantee and that the pb handler could be called before northd has reinstated the requested_chassis pointers. The other alternative I can think of is to compare NB_Global:sb_cfg_timestamp to the controllers knowledge of when it registered its chassis record, which should be a good indication of northd having run its loop. Would that work for you? > > > + return; > > > + } > > > + const struct ovsrec_interface *iface_rec; > > > + OVSREC_INTERFACE_TABLE_FOR_EACH (iface_rec, > > > + plug_ctx_in->iface_table) { > > > + plug_handle_iface(iface_rec, plug_ctx_in, plug_ctx_out); > > > + } > > > + const struct sbrec_port_binding *pb; > > > + SBREC_PORT_BINDING_TABLE_FOR_EACH (pb, > > > + plug_ctx_in->port_binding_table) { > > This loop can be costly in a large scale environment. Can we consider only > the port_bindings with requested-chassis equal to the current chassis using > OVSDB index? Since the previous loop in this function already considers all > interfaces on this chassis, unplugging a VIF that doesn't have port-binding > should have already been handled. Yes, we should definitively use an index here, thank you for pointing that out. > > > + enum en_lport_type lport_type = get_lport_type(pb); > > > + if (lport_type == LP_VIF) { > > > + plug_handle_lport_vif(pb, plug_ctx_in, plug_ctx_out); > > > + } > > > + } > > > +} > > > + > > > +bool > > > +plug_handle_port_binding_changes(struct plug_ctx_in *plug_ctx_in, > > > + struct plug_ctx_out *plug_ctx_out) > > > +{ > > > + const struct sbrec_port_binding *pb; > > > + bool handled = true; > > > + > > > + /* handle deleted lports */ > > > + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED ( > > > + pb, > > > + plug_ctx_in->port_binding_table) { > > > + if (!sbrec_port_binding_is_deleted(pb)) { > > > + continue; > > > + } > > > + > > > + enum en_lport_type lport_type = get_lport_type(pb); > > > + if (lport_type == LP_VIF) { > > > + struct local_binding *lbinding = local_binding_find( > > > + plug_ctx_in->local_bindings, pb->logical_port); > > > + if (lbinding && lbinding->iface > > > + && !plug_iface_touched_this_txn(plug_ctx_out, > > > + pb->logical_port)) { > > > + handled &= consider_unplug_iface(lbinding->iface, pb, > > > + plug_ctx_in, > > > plug_ctx_out); > > > + } > > > + } > > > + } > > > + > > > + /* handle any new or updated lports */ > > > + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED ( > > > + pb, > > > + plug_ctx_in->port_binding_table) { > > > + if (sbrec_port_binding_is_deleted(pb)) { > > > + continue; > > > + } > > > + enum en_lport_type lport_type = get_lport_type(pb); > > > + if (lport_type == LP_VIF) { > > > + handled &= plug_handle_lport_vif(pb, plug_ctx_in, > > > plug_ctx_out); > > > + } > > > + } > > > + > > > + return handled; > > > +} > > > + > > > +bool > > > +plug_handle_ovs_interface_changes(struct plug_ctx_in *plug_ctx_in, > > > + struct plug_ctx_out *plug_ctx_out) > > > +{ > > > + bool handled = true; > > > + const struct ovsrec_interface *iface_rec; > > > + > > > + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, > > > + plug_ctx_in->iface_table) { > > > + if (ovsrec_interface_is_deleted(iface_rec)) { > > Why would this be ignored? If a VIF is deleted by mistake, and if it is > supposed to be plugged on this chassis, we should replug it, right? Consider > if the run() function was triggered instead of this change handler, it would > be replugged, right? The change handlers in I-P are supposed to generate same > result as if the recompute happens (although more efficient). You're right, I'll revisit. I think I conflated this with interfaces we have deleted ourselves, for which re-processing would cause issues. -- Frode Nordahl > Thanks, > Han > > > > + continue; > > > + } > > > + handled &= plug_handle_iface(iface_rec, plug_ctx_in, > > > plug_ctx_out); > > > + } > > > + return handled; > > > +} > > > + > > > +static void > > > +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 plug_port_ctx *plug_port_ctx = node->data; > > > + if (txn_success) { > > > + plug_port_finish(plug_port_ctx->plug, > > > + &plug_port_ctx->plug_port_ctx_in, > > > + NULL); > > > + } > > > + shash_delete(deleted_iface_ids, node); > > > + destroy_port_ctx(plug_port_ctx); > > > + } > > > +} > > > + > > > +void > > > +plug_clear_deleted(struct shash *deleted_iface_ids) { > > > + plug_finish_deleted__(deleted_iface_ids, false); > > > +} > > > + > > > +void > > > +plug_finish_deleted(struct shash *deleted_iface_ids) { > > > + plug_finish_deleted__(deleted_iface_ids, true); > > > +} > > > + > > > +static void > > > +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 plug_port_ctx *plug_port_ctx = node->data; > > > + if (txn_success) { > > > + plug_port_finish(plug_port_ctx->plug, > > > + &plug_port_ctx->plug_port_ctx_in, > > > + &plug_port_ctx->plug_port_ctx_out); > > > + } > > > + plug_port_ctx_destroy(plug_port_ctx->plug, > > > + &plug_port_ctx->plug_port_ctx_in, > > > + &plug_port_ctx->plug_port_ctx_out); > > > + shash_delete(changed_iface_ids, node); > > > + destroy_port_ctx(plug_port_ctx); > > > + } > > > +} > > > + > > > +void > > > +plug_clear_changed(struct shash *deleted_iface_ids) { > > > + plug_finish_changed__(deleted_iface_ids, false); > > > +} > > > + > > > +void > > > +plug_finish_changed(struct shash *deleted_iface_ids) { > > > + plug_finish_changed__(deleted_iface_ids, true); > > > +} > > > diff --git a/controller/plug.h b/controller/plug.h > > > new file mode 100644 > > > index 000000000..5647a25ac > > > --- /dev/null > > > +++ b/controller/plug.h > > > @@ -0,0 +1,83 @@ > > > +/* > > > + * 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 PLUG_H > > > +#define PLUG_H 1 > > > + > > > +/* > > > + * Plug, the controller internal interface to the plug provider > > > infrastructure. > > > + */ > > > + > > > +#include "openvswitch/shash.h" > > > +#include "smap.h" > > > + > > > +#ifdef __cplusplus > > > +extern "C" { > > > +#endif > > > + > > > +struct plug_ctx_in { > > > + struct ovsdb_idl_txn *ovs_idl_txn; > > > + struct ovsdb_idl_index *sbrec_port_binding_by_name; > > > + 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 sbrec_port_binding_table *port_binding_table; > > > + const struct shash *local_bindings; > > > + bool pb_handler_has_run; > > > +}; > > > + > > > +struct plug_ctx_out { > > > + struct shash *deleted_iface_ids; > > > + struct shash *changed_iface_ids; > > > +}; > > > + > > > +struct plug_class; > > > +struct plug_port_ctx_out; > > > +struct plug_port_ctx_in; > > > + > > > +const struct sset * plug_get_maintained_iface_options( > > > + const struct plug_class *plug_class); > > > + > > > +bool plug_port_prepare(const struct plug_class *, > > > + const struct plug_port_ctx_in *, > > > + struct plug_port_ctx_out *); > > > +void plug_port_finish(const struct plug_class *, > > > + const struct plug_port_ctx_in *, > > > + struct plug_port_ctx_out *); > > > +void plug_port_ctx_destroy(const struct plug_class *, > > > + const struct plug_port_ctx_in *, > > > + struct plug_port_ctx_out *); > > > + > > > +struct ovsdb_idl; > > > + > > > +void plug_register_ovs_idl(struct ovsdb_idl *ovs_idl); > > > +void plug_run(struct plug_ctx_in *, struct plug_ctx_out *); > > > +bool plug_handle_port_binding_changes(struct plug_ctx_in *, > > > + struct plug_ctx_out *); > > > +bool plug_handle_ovs_interface_changes(struct plug_ctx_in *, > > > + struct plug_ctx_out *); > > > +void plug_clear_changed(struct shash *deleted_iface_ids); > > > +void plug_finish_changed(struct shash *changed_iface_ids); > > > +void plug_clear_deleted(struct shash *deleted_iface_ids); > > > +void plug_finish_deleted(struct shash *changed_iface_ids); > > > + > > > +#ifdef __cplusplus > > > +} > > > +#endif > > > + > > > +#endif /* plug.h */ > > > diff --git a/controller/test-plug.c b/controller/test-plug.c > > > new file mode 100644 > > > index 000000000..683aa7df2 > > > --- /dev/null > > > +++ b/controller/test-plug.c > > > @@ -0,0 +1,70 @@ > > > +/* 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 "plug.h" > > > +#include "plug-provider.h" > > > +#include "smap.h" > > > +#include "sset.h" > > > +#include "tests/ovstest.h" > > > + > > > +static void > > > +test_plug(struct ovs_cmdl_context *ctx OVS_UNUSED) > > > +{ > > > + const struct plug_class *plug_class; > > > + > > > + ovs_assert(plug_provider_unregister("dummy") == EINVAL); > > > + > > > + ovs_assert(!plug_provider_register(&plug_dummy_class)); > > > + plug_class = plug_provider_get("dummy"); > > > + ovs_assert(plug_provider_register(&plug_dummy_class) == EEXIST); > > > + > > > + > > > ovs_assert(sset_contains(plug_get_maintained_iface_options(plug_class), > > > + "plug-dummy-option")); > > > + > > > + struct plug_port_ctx_in ctx_in = { > > > + .op_type = PLUG_OP_CREATE, > > > + .lport_name = "lsp1", > > > + .lport_options = SMAP_INITIALIZER(&ctx_in.lport_options), > > > + }; > > > + struct plug_port_ctx_out ctx_out; > > > + plug_port_prepare(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, "plug-dummy-option"), "value")); > > > + > > > + plug_port_finish(plug_class, &ctx_in, &ctx_out); > > > + plug_port_ctx_destroy(plug_class, &ctx_in, &ctx_out); > > > + plug_provider_destroy_all(); > > > +} > > > + > > > +static void > > > +test_plug_main(int argc, char *argv[]) > > > +{ > > > + set_program_name(argv[0]); > > > + static const struct ovs_cmdl_command commands[] = { > > > + {"run", NULL, 0, 0, test_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-plug", test_plug_main); > > > diff --git a/lib/automake.mk b/lib/automake.mk > > > index 9f9f447d5..0c320c6f9 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_PLUG_PROVIDER > > > +lib_libovn_la_LDFLAGS += $(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/plug-provider.h \ > > > + lib/plug-provider.c \ > > > + lib/plug_providers/dummy/plug-dummy.c > > > nodist_lib_libovn_la_SOURCES = \ > > > lib/ovn-dirs.c \ > > > lib/ovn-nb-idl.c \ > > > diff --git a/lib/plug-provider.c b/lib/plug-provider.c > > > new file mode 100644 > > > index 000000000..e7e463423 > > > --- /dev/null > > > +++ b/lib/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 "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(plug_provider); > > > + > > > +#ifdef ENABLE_PLUG > > > +static const struct plug_class *base_plug_classes[] = { > > > +}; > > > +#endif > > > + > > > +static struct shash plug_classes = SHASH_INITIALIZER(&plug_classes); > > > + > > > +/* Protects the 'plug_classes' shash. */ > > > +static struct ovs_mutex plug_classes_mutex = OVS_MUTEX_INITIALIZER; > > > + > > > +/* Initialize the the plug infrastructure by registering known plug > > > classes */ > > > +void > > > +plug_provider_initialize(void) > > > +{ > > > + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; > > > + > > > + if (ovsthread_once_start(&once)) { > > > +#ifdef ENABLE_PLUG > > > + /* Register built-in plug provider classes */ > > > + for (int i = 0; i < ARRAY_SIZE(base_plug_classes); i++) { > > > + plug_provider_register(base_plug_classes[i]); > > > + } > > > +#endif > > > +#ifdef HAVE_PLUG_PROVIDER > > > + /* Register external plug provider classes. > > > + * > > > + * Note that we cannot use the ARRAY_SIZE macro here as > > > + * 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 plug_class **pp = plug_provider_classes; > > > + pp && *pp; > > > + pp++) > > > + { > > > + plug_provider_register(*pp); > > > + } > > > +#endif > > > + ovsthread_once_done(&once); > > > + } > > > +} > > > + > > > +static int > > > +plug_provider_register__(const struct plug_class *new_class) > > > +{ > > > + struct plug_class *plug_class; > > > + int error; > > > + > > > + if (shash_find(&plug_classes, new_class->type)) { > > > + VLOG_WARN("attempted to register duplicate plug provider: %s", > > > + new_class->type); > > > + return EEXIST; > > > + } > > > + > > > + error = new_class->init ? new_class->init() : 0; > > > + if (error) { > > > + VLOG_WARN("failed to initialize %s plug class: %s", > > > + new_class->type, ovs_strerror(error)); > > > + return error; > > > + } > > > + > > > + plug_class = xmalloc(sizeof *plug_class); > > > + memcpy(plug_class, new_class, sizeof *plug_class); > > > + > > > + shash_add(&plug_classes, new_class->type, plug_class); > > > + > > > + return 0; > > > +} > > > + > > > +/* Register the new plug provider referred to in 'new_class' and perform > > > any > > > + * class level initialization as specified in its plug_class. */ > > > +int > > > +plug_provider_register(const struct plug_class *new_class) > > > +{ > > > + int error; > > > + > > > + ovs_mutex_lock(&plug_classes_mutex); > > > + error = plug_provider_register__(new_class); > > > + ovs_mutex_unlock(&plug_classes_mutex); > > > + > > > + return error; > > > +} > > > + > > > +static int > > > +plug_provider_unregister__(const char *type) > > > +{ > > > + int error; > > > + struct shash_node *node; > > > + struct plug_class *plug_class; > > > + > > > + node = shash_find(&plug_classes, type); > > > + if (!node) { > > > + return EINVAL; > > > + } > > > + > > > + plug_class = node->data; > > > + error = plug_class->destroy ? plug_class->destroy() : 0; > > > + if (error) { > > > + VLOG_WARN("failed to destroy %s plug class: %s", > > > + plug_class->type, ovs_strerror(error)); > > > + return error; > > > + } > > > + > > > + shash_delete(&plug_classes, node); > > > + free(plug_class); > > > + > > > + return 0; > > > +} > > > + > > > +/* Unregister the plug provider identified by 'type' and perform any > > > class > > > + * level de-initialization as specified in its plug_class. */ > > > +int > > > +plug_provider_unregister(const char *type) > > > +{ > > > + int error; > > > + > > > + ovs_mutex_lock(&plug_classes_mutex); > > > + error = plug_provider_unregister__(type); > > > + ovs_mutex_unlock(&plug_classes_mutex); > > > + > > > + return error; > > > +} > > > + > > > +/* Check whether there are any plug providers registered */ > > > +bool > > > +plug_provider_has_providers(void) > > > +{ > > > + return !shash_is_empty(&plug_classes); > > > +} > > > + > > > +const struct plug_class * > > > +plug_provider_get(const char *type) > > > +{ > > > + struct plug_class *plug_class; > > > + > > > + ovs_mutex_lock(&plug_classes_mutex); > > > + plug_class = shash_find_data(&plug_classes, type); > > > + ovs_mutex_unlock(&plug_classes_mutex); > > > + > > > + return plug_class; > > > +} > > > + > > > +/* Iterate over 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 > > > +plug_provider_run_all(void) > > > +{ > > > + struct shash_node *node, *next; > > > + bool changed = false; > > > + > > > + SHASH_FOR_EACH_SAFE (node, next, &plug_classes) { > > > + struct plug_class *plug_class = node->data; > > > + if (plug_class->run && plug_class->run(plug_class)) { > > > + changed = true; > > > + } > > > + } > > > + return changed; > > > +} > > > + > > > +/* De-initialize and unregister the plug provider classes. */ > > > +void > > > +plug_provider_destroy_all(void) > > > +{ > > > + struct shash_node *node, *next; > > > + > > > + SHASH_FOR_EACH_SAFE (node, next, &plug_classes) { > > > + struct plug_class *plug_class = node->data; > > > + plug_provider_unregister(plug_class->type); > > > + } > > > +} > > > diff --git a/lib/plug-provider.h b/lib/plug-provider.h > > > new file mode 100644 > > > index 000000000..a5ed22684 > > > --- /dev/null > > > +++ b/lib/plug-provider.h > > > @@ -0,0 +1,164 @@ > > > +/* > > > + * 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 PLUG_PROVIDER_H > > > +#define PLUG_PROVIDER_H 1 > > > + > > > +/* Interface for plug providers. > > > + * > > > + * A 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 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 plug_class; > > > +struct ovsdb_idl_txn; > > > +struct ovsrec_bridge; > > > + > > > +enum plug_op_type { > > > + PLUG_OP_CREATE = 1, /* Port is created or updated */ > > > + PLUG_OP_REMOVE, /* Port is removed from this chassis */ > > > +}; > > > + > > > +struct plug_port_ctx_in { > > > + /* Operation being performed */ > > > + enum 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 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 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 plug_port_ctx { > > > + const struct plug_class *plug; > > > + struct plug_port_ctx_in plug_port_ctx_in; > > > + struct plug_port_ctx_out plug_port_ctx_out; > > > +}; > > > + > > > +struct plug_class { > > > + /* Type of plugger in this class. */ > > > + const char *type; > > > + > > > + /* Called when the plug provider is registered, typically at program > > > + * startup. > > > + * > > > + * This function may be set to null if a plug class needs no > > > + * initialization at registration time. */ > > > + int (*init)(void); > > > + > > > + /* Called when the 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 plugger, if any is necessary. > > > Returns > > > + * 'true; if anything (i.e. lookup tables) changed, 'false' > > > otherwise. > > > + * > > > + * A return value of 'true' will cause further processing in the > > > + * incremental processing engine, a return value of 'false' will set > > > the > > > + * plug_provider_lookup node as unchanged. */ > > > + bool (*run)(struct plug_class *); > > > + > > > + /* Retrieve Interface options this plugger will maintain. This set > > > is used > > > + * to know which items to remove when maintaining the database > > > record. */ > > > + const struct sset * (*plug_get_maintained_iface_options)(void); > > > + > > > + /* Pass plug_port_ctx_in to plug implementation to prepare for port > > > + * creation/update. > > > + * > > > + * The plug implemantation can perform lookup or any per port > > > + * initialization and should fill plug_port_ctx_out with data > > > required for > > > + * port/interface creation. The plug implementation should return > > > true if > > > + * it wants the caller to create/update a port/interface, false > > > otherwise. > > > + * > > > + * Data in the plug_port_ctx_out struct is owned by the plugging > > > library, > > > + * and a call must be made to the plug_port_ctx_destroy callback to > > > free > > > + * up any allocations when done with port creation/update. > > > + */ > > > + bool (*plug_port_prepare)(const struct plug_port_ctx_in *, > > > + struct plug_port_ctx_out *); > > > + > > > + /* Notify plugging library that port update is done. */ > > > + void (*plug_port_finish)(const struct plug_port_ctx_in *, > > > + struct plug_port_ctx_out *); > > > + > > > + /* Free any allocations made by the plug_port_prepare callback. */ > > > + void (*plug_port_ctx_destroy)(const struct plug_port_ctx_in *, > > > + struct plug_port_ctx_out *); > > > +}; > > > + > > > +extern const struct plug_class plug_dummy_class; > > > +#ifdef HAVE_PLUG_PROVIDER > > > +extern const struct plug_class *plug_provider_classes[]; > > > +#endif > > > + > > > +void plug_provider_initialize(void); > > > +int plug_provider_register(const struct plug_class *); > > > +int plug_provider_unregister(const char *type); > > > +bool plug_provider_has_providers(void); > > > +const struct plug_class * plug_provider_get(const char *); > > > +bool plug_provider_run_all(void); > > > +void plug_provider_destroy_all(void); > > > +void plug_dummy_enable(void); > > > + > > > +#ifdef __cplusplus > > > +} > > > +#endif > > > + > > > +#endif /* plug-provider.h */ > > > diff --git a/lib/plug_providers/dummy/plug-dummy.c > > > b/lib/plug_providers/dummy/plug-dummy.c > > > new file mode 100644 > > > index 000000000..0909bab46 > > > --- /dev/null > > > +++ b/lib/plug_providers/dummy/plug-dummy.c > > > @@ -0,0 +1,119 @@ > > > +/* > > > + * 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/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(plug_dummy); > > > + > > > +static struct sset plug_dummy_maintained_iface_options; > > > + > > > +static int > > > +plug_dummy_init(void) > > > +{ > > > + sset_init(&plug_dummy_maintained_iface_options); > > > + sset_add(&plug_dummy_maintained_iface_options, "plug-dummy-option"); > > > + > > > + return 0; > > > +} > > > + > > > +static int > > > +plug_dummy_destroy(void) > > > +{ > > > + sset_destroy(&plug_dummy_maintained_iface_options); > > > + > > > + return 0; > > > +} > > > + > > > +static const struct sset* > > > +plug_dummy_get_maintained_iface_options(void) > > > +{ > > > + return &plug_dummy_maintained_iface_options; > > > +} > > > + > > > +static bool > > > +plug_dummy_run(struct plug_class *plug) > > > +{ > > > + VLOG_DBG("plug_dummy_run(%p)", plug); > > > + > > > + return false; > > > +} > > > + > > > +static bool > > > +plug_dummy_port_prepare(const struct plug_port_ctx_in *ctx_in, > > > + struct plug_port_ctx_out *ctx_out) > > > +{ > > > + VLOG_DBG("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, "plug-dummy-option", "value"); > > > + } > > > + > > > + return true; > > > +} > > > + > > > +static void > > > +plug_dummy_port_finish(const struct plug_port_ctx_in *ctx_in, > > > + struct plug_port_ctx_out *ctx_out OVS_UNUSED) > > > +{ > > > + VLOG_DBG("plug_dummy_port_finish: %s", ctx_in->lport_name); > > > +} > > > + > > > +static void > > > +plug_dummy_port_ctx_destroy(const struct plug_port_ctx_in *ctx_in, > > > + struct plug_port_ctx_out *ctx_out) > > > +{ > > > + VLOG_DBG("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 plug_class plug_dummy_class = { > > > + .type = "dummy", > > > + .init = plug_dummy_init, > > > + .destroy = plug_dummy_destroy, > > > + .plug_get_maintained_iface_options = > > > + plug_dummy_get_maintained_iface_options, > > > + .run = plug_dummy_run, > > > + .plug_port_prepare = plug_dummy_port_prepare, > > > + .plug_port_finish = plug_dummy_port_finish, > > > + .plug_port_ctx_destroy = plug_dummy_port_ctx_destroy, > > > +}; > > > + > > > +void > > > +plug_dummy_enable(void) > > > +{ > > > + plug_provider_register(&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..982db4984 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-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-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/plug.$(OBJEXT) \ > > > northd/ipam.$(OBJEXT) > > > > > > # Python tests. > > > diff --git a/tests/ovn-plug.at b/tests/ovn-plug.at > > > new file mode 100644 > > > index 000000000..d5c6a1b6d > > > --- /dev/null > > > +++ b/tests/ovn-plug.at > > > @@ -0,0 +1,8 @@ > > > +# > > > +# Unit tests for the lib/plug.c module. > > > +# > > > +AT_BANNER([OVN unit tests - plug]) > > > + > > > +AT_SETUP([unit test -- plugging infrastructure tests]) > > > +AT_CHECK([ovstest test-plug run], [0], []) > > > +AT_CLEANUP > > > -- > > > 2.32.0 > > > > > > _______________________________________________ > > > dev mailing list > > > [email protected] > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
