Re: [ovs-dev] [PATCH v2] ovsdb: Expose vhost-user socket directory in ovsdb

2016-07-20 Thread Mooney, Sean K
Hi sorry for the delay
Replies inline.

> -Original Message-
> From: Aaron Conole [mailto:acon...@redhat.com]
> Sent: Monday, July 11, 2016 6:44 PM
> To: Mooney, Sean K 
> Cc: Flavio Leitner ; dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v2] ovsdb: Expose vhost-user socket directory in
> ovsdb
> 
> "Mooney, Sean K"  writes:
> 
> > "Wojciechowicz, RobertX"  writes:
> >
> >> Hi Ben,
> >>
> >>
> >>> -Original Message-
> >>> From: Ben Pfaff [mailto:blp at ovn.org]
> >>> Sent: Tuesday, July 5, 2016 5:07 PM
> >>> To: Wojciechowicz, RobertX 
> >>> Cc: dev at openvswitch.org
> >>> Subject: Re: [ovs-dev] [PATCH v2] ovsdb: Expose vhost-user socket
> >>> directory in ovsdb
> >>>
> >>> On Mon, Jul 04, 2016 at 07:22:40AM +, Wojciechowicz, RobertX wrote:
> >>> > Hi,
> >>> >
> >>> > > -Original Message-
> >>> > > From: Ben Pfaff [mailto:blp at ovn.org]
> >>> > > Sent: Saturday, July 2, 2016 2:49 AM
> >>> > > To: Wojciechowicz, RobertX 
> >>> > > Cc: dev at openvswitch.org
> >>> > > Subject: Re: [ovs-dev] [PATCH v2] ovsdb: Expose vhost-user
> >>> > > socket
> >>> directory
> >>> > > in ovsdb
> >>> > >
> >>> > > On Mon, Jun 20, 2016 at 10:16:51AM +, Wojciechowicz, RobertX
> >>> wrote:
> >>> > > > Hi,
> >>> > > >
> >>> > > > > -Original Message-
> >>> > > > > From: Ben Pfaff [mailto:blp at ovn.org]
> >>> > > > > Sent: Wednesday, June 8, 2016 10:41 PM
> >>> > > > > To: Wojciechowicz, RobertX  >>> > > > > intel.com>
> >>> > > > > Cc: dev at openvswitch.org
> >>> > > > > Subject: Re: [ovs-dev] [PATCH v2] ovsdb: Expose vhost-user
> >>> > > > > socket
> >>> > > directory
> >>> > > > > in ovsdb
> >>> > > > >
> >>> > > > > On Thu, Jun 02, 2016 at 11:25:56AM +0100, Robert
> >>> > > > > Wojciechowicz
> >>> wrote:
> >>> > > > > > In order to correctly interoperate with Openstack and ODL,
> >>> > > > > > the vhost-user socket directory must be exposed from OVS
> >>> > > > > > via
> >>> OVSDB.
> >>> > > > > > Different distros may package OVS in different ways, so
> >>> > > > > > the locations of these sockets may vary depending on how
> >>> > > > > > ovs-vswitchd has been started. Some clients need
> >>> > > > > > information
> >>> where
> >>> > > > > > the sockets are located when instantiating Qemu virtual 
> >>> > > > > > machines.
> >>> > > > > > The full vhost-user socket directory is constructed from
> >>> > > > > > current OVS working directory and optionally from specified
> subdirectory.
> >>> > > > > > This patch exposes vhost-user socket directory in
> >>> > > > > > Open_vSwitch table in other_config column in two following keys:
> >>> > > > > > 1. ovs-run-dir- OVS working directory
> >>> > > > > > 2. vhost-sock-dir - subdirectory of ovs-run-dir (might be
> >>> > > > > > empty)
> >>> > > > > >
> >>> > > > > > Signed-off-by: Robert Wojciechowicz
> >>> > > 
> >>> > > > > >
> >>> > > > > > v1->v2
> >>> > > > > > - moving vswitch-idl.h dependency inside #ifdef block
> >>> > > > > > - sock_dir_subcomponent initialization with ""
> >>> > > > >
> >>> > > > > Same comment as v1: architecturally, ovs-vswitchd only reads
> >>> > > > > other-config columns, it never writes to them.  Please fix.
> >>> > > >
> >>> > > > If ovs-vswitchd cannot writes to other-config then the only
> >>> > > > place for writing default values to this column I can think of
> >>> > > > is vswitch startup script ovs-ctl.
> >>> &

Re: [ovs-dev] [PATCH RFC 1/1] netdev-dpdk: add DPDK pdump capability

2016-07-29 Thread Mooney, Sean K
This looks like a nice useablity feature to add.
Two questions inline 
Regards sean.

> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Ciara Loftus
> Sent: Friday, July 29, 2016 10:58 AM
> To: dev@openvswitch.org
> Subject: [ovs-dev] [PATCH RFC 1/1] netdev-dpdk: add DPDK pdump capability
> 
> This commit provides the ability to 'listen' on DPDK ports and save packets 
> to a
> pcap file with a DPDK app that uses the librte_pdump library. One such app is 
> the
> 'pdump' app that can be found in the DPDK 'app' directory. Instructions on 
> how to
> use this can be found in INSTALL.DPDK-ADVANCED.md
> 
> The pdump feature is optional. Should you wish to use it, pcap libraries must 
> to
> be installed on the system and the CONFIG_RTE_LIBRTE_PMD_PCAP=y and
> CONFIG_RTE_LIBRTE_PDUMP=y options set in DPDK. Additionally you must set
> the 'dpdk-pdump' ovs other_config DB value to 'true'.
> 
> Signed-off-by: Ciara Loftus 
> ---
>  INSTALL.DPDK-ADVANCED.md | 27 +--
>  NEWS |  1 +
>  acinclude.m4 | 23 +++
>  lib/netdev-dpdk.c| 19 +++
>  vswitchd/vswitch.xml | 12 
>  5 files changed, 80 insertions(+), 2 deletions(-)
> 
> diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md index
> ec1de29..0ffafa3 100644
> --- a/INSTALL.DPDK-ADVANCED.md
> +++ b/INSTALL.DPDK-ADVANCED.md
> @@ -11,7 +11,8 @@ OVS DPDK ADVANCED INSTALL GUIDE  6. [Vhost
> Walkthrough](#vhost)  7. [QOS](#qos)  8. [Rate Limiting](#rl) -9.
> [Vsperf](#vsperf)
> +9. [Pdump](#pdump)
> +10. [Vsperf](#vsperf)
> 
>  ##  1. Overview
> 
> @@ -827,7 +828,29 @@ To clear the ingress policer configuration from the port
> use the following:
> 
>  For more details regarding ingress-policer see the vswitch.xml.
> 
> -##  9. Vsperf
> +##  9. Pdump
> +
> +Pdump allows you to listen on DPDK ports and view the traffic that is
> +passing on them. To use this utility, one must have libpcap installed
> +on the system. Furthermore, DPDK must be built with
> +CONFIG_RTE_LIBRTE_PDUMP=y and CONFIG_RTE_LIBRTE_PMD_PCAP=y. And
> +finally, the following database value must be set before launching the 
> switch,
> like so:
> +
> +`ovs-vsctl set Open_vSwitch . other_config:dpdk-pdump=true`
> +
> +To use pdump, simply launch OVS as usual. Then, navigate to the 'app/pdump'
> +directory in DPDK, 'make' the application and run like so:
> +
> +`sudo ./build/app/dpdk_pdump -- --pdump
> +'port=0,queue=0,rx-dev=/tmp/rx.pcap'`
[Mooney, Sean K] can the dpdk_pdump utility dump non dpdk physical ports such as
Vhost-user ports or dump all queues on a port at the same time? 
Am I correct in saying that Port=0 in this case indicate the first index in the 
dpdk port
list which is dpdk0 from an ovs perspective or is this the port id as shown in 
ovs-appctl dpctl/show?
> +
> +The above command captures traffic received on queue 0 of port 0 and
> +stores it in /tmp/rx.pcap. Other combinations of port numbers, queues
> +numbers and pcap locations are of course also available to use.
> +
> +A small performance decrease is seen when dpdk-pdump=true. This
> +decrease is larger when using a monitoring application like the DPDK pdump
> app.
> +
> +##  10. Vsperf
> 
>  Vsperf project goal is to develop vSwitch test framework that can be used to
> validate the suitability of different vSwitch implementations in a Telco
> deployment diff --git a/NEWS b/NEWS index 32975b0..f59b3b0 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -64,6 +64,7 @@ Post-v2.5.0
>   * Basic connection tracking for the userspace datapath (no ALG,
> fragmentation or NAT support yet)
>   * Support for DPDK 16.07
> + * Optional support for DPDK pdump enabled.
> - Increase number of registers to 16.
> - ovs-benchmark: This utility has been removed due to lack of use and
>   bitrot.
> diff --git a/acinclude.m4 b/acinclude.m4 index faf79eb..0c1dafd 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -211,6 +211,29 @@ AC_DEFUN([OVS_CHECK_DPDK], [
> 
>  AC_SEARCH_LIBS([get_mempolicy],[numa],[],[AC_MSG_ERROR([unable to
> find libnuma, install the dependency package])])
> 
> +AC_COMPILE_IFELSE([
> +  AC_LANG_PROGRAM(
> +[
> +  #include 
> +#if RTE_LIBRTE_PMD_PCAP
> +#error
> +#endif
> +], [])
> +  ], [],
> +  [AC_SEARCH_LIBS([pcap_dump],[pcap],[],[AC_MSG_ERROR([unable to find
> libpcap, install the dependency package])])
> +   DPDK_EXTRA_LIB="-lpcap"
> +   AC_COMPILE_IFELSE([
> + AC_LANG_PR

[ovs-dev] [PATCH v2] ovsdb: Expose vhost-user socket directory in ovsdb

2016-07-08 Thread Mooney, Sean K


"Wojciechowicz, RobertX"  writes:

> Hi Ben,
>
>
>> -Original Message-
>> From: Ben Pfaff [mailto:blp at ovn.org]
>> Sent: Tuesday, July 5, 2016 5:07 PM
>> To: Wojciechowicz, RobertX 
>> Cc: dev at openvswitch.org
>> Subject: Re: [ovs-dev] [PATCH v2] ovsdb: Expose vhost-user socket directory
>> in ovsdb
>>
>> On Mon, Jul 04, 2016 at 07:22:40AM +, Wojciechowicz, RobertX wrote:
>> > Hi,
>> >
>> > > -Original Message-
>> > > From: Ben Pfaff [mailto:blp at ovn.org]
>> > > Sent: Saturday, July 2, 2016 2:49 AM
>> > > To: Wojciechowicz, RobertX 
>> > > Cc: dev at openvswitch.org
>> > > Subject: Re: [ovs-dev] [PATCH v2] ovsdb: Expose vhost-user socket
>> directory
>> > > in ovsdb
>> > >
>> > > On Mon, Jun 20, 2016 at 10:16:51AM +, Wojciechowicz, RobertX
>> wrote:
>> > > > Hi,
>> > > >
>> > > > > -Original Message-
>> > > > > From: Ben Pfaff [mailto:blp at ovn.org]
>> > > > > Sent: Wednesday, June 8, 2016 10:41 PM
>> > > > > To: Wojciechowicz, RobertX 
>> > > > > Cc: dev at openvswitch.org
>> > > > > Subject: Re: [ovs-dev] [PATCH v2] ovsdb: Expose vhost-user socket
>> > > directory
>> > > > > in ovsdb
>> > > > >
>> > > > > On Thu, Jun 02, 2016 at 11:25:56AM +0100, Robert Wojciechowicz
>> wrote:
>> > > > > > In order to correctly interoperate with Openstack and ODL,
>> > > > > > the vhost-user socket directory must be exposed from OVS via
>> OVSDB.
>> > > > > > Different distros may package OVS in different ways,
>> > > > > > so the locations of these sockets may vary depending on how
>> > > > > > ovs-vswitchd has been started. Some clients need information
>> where
>> > > > > > the sockets are located when instantiating Qemu virtual machines.
>> > > > > > The full vhost-user socket directory is constructed from current
>> > > > > > OVS working directory and optionally from specified subdirectory.
>> > > > > > This patch exposes vhost-user socket directory in Open_vSwitch
>> > > > > > table in other_config column in two following keys:
>> > > > > > 1. ovs-run-dir- OVS working directory
>> > > > > > 2. vhost-sock-dir - subdirectory of ovs-run-dir (might be empty)
>> > > > > >
>> > > > > > Signed-off-by: Robert Wojciechowicz
>> > > 
>> > > > > >
>> > > > > > v1->v2
>> > > > > > - moving vswitch-idl.h dependency inside #ifdef block
>> > > > > > - sock_dir_subcomponent initialization with ""
>> > > > >
>> > > > > Same comment as v1: architecturally, ovs-vswitchd only reads
>> > > > > other-config columns, it never writes to them.  Please fix.
>> > > >
>> > > > If ovs-vswitchd cannot writes to other-config then the only place
>> > > > for writing default values to this column I can think of is vswitch
>> > > > startup script ovs-ctl.
>> > > > Basically I tested in my environment the below solution
>> > > > and it seems to solve our issue.
>> > > > Is it acceptable approach?
>> > >
>> > > It looks like you're trying to use other-config to report something,
>> > > instead of to configure something.  That's not what it's for.
>> >
>> > Actually I'm trying to add missing information to the OVSDB.
>> > By default ovs-vswitchd is already configured that vhost-user
>> > sockects are created in the rundir, but this information
>> > is not available in the OVSDB. Third-party scripts, which need
>> > this information are forced to take some guesses about this.
>> > Basically this approach is very similar to storing hostname
>> > in this patch:
>> > http://openvswitch.org/pipermail/dev/2016-March/068511.html
>>
>> There is a difference between external-ids and other-config.
>> other-config is to configure the switch.  That patch uses external-ids.
>
> [RW] Yes, of course, but my point is that the configuration
> currently looks as follows:
> 1. start ovsdb
> 2. vhost-sock-dir is not configured
> 3. start ovs-vswitchd
> 4. ovs-vswitchd in the function dpdk_init__ configures vhost-sock-dir
> from ovs_rundir() and sock_dir_subcomponent
> 5. vhost-sock-dir is now configured, but still there is no information
> in the ovsdb

I don't understand this flow.  Can you tell me what you mean by
vhost-sock-dir is configured but not configured?

[sean-k-mooney]
@aaron Following your change to move the dpdk and vhost-user socket dir 
paramtater
To the db if a non default value for vhost-sockt-dir is used it will be present 
in the ovsdb.
This is good as it can be discovered remotely by odl and reported to openstack 
so that i
can tell libvirt what the relative path is from the ovs run dir.
That is where we have a problem. The ovs run dir is both a compile time constant
That is set via configure flags and can be overridden on the commandline when 
starting ovs.
Because this value is never stored in the ovsdb odl cannot discover it remotely.
As different disto have different conventions for where the run directory 
should be loacated
/var/run/openvswitch vs /usr/var/run/openvswitch we cannot support multiple 
distros
in the same deployment with odl as a controller.

In an openstack deployment when you use odl o

Re: [ovs-dev] [PATCH v9 0/6] Convert DPDK configuration from command line to DB based

2016-03-03 Thread Mooney, Sean K
> -Original Message-
> From: Aaron Conole [mailto:acon...@redhat.com]
> Sent: Wednesday, March 2, 2016 10:52 PM
> To: dev@openvswitch.org
> Cc: Mooney, Sean K ; Flavio Leitner
> ; Daniele Di Proietto ; Andy
> Zhou ; Traynor, Kevin 
> Subject: Re: [ovs-dev] [PATCH v9 0/6] Convert DPDK configuration from
> command line to DB based
> 
> 
> Ping re: this series. At this point, it has lingered for a while; I have
> added some updates to concerns in the following links:
> 
> http://openvswitch.org/pipermail/dev/2016-February/066073.html
> http://openvswitch.org/pipermail/dev/2016-February/066502.html
Hi I tested your v9 patches with a modified version of our devstack plugin it 
worked
As expected. The required modification were trivial as expected so I think the 
same should be
True of any other deployment tool.
> http://openvswitch.org/pipermail/dev/2016-February/066990.html
We have never used ovs-ctl to start/stop ovs partly because of this issue so
Its nice to see this change. 
> 
> Is there anything else I can do? I'm planning on rebasing this series,
> and including the minor fixes from Kevin in my v10. Do I need to include
> anything else?

It probably not for this patch series but maybe as a follow up change it would 
be
Nice to initializes the ovs logging sooner(or possibly configure dpdk to use 
it?). 

You may have noticed that we start ovs as follows 
screen -dms ovs-vswitchd sudo sg $qemu_group -c "umask 002; 
${OVS_INSTALL_DIR}/sbin/ovs-vswitchd -- unix:$OVS_DB_SOCKET 2>&1 | tee 
${OVS_LOG_DIR}/ovs-vswitchd.log

this is because if you use the standard way of logging to a file with ovs it 
does not log the dpdk eal init output so if you have an issue
in dpdk setup it will not appear in the log. From a usability point of view it 
would be nice the dpdk init logs were captures by the standard
logging mechanism. So no other then the change you already plan to make when 
rebasing there is nothing else from me at least.

 
> Aaron Conole  writes:
> > Currently, configuration of DPDK parameters is done via the command
> > line through a --dpdk **OPTIONS** -- command line argument. This has a
> > number of challenges, including:
> > * It must be the first option passed to ovs-vswitchd
> > * It is the only datapath feature in OVS to be configured on the
> > command line
> > * It requires specialized knowledge of sub-component command switches
> > * It also inteprets non-EAL arguments (confusing users)
> > * It is a broken model.
> >
> >
> > This series brings the following changes to openvswitch:
> > * All DPDK options are taken from the ovs database rather than the
> >   command line
> > * Non-EAL arguments also have separate database entries
> > * DPDK lcores are optionally auto-assigned to a single core based on
> the
> >   bridge coremask.
> > * DPDK options have default behaviors
> > * Updated documentation
> >
> > v2:
> > * Dropped the vhost-user socket configuration options. Those can be
> re-added
> >   as an extension
> > * Incorporated feedback from Kevin Traynor.
> >
> > v3:
> > * Went back to a global dpdk-init
> > * Language cleanup and various minor fixes
> >
> > v4:
> > * Added a way to pass arbitrary eal arguments
> >
> > v5:
> > * Restore the socket-mem default, and fix up the ovs-dev.py script,
> along
> >   with the manpage for ovsdb-server
> >
> > v6:
> > * Correct a documentation issue with INSTALL.DPDK.md
> > * Correct a non-dpdk enabled OVS incorrect warning variable
> > * Remove an excess whitespace
> >
> > v7:
> > * After testing by Christian with dpdk-alloc-mem
> >
> > v8:
> > * Confirmed ``make check`` operation with and without dpdk.
> >   Retested on live-host
> >
> > v9:
> > * Cleanup of comments
> > * Cleanup of one place where headers are specified
> > * Mark the dpdk coremask and numa config as optional
> > * Added 5/6 to scan the extras and warn the user when conflicting
> >   DB entries are present
> > * Acks given for all but patch 5/6
> >
> > Aaron Conole (5):
> >   netdev-dpdk: Restore thread affinity after DPDK init
> >   netdev-dpdk: Convert initialization from cmdline to db
> >   netdev-dpdk: Autofill lcore coremask if absent
> >   netdev-dpdk: Allow arbitrary eal arguments
> >   NEWS: Announce the DPDK EAL configuration change
> >
> >  FAQ.md |   6 +-
> >  INSTALL.DPDK.md|  90 ++---
> >  NEWS   |   5 +
> >  lib/netdev-dpdk.c  | 327
> ++---
> >  lib/netdev-dpdk.h  |  22 ++-
> >  utilities/ovs-dev.py   |   7 +-
> >  vswitchd/bridge.c  |   3 +
> >  vswitchd/ovs-vswitchd.8.in |   5 +-
> >  vswitchd/ovs-vswitchd.c|  25 +---
> >  vswitchd/vswitch.xml   | 128 +-
> >  10 files changed, 515 insertions(+), 107 deletions(-)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v11 2/8] util: Add a path canonicalizer

2016-04-12 Thread Mooney, Sean K


> -Original Message-
> From: Aaron Conole [mailto:acon...@redhat.com]
> Sent: Tuesday, April 12, 2016 1:12 AM
> To: Ben Pfaff 
> Cc: dev@openvswitch.org; Flavio Leitner ; Traynor,
> Kevin ; Panu Matilainen ;
> Wojciechowicz, RobertX ; Mooney, Sean K
> ; Andy Zhou ; Daniele Di
> Proietto ; Zoltan Kiss ;
> Christian Ehrhardt 
> Subject: Re: [PATCH v11 2/8] util: Add a path canonicalizer
> 
> Hi Ben,
> 
> Ben Pfaff  writes:
> 
> > (Yow, that's a lot of CCs.)
> 
> Lots of cooks in the kitchen on this one.
> 
> > On Fri, Apr 01, 2016 at 11:31:31AM -0400, Aaron Conole wrote:
> >> This commit adds a new function (ovs_realpath) to perform the role of
> >> realpath on various operating systems. The purpose is to ensure that
> >> a given path to file exists, and to return a completely resolved path
> >> (sans '.' and '..').
> >>
> >> Signed-off-by: Aaron Conole 
> >
> > Path canonicalization is a pretty big hammer.  In other cases where
> > OVS relies on an absolute path, it uses textual comparison on a prefix
> > of the name (representing a directory) and rejects use of ".."
> > following the prefix.  This is pretty easy to get right, and it is not
> > as heavyweight, and it does not have to actually do file system
> > operations (stat, readlink, ...), and its verdict can't change as a
> > result of changes to the file system (e.g. new or modified or deleted
> > symlinks, NFS servers that are down), and so on.
> >
> > Do you think we really need path canonicalization?
> 
> I was nervous about a user putting escapes in the code. Unlike with,
> say, vhost user filenames (where we just blanket deny '/' because the
> semantic is of a file) this is not a file specification, but a directory
> specification. That implies that we would have to keep state and test
> for /../, and ../ (at the beginning of the string), at the least.
> 
> If you think it's safe to merely test for the presence of these and
> bail, I'm okay with it, but I didn't want to leave any possibility that
> a malicious DB user could escape out of the rundir when changing the
> vhost-socket dir.
[Mooney, Sean K] 
currently  it is possible to use any dir for the vhost-socket dir if the 
absolute path is 
specified on the ovs-vswitchd command line correct?
I know for a time we used /tmp/ for the sockets. I have also seen 
/dev/vhostuser/ used.
The run dir (/var/run/openvswitch or /usr/var/run/openvswitch) makes the most 
sense
To me as a default.

More of a clarifying question but are you proposing restricting the localtions
where the vhost-user sockets can be stored or constraining the path format to 
allow/deny
relative path specifiers such as "." and ".." and converting that path in an 
canonical absolute path?

I don't have a strong preference but just wanted to make sure I understand what 
is being proposed
And what flexibility we would be gaining/losing.

> 
> I do agree it's heavy.
> 
> > Thanks,
> 
> Thanks for the review!
> 
> > Ben.

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v11 2/8] util: Add a path canonicalizer

2016-04-12 Thread Mooney, Sean K


> -Original Message-
> From: Aaron Conole [mailto:acon...@redhat.com]
> Sent: Tuesday, April 12, 2016 2:18 PM
> To: Mooney, Sean K 
> Cc: Ben Pfaff ; dev@openvswitch.org; Flavio Leitner
> ; Traynor, Kevin ; Panu
> Matilainen ; Wojciechowicz, RobertX
> ; Andy Zhou ; Daniele Di
> Proietto ; Zoltan Kiss ;
> Christian Ehrhardt 
> Subject: Re: [PATCH v11 2/8] util: Add a path canonicalizer
> 
> "Mooney, Sean K"  writes:
> 
> >> -Original Message-
> >> From: Aaron Conole [mailto:acon...@redhat.com]
> >> Sent: Tuesday, April 12, 2016 1:12 AM
> >> To: Ben Pfaff 
> >> Cc: dev@openvswitch.org; Flavio Leitner ; Traynor,
> >> Kevin ; Panu Matilainen
> >> ; Wojciechowicz, RobertX
> >> ; Mooney, Sean K
> >> ; Andy Zhou ; Daniele Di
> >> Proietto ; Zoltan Kiss
> >> ; Christian Ehrhardt
> >> 
> >> Subject: Re: [PATCH v11 2/8] util: Add a path canonicalizer
> >>
> >> Hi Ben,
> >>
> >> Ben Pfaff  writes:
> >>
> >> > (Yow, that's a lot of CCs.)
> >>
> >> Lots of cooks in the kitchen on this one.
> >>
> >> > On Fri, Apr 01, 2016 at 11:31:31AM -0400, Aaron Conole wrote:
> >> >> This commit adds a new function (ovs_realpath) to perform the role
> >> >> of realpath on various operating systems. The purpose is to ensure
> >> >> that a given path to file exists, and to return a completely
> >> >> resolved path (sans '.' and '..').
> >> >>
> >> >> Signed-off-by: Aaron Conole 
> >> >
> >> > Path canonicalization is a pretty big hammer.  In other cases where
> >> > OVS relies on an absolute path, it uses textual comparison on a
> >> > prefix of the name (representing a directory) and rejects use of
> ".."
> >> > following the prefix.  This is pretty easy to get right, and it is
> >> > not as heavyweight, and it does not have to actually do file system
> >> > operations (stat, readlink, ...), and its verdict can't change as a
> >> > result of changes to the file system (e.g. new or modified or
> >> > deleted symlinks, NFS servers that are down), and so on.
> >> >
> >> > Do you think we really need path canonicalization?
> >>
> >> I was nervous about a user putting escapes in the code. Unlike with,
> >> say, vhost user filenames (where we just blanket deny '/' because the
> >> semantic is of a file) this is not a file specification, but a
> >> directory specification. That implies that we would have to keep
> >> state and test for /../, and ../ (at the beginning of the string), at
> the least.
> >>
> >> If you think it's safe to merely test for the presence of these and
> >> bail, I'm okay with it, but I didn't want to leave any possibility
> >> that a malicious DB user could escape out of the rundir when changing
> >> the vhost-socket dir.
> > [Mooney, Sean K]
> > currently it is possible to use any dir for the vhost-socket dir if
> > the absolute path is specified on the ovs-vswitchd command line
> > correct?
> > I know for a time we used /tmp/ for the sockets. I have also seen
> > /dev/vhostuser/ used.
> > The run dir (/var/run/openvswitch or /usr/var/run/openvswitch) makes
> > the most sense To me as a default.
> >
> > More of a clarifying question but are you proposing restricting the
> > localtions where the vhost-user sockets can be stored or constraining
> > the path format to allow/deny relative path specifiers such as "." and
> > ".." and converting that path in an canonical absolute path?
> >
> > I don't have a strong preference but just wanted to make sure I
> > understand what is being proposed And what flexibility we would be
> > gaining/losing.
> 
> Sure; this would restrict all paths specified in the string to being
> subpaths of the ovs_rundir(). So, your case of using /tmp or
> /dev/vhostuser/ would not work. On the other hand, a path like
> vhostuser/ would be treated as /var/run/openvswitch/vhostuser/. The code
> is merely attempting to prevent arbitrary directory escapes by using the
> realpath system call.
> 
> I added this after
> http://openvswitch.org/pipermail/dev/2016-March/068743.html
[Mooney, Sean K] 
Ah ok that restriction makes sense in that context.
We swapped to using the ovs run directory (e.g. /var/run/openvswitch/)
when the vhost-user patches were merge to ovs so this will not affect
the OpenStack integration with Neutron or odl so this seems ok to me.

Thanks for clarifying.

> 
> Thanks,
> -Aaron
> 
> >>
> >> I do agree it's heavy.
> >>
> >> > Thanks,
> >>
> >> Thanks for the review!
> >>
> >> > Ben.
> >

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v4 3/3] netdev-dpdk: vHost client mode and reconnect

2016-08-15 Thread Mooney, Sean K
Hi Daniele
Sorry to top post but I have just read back over the last
couple of revisions of this patch.
I noticed that you requested that the vhost-driver-mode flag be removed
From the Open_vSwitch table. The vhost-driver-mode flag was included in
The original patch for two reasons 1 to configure the global driver mode
And 2 to provide a way to detect if reconnect/qemu server mode was available.

Without the global flag or a similar mechanism to expose the capability via
The ovsdb I cannot complete the OpenStack integrations.
https://review.openstack.org/#/c/344997/

I have one proposal which is to store the feature list currently in the faq 
https://github.com/openvswitch/ovs/blob/master/FAQ.md#q-are-all-features-available-with-all-datapaths
in the ovsdb. This can be retrieve remotely via the ovs db by openstack or any 
other orchestrator to
make dession based on the feature detected.
 
If you have another suggestion I would be glad to adapt my OpenStack change
To use another mechanism to detect the support for reconnect but without the
vhost-driver-mode flag I am currently blocked.

I will make as a separate thread not to discuss feature discovery in general
Not to  distract from this review
Regards
Sean.


> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di
> Proietto
> Sent: Saturday, August 13, 2016 1:41 AM
> To: Loftus, Ciara 
> Cc: dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v4 3/3] netdev-dpdk: vHost client mode
> and reconnect
> 
> Thanks for the patch, I tried it and it makes possible to restart
> vswitchd and qemu.
> 
> I believe that now vhost_server_id and vhost_client_id are not constant
> for the lifetime of the struct and must be protected with dev->mutex.
> 
> The following incremental on top of your patch does that and remove
> extra parentheses from sizeof operator:
> 
> -/* Identifiers used to distinguish vhost devices from each other.
> They
> do
> - * not change during the lifetime of a struct netdev_dpdk. They
> can be
> read
> - * without holding any mutex. */
> -const char vhost_server_id[PATH_MAX];
> -const char vhost_client_id[PATH_MAX];
> +/* Identifiers used to distinguish vhost devices from each other.
> */
> +char vhost_server_id[PATH_MAX];
> +char vhost_client_id[PATH_MAX];
> 
>  /* In dpdk_list. */
>  struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex); @@ -837,6
> +835,7 @@ dpdk_dev_parse_name(const char dev_name[], const char
> prefix[],
>   * use */
>  static const char *
>  get_vhost_id(struct netdev_dpdk *dev)
> +OVS_REQUIRES(dev->mutex)
>  {
>  return dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT ?
> dev->vhost_client_id : dev->vhost_server_id; @@ -867,20
> +866,20 @@ netdev_dpdk_vhost_construct(struct netdev *netdev)
>  /* Take the name of the vhost-user port and append it to the
> location where
>   * the socket is to be created, then register the socket.
>   */
> -snprintf(CONST_CAST(char *, dev->vhost_server_id),
> - sizeof(dev->vhost_server_id), "%s/%s", vhost_sock_dir,
> name);
> +snprintf(dev->vhost_server_id, sizeof dev->vhost_server_id,
> "%s/%s",
> + vhost_sock_dir, name);
> 
> -err = rte_vhost_driver_register(get_vhost_id(dev),
> +err = rte_vhost_driver_register(dev->vhost_server_id,
>  dev->vhost_driver_flags);
>  if (err) {
>  VLOG_ERR("vhost-user socket device setup failure for socket
> %s\n",
> - get_vhost_id(dev));
> + dev->vhost_server_id);
>  } else {
>  if (!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT)) {
>  /* OVS server mode - add this socket to list for deletion
> */
> -fatal_signal_add_file_to_unlink(get_vhost_id(dev));
> +fatal_signal_add_file_to_unlink(dev->vhost_server_id);
>  VLOG_INFO("Socket %s created for vhost-user port %s\n",
> -  get_vhost_id(dev), name);
> +  dev->vhost_server_id, name);
>  }
>  err = netdev_dpdk_init(netdev, -1, DPDK_DEV_VHOST);
>  }
> @@ -935,17 +934,19 @@ netdev_dpdk_destruct(struct netdev *netdev)
>   * try to acquire 'dpdk_mutex' and possibly 'dev->mutex'.  To avoid a
>   * deadlock, none of the mutexes must be held while calling this
> function.
> */
>  static int
> -dpdk_vhost_driver_unregister(struct netdev_dpdk *dev)
> +dpdk_vhost_driver_unregister(struct netdev_dpdk *dev OVS_UNUSED,
> + const char *vhost_id)
>  OVS_EXCLUDED(dpdk_mutex)
>  OVS_EXCLUDED(dev->mutex)
>  {
> -return rte_vhost_driver_unregister(get_vhost_id(dev));
> +return rte_vhost_driver_unregister(vhost_id);
>  }
> 
>  static void
>  netdev_dpdk_vhost_destruct(struct netdev *netdev)  {
>  struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> +char *vhost_id;
> 
>  ovs_mutex_lock(&dpdk_mutex);
>  ovs_mutex_lock(&de

[ovs-dev] Remote OVS feature discovery

2016-08-15 Thread Mooney, Sean K
Hi I would like to bring up a recurring problem that
Has arisen several times with enabling new ovs features in
OpenStack.

OpenStack neutron  supports many different network backend
Including ovs which is currently the most common .
As OpenStack has to support multiple versions of ovs both to provide
Multi distro support and to support rolling upgrades new ovs features
That are consumed in openstack must be enabled dynamically based on the
Capabilities of the vswitch on the target server.

An example of this today is vhost-user support. if ovs is compiled with dpdk 
support
And started with dpdk enabled the ifaces_types field in the Open_vSwitch table 
will
Contain the dpdk_vhostuser port type. If the dpdk_vhostuser port type is found 
and the bridges
Datapath type Is netdev then vhost-user is enabled for that platform in neutron.

There are many other feature that cannot be detected remotely today.
The ovs FAQ lists a number of features that are available on different data 
paths that openstack and other
Systems may care to know about so that it can use that feature when present and
take appropriate action when not. e.g. place vm on a different host running a 
different ovs version or fallback
to a different code path if a new feature is not available
https://github.com/openvswitch/ovs/blob/master/FAQ.md#q-are-all-features-available-with-all-datapaths

what I would like to ask is if we can introduce a new ovsdb table to declare 
the features available in the current ovs version.
This would involve converting the current tables in the FAQ feature section in 
to static entries in the ovsdb schema in a new
Feature table.
Each entry in the table would be readonly and the table can be upgraded in each 
release by ovsdb-tool convert to migrate/upgreade
The schema of the current db.

Note the reason I am suggesting using the ovsdb to declare the features is that 
for odl and to a less extent ovn and openstack
The ovsdb is the only interface that can be used to interact with the vswitch 
remotely.
In the special case of odl neither odl or openstack neutron have an agent on 
the server running ovs
so only ovsdb and openflow can be used. As a result tools such as ovs-appctl or 
any of the other ovs-* tools cannot be used
to support this usecase.

Currently there are 3 feature that will require this or a similar feature to 
enable in openstack.
Those feature are:
Vhost-user reconnect /  vhost- user  qemu:server dpdk:client mode
UserSpace jumbo frames support
Connection tracking

Other features that this would be required for in the future include ovs 
NAT,QOS polices,NSH and any other feature that is not
Enabled on all datapath concurrently. I would expect that ovn would need a 
subset of this information to be reported in the chassis table
also in the future to enable more advanced use cases.

Regards
Sean.



___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] NSH Option 2 implementation

2016-08-15 Thread Mooney, Sean K


> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Jesse Gross
> Sent: Monday, August 15, 2016 6:12 PM
> To: Jan Scheurich 
> Cc: dev@openvswitch.org; Manuel Buil ; Simon
> Horman ; László Sürü
> ; Miguel Angel Muñoz Gonzalez
> ; Multanen, Eric W
> 
> Subject: Re: [ovs-dev] NSH Option 2 implementation
> 
> On Fri, Aug 12, 2016 at 1:01 AM, Jan Scheurich
>  wrote:
> > The only clean way to avoid that now would be to maintain the
> monolithic push/pop_nsh semantic of the Yi Yang patch and always
> push/pop NSH header and outer MAC header together. Together with
> Simon's patch we could still achieve NSH over VXLAN-GPE, if the VXLAN-
> GPE tunnel is configured as non-Ethernet tunnel, as the unneeded
> Ethernet header would be popped at transmission to the tunnel.
> >
> > I believe we to decide whether we want to stick to the Ethernet-only
> pipeline or start implementing support for packet-type aware pipeline
> in OVS for the introduction of NSH. Anything in between would be
> conceptually problematic and not in line with OF 1.5.
> 
> I'm still not enthusiastic about having a combined push NSH/Ethernet
> action. I don't think that it even really supports the full NSH use
> case since at least some of the patches sent are not using VXLAN-GPE
> but directly insert the header over Ethernet. I wouldn't want to
> introduce an action that would immediately need to be replaced since we
> won't be able to change any of the public interface semantics once this
> goes in.
[Mooney, Sean K] sorry I don’t know the context of the rest of the thread
But from an neutron ovs agent point of view I think having separate push/pop 
nsh and push
Pop Ethernet would make the most sense. nsh with Ethernet transport is much
More appealing give how openstack tenant network already work.
Vxlan-gpe with openstack would end in a double encaped packet on the wire.
The vxlan-gpe encapsulation + the tenant network 
encapsulation(flat,vlan,vxlan,geneve...).
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] Remote OVS feature discovery

2016-08-15 Thread Mooney, Sean K


From: Daniele Di Proietto [mailto:diproiet...@ovn.org]
Sent: Monday, August 15, 2016 6:29 PM
To: Mooney, Sean K 
Cc: dev@openvswitch.org; Loftus, Ciara 
Subject: Re: [ovs-dev] Remote OVS feature discovery

Hi Sean,
I'm not familiar with OpenStack, so I'm not sure that my comments make sense 
for every possible use case.
I'm not 100% sure an explicit feature discovery interface is required.  If an 
interface is well designed it should be possible to discover features by 
"probing".  We never had any problem doing that with the datapath netlink 
interface, see for example check_support() in ofproto/ofproto-dpif.c
[Mooney, Sean K] in the past OpenStack neutron has not allowed the use of 
active probing to detect features. We can query the ovsdb but not write to it.
Going feature by feature:
1. Connection tracking
This can easily be detected by trying to set up a flow with a connection 
tracking action.  If the flow setup succeeds it means that the datapath 
supports connection tracking.
[Mooney, Sean K] this approach has been reject by the neutron core team in the 
past though It is something we could bring  up again.
It was previously seen as a potential security risk though if you used a 
dedicate openflow table that is never otherwise used then
It may  be acceptable. Its been about 2 years since I last suggested actively 
probing in that case to detect vhost-user before the
iface_types field was added.

2. Nat
Same as connection tracking
3. Vhost-client mode
The interface needs to be redefined.  The fact that's not easy to do feature 
detection probably means that the interface is not well defined
4. Jumbo frames
We added another column in the Interface table.  By looking at the schema it 
should be possible to determine if the datapath supports the feature.
[Mooney, Sean K] yes in this case a ovs-vsctl get or similar to check if the 
column is defined should work well though we can’t check the schema directly 
and must query the
Ovsdb. This does not work for vhost-client mode as it would be storing the 
vhost-user socket path in the other_config section of the port as far as I 
recall.


I'm sure we should be able to do the same with NSH and all the other new 
features.
[Mooney, Sean K] for nsh assuming we have openflow push/pop action we could 
possible do an active probe when we started the ovs neutron agent but again 
last time I suggested this in
Neutron active probes were not accepted.
if ovs-vsctl --dry-run or ovs-ofctl --dry-run can be used to test if a feature 
can be enabled then that might be accepted in neutron unfortunately this is not 
the case
for example  sudo ovs-vsctl --dry-run --no-wait add-port br-int fake-port -- 
set interface fake-port type=fake-type ; echo $?
will print 0 as the command will always succeed as the ovsdb does not validate 
the type is one of the registered types. The validation
if added would have to be done server side as the client would not always be 
used i.e when odl is managing ovs.

This is just my opinion based on my experience so far.  If some of the 
developers think this is too hacky, maybe it's fine to explicitly export the 
supported features.
[Mooney, Sean K] well the main reason I brought this up is often features are 
added to ovs that require active probing to detect which to date were not 
allowed to use in neutron.
the other drawback to this is that for every new feature that is added to ovs 
we have to come up with a new way to detect if its there or not.
im not sure if exporting  the supported features explicitly is the right 
solution but  it would provide a standardized interface to check for feature x.
this is more of an open question in general is remote passive ovs feature 
discovery something that people think is useful or is active probing the only
thing that ovs will support in this regard.
Thanks,
Daniele



2016-08-15 9:55 GMT-07:00 Mooney, Sean K 
mailto:sean.k.moo...@intel.com>>:
Hi I would like to bring up a recurring problem that
Has arisen several times with enabling new ovs features in
OpenStack.

OpenStack neutron  supports many different network backend
Including ovs which is currently the most common .
As OpenStack has to support multiple versions of ovs both to provide
Multi distro support and to support rolling upgrades new ovs features
That are consumed in openstack must be enabled dynamically based on the
Capabilities of the vswitch on the target server.

An example of this today is vhost-user support. if ovs is compiled with dpdk 
support
And started with dpdk enabled the ifaces_types field in the Open_vSwitch table 
will
Contain the dpdk_vhostuser port type. If the dpdk_vhostuser port type is found 
and the bridges
Datapath type Is netdev then vhost-user is enabled for that platform in neutron.

There are many other feature that cannot be detected remotely today.
The ovs FAQ lists a number of features that are available on different data 
paths that openstack and other
Sys

Re: [ovs-dev] Remote OVS feature discovery

2016-08-16 Thread Mooney, Sean K


> -Original Message-
> From: Aaron Conole [mailto:acon...@redhat.com]
> Sent: Monday, August 15, 2016 9:57 PM
> To: Daniele Di Proietto 
> Cc: Mooney, Sean K ; dev@openvswitch.org
> Subject: Re: [ovs-dev] Remote OVS feature discovery
> 
> Daniele Di Proietto  writes:
> 
> > 2016-08-15 11:39 GMT-07:00 Mooney, Sean K :
> >
> >>
> >>
> >>
> >>
> >> *From:* Daniele Di Proietto [mailto:diproiet...@ovn.org]
> >> *Sent:* Monday, August 15, 2016 6:29 PM
> >> *To:* Mooney, Sean K 
> >> *Cc:* dev@openvswitch.org; Loftus, Ciara 
> >> *Subject:* Re: [ovs-dev] Remote OVS feature discovery
> >>
> >>
> >>
> >> Hi Sean,
> >>
> >> I'm not familiar with OpenStack, so I'm not sure that my comments
> >> make sense for every possible use case.
> >>
> >> I'm not 100% sure an explicit feature discovery interface is
> >> required.  If an interface is well designed it should be possible to
> >> discover features by "probing".  We never had any problem doing that
> >> with the datapath netlink interface, see for example check_support()
> >> in ofproto/ofproto-dpif.c
> >>
> >> *[Mooney, Sean K] in the past OpenStack neutron has not allowed the
> >> use of active probing to detect features. We can query the ovsdb but
> >> not write to
> >> it.*
> >>
> >> Going feature by feature:
> >>
> >> 1. Connection tracking
> >>
> >> This can easily be detected by trying to set up a flow with a
> >> connection tracking action.  If the flow setup succeeds it means
> that
> >> the datapath supports connection tracking.
> >>
> >>
> >>
> >>
> >>
> >> *[Mooney, Sean K] this approach has been reject by the neutron core
> >> team in the past though It is something we could bring  up again. It
> >> was previously seen as a potential security risk though if you used
> a
> >> dedicate openflow table that is never otherwise used then It may  be
> >> acceptable. Its been about 2 years since I last suggested actively
> >> probing in that case to detect vhost-user before the iface_types
> >> field was added.*
> >>
> > I see.  I'm sure we can come up with other ways to probe the
> conntrack
> > feature that don't require changes to a table.  Perhaps we need some
> > changes in OvS, but I don't see why we should involve ovsdb into
> > something that can be handled entirely in OpenFlow.
> >
> >
> >>
> >>
> >> 2. Nat
> >>
> >> Same as connection tracking
> >>
> >> 3. Vhost-client mode
> >>
> >> The interface needs to be redefined.  The fact that's not easy to do
> >> feature detection probably means that the interface is not well
> >> defined
> >>
> >> 4. Jumbo frames
> >>
> >> We added another column in the Interface table.  By looking at the
> >> schema it should be possible to determine if the datapath supports
> the feature.
> >>
> >> *[Mooney, Sean K] yes in this case a ovs-vsctl get or similar to
> >> check if the column is defined should work well though we can’t
> check
> >> the schema directly and must query the*
> >>
> >> *Ovsdb. This does not work for vhost-client mode as it would be
> >> storing the vhost-user socket path in the other_config section of
> the
> >> port as far as I recall.*
> >>
> >
> > Right, the interface to vhostuser client mode doesn't allow that,
> > that's why I suggested changing the interface in response to your
> comments.
> >
> >
> >>
> >>
> >> I'm sure we should be able to do the same with NSH and all the other
> >> new features.
> >>
> >> *[Mooney, Sean K] for nsh assuming we have openflow push/pop action
> >> we could possible do an active probe when we started the ovs neutron
> >> agent but again last time I suggested this in*
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >> *Neutron active probes were not accepted. if ovs-vsctl --dry-run or
> >> ovs-ofctl --dry-run can be used to test if a feature can be enabled
> >> then that might be accepted in neutron unfortunately this is not the
> >> case for example  sudo ovs-vsctl --dry-run --no-wait add-port br-int
> >> fake-port -- set interface fake-port type=fake-type ;

Re: [ovs-dev] [PATCH v4 3/3] netdev-dpdk: vHost client mode and reconnect

2016-08-17 Thread Mooney, Sean K
hi Daniele. Sorry I missed this message but Ciara just pointed this out to me.

From: Daniele Di Proietto [mailto:diproiet...@ovn.org]
Sent: Monday, August 15, 2016 6:10 PM
To: Mooney, Sean K 
Cc: Loftus, Ciara ; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v4 3/3] netdev-dpdk: vHost client mode and 
reconnect

Hi Sean,

2016-08-15 9:04 GMT-07:00 Mooney, Sean K 
mailto:sean.k.moo...@intel.com>>:
Hi Daniele
Sorry to top post but I have just read back over the last
couple of revisions of this patch.

No problem, thanks for stepping in.  As I said many times during the review of 
this series I'm not sure what the best interface would be, and I really 
appreciate any feedback on this.

I noticed that you requested that the vhost-driver-mode flag be removed
From the Open_vSwitch table. The vhost-driver-mode flag was included in
The original patch for two reasons 1 to configure the global driver mode

In my opinion Open vSwitch is not the right place to store this global 
configuration parameter.  I think we should put it as high in the stack as 
possible.  Isn't there another place to store it in OpenStack?
[Mooney, Sean K] OpenStack make the choice instead of requiring ovs to store 
this information as a global parameter as long as we can detect if the feature 
is available.

And 2 to provide a way to detect if reconnect/qemu server mode was available.

I see your point, we need feature detection for this.
Perhaps we can use another type for client ports, like "dpdkvhostuserclient".  
I think it make senses to have a separate class, since the interface is 
different anyway.  It would be easy then to detect the feature based on the 
available iface_types.
[Mooney, Sean K] Yes if we keep the current dpdkvhostuser port type for 
qemu:clinet dpdk:server mode
and introduced a "dpdkvhostuserclient" for qemu the new qemu:server dpdk:clinet 
mode of vhost-user that would work perfectly for my usecase.
it would be a trivial change in openstack and this should work equally well in 
odl and ovn.
Could this be included in the 2.6 release?
What do you think?
Thanks,
Daniele


Without the global flag or a similar mechanism to expose the capability via
The ovsdb I cannot complete the OpenStack integrations.
https://review.openstack.org/#/c/344997/

I have one proposal which is to store the feature list currently in the faq
https://github.com/openvswitch/ovs/blob/master/FAQ.md#q-are-all-features-available-with-all-datapaths
in the ovsdb. This can be retrieve remotely via the ovs db by openstack or any 
other orchestrator to
make dession based on the feature detected.

If you have another suggestion I would be glad to adapt my OpenStack change
To use another mechanism to detect the support for reconnect but without the
vhost-driver-mode flag I am currently blocked.

I will make as a separate thread not to discuss feature discovery in general
Not to  distract from this review
Regards
Sean.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] netdev-dpdk: Add new 'dpdkvhostuserclient' port type

2016-08-19 Thread Mooney, Sean K
Hi I have updated my openstack changes
https://review.openstack.org/#/c/344997/ (neutron)
https://review.openstack.org/#/c/357555/  (os-vif)
https://review.openstack.org/#/c/334048/ (nova)
to work with this change and tested it with the v1 patch.
As far as I can tell the only change in v2 is in the install.dpdk-advanced and 
Commit message but I can retest with v2 also if desired.

Time permitting assuming this change is accepted I will also submit a patch to 
networking-ovn
And networking-odl Next week to complete enabling the feature in each of the
Main ovs compatible neutron backends.

> -Original Message-
> From: Loftus, Ciara
> Sent: Friday, August 19, 2016 10:23 AM
> To: dev@openvswitch.org
> Cc: diproiet...@vmware.com; Mooney, Sean K ;
> Loftus, Ciara 
> Subject: [PATCH v2] netdev-dpdk: Add new 'dpdkvhostuserclient' port
> type
> 
> The 'dpdkvhostuser' port type no longer supports both server and client
> mode. Instead, 'dpdkvhostuser' ports are always 'server' mode and
> 'dpdkvhostuserclient' ports are always 'client' mode.
> 
> Suggested-by: Daniele Di Proietto 
> Signed-off-by: Ciara Loftus 
> ---
>  INSTALL.DPDK-ADVANCED.md | 102 +++
>  NEWS |   1 +
>  lib/netdev-dpdk.c| 176 ++-
> 
>  vswitchd/vswitch.xml |   8 +--
>  4 files changed, 159 insertions(+), 128 deletions(-)
> 
> diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md index
> 857c805..d7b9873 100755
> --- a/INSTALL.DPDK-ADVANCED.md
> +++ b/INSTALL.DPDK-ADVANCED.md
> @@ -461,6 +461,21 @@ For users wanting to do packet forwarding using
> kernel stack below are the steps
>   ```
> 
>  ##  6. Vhost Walkthrough
> +
> +Two types of vHost User ports are available in OVS:
> +
> +1. vhost-user (dpdkvhostuser ports)
> +
> +2. vhost-user-client (dpdkvhostuserclient ports)
> +
> +vHost User uses a client-server model. The server
> +creates/manages/destroys the vHost User sockets, and the client
> +connects to the server. Depending on which port type you use,
> +dpdkvhostuser or dpdkvhostuserclient, a different configuration of the
> client-server model is used.
> +
> +For vhost-user ports, OVS DPDK acts as the server and QEMU the client.
> +For vhost-user-client ports, OVS DPDK acts as the client and QEMU the
> server.
> +
>  ### 6.1 vhost-user
> 
>- Prerequisites:
> @@ -570,49 +585,6 @@ For users wanting to do packet forwarding using
> kernel stack below are the steps
> where `-L`: Changes the numbers of channels of the specified
> network device
> and `combined`: Changes the number of multi-purpose channels.
> 
> -4. OVS vHost client-mode & vHost reconnect (OPTIONAL)
> -
> -   By default, OVS DPDK acts as the vHost socket server for
> dpdkvhostuser
> -   ports and QEMU acts as the vHost client. This means OVS creates
> and
> -   manages the vHost socket and QEMU is the client which connects
> to the
> -   vHost server (OVS). In QEMU v2.7 the option is available for
> QEMU to act
> -   as the vHost server meaning the roles can be reversed and OVS
> can become
> -   the vHost client. To enable client mode for a given
> dpdkvhostuserport,
> -   one must specify a valid 'vhost-server-path' like so:
> -
> -   ```
> -   ovs-vsctl set Interface dpdkvhostuser0 options:vhost-server-
> path=/path/to/socket
> -   ```
> -
> -   Setting this value automatically switches the port to client
> mode (from
> -   OVS' perspective). 'vhost-server-path' reflects the full path
> of the
> -   socket that has been or will be created by QEMU for the given
> vHost User
> -   port. Once a path is specified, the port will remain in
> 'client' mode
> -   for the remainder of it's lifetime ie. it cannot be reverted
> back to
> -   server mode.
> -
> -   One must append ',server' to the 'chardev' arguments on the
> QEMU command
> -   line, to instruct QEMU to use vHost server mode for a given
> interface,
> -   like so:
> -
> -   
> -   -chardev socket,id=char0,path=/path/to/socket,server
> -   
> -
> -   If the corresponding dpdkvhostuser port has not yet been
> configured in
> -   OVS with vhost-server-path=/path/to/socket, QEMU will print a
> log
> -   similar to the following:
> -
> -   `QEMU waiting for connection on:
> disconnected:unix:/path/to/socket,server`
> -
> -   QEMU will wait until the port is created sucessfully in OVS to
> boot the
> -   VM.

Re: [ovs-dev] [PATCH v4 3/3] netdev-dpdk: Support user-defined socket attribs

2016-08-19 Thread Mooney, Sean K


> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Aaron Conole
> Sent: Saturday, August 20, 2016 12:48 AM
> To: dev@openvswitch.org; Ben Pfaff ; Daniele Di Proietto
> 
> Subject: [ovs-dev] [PATCH v4 3/3] netdev-dpdk: Support user-defined socket 
> attribs
> 
> Currently, when vhost-user server socket devices are created, they inherit the
> running umask and uid/gid of the vswitchd process. This leads to difficulties 
> when
> using vhost_user consumers (such as qemu).
> 
> This patch introduces two new database entries, 'vhost-sock-owner' to set the
> ownership, and 'vhost-sock-perms' to set the permissions bits for all 
> vhost_user
> server sockets.
[Mooney, Sean K] will they default to the user and group of the vswitchd 
process if
Not set to maintain backwards compatibility?
> 
> Signed-off-by: Aaron Conole 
> ---
> v3->v4:
> * Rebased on upstream, the dev->vhost_id had to move to dev->vhost_server_id
> 
>  INSTALL.DPDK.md  |  8 
>  lib/netdev-dpdk.c| 37 +
>  vswitchd/vswitch.xml | 23 +++
>  3 files changed, 68 insertions(+)
> 
> diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md index 30e9258..93bc380 100644
> --- a/INSTALL.DPDK.md
> +++ b/INSTALL.DPDK.md
> @@ -223,6 +223,14 @@ advanced install guide [INSTALL.DPDK-ADVANCED.md]
>   * vhost-sock-dir
>   Option to set the path to the vhost_user unix socket files.
> 
> + * vhost-sock-owner
> + Option to set the file-system ownership of the vhost_user unix socket
> + files.
> +
> + * vhost-sock-dir
> + Option to set the file-system permissions of the vhost_user unix socket
> + files.
> +
>   NOTE: Changing any of these options requires restarting the ovs-vswitchd
>   application.
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 6d334db..6cac2ea 
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -31,6 +31,7 @@
>  #include 
>  #include 
> 
> +#include "chutil.h"
>  #include "dirs.h"
>  #include "dp-packet.h"
>  #include "dpif-netdev.h"
> @@ -141,6 +142,10 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF /
> ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
>* yet mapped to another queue. */
> 
>  static char *vhost_sock_dir = NULL;   /* Location of vhost-user sockets */
> +static char *vhost_sock_def_owner = NULL; /* Default owner of vhost-user
> +   * sockets */ static char
> +*vhost_sock_def_perms = NULL; /* Default permissions of
> +   * vhost-user sockets */
> 
>  #define VHOST_ENQ_RETRY_NUM 8
>  #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ) @@ -
> 889,6 +894,30 @@ get_vhost_id(struct netdev_dpdk *dev)  }
> 
>  static int
> +vhost_set_permissions(struct netdev_dpdk *dev) OVS_REQUIRES(dpdk_mutex)
> +{
> +int err = 0;
> +
> +/* ovs_kchown and ovs_kchmod are robust enough to deal with null or
> + * empty strings.  However, since they have the potential to race,
> + * only attempt them if the user actually requested a change. */
> +
> +if (vhost_sock_def_owner &&
> +(err = ovs_kchown(dev->vhost_server_id, vhost_sock_def_owner))) {
> +VLOG_ERR("dpdk: vhost-user socket (%s) ownership change failed 
> (%s).",
> + dev->vhost_server_id, ovs_strerror(err));
> +}
> +
> +if (!err && vhost_sock_def_perms &&
> +(err = ovs_kchmod(dev->vhost_server_id, vhost_sock_def_perms))) {
> +VLOG_ERR("dpdk: vhost-user socket (%s) permissions failed (%s).",
> + dev->vhost_server_id, ovs_strerror(err));
> +}
> +return err;
> +}
> +
> +
> +static int
>  netdev_dpdk_vhost_construct(struct netdev *netdev)  {
>  struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); @@ -932,10 +961,14 @@
> netdev_dpdk_vhost_construct(struct netdev *netdev)
>  err = netdev_dpdk_init(netdev, -1, DPDK_DEV_VHOST);
>  }
> 
> +if (!err) {
> +err = vhost_set_permissions(dev);
> +}
>  ovs_mutex_unlock(&dpdk_mutex);
>  return err;
>  }
> 
> +
>  static int
>  netdev_dpdk_construct(struct netdev *netdev)  { @@ -3363,6 +3396,10 @@
> dpdk_init__(const struct smap *ovs_other_config)
>  } else {
>  vhost_sock_dir = sock_dir_subcomponent;
>  }
> +process_vhost_flags("vhost-sock-owner", NULL, NAME_MAX, ovs_other_config,
> +&vhost_sock_def_owner);
>

Re: [ovs-dev] [PATCH v4 0/3] vhost-user: Add the ability to control ownership/permissions

2016-08-19 Thread Mooney, Sean K


> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Aaron Conole
> Sent: Saturday, August 20, 2016 12:48 AM
> To: dev@openvswitch.org; Ben Pfaff ; Daniele Di Proietto
> 
> Subject: [ovs-dev] [PATCH v4 0/3] vhost-user: Add the ability to control
> ownership/permissions
> 
> Currently, when using Open vSwitch with DPDK and qemu guests, the
> recommended method for joining the guests is via the dpdkvhostuser interface. 
> This
> interface uses Unix Domain sockets to communicate. When these sockets are
> created, they inherit the permissions and ownership from the vswitchd process.
> This can lead to an undesirable state where the QEMU process cannot use the
> socket file until manual intervention is performed (via `chown` and/or `chmod`
> calls).
> 
> 
> This patchset gives the ability to set the permissions and ownership of all
> dpdkvhostuser sockets from the database, avoiding the manual intervention
> required to connect QEMU and OVS via DPDK.
[Mooney, Sean K] technically you don’t need to do any manual intervention today 
if you
Start the ovs-vswitchd process with sudo sg   -c "umask 200; 
ovs-vswitchd .."
i.e. start it with the same group as qemu process and allow read write acess to 
members of the
same group.
This is how we have deployed ovs with dpdk in the networking-ovs-dpdk devstack 
plug for more then 2 years.

The new parameters make this simpler though as you no longer need to use the 
linux sg and umask command
To adjust the socket permissions of all files created by the vswitchd process. 
Its also likely more secure
As the permission change is limited to the vhost-user socket files.

> 
> 
> The first patch adds chmod and chown calls to lib, with unit tests.  The 
> second patch
> adds a hardness amplification version as described in the paper "Portably 
> Solving
> File TOCTTOU Races with Hardness Amplification"
> found at
> https://www.usenix.org/legacy/event/fast08/tech/full_papers/tsafrir/tsafrir_html/i
> ndex.html, while the third patch hooks those calls into the
> netdev_dpdk_vhost_user_construct function, after the socket is created.
> 
> 
> Changes from v3:
> * Replaced patch 2/3 with hardness amplification version.  Retested on RHEL7
>   and validated the travis builds.
> 
> Changes from v2:
> * Added a new 2nd patch to series for chmod/chown on already opened files.
>   There exist known implementations for other systems, including FreeBSD, but
>   only linux is implemented.  ENOTSUP is set when these calls fail on 
> non-linux
>   systems.
> 
> Aaron Conole (3):
>   chutil: introduce a new change-utils lib
>   chutil: Add hardness amplification versions of chmod/chown
>   netdev-dpdk: Support user-defined socket attribs
> 
>  INSTALL.DPDK.md  |   8 +
>  configure.ac |   2 +-
>  lib/automake.mk  |   2 +
>  lib/chutil-unix.c| 652
> +++
>  lib/chutil.h |  36 +++
>  lib/daemon-unix.c| 149 +---
>  lib/netdev-dpdk.c|  37 +++
>  tests/automake.mk|   2 +
>  tests/library.at |   5 +
>  tests/test-chutil.c  | 297 +++  vswitchd/vswitch.xml |  
> 23
> ++
>  11 files changed, 1068 insertions(+), 145 deletions(-)  create mode 100644
> lib/chutil-unix.c  create mode 100644 lib/chutil.h  create mode 100644 
> tests/test-
> chutil.c
> 
> --
> 2.5.5
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] Add support for 802.1ad (QinQ tunneling)

2016-08-19 Thread Mooney, Sean K
Hi carl just reading thought this full thread now
But there are probably 2 usecase that we would like to use this for in neutron.
Vlan transparency for the ovs neutron network and as a qinq type driver.
We also might want to extend the vlan aware vms spec to support qinq at some 
point
So once this is available in ovs I think there are several places it could be 
beneficial
To consume it in neutron.

> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Carl Baldwin
> Sent: Wednesday, July 13, 2016 7:36 PM
> To: Xiao Liang 
> Cc: dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] Add support for 802.1ad (QinQ tunneling)
> 
> On Sat, Jun 25, 2016 at 4:13 AM, Xiao Liang  wrote:
> 
> > Hi,
> >
> > I'm looking for QinQ support in OVS for some time. And found
> > patches[1-3] by Thomas, and Gayathri. What I want is to use QinQ
> > tunneling in neutron networking, which needs vlan_mode configuration
> > of ports (as in Gayathri's
> > patch) as well as openflow VLAN manipulating action support (as in
> > Thomas's patch). Unfortunately, these patches are not compatible, so I
> > made this patch.
> > It's based on Thomas's kernel patch and reference the idea from
> > Gayathri, and is (in my tests) backward compatible. Although the
> > kernel patch not merged yet, I believe the nlmsg will not change.
> >
> 
> I haven't seen much discussion on the kernel patches that you reference since 
> they
> were proposed in November.  I'm unfamiliar with the process for landing 
> changes
> like this which span the linux and ovs repositories.  Is there effort being 
> made
> elsewhere to get these to land?  Where might I look to follow that effort?  
> Is there
> anything I can do to help with the process?
> 
> Carl
> 
> References:
> > [1] openvswitch: 802.1ad uapi changes, Thomas F Herbert,
> > https://patchwork.ozlabs.org/patch/540673/
> > Check for vlan ethernet types for 8021.q or 802.1ad,
> > https://patchwork.ozlabs.org/patch/540668/
> > openvswitch: 802.1AD: Flow handling, actions, vlan parsing and
> > netlink attributes, https://patchwork.ozlabs.org/patch/540671/
> > [2] Add 802.1ad (qinq) support, Thomas F Herbert,
> > http://openvswitch.org/pipermail/dev/2015-October/060874.html
> > [3] 802.1ad support in OVS & OVS-DPDK, Gayathri Manepalli,
> > http://openvswitch.org/pipermail/dev/2016-February/066265.html
> >
> >
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v4 1/4] Add support for 802.1ad (QinQ tunneling)

2016-08-19 Thread Mooney, Sean K


> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Ben Pfaff
> Sent: Friday, August 19, 2016 9:57 PM
> To: Xiao Liang ; dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v4 1/4] Add support for 802.1ad (QinQ tunneling)
> 
> On Fri, Aug 19, 2016 at 04:42:18PM -0400, Eric Garver wrote:
> > On Fri, Aug 19, 2016 at 01:24:10PM -0700, Ben Pfaff wrote:
> > > On Fri, Aug 19, 2016 at 04:19:31PM -0400, Eric Garver wrote:
> > > > On Sat, Aug 06, 2016 at 08:04:44PM -0700, Ben Pfaff wrote:
> > > > > On Sun, Aug 07, 2016 at 10:54:00AM +0800, Xiao Liang wrote:
> > > > > > On Thu, Aug 4, 2016 at 6:07 AM, Ben Pfaff  wrote:
> > > > > > > Thanks for the replies, I have some further responses below.
> > > > > > >
> > > > > > > On Sun, Jul 31, 2016 at 08:22:47AM +0800, Xiao Liang wrote:
> > > > > > >> On Thu, Jul 28, 2016 at 2:40 AM, Ben Pfaff  wrote:
> > > > > > >> > I'm concerned about backward compatibility.  Consider
> > > > > > >> > some application built on Open vSwitch using OpenFlow.
> > > > > > >> > Today, it can distinguish single-tagged and double-tagged
> > > > > > >> > packets (that use outer Ethertype 0x8100), as follows:
> > > > > > >> >
> > > > > > >> > - A single-tagged packet has vlan_tci != 0 and some 
> > > > > > >> > non-VLAN
> > > > > > >> >   dl_type.
> > > > > > >> >
> > > > > > >> > - A double-tagged packet has vlan_tci != 0 and a VLAN 
> > > > > > >> > dl_type.
> > > > > > >> >
> > > > > > >> > With this patch, this won't work, because neither kind of
> > > > > > >> > packet has a VLAN dl_type.  Instead, applications need to
> > > > > > >> > first match on the outer VLAN, then pop it off, then
> > > > > > >> > match on the inner VLAN.  This difference could lead to
> > > > > > >> > security problems in applications.  An application might,
> > > > > > >> > for example, want to pop an outer VLAN and forward the
> > > > > > >> > packet, but only if there is no inner VLAN.  If it is 
> > > > > > >> > implemented
> according to the previous rules, then it will not notice the inner VLAN.
> > > > > > >>
> > > > > > >> Maybe some applications are implemented this way, but they
> > > > > > >> are probably wrong. OpenFlow says eth_type is "ethernet
> > > > > > >> type of the OpenFlow packet payload, after VLAN tags", so
> > > > > > >> it is the payload ethtype for a double-tagged packet. It's
> > > > > > >> the same for single-tagged
> > > > > > >> packet: application must explicitly match vlan_tci to
> > > > > > >> decide whether it has VLAN tag.
> > > > > > >
> > > > > > > OpenFlow does say that, but it's inconsistent with
> > > > > > > long-standing Open vSwitch practice and will cause backward
> > > > > > > incompatibility and, worse, security problems.  If we need
> > > > > > > the official OpenFlow behavior then I think we'll need to add a 
> > > > > > > feature
> switch to turn on that behavior.
> > > > > >
> > > > > > It's a good idea to add a switch. I think QinQ can be disabled
> > > > > > and fallback to current behavior if the switch is off, since
> > > > > > these legacy applications are not written for QinQ.
> > > > >
> > > > > OK.  I'm happy with that solution, as long as the implementation
> > > > > is clean.
> > > >
> > > > Is a new flag, i.e. OVS_DP_F_8021AD, passed via
[Mooney, Sean K]  am I correct in assuming that this new flag will be set in 
the ovsdb
Either on the bridge or prereably globally in the other_config section fo the 
Open_vSwitch table.
Both will allow easy detection of the feature form openstack so we can detect 
if qinq can be used.
The openstack ovs neutron agent currently uses vlans for tenant isolatation so 
this would enable
Vlan transparency when qinq is available.
> > > > OVS_DP_ATTR_USER_FEATURES an appropriate way to communicate this
> > > > to the kernel?
> > >
> > > Why does the kernel need to know?
> >
> > When extracting the key from a double tagged frame how does the kernel
> > know whether the second tag should be classified as second VLAN or an
> > Ethertype?
> 
> The kernel should always classify it as a second VLAN.
> datapath/README.md explains this in detail, under "Basic rule for evolving 
> flow
> keys".
[Mooney, Sean K] out of interest would it be possible to support 3 laryers of 
vlans and still be able to match on the inner packet headers.
As I said about neutron currently uses on level of vlans for tenant isolation
Having qinq would allow the tenant to send one level of vlan and neutron to use 
one
Layer of vlans for isolation. Support 3 layares fo vlans in ovs would allow the 
guest to use qinq 
Neutron to use vlans for isolation at the same time. 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v4 1/4] Add support for 802.1ad (QinQ tunneling)

2016-09-09 Thread Mooney, Sean K


> -Original Message-
> From: Xiao Liang [mailto:shaw.l...@gmail.com]
> Sent: Friday, September 9, 2016 5:27 AM
> To: Mooney, Sean K 
> Cc: Ben Pfaff ; dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v4 1/4] Add support for 802.1ad (QinQ
> tunneling)
> 
> On Sat, Aug 20, 2016 at 9:41 AM, Mooney, Sean K
>  wrote:
> >
> >
> >> -Original Message-
> >> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Ben
> Pfaff
> >> Sent: Friday, August 19, 2016 9:57 PM
> >> To: Xiao Liang ; dev@openvswitch.org
> >> Subject: Re: [ovs-dev] [PATCH v4 1/4] Add support for 802.1ad (QinQ
> >> tunneling)
> >>
> >> On Fri, Aug 19, 2016 at 04:42:18PM -0400, Eric Garver wrote:
> >> > On Fri, Aug 19, 2016 at 01:24:10PM -0700, Ben Pfaff wrote:
> >> > > On Fri, Aug 19, 2016 at 04:19:31PM -0400, Eric Garver wrote:
> >> > > > On Sat, Aug 06, 2016 at 08:04:44PM -0700, Ben Pfaff wrote:
> >> > > > > On Sun, Aug 07, 2016 at 10:54:00AM +0800, Xiao Liang wrote:
> >> > > > > > On Thu, Aug 4, 2016 at 6:07 AM, Ben Pfaff 
> wrote:
> >> > > > > > > Thanks for the replies, I have some further responses
> below.
> >> > > > > > >
> >> > > > > > > On Sun, Jul 31, 2016 at 08:22:47AM +0800, Xiao Liang
> wrote:
> >> > > > > > >> On Thu, Jul 28, 2016 at 2:40 AM, Ben Pfaff
>  wrote:
> >> > > > > > >> > I'm concerned about backward compatibility.  Consider
> >> > > > > > >> > some application built on Open vSwitch using
> OpenFlow.
> >> > > > > > >> > Today, it can distinguish single-tagged and
> >> > > > > > >> > double-tagged packets (that use outer Ethertype
> 0x8100), as follows:
> >> > > > > > >> >
> >> > > > > > >> > - A single-tagged packet has vlan_tci != 0 and
> some non-VLAN
> >> > > > > > >> >   dl_type.
> >> > > > > > >> >
> >> > > > > > >> > - A double-tagged packet has vlan_tci != 0 and a
> VLAN dl_type.
> >> > > > > > >> >
> >> > > > > > >> > With this patch, this won't work, because neither
> kind
> >> > > > > > >> > of packet has a VLAN dl_type.  Instead, applications
> >> > > > > > >> > need to first match on the outer VLAN, then pop it
> >> > > > > > >> > off, then match on the inner VLAN.  This difference
> >> > > > > > >> > could lead to security problems in applications.  An
> >> > > > > > >> > application might, for example, want to pop an outer
> >> > > > > > >> > VLAN and forward the packet, but only if there is no
> >> > > > > > >> > inner VLAN.  If it is implemented
> >> according to the previous rules, then it will not notice the inner
> VLAN.
> >> > > > > > >>
> >> > > > > > >> Maybe some applications are implemented this way, but
> >> > > > > > >> they are probably wrong. OpenFlow says eth_type is
> >> > > > > > >> "ethernet type of the OpenFlow packet payload, after
> >> > > > > > >> VLAN tags", so it is the payload ethtype for a
> >> > > > > > >> double-tagged packet. It's the same for single-tagged
> >> > > > > > >> packet: application must explicitly match vlan_tci to
> >> > > > > > >> decide whether it has VLAN tag.
> >> > > > > > >
> >> > > > > > > OpenFlow does say that, but it's inconsistent with
> >> > > > > > > long-standing Open vSwitch practice and will cause
> >> > > > > > > backward incompatibility and, worse, security problems.
> >> > > > > > > If we need the official OpenFlow behavior then I think
> >> > > > > > > we'll need to add a feature
> >> switch to turn on that behavior.
> >> > > > > >
> >> > > > > > It's a good idea to add a switch. I think QinQ can be
> >> > > > > > disabled and fallback to current behavior if 

Re: [ovs-dev] [PATCH v2] netdev-dpdk: Add new 'dpdkvhostuserclient' port type

2016-09-15 Thread Mooney, Sean K
Hi I just wanted to follow up on the status of this patch.

Without this patch support for vhost user reconnect will be blocked in 
openstack.

At this point It is now too late to include support for this feature in the 
newton release
In q4 but I would like to enable it early in the Openstack Ocata cycle if 
possible.

Will this be in ovs 2.6?
Regard
sean


> -Original Message-
> From: Mooney, Sean K
> Sent: Saturday, August 20, 2016 12:22 AM
> To: Loftus, Ciara ; dev@openvswitch.org
> Cc: diproiet...@vmware.com; Mooney, Sean K 
> Subject: RE: [PATCH v2] netdev-dpdk: Add new 'dpdkvhostuserclient' port
> type
> 
> Hi I have updated my openstack changes
> https://review.openstack.org/#/c/344997/ (neutron)
> https://review.openstack.org/#/c/357555/  (os-vif)
> https://review.openstack.org/#/c/334048/ (nova) to work with this
> change and tested it with the v1 patch.
> As far as I can tell the only change in v2 is in the install.dpdk-
> advanced and Commit message but I can retest with v2 also if desired.
> 
> Time permitting assuming this change is accepted I will also submit a
> patch to networking-ovn And networking-odl Next week to complete
> enabling the feature in each of the Main ovs compatible neutron
> backends.
> 
> > -Original Message-
> > From: Loftus, Ciara
> > Sent: Friday, August 19, 2016 10:23 AM
> > To: dev@openvswitch.org
> > Cc: diproiet...@vmware.com; Mooney, Sean K ;
> > Loftus, Ciara 
> > Subject: [PATCH v2] netdev-dpdk: Add new 'dpdkvhostuserclient' port
> > type
> >
> > The 'dpdkvhostuser' port type no longer supports both server and
> > client mode. Instead, 'dpdkvhostuser' ports are always 'server' mode
> > and 'dpdkvhostuserclient' ports are always 'client' mode.
> >
> > Suggested-by: Daniele Di Proietto 
> > Signed-off-by: Ciara Loftus 
> > ---
> >  INSTALL.DPDK-ADVANCED.md | 102 +++
> >  NEWS |   1 +
> >  lib/netdev-dpdk.c| 176 ++---
> --
> > 
> >  vswitchd/vswitch.xml |   8 +--
> >  4 files changed, 159 insertions(+), 128 deletions(-)
> >
> > diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md
> index
> > 857c805..d7b9873 100755
> > --- a/INSTALL.DPDK-ADVANCED.md
> > +++ b/INSTALL.DPDK-ADVANCED.md
> > @@ -461,6 +461,21 @@ For users wanting to do packet forwarding using
> > kernel stack below are the steps
> >   ```
> >
> >  ##  6. Vhost Walkthrough
> > +
> > +Two types of vHost User ports are available in OVS:
> > +
> > +1. vhost-user (dpdkvhostuser ports)
> > +
> > +2. vhost-user-client (dpdkvhostuserclient ports)
> > +
> > +vHost User uses a client-server model. The server
> > +creates/manages/destroys the vHost User sockets, and the client
> > +connects to the server. Depending on which port type you use,
> > +dpdkvhostuser or dpdkvhostuserclient, a different configuration of
> > +the
> > client-server model is used.
> > +
> > +For vhost-user ports, OVS DPDK acts as the server and QEMU the
> client.
> > +For vhost-user-client ports, OVS DPDK acts as the client and QEMU
> the
> > server.
> > +
> >  ### 6.1 vhost-user
> >
> >- Prerequisites:
> > @@ -570,49 +585,6 @@ For users wanting to do packet forwarding using
> > kernel stack below are the steps
> > where `-L`: Changes the numbers of channels of the specified
> > network device
> > and `combined`: Changes the number of multi-purpose channels.
> >
> > -4. OVS vHost client-mode & vHost reconnect (OPTIONAL)
> > -
> > -   By default, OVS DPDK acts as the vHost socket server for
> > dpdkvhostuser
> > -   ports and QEMU acts as the vHost client. This means OVS
> creates
> > and
> > -   manages the vHost socket and QEMU is the client which
> connects
> > to the
> > -   vHost server (OVS). In QEMU v2.7 the option is available for
> > QEMU to act
> > -   as the vHost server meaning the roles can be reversed and OVS
> > can become
> > -   the vHost client. To enable client mode for a given
> > dpdkvhostuserport,
> > -   one must specify a valid 'vhost-server-path' like so:
> > -
> > -   ```
> > -   ovs-vsctl set Interface dpdkvhostuser0 options:vhost-server-
> > path=/path/to/socket
> > -   ```
> > -
> > -   Setting this value automatically switches the port to client
> > mode (from
>

Re: [ovs-dev] [PATCH v2] netdev-dpdk: Add new 'dpdkvhostuserclient' port type

2016-09-26 Thread Mooney, Sean K
Hi I am on vaction until next week so didn’t see this till now.

Thanks for appling the patch to master and the 2.6 branch.
I will rebase my openstack patches next week when I get back and resubmit for 
ocata.

Regards
Sean.

From: Daniele Di Proietto [mailto:diproiet...@ovn.org]
Sent: Monday, September 19, 2016 10:11 PM
To: Mooney, Sean K 
Cc: Loftus, Ciara ; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v2] netdev-dpdk: Add new 'dpdkvhostuserclient' 
port type

Apologies for the delay, applied to master and branch-2.6
Thanks,
Daniele

2016-09-15 6:53 GMT-07:00 Mooney, Sean K 
mailto:sean.k.moo...@intel.com>>:
Hi I just wanted to follow up on the status of this patch.

Without this patch support for vhost user reconnect will be blocked in 
openstack.

At this point It is now too late to include support for this feature in the 
newton release
In q4 but I would like to enable it early in the Openstack Ocata cycle if 
possible.

Will this be in ovs 2.6?
Regard
sean


> -Original Message-----
> From: Mooney, Sean K
> Sent: Saturday, August 20, 2016 12:22 AM
> To: Loftus, Ciara mailto:ciara.lof...@intel.com>>; 
> dev@openvswitch.org<mailto:dev@openvswitch.org>
> Cc: diproiet...@vmware.com<mailto:diproiet...@vmware.com>; Mooney, Sean K 
> mailto:sean.k.moo...@intel.com>>
> Subject: RE: [PATCH v2] netdev-dpdk: Add new 'dpdkvhostuserclient' port
> type
>
> Hi I have updated my openstack changes
> https://review.openstack.org/#/c/344997/ (neutron)
> https://review.openstack.org/#/c/357555/  (os-vif)
> https://review.openstack.org/#/c/334048/ (nova) to work with this
> change and tested it with the v1 patch.
> As far as I can tell the only change in v2 is in the install.dpdk-
> advanced and Commit message but I can retest with v2 also if desired.
>
> Time permitting assuming this change is accepted I will also submit a
> patch to networking-ovn And networking-odl Next week to complete
> enabling the feature in each of the Main ovs compatible neutron
> backends.
>
> > -Original Message-
> > From: Loftus, Ciara
> > Sent: Friday, August 19, 2016 10:23 AM
> > To: dev@openvswitch.org<mailto:dev@openvswitch.org>
> > Cc: diproiet...@vmware.com<mailto:diproiet...@vmware.com>; Mooney, Sean K 
> > mailto:sean.k.moo...@intel.com>>;
> > Loftus, Ciara mailto:ciara.lof...@intel.com>>
> > Subject: [PATCH v2] netdev-dpdk: Add new 'dpdkvhostuserclient' port
> > type
> >
> > The 'dpdkvhostuser' port type no longer supports both server and
> > client mode. Instead, 'dpdkvhostuser' ports are always 'server' mode
> > and 'dpdkvhostuserclient' ports are always 'client' mode.
> >
> > Suggested-by: Daniele Di Proietto 
> > mailto:diproiet...@vmware.com>>
> > Signed-off-by: Ciara Loftus 
> > mailto:ciara.lof...@intel.com>>
> > ---
> >  INSTALL.DPDK-ADVANCED.md<http://INSTALL.DPDK-ADVANCED.md> | 102 
> > +++
> >  NEWS |   1 +
> >  lib/netdev-dpdk.c| 176 ++---
> --
> > 
> >  vswitchd/vswitch.xml |   8 +--
> >  4 files changed, 159 insertions(+), 128 deletions(-)
> >
> > diff --git a/INSTALL.DPDK-ADVANCED.md<http://INSTALL.DPDK-ADVANCED.md> 
> > b/INSTALL.DPDK-ADVANCED.md<http://INSTALL.DPDK-ADVANCED.md>
> index
> > 857c805..d7b9873 100755
> > --- a/INSTALL.DPDK-ADVANCED.md<http://INSTALL.DPDK-ADVANCED.md>
> > +++ b/INSTALL.DPDK-ADVANCED.md<http://INSTALL.DPDK-ADVANCED.md>
> > @@ -461,6 +461,21 @@ For users wanting to do packet forwarding using
> > kernel stack below are the steps
> >   ```
> >
> >  ##  6. Vhost Walkthrough
> > +
> > +Two types of vHost User ports are available in OVS:
> > +
> > +1. vhost-user (dpdkvhostuser ports)
> > +
> > +2. vhost-user-client (dpdkvhostuserclient ports)
> > +
> > +vHost User uses a client-server model. The server
> > +creates/manages/destroys the vHost User sockets, and the client
> > +connects to the server. Depending on which port type you use,
> > +dpdkvhostuser or dpdkvhostuserclient, a different configuration of
> > +the
> > client-server model is used.
> > +
> > +For vhost-user ports, OVS DPDK acts as the server and QEMU the
> client.
> > +For vhost-user-client ports, OVS DPDK acts as the client and QEMU
> the
> > server.
> > +
> >  ### 6.1 vhost-user
> >
> >- Prerequisites:
> > @@ -570,49 +585,6 @@ For users wanting to do packet forwarding using
> > kernel stack below are 

Re: [ovs-dev] [PATCH v8 0/5] Convert DPDK configuration from command line to DB based

2016-02-10 Thread Mooney, Sean K
Overall I like this change but as a user of ovs with dpdk and
A maintainer of a plugin to deploy it I have some comments inline.

> -Original Message-
> From: Aaron Conole [mailto:acon...@bytheb.org]
> Sent: Wednesday, February 10, 2016 1:23 PM
> To: Traynor, Kevin
> Cc: dev@openvswitch.org; Flavio Leitner; Mooney, Sean K
> Subject: Re: [ovs-dev] [PATCH v8 0/5] Convert DPDK configuration from
> command line to DB based
> 
> "Traynor, Kevin"  writes:
> >> -Original Message-
> >> From: Aaron Conole [mailto:acon...@redhat.com]
> >> Sent: Friday, January 29, 2016 5:57 PM
> >> To: dev@openvswitch.org
> >> Cc: Flavio Leitner ; Panu Matilainen
> >> ; Traynor, Kevin ;
> >> Zoltan Kiss ; Christian Ehrhardt
> >> 
> >> Subject: [PATCH v8 0/5] Convert DPDK configuration from command line
> >> to DB based
> >>
> >> Currently, configuration of DPDK parameters is done via the command
> >> line through a --dpdk **OPTIONS** -- command line argument. This has
> >> a number of challenges, including:
> >> * It must be the first option passed to ovs-vswitchd
> >> * It breaks from the way most other things are configured in OVS
> >> * It doesn't allow an easy way to populate defaults
> >>
> >>
> >> This series brings the following changes to openvswitch:
> >> * All DPDK options are taken from the ovs database rather than the
> >>   command line
> >> * DPDK lcores are optionally auto-assigned to a single core based on
> the
> >>   bridge coremask.
> >> * Updated documentation
> >>
> >> v2:
> >> * Dropped the vhost-user socket configuration options. Those can be
> re-added
> >>   as an extension
> >> * Incorporated feedback from Kevin Traynor.
> >>
> >> v3:
> >> * Went back to a global dpdk-init
> >> * Language cleanup and various minor fixes
> >>
> >> v4:
> >> * Added a way to pass arbitrary eal arguments
> >>
> >> v5:
> >> * Restore the socket-mem default, and fix up the ovs-dev.py script,
> along
> >>   with the manpage for ovsdb-server
> >>
> >> v6:
> >> * Correct a documentation issue with INSTALL.DPDK.md
> >> * Correct a non-dpdk enabled OVS incorrect warning variable
> >> * Remove an excess whitespace
> >>
> >> v7:
> >> * After testing by Christian with dpdk-alloc-mem
> >>
> >> v8:
> >> * Confirmed ``make check`` operation with and without dpdk.
> >>   Retested on live-host
> >
> > Hi,
> >
> > I've done some testing on this patchset and I couldn't find any
> > issues.
> 
> Cool; does that mean I have your Tested-by? :)
> 
> >  - tested that -c and -n defaults and explicit values are catered for
> >  - tested dpdk-init=t/f leads to dpdk initialization or not
> >  - tested that use of both dpdk-socket-mem and dpdk-alloc-mem is
> > caught
> >  - tested that a string can be passed in through extra_args
> >  - tested the code won't catch using a db entry dpdk-socket-mem and
> also
> >putting --socket-mem in extra_args, however dpdk will barf
> >
> > On command line args vs. db entries vs. a string of args in the db, if
> > there is doubt on this then let's debate further. This will change how
> > ovs with dpdk is used, so better debate it out and get it right.
> 
> I don't think there's any real doubt. I think the approach is the best
> way to do this. I have agreement from almost everyone else, I think?
> Anyone still need to be convinced?
From an orchestration point of view the explcit values are nice but form a 
Deployment point of view I favor only having dpdk_init:true/false and 
dpdk_extra:

In particular if is set a value in dpdk_extra and in the future you add a 
explicit entry in the db to 
Hold that value I never want it to brake my deployment code which still uses 
dpdk_extra. Where db entries
Exist for an item it would make my life eaiser if they were always set 
automatically when the vswitchd starts

e.g. if I never spcify the vhostuser_socket_dir it would be nice if it was set 
by ovsvswitchd to the compile time default.
Similar if I override the default of a value in dpdk_extra then it would be 
nice if when an explicit value for that item exits
In the db that it be updated to match the value specified in the dpdk_extra 
args.

If this is not the case I cannot true the explcit values in the ovsdb as they 
may not be valid.

> 
> > There's one or two of the db entries that may be able to reused later
> > for

Re: [ovs-dev] [PATCH v8 0/5] Convert DPDK configuration from command line to DB based

2016-02-16 Thread Mooney, Sean K
Hi sorry to jump back to this mail as I know the v9 patches are in flight.
I was on vacation for the last few days but my responses are inline.
I'll try to look at the revised patches also but I think you have addressed 
most of my concerns. 

> -Original Message-
> From: Aaron Conole [mailto:acon...@redhat.com]
> Sent: Thursday, February 11, 2016 6:13 PM
> To: Mooney, Sean K 
> Cc: Traynor, Kevin ; dev@openvswitch.org;
> Flavio Leitner 
> Subject: Re: [ovs-dev] [PATCH v8 0/5] Convert DPDK configuration from
> command line to DB based
> 
> Hi Sean,
> 
> "Mooney, Sean K"  writes:
> 
> > Overall I like this change but as a user of ovs with dpdk and A
> > maintainer of a plugin to deploy it I have some comments inline.
> 
> AWESOME! Seriously, I will probably be bothering you a lot because you
> are the person I want to hear from the most on this patchset. :)
> 
> >> -Original Message-
> >> From: Aaron Conole [mailto:acon...@bytheb.org]
> >> Sent: Wednesday, February 10, 2016 1:23 PM
> >> To: Traynor, Kevin
> >> Cc: dev@openvswitch.org; Flavio Leitner; Mooney, Sean K
> >> Subject: Re: [ovs-dev] [PATCH v8 0/5] Convert DPDK configuration from
> >> command line to DB based
> >>
> >> "Traynor, Kevin"  writes:
> >> >> -Original Message-
> >> >> From: Aaron Conole [mailto:acon...@redhat.com]
> >> >> Sent: Friday, January 29, 2016 5:57 PM
> >> >> To: dev@openvswitch.org
> >> >> Cc: Flavio Leitner ; Panu Matilainen
> >> >> ; Traynor, Kevin ;
> >> >> Zoltan Kiss ; Christian Ehrhardt
> >> >> 
> >> >> Subject: [PATCH v8 0/5] Convert DPDK configuration from command
> >> >> line to DB based
> >> >>
> >> >> Currently, configuration of DPDK parameters is done via the
> >> >> command line through a --dpdk **OPTIONS** -- command line
> >> >> argument. This has a number of challenges, including:
> >> >> * It must be the first option passed to ovs-vswitchd
> >> >> * It breaks from the way most other things are configured in OVS
> >> >> * It doesn't allow an easy way to populate defaults
> >> >>
> >> >>
> >> >> This series brings the following changes to openvswitch:
> >> >> * All DPDK options are taken from the ovs database rather than the
> >> >>   command line
> >> >> * DPDK lcores are optionally auto-assigned to a single core based
> >> >> on
> >> the
> >> >>   bridge coremask.
> >> >> * Updated documentation
> >> >>
> >> >> v2:
> >> >> * Dropped the vhost-user socket configuration options. Those can
> >> >> be
> >> re-added
> >> >>   as an extension
> >> >> * Incorporated feedback from Kevin Traynor.
> >> >>
> >> >> v3:
> >> >> * Went back to a global dpdk-init
> >> >> * Language cleanup and various minor fixes
> >> >>
> >> >> v4:
> >> >> * Added a way to pass arbitrary eal arguments
> >> >>
> >> >> v5:
> >> >> * Restore the socket-mem default, and fix up the ovs-dev.py
> >> >> script,
> >> along
> >> >>   with the manpage for ovsdb-server
> >> >>
> >> >> v6:
> >> >> * Correct a documentation issue with INSTALL.DPDK.md
> >> >> * Correct a non-dpdk enabled OVS incorrect warning variable
> >> >> * Remove an excess whitespace
> >> >>
> >> >> v7:
> >> >> * After testing by Christian with dpdk-alloc-mem
> >> >>
> >> >> v8:
> >> >> * Confirmed ``make check`` operation with and without dpdk.
> >> >>   Retested on live-host
> >> >
> >> > Hi,
> >> >
> >> > I've done some testing on this patchset and I couldn't find any
> >> > issues.
> >>
> >> Cool; does that mean I have your Tested-by? :)
> >>
> >> >  - tested that -c and -n defaults and explicit values are catered
> >> > for
> >> >  - tested dpdk-init=t/f leads to dpdk initialization or not
> >> >  - tested that use of both dpdk-socket-mem and dpdk-alloc-mem is
> >> > caught
> >> >  - tested that a string can be passed in through extra_args
> >> >  - te