On 04/25/2018 06:05 PM, Aaron Conole wrote:
> Kevin Traynor <[email protected]> writes:
>
> Thanks for the review, Kevin!
>
>> On 04/18/2018 07:30 PM, Aaron Conole wrote:
>>> 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'.
>>>
>>> Signed-off-by: Aaron Conole <[email protected]>
>>> ---
>>> 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(-)
>>>
>>> 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..23fabeffd 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 eithr ``true`` or ``try``.
>>
>> s/eithr/either/
>
> d'oh! Thanks.
>
>>> +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.
>>>
>>> ``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 641474cde..37c1eac90 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,
>>> @@ -473,7 +476,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 should_try = !strcmp(dpdk_init_val, "try");
>>
>> strncmp?
>
> I will change this (and the following call) to strcasecmp() - does that
> make sense? I think strncmp doesn't make sense since someone could
> pass: "try this on for size" and still have it match. It would be
> really funny to see in the ovsdb once, though. Maybe strncasecmp?
> WDYT?
Yeah, the n versions don't make sense. It's probably fine the way it is
then - whatever you think is fine with me.
>
>>> + if (!strcmp(dpdk_init_val, "true") || should_try) {
>>> static struct ovsthread_once once_enable =
>>> OVSTHREAD_ONCE_INITIALIZER;
>>>
>>> if (ovsthread_once_start(&once_enable)) {
>>> @@ -482,7 +489,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 (!should_try) {
>>
>> nit: 'if (!should_try)' kind of reads like 'if (should_not_try)'
>> Maybe 'try_only' or 'only_try' or something else would be better
>
> Okay. I'll change it.
>
>>> ovs_abort(rte_errno, "Cannot init EAL");
>>> }
>>> ovsthread_once_done(&once_enable);
>>> @@ -492,6 +499,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 *
>>> @@ -519,3 +527,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">
>>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
