[ovs-dev] [PATCH 2/2] dpdk: reflect status and version in the database
Stokes, Ian
ian.stokes at intel.com
Tue May 22 15:32:03 UTC 2018
> "Stokes, Ian" <ian.stokes at intel.com> writes:
>
> >> The normal way of retrieving the running DPDK status involves parsing
> >> log files and issuing various incantations of ovs-vsctl and
> >> ovs-appctl commands to determine whether the rte_eal_init successfully
> started.
> >>
> >> This commit adds two new records to reflect the dpdk version, and the
> >> dpdk initialization status.
> >>
> >> To support this, the other_config:dpdk-init configuration block
> >> supports the 'true' and 'try' keywords now, instead of just 'true'.
> >>
> >
> > Look ok overall, minor comments below that can be addressed on commit
> > to DPDK_MERGE if you agree with them.
>
> Okay. Whichever you prefer.
>
> >> Signed-off-by: Aaron Conole <aconole at redhat.com>
> >> ---
> >> Documentation/faq/configuration.rst | 8 +++++---
> >> Documentation/intro/install/dpdk.rst | 27 ++++++++++++++++++++++++---
> >> lib/dpdk-stub.c | 10 ++++++++++
> >> lib/dpdk.c | 21 +++++++++++++++++++--
> >> lib/dpdk.h | 3 ++-
> >> vswitchd/bridge.c | 5 +++++
> >> vswitchd/vswitch.ovsschema | 11 ++++++++---
> >> vswitchd/vswitch.xml | 11 +++++++++++
> >> 8 files changed, 84 insertions(+), 12 deletions(-)
> >
> > We should mention this in news as it seems like a relevant enough
> > change for users.
>
> Okay. I can submit a follow up patch to this (if you want), or I can
> submit a v2 of this patch if you want. Let me know how you'd like it.
>
A v2 will be fine.
> >>
> >> diff --git a/Documentation/faq/configuration.rst
> >> b/Documentation/faq/configuration.rst
> >> index 1c93a55cc..0213c58dd 100644
> >> --- a/Documentation/faq/configuration.rst
> >> +++ b/Documentation/faq/configuration.rst
> >> @@ -102,9 +102,11 @@ Q: How do I configure a DPDK port as an access
> port?
> >>
> >> A: Firstly, you must have a DPDK-enabled version of Open vSwitch.
> >>
> >> - If your version is DPDK-enabled it will support the other-
> >> config:dpdk-init
> >> - configuration in the database and will display lines with
> "EAL:..."
> >> during
> >> - startup when other_config:dpdk-init is set to 'true'.
> >> + If your version is DPDK-enabled it may support the dpdk_version
> and
> >> + dpdk_initialized keys in the configuration database. Earlier
> >> versions
> >> + of Open vSwitch only supported the other-config:dpdk-init key in
> the
> >> + configuration in the database. All versions will display lines
> with
> >> + "EAL:..." during startup when other_config:dpdk-init is set to
> >> 'true'.
> >>
> >> Secondly, when adding a DPDK port, unlike a system port, the
> >> type for the
> >> interface and valid dpdk-devargs must be specified. For example::
> >> diff --git a/Documentation/intro/install/dpdk.rst
> >> b/Documentation/intro/install/dpdk.rst
> >> index fea48908d..8fb0c4163 100644
> >> --- a/Documentation/intro/install/dpdk.rst
> >> +++ b/Documentation/intro/install/dpdk.rst
> >> @@ -208,7 +208,8 @@ Open vSwitch should be started as described in
> >> :doc:`general` with the exception of ovs-vswitchd, which requires
> >> some special configuration to enable DPDK functionality. DPDK
> >> configuration arguments can be passed to ovs-vswitchd via the
> >> ``other_config`` column of the ``Open_vSwitch`` table. At a minimum,
> >> -the ``dpdk-init`` option must be set to ``true``. For example::
> >> +the ``dpdk-init`` option must be set to either ``true`` or ``try``.
> >> +For example::
> >>
> >> $ export PATH=$PATH:/usr/local/share/openvswitch/scripts
> >> $ export DB_SOCK=/usr/local/var/run/openvswitch/db.sock
> >> @@ -219,8 +220,12 @@ There are many other configuration options, the
> >> most important of which are listed below. Defaults will be provided
> >> for all values not explicitly set.
> >>
> >> ``dpdk-init``
> >> - Specifies whether OVS should initialize and support DPDK ports.
> >> This is a
> >> - boolean, and defaults to false.
> >> + Specifies whether OVS should initialize and support DPDK ports.
> >> + This field can either be ``true`` or ``try``.
> >> + A value of ``true`` will cause the ovs-vswitchd process to abort
> >> + on initialization failure.
> >> + A value of ``try`` will imply that the ovs-vswitchd process should
> >> + continue running even if the EAL initialization fails.
> >>
> > .. versionchanged:: 2.10.0
> >
> > We've started to track changes in behavior in the docs, I'm wondering
> > if one should be added here but it may look strange as 2.10 above has
> > not been released yet. It's that or we have a 'to-do' to track it when
> > the release is out. Thoughts?
>
> I guess it would make more sense to use just write the document ovs
> version instead (rather than which version changed it). And that can be
> accomplished with @VERSION@ tag and using a .in file.
>
> Anyway, I missed that particular part of the docs, but that's because I
> think some of it was modified between this patch being submitted and
> interrim changes. I can rebase.
>
> I don't think it makes sense to change this version number though (do
> you?) since as you note the version hasn't been released.
>
I agree that 2.10 doesn't make sense. I've haven't used the @version and .in approach before but it sounds interesting. There is a minor nit to be fair though, I won't block on it as it is either.
Ian
> > Ian
> >> ``dpdk-lcore-mask``
> >> Specifies the CPU cores on which dpdk lcore threads should be
> >> spawned and @@ -257,6 +262,22 @@ See the section ``Performance
> >> Tuning`` for important DPDK customizations.
> >> Validating
> >> ----------
> >>
> >> +DPDK support can be confirmed by validating the ``dpdk_initialized``
> >> +boolean value from the ovsdb. A value of ``true`` means that the
> >> +DPDK EAL initialization succeeded::
> >> +
> >> + $ ovs-vsctl get Open_vSwitch . dpdk_initialized true
> >> +
> >> +Additionally, the library version linked to ovs-vswitchd can be
> >> +confirmed with either the ovs-vswitchd logs, or by running either of
> >> +the
> >> commands::
> >> +
> >> + $ ovs-vswitchd --version
> >> + ovs-vswitchd (Open vSwitch) 2.9.0
> >> + DPDK 17.11.0
> >> + $ ovs-vsctl get Open_vSwitch . dpdk_version "DPDK 17.11.0"
> >> +
> >> At this point you can use ovs-vsctl to set up bridges and other Open
> >> vSwitch features. Seeing as we've configured the DPDK datapath, we
> >> will use DPDK-type ports. For example, to create a userspace bridge
> >> named ``br0`` and add two diff --git a/lib/dpdk-stub.c
> >> b/lib/dpdk-stub.c index
> >> 041cd0cbb..1df1c5848 100644
> >> --- a/lib/dpdk-stub.c
> >> +++ b/lib/dpdk-stub.c
> >> @@ -21,6 +21,7 @@
> >> #include "smap.h"
> >> #include "ovs-thread.h"
> >> #include "openvswitch/vlog.h"
> >> +#include "vswitch-idl.h"
> >>
> >> VLOG_DEFINE_THIS_MODULE(dpdk);
> >>
> >> @@ -59,3 +60,12 @@ void
> >> print_dpdk_version(void)
> >> {
> >> }
> >> +
> >> +void
> >> +dpdk_status(const struct ovsrec_open_vswitch *cfg) {
> >> + if (cfg) {
> >> + ovsrec_open_vswitch_set_dpdk_initialized(cfg, false);
> >> + ovsrec_open_vswitch_set_dpdk_version(cfg, "none");
> >> + }
> >> +}
> >> diff --git a/lib/dpdk.c b/lib/dpdk.c
> >> index 8bb686c43..09afd8cc2 100644
> >> --- a/lib/dpdk.c
> >> +++ b/lib/dpdk.c
> >> @@ -37,6 +37,7 @@
> >> #include "openvswitch/dynamic-string.h"
> >> #include "openvswitch/vlog.h"
> >> #include "smap.h"
> >> +#include "vswitch-idl.h"
> >>
> >> VLOG_DEFINE_THIS_MODULE(dpdk);
> >>
> >> @@ -44,6 +45,8 @@ static FILE *log_stream = NULL; /* Stream for
> DPDK
> >> log redirection */
> >>
> >> static char *vhost_sock_dir = NULL; /* Location of vhost-user
> sockets
> >> */
> >> static bool vhost_iommu_enabled = false; /* Status of vHost IOMMU
> >> support */
> >> +static bool dpdk_initialized = false; /* Indicates successful
> >> initialization
> >> + * of DPDK. */
> >>
> >> static int
> >> process_vhost_flags(char *flag, const char *default_val, int size,
> >> @@ -
> >> 474,7 +477,11 @@ dpdk_init(const struct smap *ovs_other_config)
> >> return;
> >> }
> >>
> >> - if (smap_get_bool(ovs_other_config, "dpdk-init", false)) {
> >> + const char *dpdk_init_val = smap_get_def(ovs_other_config,
> >> + "dpdk-
> >> init",
> >> + "false");
> >> +
> >> + bool try_only = !strcmp(dpdk_init_val, "try");
> >> + if (!strcmp(dpdk_init_val, "true") || try_only) {
> >> static struct ovsthread_once once_enable =
> >> OVSTHREAD_ONCE_INITIALIZER;
> >>
> >> if (ovsthread_once_start(&once_enable)) { @@ -483,7 +490,7
> >> @@ dpdk_init(const struct smap *ovs_other_config)
> >> enabled = dpdk_init__(ovs_other_config);
> >> if (enabled) {
> >> VLOG_INFO("DPDK Enabled - initialized");
> >> - } else {
> >> + } else if (!try_only) {
> >> ovs_abort(rte_errno, "Cannot init EAL");
> >> }
> >> ovsthread_once_done(&once_enable);
> >> @@ -493,6 +500,7 @@ dpdk_init(const struct smap *ovs_other_config)
> >> } else {
> >> VLOG_INFO_ONCE("DPDK Disabled - Use other_config:dpdk-init
> >> to enable");
> >> }
> >> + dpdk_initialized = enabled;
> >> }
> >>
> >> const char *
> >> @@ -520,3 +528,12 @@ print_dpdk_version(void) {
> >> puts(rte_version());
> >> }
> >> +
> >> +void
> >> +dpdk_status(const struct ovsrec_open_vswitch *cfg) {
> >> + if (cfg) {
> >> + ovsrec_open_vswitch_set_dpdk_initialized(cfg,
> dpdk_initialized);
> >> + ovsrec_open_vswitch_set_dpdk_version(cfg, rte_version());
> >> + }
> >> +}
> >> diff --git a/lib/dpdk.h b/lib/dpdk.h
> >> index b04153591..efdaa637c 100644
> >> --- a/lib/dpdk.h
> >> +++ b/lib/dpdk.h
> >> @@ -33,11 +33,12 @@
> >> #endif /* DPDK_NETDEV */
> >>
> >> struct smap;
> >> +struct ovsrec_open_vswitch;
> >>
> >> void dpdk_init(const struct smap *ovs_other_config); void
> >> dpdk_set_lcore_id(unsigned cpu); const char
> >> *dpdk_get_vhost_sock_dir(void); bool dpdk_vhost_iommu_enabled(void);
> >> void print_dpdk_version(void);
> >> -
> >> +void dpdk_status(const struct ovsrec_open_vswitch *);
> >> #endif /* dpdk.h */
> >> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index
> >> d90997e3a..ef04b015f 100644
> >> --- a/vswitchd/bridge.c
> >> +++ b/vswitchd/bridge.c
> >> @@ -407,6 +407,8 @@ bridge_init(const char *remote)
> >> ovsdb_idl_omit(idl, &ovsrec_open_vswitch_col_db_version);
> >> ovsdb_idl_omit(idl, &ovsrec_open_vswitch_col_system_type);
> >> ovsdb_idl_omit(idl, &ovsrec_open_vswitch_col_system_version);
> >> + ovsdb_idl_omit_alert(idl, &ovsrec_open_vswitch_col_dpdk_version);
> >> + ovsdb_idl_omit_alert(idl,
> >> + &ovsrec_open_vswitch_col_dpdk_initialized);
> >>
> >> ovsdb_idl_omit_alert(idl, &ovsrec_bridge_col_datapath_id);
> >> ovsdb_idl_omit_alert(idl, &ovsrec_bridge_col_datapath_version);
> >> @@ -2836,10 +2838,13 @@ run_status_update(void)
> >> * previous one is not done. */
> >> seq = seq_read(connectivity_seq_get());
> >> if (seq != connectivity_seqno || status_txn_try_again) {
> >> + const struct ovsrec_open_vswitch *cfg =
> >> + ovsrec_open_vswitch_first(idl);
> >> struct bridge *br;
> >>
> >> connectivity_seqno = seq;
> >> status_txn = ovsdb_idl_txn_create(idl);
> >> + dpdk_status(cfg);
> >> HMAP_FOR_EACH (br, node, &all_bridges) {
> >> struct port *port;
> >>
> >> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> >> index 90e50b626..80f17e89b 100644
> >> --- a/vswitchd/vswitch.ovsschema
> >> +++ b/vswitchd/vswitch.ovsschema
> >> @@ -1,6 +1,6 @@
> >> {"name": "Open_vSwitch",
> >> - "version": "7.15.1",
> >> - "cksum": "3682332033 23608",
> >> + "version": "7.16.0",
> >> + "cksum": "2403910601 23776",
> >> "tables": {
> >> "Open_vSwitch": {
> >> "columns": {
> >> @@ -47,7 +47,12 @@
> >> "min": 0, "max": "unlimited"}},
> >> "iface_types": {
> >> "type": {"key": {"type": "string"},
> >> - "min": 0, "max": "unlimited"}}},
> >> + "min": 0, "max": "unlimited"}},
> >> + "dpdk_initialized": {
> >> + "type": "boolean"},
> >> + "dpdk_version": {
> >> + "type": {"key": {"type": "string"},
> >> + "min": 0, "max": 1}}},
> >> "isRoot": true,
> >> "maxRows": 1},
> >> "Bridge": {
> >> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index
> >> 9c2a8263e..37c7f4f80 100644
> >> --- a/vswitchd/vswitch.xml
> >> +++ b/vswitchd/vswitch.xml
> >> @@ -466,6 +466,11 @@
> >> configuration changes.
> >> </column>
> >>
> >> + <column name="dpdk_initialized">
> >> + True if <ref column="other_config" key="dpdk-init"/> is set to
> >> + true and the DPDK library is successfully initialized.
> >> + </column>
> >> +
> >> <group title="Statistics">
> >> <p>
> >> The <code>statistics</code> column contains key-value
> >> pairs that @@ -649,6 +654,12 @@
> >> </p>
> >> </column>
> >>
> >> + <column name="dpdk_version">
> >> + <p>
> >> + The version of the linked DPDK library.
> >> + </p>
> >> + </column>
> >> +
> >> </group>
> >>
> >> <group title="Capabilities">
> >> --
> >> 2.14.3
More information about the dev
mailing list