[ovs-dev] [PATCH ovn v3 5/7] lib: Add infrastructure for plugging providers.

Frode Nordahl frode.nordahl at canonical.com
Fri Aug 27 21:37:11 UTC 2021


fre. 27. aug. 2021, 22:01 skrev Han Zhou <zhouhan at gmail.com>:

>
>
> On Fri, Aug 27, 2021 at 1:47 AM Frode Nordahl <frode.nordahl at canonical.com>
> wrote:
> >
> > On Fri, Aug 27, 2021 at 9:29 AM Han Zhou <zhouhan at gmail.com> wrote:
> > >
> > >
> > >
> > > On Thu, Aug 19, 2021 at 4:09 AM Frode Nordahl <
> frode.nordahl at canonical.com> wrote:
> > > >
> > > > New module contains the infrastructure for registering and
> > > > instantiating plugging classes which may be hosted inside or
> > > > outside the core OVN repository.  The data structures and functions
> > > > for interacting with these plugging classes also live there.
> > > >
> > > > Extend build system to allow linking a externally built plugging
> > > > provider.
> > > >
> > >
> > > Thanks Frode. I think documentation is missing for this plugging
> infrastructure. I'd suggest to add a section in ovn-architecture to briefly
> describe the feature, and then a specific document in Documentation/topics
> (or Documentation/tutorials) for the details of how the infrastructure
> works, what are the default providers and how to add a new one, etc.
> >
> > Yes documentation is indeed something I have forgotten to add to the
> > patch set. Thank you for your suggestions as to where to place it,
> > will follow up on that.
> >
> > > Please see below for some more comments.
> > >
> > > > Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
> > > > ---
> > > >  acinclude.m4        |  34 +++++
> > > >  configure.ac        |   1 +
> > > >  lib/automake.mk     |  12 +-
> > > >  lib/plug-dummy.c    | 144 +++++++++++++++++++
> > > >  lib/plug-dummy.h    |  33 +++++
> > > >  lib/plug-provider.h | 109 ++++++++++++++
> > > >  lib/plug.c          | 342
> ++++++++++++++++++++++++++++++++++++++++++++
> > > >  lib/plug.h          | 101 +++++++++++++
> > > >  lib/test-plug.c     |  80 +++++++++++
> > > >  tests/automake.mk   |   4 +-
> > > >  tests/ovn-plug.at   |   8 ++
> > > >  11 files changed, 866 insertions(+), 2 deletions(-)
> > > >  create mode 100644 lib/plug-dummy.c
> > > >  create mode 100644 lib/plug-dummy.h
> > > >  create mode 100644 lib/plug-provider.h
> > > >  create mode 100644 lib/plug.c
> > > >  create mode 100644 lib/plug.h
> > > >  create mode 100644 lib/test-plug.c
> > > >  create mode 100644 tests/ovn-plug.at
> > > >
> > > > diff --git a/acinclude.m4 b/acinclude.m4
> > > > index e7f829520..ba7a01c49 100644
> > > > --- a/acinclude.m4
> > > > +++ b/acinclude.m4
> > > > @@ -441,3 +441,37 @@ 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 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
> > > > +])
> > > > diff --git a/configure.ac b/configure.ac
> > > > index df0b98295..73e3bfe8c 100644
> > > > --- a/configure.ac
> > > > +++ b/configure.ac
> > > > @@ -172,6 +172,7 @@ OVS_ENABLE_SPARSE
> > > >  OVS_CHECK_DDLOG([0.38])
> > > >  OVS_CHECK_PRAGMA_MESSAGE
> > > >  OVN_CHECK_OVS
> > > > +OVN_CHECK_PLUG_PROVIDER
> > > >  OVS_CTAGS_IDENTIFIERS
> > > >  AC_SUBST([OVS_CFLAGS])
> > > >  AC_SUBST([OVS_LDFLAGS])
> > > > diff --git a/lib/automake.mk b/lib/automake.mk
> > > > index ddfe33948..086fbd62d 100644
> > > > --- a/lib/automake.mk
> > > > +++ b/lib/automake.mk
> > > > @@ -3,6 +3,11 @@ lib_libovn_la_LDFLAGS = \
> > > >          $(OVS_LTINFO) \
> > > >          -Wl,--version-script=$(top_builddir)/lib/libovn.sym \
> > > >          $(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 \
> > > > @@ -32,7 +37,12 @@ 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.h \
> > > > +       lib/plug.c \
> > > > +       lib/plug-dummy.h \
> > > > +       lib/plug-dummy.c
> > > >  nodist_lib_libovn_la_SOURCES = \
> > > >         lib/ovn-dirs.c \
> > > >         lib/ovn-nb-idl.c \
> > > > diff --git a/lib/plug-dummy.c b/lib/plug-dummy.c
> > > > new file mode 100644
> > > > index 000000000..efa413b19
> > > > --- /dev/null
> > > > +++ b/lib/plug-dummy.c
> > > > @@ -0,0 +1,144 @@
> > > > +/*
> > > > + * 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-dummy.h"
> > > > +#include "plug-provider.h"
> > > > +#include "plug.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);
> > > > +
> > > > +struct plug_dummy {
> > > > +    struct plug plug;
> > > > +};
> > > > +
> > > > +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");
> > >
> > > Initializing maintained_iface_options in the init() function looks
> unnatural. In theory the provider doesn't know when will the init()
> interface will be called and has no idea if maintained_iface_options will
> be accessed before the init() function is called, unless this is clearly
> specified as a contract. I think it may be simpler to have two arguments,
> const char ** maintained_iface_options and size_t
> n_maintained_iface_options, to pass this information, then the provider can
> define a static string array and no dynamic initialization/destroy of this
> member is needed.
> >
> > What I was thinking is that the contract is formed as comments in the
> > struct plug_class in plug-provider.h, there the init function is
> > documented as a one time call which will be made at plugin register
> > time. Before registration it is not possible to open or make any calls
> > to the plugin so as such the call order is guaranteed.
> >
> > I agree that a array of strings and size could be used for static
> > information like this, but that would mean we would either have to
> > transform it into a sset or make the maintain_interface_smap_column
> > function in ovsport.c (patch 3) take array of strings + size instead
> > of sset as argument. That is doable, but it feels like the sset type
> > encodes the necessary information (data + size) better as well as
> > providing safe iteration macros?
> >
>
> Alternatively it can be a static string array with a NULL terminating
> element, but I agree with you the sset is convenient to use. If we decide
> to go with the dynamic approach, probably it is better to add this detail
> in the documentation that the sset needs to be initialized in the init()
> callback, at least a recommended way.
>

Yes, this should definitely be documented either way.

> > > +
> > > > +    return 0;
> > > > +}
> > > > +
> > > > +static int
> > > > +plug_dummy_destroy(void)
> > > > +{
> > > > +    sset_destroy(&plug_dummy_maintained_iface_options);
> > > > +
> > > > +    return 0;
> > > > +}
> > > > +
> > > > +static int
> > > > +plug_dummy_open(const struct plug_class *class, struct plug **plugp)
> > > > +{
> > > > +    struct plug_dummy *plug;
> > > > +
> > > > +    plug = xmalloc(sizeof *plug);
> > > > +    plug->plug.plug_class = class;
> > > > +    *plugp = &plug->plug;
> > > > +
> > > > +    VLOG_DBG("plug_dummy_open(%p)", plug);
> > > > +    return 0;
> > > > +}
> > > > +
> > > > +static int
> > > > +plug_dummy_close(struct plug *plug)
> > > > +{
> > > > +    VLOG_DBG("plug_dummy_close(%p)", plug);
> > > > +    free(plug);
> > > > +
> > > > +    return 0;
> > > > +}
> > > > +
> > > > +static bool
> > > > +plug_dummy_run(struct plug *plug)
> > > > +{
> > > > +    VLOG_DBG("plug_dummy_run(%p)", plug);
> > > > +
> > > > +    return true;
> > > > +}
> > > > +
> > > > +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");
> > > > +        ctx_out->iface_options = xmalloc(sizeof
> *ctx_out->iface_options);
> > > > +        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);
> > > > +    free(ctx_out->iface_options);
> > > > +}
> > > > +
> > > > +const struct plug_class plug_dummy_class = {
> > > > +    .type = "dummy",
> > > > +    .maintained_iface_options =
> &plug_dummy_maintained_iface_options,
> > > > +    .init = plug_dummy_init,
> > > > +    .destroy = plug_dummy_destroy,
> > > > +    .open = plug_dummy_open,
> > > > +    .close = plug_dummy_close,
> > > > +    .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_register_provider(&plug_dummy_class);
> > > > +}
> > > > +
> > > > diff --git a/lib/plug-dummy.h b/lib/plug-dummy.h
> > > > new file mode 100644
> > > > index 000000000..6ea33671e
> > > > --- /dev/null
> > > > +++ b/lib/plug-dummy.h
> > > > @@ -0,0 +1,33 @@
> > > > +/*
> > > > + * 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_DUMMY_H
> > > > +#define PLUG_DUMMY_H 1
> > > > +
> > > > +/*
> > > > + * The dummy plugger, allows for experimenting with plugging in a
> sandbox */
> > > > +
> > > > +#ifdef  __cplusplus
> > > > +extern "C" {
> > > > +#endif
> > > > +
> > > > +void plug_dummy_enable(void);
> > > > +
> > > > +#ifdef  __cplusplus
> > > > +}
> > > > +#endif
> > > > +
> > > > +#endif /* plug-dummy.h */
> > > > diff --git a/lib/plug-provider.h b/lib/plug-provider.h
> > > > new file mode 100644
> > > > index 000000000..5ffbe088f
> > > > --- /dev/null
> > > > +++ b/lib/plug-provider.h
> > > > @@ -0,0 +1,109 @@
> > > > +/*
> > > > + * 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
> > > > +
> > > > +/* Provider interface to pluggers.  A plugger 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).
> > > > + */
> > > > +
> > > > +#include <stdbool.h>
> > > > +
> > > > +#include "plug.h"
> > > > +
> > > > +#ifdef __cplusplus
> > > > +extern "C" {
> > > > +#endif
> > > > +
> > > > +struct plug {
> > > > +    const struct plug_class *plug_class;
> > > > +};
> > >
> > > I don't see the point of having struct plug_class and struct plug for
> instances, because from the code below it is not possible to have more than
> one instance of each class. And the open() may be just replaced by an
> interface like get_provider(). The close() and reference counters seem not
> quite useful. Correct me if I am wrong, but I don't really see the close()
> get called in the later patches.
> >
> > This pattern is taken from the OVS dpif provider interface and allows
> > for each plug class to compose their own data structure for any per
> > instance data they may need to carry.
> >
> > Examples from OVS:
> >
> https://github.com/openvswitch/ovs/blob/bfee9f6c011518c7690d3ce3b290a2b7189a377d/lib/dpif-netdev.c#L454-L459
> >
> https://github.com/openvswitch/ovs/blob/bfee9f6c011518c7690d3ce3b290a2b7189a377d/lib/dpif-netlink.c#L208-L227
> >
> > And you're right that the current code will only ever provide you with
> > one instance of each class, but I thought it would be useful to have
> > the infrastructure in place should we ever introduce multiple threads
> > to handle port bindings.
> >
> > The OVS code uses a name for each instance to allow for multiple
> instances:
> >
> https://github.com/openvswitch/ovs/blob/master/lib/dpif-provider.h#L169-L170
> >
>
> For my understanding the OVS dpif provider can have multiple instances for
> the same class not for multithreading, but for real datapath entitiies,
> e.g. class XYZ implements a DP provider that uses a specific HW switch
> datapath, and you can have mulitple such HW switches, each maps to an OVS
> dpif instance.
>
> For handling port bindings with multi-threading, I don't think it requires
> multiple instances. So I think it can be simply a "static" class with
> static members and methods, from an OOP point of view. Of course it makes
> sense to have the instances structure if we foresee that in the future it
> is possible to have a need for multiple plugging provider instances for the
> same class, but I just can't imagine what would be that use case for now.
>

Yes, you have convinced me, let's get rid of the extra complexity.

> I could do something similar here and either feed in a static name or
> > make a call to pthread_getname_np(3) if any thread names are currently
> > set in OVN?
> >
> > The controller would have no knowledge about which plug provdiers are
> > actually in use before it encounters them when processing port
> > bindings. So I think it makes sense to have a two stage approach here
> > where the plugins are registered at startup and activated dynamically
> > as the port binding module processes ports. This is also why you see
> > no call to close in the binding module.
>
> Have open() without corresponding close() perhaps already suggesting that
> the open() is not necessary.
>

Indeed, I should have made that call when copying the pattern and not
bringing over the name component.

>
> > The close() function is called via a call to plug_destroy_all() at
> > ovn-controller exit in patch 6.
>
> Yes, but open is called every time when binding a port. The refcount would
> just keep increasing and loses its meaning.
>

Actually, the open would return the same object without increasing the
refcount, but as stated above you have convinced me that we should get rid
of the extra complexity. Thank you for the persistence.

--
Frode Nordahl

Thanks,
> Han
>
> >
> > > > +
> > > > +struct plug_class {
> > > > +    /* Type of plugger in this class. */
> > > > +    const char *type;
> > > > +
> > > > +    /* Interface options this plugger will maintain.  This set is
> used
> > > > +     * to know which items to remove when maintaining the database
> record. */
> > > > +    struct sset *maintained_iface_options;
> > > > +
> > > > +    /* 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);
> > > > +
> > > > +    /* Creates a new plug class instance.
> > > > +     *
> > > > +     * If successful, stores a pointer to the plug instance in
> '*plugp' */
> > > > +    int (*open)(const struct plug_class *class, struct plug
> **plugp);
> > > > +
> > > > +    /* Closes plug class instance and frees associated memory. */
> > > > +    int (*close)(struct plug *plug);
> > > > +
> > > > +    /* Performs periodic work needed by plugger, if any is
> necessary.  Returns
> > > > +     * true if something changed, false otherwise.
> > > > +     *
> > > > +     * Note that work performed by plugger in this function must
> under no
> > > > +     * circumstances block. */
> > > > +    bool (*run)(struct plug *plug);
> > > > +
> > > > +    /* 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
> > > > +
> > > > +#ifdef  __cplusplus
> > > > +}
> > > > +#endif
> > > > +
> > > > +#endif /* plug-provider.h */
> > > > diff --git a/lib/plug.c b/lib/plug.c
> > > > new file mode 100644
> > > > index 000000000..8d717bb37
> > > > --- /dev/null
> > > > +++ b/lib/plug.c
> > > > @@ -0,0 +1,342 @@
> > > > +/*
> > > > + * 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 "plug.h"
> > > > +
> > > > +#include <errno.h>
> > > > +#include <stdint.h>
> > > > +#include <string.h>
> > > > +
> > > > +#include "openvswitch/vlog.h"
> > > > +#include "openvswitch/shash.h"
> > > > +#include "smap.h"
> > > > +#include "sset.h"
> > > > +
> > > > +VLOG_DEFINE_THIS_MODULE(plug);
> > > > +
> > > > +struct registered_plug_class {
> > > > +    const struct plug_class *plug_class;
> > > > +    int refcount;
> > > > +};
> > > > +static struct shash plug_classes = SHASH_INITIALIZER(&plug_classes);
> > > > +static struct shash plug_instances =
> SHASH_INITIALIZER(&plug_instances);
> > > > +
> > > > +/* Protects 'plug_classes', including the refcount. */
> > > > +static struct ovs_mutex plug_classes_mutex = OVS_MUTEX_INITIALIZER;
> > > > +/* Protects 'plug_instances' */
> > > > +static struct ovs_mutex plug_instances_mutex =
> OVS_MUTEX_INITIALIZER;
> > > > +
> > > > +/* Initialize the the plug infrastructure by registering known plug
> classes */
> > > > +static void
> > > > +plug_initialize(void)
> > > > +{
> > > > +    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> > > > +
> > > > +    if (ovsthread_once_start(&once)) {
> > > > +#ifdef HAVE_PLUG_PROVIDER
> > > > +        for (const struct plug_class **pp = plug_provider_classes;
> > > > +             pp && *pp;
> > > > +             pp++)
> > > > +        {
> > > > +            plug_register_provider(*pp);
> > > > +        }
> > > > +#endif
> > > > +        ovsthread_once_done(&once);
> > > > +    }
> > > > +}
> > > > +
> > > > +static int
> > > > +plug_register_provider__(const struct plug_class *new_class)
> > > > +{
> > > > +    struct registered_plug_class *rc;
> > > > +    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;
> > > > +    }
> > > > +
> > > > +    rc = xmalloc(sizeof *rc);
> > > > +    rc->plug_class = new_class;
> > > > +    rc->refcount = 0;
> > > > +
> > > > +    shash_add(&plug_classes, new_class->type, rc);
> > > > +
> > > > +    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_register_provider(const struct plug_class *new_class)
> > > > +{
> > > > +    int error;
> > > > +
> > > > +    ovs_mutex_lock(&plug_classes_mutex);
> > > > +    error = plug_register_provider__(new_class);
> > > > +    ovs_mutex_unlock(&plug_classes_mutex);
> > > > +
> > > > +    return error;
> > > > +}
> > > > +
> > > > +static int
> > > > +plug_unregister_provider__(const char *type)
> > > > +{
> > > > +    int error;
> > > > +    struct shash_node *node;
> > > > +    struct registered_plug_class *rc;
> > > > +
> > > > +    node = shash_find(&plug_classes, type);
> > > > +    if (!node) {
> > > > +        return EINVAL;
> > > > +    }
> > > > +
> > > > +    rc = node->data;
> > > > +    if (rc->refcount) {
> > > > +        VLOG_WARN("attempted to unregister in use plug provider:
> %s", type);
> > > > +        return EBUSY;
> > > > +    }
> > > > +
> > > > +    error = rc->plug_class->destroy ? rc->plug_class->destroy() : 0;
> > > > +    if (error) {
> > > > +        VLOG_WARN("failed to destroy %s plug class: %s",
> > > > +                  rc->plug_class->type, ovs_strerror(error));
> > > > +        return error;
> > > > +    }
> > > > +
> > > > +    shash_delete(&plug_classes, node);
> > > > +    free(rc);
> > > > +
> > > > +    return 0;
> > > > +}
> > > > +
> > > > +/* Unregister the plug provider identified by 'type' and perform
> any class
> > > > + * level de-initialization as specified in its plug_class. */
> > > > +int
> > > > +plug_unregister_provider(const char *type)
> > > > +{
> > > > +    int error;
> > > > +
> > > > +    plug_initialize();
> > >
> > > Sorry that I don't understand why need to call initialization when
> unregistering.
> >
> > To be honest I probably copied this over from the OVS dpif unregister
> > provider function, it is probably just a safeguard and I agree it can
> > probably be removed in our case. Will revisit.
> >
> > > > +
> > > > +    ovs_mutex_lock(&plug_classes_mutex);
> > > > +    error = plug_unregister_provider__(type);
> > > > +    ovs_mutex_unlock(&plug_classes_mutex);
> > > > +
> > > > +    return error;
> > > > +}
> > > > +
> > > > +static void
> > > > +plug_class_unref(struct registered_plug_class *rc)
> > > > +{
> > > > +    ovs_mutex_lock(&plug_classes_mutex);
> > > > +    ovs_assert(rc->refcount);
> > > > +    rc->refcount--;
> > > > +    ovs_mutex_unlock(&plug_classes_mutex);
> > > > +}
> > > > +
> > > > +static struct registered_plug_class *
> > > > +plug_class_lookup(const char *type)
> > > > +{
> > > > +    struct registered_plug_class *rc;
> > > > +
> > > > +    ovs_mutex_lock(&plug_classes_mutex);
> > > > +    rc = shash_find_data(&plug_classes, type);
> > > > +    if (rc) {
> > > > +        rc->refcount++;
> > >
> > > Nit: It may be better to perform the refcount increasing outside of
> this function. Usually people doesn't expect a "lookup" function to
> increase refcount. Or maybe just rename the function if we don't need a
> function for purely "lookup".
> > > (ignore this comment if we decide to remove the refcount).
> >
> > Thank you for pointing that out, yes this is indeed a side effect you
> > would not expect from a lookup. Will revisit.
> >
> > > > +    }
> > > > +    ovs_mutex_unlock(&plug_classes_mutex);
> > > > +
> > > > +    return rc;
> > > > +}
> > > > +
> > > > +static int
> > > > +plug_open__(const char *type, struct plug **plugp)
> > > > +{
> > > > +    struct plug *plug = NULL;
> > > > +    int error;
> > > > +    struct registered_plug_class *rc;
> > > > +
> > > > +    plug_initialize();
> > > > +    rc = plug_class_lookup(type);
> > > > +    if (!rc) {
> > > > +        VLOG_WARN("unable to open plug provider of unknown type:
> %s", type);
> > > > +        error = EINVAL;
> > > > +        goto out;
> > > > +    }
> > > > +
> > > > +    error = rc->plug_class->open(rc->plug_class, &plug);
> > > > +    if (error) {
> > > > +        plug_class_unref(rc);
> > > > +    }
> > > > +
> > > > +out:
> > > > +    *plugp = error ? NULL: plug;
> > > > +    return error;
> > > > +}
> > > > +
> > > > +/* Create, or retrieve the already created instance of plug class
> from a
> > > > + * previous call to plug_open, identified by 'type' and store a
> reference to it
> > > > + * in '*plugp'.
> > > > + *
> > > > + * The plug implementation will perform any initialization and
> allocations it
> > > > + * needs, and the plug infrastructure will store a reference to
> it.  Subsequent
> > > > + * calls to this function with the same 'type' parameter will
> return the same
> > > > + * object, until the instance is removed with a call to plug_close.
> */
> > > > +int
> > > > +plug_open(const char *type, struct plug **plugp)
> > > > +{
> > > > +    struct plug *instance = shash_find_data(&plug_instances, type);
> > > > +    int error;
> > > > +
> > > > +    if (instance) {
> > > > +        *plugp = instance;
> > > > +        return 0;
> > > > +    }
> > > > +
> > > > +    error = plug_open__(type, plugp);
> > > > +    if (error) {
> > > > +        return error;
> > > > +    }
> > > > +
> > > > +    ovs_mutex_lock(&plug_instances_mutex);
> > > > +    shash_add(&plug_instances, type, *plugp);
> > > > +    ovs_mutex_unlock(&plug_instances_mutex);
> > > > +
> > > > +    return 0;
> > > > +}
> > > > +
> > > > +/* Close the plug class instance previously created by a call to
> 'plug_open'.
> > > > + *
> > > > + * The plug implementation will perform any destruction of its data
> and the
> > > > + * plug infrastructure will remove its references to it. */
> > > > +void
> > > > +plug_close(struct plug *plug)
> > > > +{
> > > > +    if (plug) {
> > > > +        ovs_mutex_lock(&plug_instances_mutex);
> > > > +        shash_find_and_delete(&plug_instances,
> plug->plug_class->type);
> > > > +        ovs_mutex_unlock(&plug_instances_mutex);
> > > > +
> > > > +        struct registered_plug_class *rc;
> > > > +        rc = shash_find_data(&plug_classes, plug->plug_class->type);
> > > > +        rc->plug_class->close(plug);
> > > > +        plug_class_unref(rc);
> > > > +    }
> > > > +}
> > > > +
> > > > +/* Close any previously instantiated plug classes and unregister
> the plug
> > > > + * providers. */
> > > > +void
> > > > +plug_destroy_all(void)
> > > > +{
> > > > +    struct shash_node *node, *next;
> > > > +
> > > > +    SHASH_FOR_EACH_SAFE (node, next, &plug_instances) {
> > > > +        struct plug *plug = node->data;
> > > > +        plug_close(plug);
> > > > +    }
> > > > +
> > > > +    SHASH_FOR_EACH_SAFE (node, next, &plug_classes) {
> > > > +        struct registered_plug_class *rc = node->data;
> > > > +        plug_unregister_provider(rc->plug_class->type);
> > > > +    }
> > > > +}
> > > > +
> > > > +/* Iterate over previously instantiated plug classes and call their
> 'run'
> > > > + * function if defined.
> > > > + *
> > > > + * If any of the instances report they have changed something this
> function
> > > > + * will return 'true', otherwise it will return 'false'. */
> > > > +bool
> > > > +plug_run_instances(void)
> > > > +{
> > > > +    struct shash_node *node;
> > > > +    bool something_changed = false;
> > > > +
> > > > +    ovs_mutex_lock(&plug_instances_mutex);
> > > > +
> > > > +    SHASH_FOR_EACH (node, &plug_instances) {
> > > > +        struct plug *instance = node->data;
> > > > +        if (instance->plug_class->run(instance)) {
> > > > +            something_changed = true;
> > > > +        }
> > > > +    }
> > > > +
> > > > +    ovs_mutex_unlock(&plug_instances_mutex);
> > > > +
> > > > +    return something_changed;
> > > > +}
> > > > +
> > > > +/* Get the class level 'maintained_iface_options' set. */
> > > > +struct sset *
> > > > +plug_class_get_maintained_iface_options(const struct plug *plug)
> > > > +{
> > > > +    return plug->plug_class->maintained_iface_options;
> > > > +}
> > > > +
> > > > +/* 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 *plug,
> > > > +                  const struct plug_port_ctx_in *ctx_in,
> > > > +                  struct plug_port_ctx_out *ctx_out)
> > > > +{
> > > > +    if (ctx_out) {
> > > > +        memset(ctx_out, 0, sizeof(*ctx_out));
> > > > +    }
> > > > +    return plug->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 *plug,
> > > > +                 const struct plug_port_ctx_in *ctx_in,
> > > > +                 struct plug_port_ctx_out *ctx_out)
> > > > +{
> > > > +    plug->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 *plug,
> > > > +                      const struct plug_port_ctx_in *ctx_in,
> > > > +                      struct plug_port_ctx_out *ctx_out)
> > > > +{
> > > > +    plug->plug_class->plug_port_ctx_destroy(ctx_in, ctx_out);
> > > > +}
> > > > diff --git a/lib/plug.h b/lib/plug.h
> > > > new file mode 100644
> > > > index 000000000..6117bd52a
> > > > --- /dev/null
> > > > +++ b/lib/plug.h
> > > > @@ -0,0 +1,101 @@
> > > > +/*
> > > > + * 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 plugging interface.  This module contains the
> infrastructure for
> > > > + * registering and instantiating plugging classes which may be
> hosted inside
> > > > + * or outside the core OVN repository.  The data structures and
> functions for
> > > > + * interacting with these plugging classes also live here.
> > > > + */
> > > > +
> > > > +#include "smap.h"
> > > > +
> > > > +#ifdef  __cplusplus
> > > > +extern "C" {
> > > > +#endif
> > > > +
> > > > +struct plug;
> > > > +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;
> > > > +
> > > > +    /* Whether the chassis uses DPDK */
> > > > +    bool use_dpdk;
> >
> > > use_dpdk looks too specific here. I think the fields in this structure
> should be generic enough for all providers. Provider specific information
> should be retrieved by provider class functions themselves. I think we can
> define ovs_cfg here, and let the provider find out whatever detailed
> configuration that can be set in Open_vSwitch.
> >
> > Yes, I see what you mean. I ended up with this because I think OVN or
> > OVS code would have more knowledege about how to determine one way or
> > the other instead of every plug implementation figuring it out on
> > their own. But they can still do that if they're provided with the
> > data they need.
> >
> > I'll change to provide the br-int ovsrec_bridge record (for
> > datapath_type) and ovsrec_open_vswitch in the struct and the plug
> > provider can choose to use that with a utility function like
> > chassis_uses_dpdk or roll their own.
> >
> > Thanks!
> >
> > --
> > Frode Nordahl
> >
> > > > +
> > > > +    /* 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. */
> > > > +    const 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;
> > > > +    const 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;
> > > > +};
> > > > +
> > > > +
> > > > +int plug_register_provider(const struct plug_class *);
> > > > +int plug_unregister_provider(const char *type);
> > > > +void plug_destroy_all(void);
> > > > +int plug_open(const char *type, struct plug **);
> > > > +void plug_close(struct plug *);
> > > > +bool plug_run_instances(void);
> > > > +
> > > > +struct sset * plug_class_get_maintained_iface_options(const struct
> plug *);
> > > > +
> > > > +bool plug_port_prepare(const struct plug *,
> > > > +                       const struct plug_port_ctx_in *,
> > > > +                       struct plug_port_ctx_out *);
> > > > +void plug_port_finish(const struct plug *,
> > > > +                      const struct plug_port_ctx_in *,
> > > > +                      struct plug_port_ctx_out *);
> > > > +void plug_port_ctx_destroy(const struct plug *,
> > > > +                           const struct plug_port_ctx_in *,
> > > > +                           struct plug_port_ctx_out *);
> > > > +
> > > > +#ifdef  __cplusplus
> > > > +}
> > > > +#endif
> > > > +
> > > > +#endif /* plug.h */
> > > > diff --git a/lib/test-plug.c b/lib/test-plug.c
> > > > new file mode 100644
> > > > index 000000000..b17b08a70
> > > > --- /dev/null
> > > > +++ b/lib/test-plug.c
> > > > @@ -0,0 +1,80 @@
> > > > +/* 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-dummy.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)
> > > > +{
> > > > +    struct plug *plug;
> > > > +
> > > > +    ovs_assert(plug_unregister_provider("dummy") == EINVAL);
> > > > +    ovs_assert(plug_open("dummy", &plug) == EINVAL);
> > > > +
> > > > +    ovs_assert(!plug_register_provider(&plug_dummy_class));
> > > > +    ovs_assert(plug_register_provider(&plug_dummy_class) == EEXIST);
> > > > +    ovs_assert(!plug_run_instances());
> > > > +
> > > > +    ovs_assert(!plug_open("dummy", &plug));
> > > > +    ovs_assert(plug_unregister_provider("dummy") == EBUSY);
> > > > +
> > > > +    ovs_assert(sset_contains(
> > > > +            plug_class_get_maintained_iface_options(plug),
> > > > +            "plug-dummy-option"));
> > > > +    ovs_assert(plug_run_instances());
> > > > +
> > > > +    struct smap fake_lport_options =
> SMAP_INITIALIZER(&fake_lport_options);
> > > > +    struct plug_port_ctx_in ctx_in = {
> > > > +        .op_type = PLUG_OP_CREATE,
> > > > +        .use_dpdk = false,
> > > > +        .lport_name = "lsp1",
> > > > +        .lport_options = &fake_lport_options,
> > > > +    };
> > > > +    struct plug_port_ctx_out ctx_out;
> > > > +    plug_port_prepare(plug, &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, &ctx_in, &ctx_out);
> > > > +    plug_port_ctx_destroy(plug, &ctx_in, &ctx_out);
> > > > +    plug_close(plug);
> > > > +    plug_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/tests/automake.mk b/tests/automake.mk
> > > > index 5b890d644..ad8978541 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 \
> > > > @@ -248,6 +249,7 @@ tests_ovstest_SOURCES = \
> > > >         controller/ofctrl-seqno.c \
> > > >         controller/ofctrl-seqno.h \
> > > >         lib/test-ovn-features.c \
> > > > +       lib/test-plug.c \
> > > >         northd/test-ipam.c \
> > > >         northd/ipam.c \
> > > >         northd/ipam.h
> > > > 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
> > > > dev at openvswitch.org
> > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list