Eli Britstein via dev <[email protected]> writes:

> Currently dpdk is a special case and has dedicated calls from the bridge
> module and entries in OVS schema.
>
> The legacy mode, (dpdk-init and other configurations in other_config) is
> kept for backward compatability.
>
> A new generalized method is introduced, for example (configuration
> options are optional):
> ovs-vsctl add-library dpdk dpdk-lcore-mask=0x5

Hi Eli,

I took a quick look over this series.

I think the naming here is a bit too generic.  For example, when we're
setting up DPDK library configuration, we're not 'adding' DPDK library -
it is already compiled against the DPDK library.  It may be confusing
for a user who doesn't know how the distribution compiled their version
of OVS and think they can simply type 'add dpdk' and it will be linked.
However, there's not a dlopen/rtld type of linking going on so the user
could have a mismatched expectation.

I do think having a more cohesive configuration and status framework for
these kinds of linked libraries could make sense.  Perhaps it would be
better to just call them what they are: configuration and status.

ie: 'ovs-vsctl add-library ...' would become
    'ovs-vsctl <library>-set-config ...'

    'ovs-vsctl <library>-status'

Then things like dpdk would register in a way that we could get commands
generated for them for dpdk specific configs.

Additionally, I think we need more than just config() and init() calls
in the framework.  For example, we may want to pre-define the
configuration options allowed - that's a double edged sword, of course
(since we then need to have some kind of maintenance on config options
which is difficult).  We don't have an uninit call in the framework,
which might make sense.

We also have other strange things here.  For example, did I understand
right in 4/4 that after this patch DPDK now can take either
other_config:dpdk-... OR the new library config?  Which takes
precedence?  what happens when both are encountered?  Practically, this
is a new way of doing things for existing users so that patch needs much
more documentation and thoughts about config.

I think we will need some understanding of how this is intended to be
used going forward as well.  For example, DPDK library is really
providing two things: new port types, and a different packet buffer
implementation.  Are all linked in libraries going to provide that?  Or
should we have different kinds of specifiers for the types of libraries
we will provide these kinds of configurations for?

At least, we should discuss what this kind of user interface and the
software interface contracts should be.

> Eli Britstein (4):
>   vswitch.ovsschema: Add libraries to Open_vSwitch.
>   ovs-vsctl: Add add/del-library commands.
>   lib: Introduce library module.
>   dpdk: Implement library_class_dpdk.
>
>  lib/automake.mk            |   3 +
>  lib/dpdk.c                 |  26 ++++++++
>  lib/dpdk.h                 |   2 +
>  lib/library-provider.h     |  25 ++++++++
>  lib/library.c              | 128 +++++++++++++++++++++++++++++++++++++
>  lib/library.h              |  18 ++++++
>  utilities/ovs-vsctl.c      |  89 ++++++++++++++++++++++++++
>  vswitchd/bridge.c          |  20 ++++++
>  vswitchd/ovs-vswitchd.c    |   2 +
>  vswitchd/vswitch.ovsschema |  27 +++++++-
>  vswitchd/vswitch.xml       |  19 ++++++
>  11 files changed, 356 insertions(+), 3 deletions(-)
>  create mode 100644 lib/library-provider.h
>  create mode 100644 lib/library.c
>  create mode 100644 lib/library.h

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

Reply via email to