| Message ID | 20171225143102.24335-1-mcroce@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v3] ovs-vsctl: show DPDK version | expand |
On Mon, Dec 25, 2017 at 03:31:02PM +0100, Matteo Croce wrote: > Show DPDK version if Open vSwitch is compiled with DPDK support. > Add a `dpdk_version` column in the datamodel, change ovs-ctl and ovs-dev.py > to populate the field with the right value. > > Signed-off-by: Matteo Croce <mcroce@redhat.com> OK, this is better, but why does ovs-vswitchd need a shell helper for this? It should just set the key itself.
On Tue, Dec 26, 2017 at 6:38 PM, Ben Pfaff <blp@ovn.org> wrote: > On Mon, Dec 25, 2017 at 03:31:02PM +0100, Matteo Croce wrote: >> Show DPDK version if Open vSwitch is compiled with DPDK support. >> Add a `dpdk_version` column in the datamodel, change ovs-ctl and ovs-dev.py >> to populate the field with the right value. >> >> Signed-off-by: Matteo Croce <mcroce@redhat.com> > > OK, this is better, but why does ovs-vswitchd need a shell helper for > this? It should just set the key itself. On ovs-vswitchd startup? I think I set dpdk_version the same way ovs_version was set. I can do it in ovs-vswitchd startup but in that case ovs_version should be set in ovs-vswitchd too. Cheers,
Hi Matteo, Matteo Croce <mcroce@redhat.com> writes: > Show DPDK version if Open vSwitch is compiled with DPDK support. > Add a `dpdk_version` column in the datamodel, change ovs-ctl and ovs-dev.py > to populate the field with the right value. > > Signed-off-by: Matteo Croce <mcroce@redhat.com> > --- Sorry I missed some of the development for this - I think it's better to have the dpdk version retrieved via an appctl command. I can't think of a need for this information to be persistent - if it's for historical information when debugging, we can put it in with the logs when dpdk is started. The user cannot influence DPDK version dynamically - it is not just read-only, but it's read-only from a compile time decision. I might be misunderstanding the point of putting this in the DB, though?
On Tue, Jan 2, 2018 at 6:08 PM, Aaron Conole <aconole@redhat.com> wrote: > Hi Matteo, > > Matteo Croce <mcroce@redhat.com> writes: > >> Show DPDK version if Open vSwitch is compiled with DPDK support. >> Add a `dpdk_version` column in the datamodel, change ovs-ctl and ovs-dev.py >> to populate the field with the right value. >> >> Signed-off-by: Matteo Croce <mcroce@redhat.com> >> --- > > Sorry I missed some of the development for this - I think it's better to > have the dpdk version retrieved via an appctl command. I can't think of > a need for this information to be persistent - if it's for historical > information when debugging, we can put it in with the logs when dpdk is > started. The user cannot influence DPDK version dynamically - it is not > just read-only, but it's read-only from a compile time decision. I > might be misunderstanding the point of putting this in the DB, though? Hi Aaron, I was threating dpdk_version the same way as ovs_version. ovs_version is saved into the DB by ovs-ctl and it isn't read-only either. What about putting it only in `ovs-vswitchd --version` output and not into the DB?
On Tue, Jan 02, 2018 at 06:18:03PM +0100, Matteo Croce wrote: > On Tue, Jan 2, 2018 at 6:08 PM, Aaron Conole <aconole@redhat.com> wrote: > > Hi Matteo, > > > > Matteo Croce <mcroce@redhat.com> writes: > > > >> Show DPDK version if Open vSwitch is compiled with DPDK support. > >> Add a `dpdk_version` column in the datamodel, change ovs-ctl and ovs-dev.py > >> to populate the field with the right value. > >> > >> Signed-off-by: Matteo Croce <mcroce@redhat.com> > >> --- > > > > Sorry I missed some of the development for this - I think it's better to > > have the dpdk version retrieved via an appctl command. I can't think of > > a need for this information to be persistent - if it's for historical > > information when debugging, we can put it in with the logs when dpdk is > > started. The user cannot influence DPDK version dynamically - it is not > > just read-only, but it's read-only from a compile time decision. I > > might be misunderstanding the point of putting this in the DB, though? > > Hi Aaron, > > I was threating dpdk_version the same way as ovs_version. > ovs_version is saved into the DB by ovs-ctl and it isn't read-only either. > What about putting it only in `ovs-vswitchd --version` output and not > into the DB? One reason that ovs_version is in the database is because a controller might need to know and the controller can't necessarily run "ovs-vswitchd --version" since it might be on a different host. I don't know whether a controller needs to know the DPDK version. I assumed that it did and that that was the motivation for the commit.
Matteo Croce <mcroce@redhat.com> writes: > On Tue, Jan 2, 2018 at 6:08 PM, Aaron Conole <aconole@redhat.com> wrote: >> Hi Matteo, >> >> Matteo Croce <mcroce@redhat.com> writes: >> >>> Show DPDK version if Open vSwitch is compiled with DPDK support. >>> Add a `dpdk_version` column in the datamodel, change ovs-ctl and ovs-dev.py >>> to populate the field with the right value. >>> >>> Signed-off-by: Matteo Croce <mcroce@redhat.com> >>> --- >> >> Sorry I missed some of the development for this - I think it's better to >> have the dpdk version retrieved via an appctl command. I can't think of >> a need for this information to be persistent - if it's for historical >> information when debugging, we can put it in with the logs when dpdk is >> started. The user cannot influence DPDK version dynamically - it is not >> just read-only, but it's read-only from a compile time decision. I >> might be misunderstanding the point of putting this in the DB, though? > > Hi Aaron, > > I was threating dpdk_version the same way as ovs_version. Ahh, okay. I had forgotten that ovs_version (and other information) was in the db as well. Sorry for that - next time I'll have coffee before reviewing. :) > ovs_version is saved into the DB by ovs-ctl and it isn't read-only either. > What about putting it only in `ovs-vswitchd --version` output and not > into the DB? What about using the datapath_version field? That field already gets populated, and I think it may be okay to modify the return value of dpif_netdev_get_datapath_version function when dpdk is enabled. Just a thought.
Hi Matteo, Can we also have DPDK version in the logs (VLOG_INFO) when DPDK is initialized ? May be in dpdk_init() function. This will help in identifying the correct DPDK version from the log files while debugging issues. Warm Regards, Vishal Ajmera -----Original Message----- From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of Aaron Conole Sent: Wednesday, January 03, 2018 12:20 AM To: Matteo Croce <mcroce@redhat.com> Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH v3] ovs-vsctl: show DPDK version Matteo Croce <mcroce@redhat.com> writes: > On Tue, Jan 2, 2018 at 6:08 PM, Aaron Conole <aconole@redhat.com> wrote: >> Hi Matteo, >> >> Matteo Croce <mcroce@redhat.com> writes: >> >>> Show DPDK version if Open vSwitch is compiled with DPDK support. >>> Add a `dpdk_version` column in the datamodel, change ovs-ctl and >>> ovs-dev.py to populate the field with the right value. >>> >>> Signed-off-by: Matteo Croce <mcroce@redhat.com> >>> --- >> >> Sorry I missed some of the development for this - I think it's better >> to have the dpdk version retrieved via an appctl command. I can't >> think of a need for this information to be persistent - if it's for >> historical information when debugging, we can put it in with the logs >> when dpdk is started. The user cannot influence DPDK version >> dynamically - it is not just read-only, but it's read-only from a >> compile time decision. I might be misunderstanding the point of putting this in the DB, though? > > Hi Aaron, > > I was threating dpdk_version the same way as ovs_version. Ahh, okay. I had forgotten that ovs_version (and other information) was in the db as well. Sorry for that - next time I'll have coffee before reviewing. :) > ovs_version is saved into the DB by ovs-ctl and it isn't read-only either. > What about putting it only in `ovs-vswitchd --version` output and not > into the DB? What about using the datapath_version field? That field already gets populated, and I think it may be okay to modify the return value of dpif_netdev_get_datapath_version function when dpdk is enabled. Just a thought.
On Thu, Jan 4, 2018 at 11:50 AM, Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> wrote: > Hi Matteo, > > Can we also have DPDK version in the logs (VLOG_INFO) when DPDK is initialized ? May be in dpdk_init() function. This will help in identifying the correct DPDK version from the log files while debugging issues. > > Warm Regards, > Vishal Ajmera > Sure, makes sense. Thanks,
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in index f1b01d1d3..1e334de2b 100755 --- a/utilities/ovs-ctl.in +++ b/utilities/ovs-ctl.in @@ -76,9 +76,16 @@ set_hostname () { set_system_ids () { set ovs_vsctl set Open_vSwitch . - OVS_VERSION=`ovs-vswitchd --version | sed 's/.*) //;1q'` + OVS_VERSION=`ovs-vswitchd --version |awk '/Open vSwitch/{print $NF}'` set "$@" ovs-version="$OVS_VERSION" + DPDK_VERSION=`ovs-vswitchd --version |awk '/^DPDK/{print$2}'` + if [ -n "$DPDK_VERSION" ]; then + set "$@" dpdk-version="$DPDK_VERSION" + else + ovs_vsctl clear Open_vSwitch . dpdk_version + fi + case $SYSTEM_ID in random) id_file=$etcdir/system-id.conf diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py index 9ce0f04c7..70170e21d 100755 --- a/utilities/ovs-dev.py +++ b/utilities/ovs-dev.py @@ -284,9 +284,16 @@ def run(): "other_config:dpdk-init=true" % root_uuid) _sh("ovs-vsctl --no-wait set Open_vSwitch %s other_config:" "dpdk-extra=\"%s\"" % (root_uuid, ' '.join(options.dpdk))) + dpdk_version = _sh("ovs-vswitchd --version", capture=True) + dpdk_version = [s for s in dpdk_version if "DPDK" in s] + if dpdk_version: + dpdk_version = dpdk_version[0].decode().strip().split()[1] + _sh("ovs-vsctl --no-wait set Open_vSwitch %s dpdk_version=%s" + % (root_uuid, dpdk_version)) else: _sh("ovs-vsctl --no-wait set Open_vSwitch %s " "other_config:dpdk-init=false" % root_uuid) + _sh("ovs-vsctl --no-wait clear Open_vSwitch %s dpdk_version" % root_uuid) if options.gdb: cmd = ["gdb", "--args"] + cmd diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c index d5e07c037..e033a7d1f 100644 --- a/vswitchd/ovs-vswitchd.c +++ b/vswitchd/ovs-vswitchd.c @@ -25,6 +25,10 @@ #include <sys/mman.h> #endif +#ifdef DPDK_NETDEV +#include <rte_version.h> +#endif + #include "bridge.h" #include "command-line.h" #include "compiler.h" @@ -187,6 +191,9 @@ parse_options(int argc, char *argv[], char **unixctl_pathp) case 'V': ovs_print_version(0, 0); +#ifdef DPDK_NETDEV + puts(rte_version()); +#endif exit(EXIT_SUCCESS); case OPT_MLOCKALL: diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema index 90e50b626..7015a2687 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": "654116098 23718", "tables": { "Open_vSwitch": { "columns": { @@ -36,6 +36,9 @@ "db_version": { "type": {"key": {"type": "string"}, "min": 0, "max": 1}}, + "dpdk_version": { + "type": {"key": {"type": "string"}, + "min": 0, "max": 1}}, "system_type": { "type": {"key": {"type": "string"}, "min": 0, "max": 1}}, diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index 37d04b7cf..74890f72b 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -611,6 +611,10 @@ </p> </column> + <column name="dpdk_version"> + DPDK version number, e.g. <code>17.11</code>. + </column> + <column name="system_type"> <p> An identifier for the type of system on top of which Open vSwitch
Show DPDK version if Open vSwitch is compiled with DPDK support. Add a `dpdk_version` column in the datamodel, change ovs-ctl and ovs-dev.py to populate the field with the right value. Signed-off-by: Matteo Croce <mcroce@redhat.com> --- utilities/ovs-ctl.in | 9 ++++++++- utilities/ovs-dev.py | 7 +++++++ vswitchd/ovs-vswitchd.c | 7 +++++++ vswitchd/vswitch.ovsschema | 7 +++++-- vswitchd/vswitch.xml | 4 ++++ 5 files changed, 31 insertions(+), 3 deletions(-)