Re: [ovs-dev] [RFC 0/2] dpdk: minor refactor of the initialization step

2018-04-11 Thread Mooney, Sean K


> -Original Message-
> From: Kevin Traynor [mailto:ktray...@redhat.com]
> Sent: Wednesday, April 11, 2018 4:03 PM
> To: Aaron Conole <acon...@redhat.com>; Mooney, Sean K
> <sean.k.moo...@intel.com>
> Cc: d...@openvswitch.org; Ilya Maximets <i.maxim...@samsung.com>; Assaf
> Muller <as...@redhat.com>
> Subject: Re: [ovs-dev] [RFC 0/2] dpdk: minor refactor of the
> initialization step
> 
> On 04/11/2018 02:21 PM, Aaron Conole wrote:
> > "Mooney, Sean K" <sean.k.moo...@intel.com> writes:
> >
> >>> -Original Message-
> >>> From: Aaron Conole [mailto:acon...@redhat.com]
> >>> Sent: Monday, April 9, 2018 4:32 PM
> >>> To: Mooney, Sean K <sean.k.moo...@intel.com>
> >>> Cc: d...@openvswitch.org; Stokes, Ian <ian.sto...@intel.com>; Kevin
> >>> Traynor <ktray...@redhat.com>; Ilya Maximets
> >>> <i.maxim...@samsung.com>; Loftus, Ciara <ciara.lof...@intel.com>;
> >>> Terry Wilson <twil...@redhat.com>; Assaf Muller <as...@redhat.com>
> >>> Subject: Re: [RFC 0/2] dpdk: minor refactor of the initialization
> >>> step
> >>>
> >>> "Mooney, Sean K" <sean.k.moo...@intel.com> writes:
> >>>
> >>>> So just from a deployment tools point of view I would like to
> point
> >>>> out that This change could break existing workflow that deploy ovs
> >>>> in
> >>> a docker container.
> >>>> Kolla ansible assumes that if the docker ovs_vswitchd container is
> >>>> still running that the is infact running in dpdk mode when we set
> >>>> dpdk-init=true.
> >>>
> >>> Is there a way to test this out and see the behavior?
> >> [Mooney, Sean K] well you could use kolla to deploy ovs-dpdk :) Am
> >> when I wrote the code I relied on the existing behavior.
> >> when kolla ansible is deploying openstack we first deploy the ovsdb.
> >> https://github.com/openstack/kolla-
> ansible/blob/4c39ea7eccd9467757226
> >> 46eec19f9ea5cbe6eb5/ansible/roles/ovs-dpdk/handlers/main.yml#L2-L37
> >> Then we start the ovs-vswitchd container
> >> https://github.com/openstack/kolla-
> ansible/blob/4c39ea7eccd9467757226
> >> 46eec19f9ea5cbe6eb5/ansible/roles/ovs-dpdk/handlers/main.yml#L55-L73
> >> finally we configure the bridges and physical interfaces.
> >> https://github.com/openstack/kolla-
> ansible/blob/4c39ea7eccd9467757226
> >> 46eec19f9ea5cbe6eb5/ansible/roles/ovs-dpdk/handlers/main.yml#L75-L90
> >>
> >> the "- name: Ensuring ovsdpdk bridges are properly setup named" task
> >> does not use --no-wait when creating bridges and adding interfaces
> so
> >> it will fail if the vswitchd is not running. This will result in
> >> ansible stopping to run any futher task on that node and reporting
> >> the error to the user. If for some reason the ensure bridge task
> >> passed the next task that check an ip is assigned to the ovs bride
> >> would fail.
> >
> > Okay - thanks for the pointers, I'll check it out.
> >
> >>>
> >>> It does seem strange that for a possible configuration error we
> >>> abort()
> >> [Mooney, Sean K] why I would expect this to be standard behavior for
> any deamon.
> >> e.g. the damon would validate it config is correct and exit if
> invalid.
> >> If we don't abort the vswitch is in an undefined state. Is is still
> >> using hugepages For example if the eal init fails after they are
> >> allocated.
> >
> > I see.  I think as of 18.11 (which won't come for 6 months), it will
> > always 'fail' in a known state.  But I'll confirm.
> >
> > One of the reasons I want this field is because there will be ways of
> > 'uninitializing'.  I think it's nice to have the ability for the user
> > to dynamically enable *and* disable dpdk.  Also, just from a
> 'selfish'
> > view, I recently was fixing a bug where dpdk initialization would
> fail
> > early with a particular hardware, and it wasn't nice to watch the
> > respawns (racy gdb attach, and coredumps all over the drive).
> >
> >>> running the vswitchd (and with --monitor set, it will continue to
> >>> abort() over and over - so I guess you're also not using the
> monitor
> >>> thread?).  In the case that an abort does happen, does the Kolla
> >>> script distinguish between issues where dpdk setup failed vs. some
> >>> other software issue

Re: [ovs-dev] [RFC 0/2] dpdk: minor refactor of the initialization step

2018-04-10 Thread Mooney, Sean K


> -Original Message-
> From: Aaron Conole [mailto:acon...@redhat.com]
> Sent: Monday, April 9, 2018 4:32 PM
> To: Mooney, Sean K <sean.k.moo...@intel.com>
> Cc: d...@openvswitch.org; Stokes, Ian <ian.sto...@intel.com>; Kevin
> Traynor <ktray...@redhat.com>; Ilya Maximets <i.maxim...@samsung.com>;
> Loftus, Ciara <ciara.lof...@intel.com>; Terry Wilson
> <twil...@redhat.com>; Assaf Muller <as...@redhat.com>
> Subject: Re: [RFC 0/2] dpdk: minor refactor of the initialization step
> 
> "Mooney, Sean K" <sean.k.moo...@intel.com> writes:
> 
> > So just from a deployment tools point of view I would like to point
> > out that This change could break existing workflow that deploy ovs in
> a docker container.
> > Kolla ansible assumes that if the docker ovs_vswitchd container is
> > still running that the is infact running in dpdk mode when we set
> > dpdk-init=true.
> 
> Is there a way to test this out and see the behavior?
[Mooney, Sean K] well you could use kolla to deploy ovs-dpdk :)
Am when I wrote the code I relied on the existing behavior.
when kolla ansible is deploying openstack we first deploy the ovsdb.
https://github.com/openstack/kolla-ansible/blob/4c39ea7eccd946775722646eec19f9ea5cbe6eb5/ansible/roles/ovs-dpdk/handlers/main.yml#L2-L37
Then we start the ovs-vswitchd container
https://github.com/openstack/kolla-ansible/blob/4c39ea7eccd946775722646eec19f9ea5cbe6eb5/ansible/roles/ovs-dpdk/handlers/main.yml#L55-L73
finally we configure the bridges and physical interfaces.
https://github.com/openstack/kolla-ansible/blob/4c39ea7eccd946775722646eec19f9ea5cbe6eb5/ansible/roles/ovs-dpdk/handlers/main.yml#L75-L90

the "- name: Ensuring ovsdpdk bridges are properly setup named" task does not 
use --no-wait when creating bridges and adding interfaces so it
will fail if the vswitchd is not running. This will result in ansible stopping 
to run any futher task on that node and reporting the error
to the user. If for some reason the ensure bridge task passed the next task 
that check an ip is assigned to the ovs bride would fail.
> 
> It does seem strange that for a possible configuration error we abort()
[Mooney, Sean K] why I would expect this to be standard behavior for any deamon.
e.g. the damon would validate it config is correct and exit if invalid.
If we don't abort the vswitch is in an undefined state. Is is still using 
hugepages
For example if the eal init fails after they are allocated.
> running the vswitchd (and with --monitor set, it will continue to
> abort() over and over - so I guess you're also not using the monitor
> thread?).  In the case that an abort does happen, does the Kolla script
> distinguish between issues where dpdk setup failed vs. some other
> software issue?
> 
> > Can I request that if you make this change you add something along
> the
> > lines of dpdk-init-is-fatal=true/false so that we can explicitly say
> which behavior we want.
> > I would not be surprised if people have built monitoring around "is
> > the ovs-vswitchd running"
> 
> I think they have, but I don't know that they use it to infer such low-
> level details (meaning a crash implies that dpdk configuration is
> wrong).
[Mooney, Sean K] they don't use is to infer that dpdk configuring is wronge
But rather that some configuration was wrong. Dpdk-init is currently considered
Fatal if it fails so it was treated the same as any other error that would have
caused the vsiwtchd process to exit. I belive in the opnfv community they used
the liveleyness of the vswitchd process and in the future dpdk keepalive
functionality to set the datapalne status filed in neutron for the host.
this allows openstack to be aware of dataplane outages.
> 
> > To infer at least at a highlevel that "everything is fine" where as
> > the log message/db field proposed Here will invalidate that.
> 
> I've added Assaf Mueller from our Open Stack team as well - maybe he
> has some additional details on those mechanisms outside of Kolla (maybe
> it exists in some kind of director / other software too, as you point
> out).
> 
> > it would be ease to check that field but its work that needs to be
> > done in multiple places.
> 
> I think such a knob wouldn't be useful.  I believe it would either have
> to be defaulted to 'dpdk-init-is-fatal=true' to abort on failure (which
> most users would want to change making it an undesirable default)
[Mooney, Sean K] I would argue against that I would never deploy with
dpdk-init-is-fatal=false. If your datapane does not start what is the point
of running ovs at all? It will not be able to forward packets.
, or
> the Kolla ansible scripts (and other detection mechanisms for dpdk
> failure - if they exist) woul

Re: [ovs-dev] [RFC 0/2] dpdk: minor refactor of the initialization step

2018-04-06 Thread Mooney, Sean K
So just from a deployment tools point of view I would like to point out that
This change could break existing workflow that deploy ovs in a docker container.
Kolla ansible assumes that if the docker ovs_vswitchd container is still 
running that the
is infact running in dpdk mode when we set dpdk-init=true.

Can I request that if you make this change you add something along the lines of
dpdk-init-is-fatal=true/false so that we can explicitly say which behavior we 
want.
I would not be surprised if people have built monitoring around "is the 
ovs-vswitchd running"
To infer at least at a highlevel that "everything is fine" where as the log 
message/db field proposed
Here will invalidate that.
it would be ease to check that field but its work that needs to be done in 
multiple places.

> -Original Message-
> From: Aaron Conole [mailto:acon...@redhat.com]
> Sent: Thursday, April 5, 2018 10:23 PM
> To: d...@openvswitch.org
> Cc: Stokes, Ian <ian.sto...@intel.com>; Kevin Traynor
> <ktray...@redhat.com>; Ilya Maximets <i.maxim...@samsung.com>; Loftus,
> Ciara <ciara.lof...@intel.com>; Mooney, Sean K
> <sean.k.moo...@intel.com>; Terry Wilson <twil...@redhat.com>
> Subject: [RFC 0/2] dpdk: minor refactor of the initialization step
> 
> Sometimes, DPDK initialization can fail, but ovs-vswitchd will abort in
> that case.  When that occurs, ovs-vswitchd will be restarted by the
> monitor and immediately abort.  This is rather unfriendly to users, who
> would prefer to possibly correct the issue or at least, not have lots
> of processes continually spawning.
> 
> This series accepts that rte_eal_init() can and does fail for real.  It
> reflects the initialization status in the database, as well as adding
> the DPDK version (where appropriate).
> 
> Submitted as RFC to spawn discussion around the type to reflect for the
> initialized information.  Presented here as a boolean - however, it
> might be more interesting to be a 'string' and have more elaborate
> details (ex: 'failed - ovs_strerror(rte_errno)' or 'uninitialized' or
> 'initialized').
> 
> Aaron Conole (2):
>   dpdk: allow init to fail
>   dpdk: reflect status and version in the database
> 
>  lib/dpdk-stub.c| 10 ++
>  lib/dpdk.c | 31 +--
>  lib/dpdk.h |  3 ++-
>  vswitchd/bridge.c  |  5 +
>  vswitchd/vswitch.ovsschema | 11 ---
>  vswitchd/vswitch.xml   | 11 +++
>  6 files changed, 61 insertions(+), 10 deletions(-)
> 
> --
> 2.14.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH 2/2] Adding configuration option to whitelist DPDK physical ports.

2017-12-07 Thread Mooney, Sean K


> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Chandran, Sugesh
> Sent: Thursday, December 7, 2017 5:07 PM
> To: O Mahony, Billy <billy.o.mah...@intel.com>; d...@openvswitch.org;
> b...@ovn.org
> Subject: Re: [ovs-dev] [PATCH 2/2] Adding configuration option to
> whitelist DPDK physical ports.
> 
> 
> 
> Regards
> _Sugesh
> 
> > -Original Message-
> > From: O Mahony, Billy
> > Sent: Thursday, December 7, 2017 11:47 AM
> > To: Chandran, Sugesh <sugesh.chand...@intel.com>;
> d...@openvswitch.org;
> > b...@ovn.org
> > Subject: RE: [ovs-dev] [PATCH 2/2] Adding configuration option to
> > whitelist DPDK physical ports.
> >
> > Hi Sugesh,
> >
> > > -Original Message-
> > > From: Chandran, Sugesh
> > > Sent: Wednesday, December 6, 2017 6:23 PM
> > > To: O Mahony, Billy <billy.o.mah...@intel.com>;
> d...@openvswitch.org;
> > > b...@ovn.org
> > > Subject: RE: [ovs-dev] [PATCH 2/2] Adding configuration option to
> > > whitelist DPDK physical ports.
> > >
> > > Thank you Billy for the review.
> > > Please find below my reply.
> > >
> > > Regards
> > > _Sugesh
> > >
> > >
> > > > -Original Message-
> > > > From: O Mahony, Billy
> > > > Sent: Wednesday, December 6, 2017 5:31 PM
> > > > To: Chandran, Sugesh <sugesh.chand...@intel.com>;
> > > > d...@openvswitch.org; b...@ovn.org
> > > > Subject: RE: [ovs-dev] [PATCH 2/2] Adding configuration option to
> > > > whitelist DPDK physical ports.
> > > >
> > > > Hi Sugesh,
> > > >
> > > > This is definitely a very useful feature. I'm looking forward to
> > > > running trex on the same DUT as my ovs-dpdk.
[Mooney, Sean K]  you can all ready to this you just need to set the whitelist 
In other_config:dpdk-extra just repeat "-w $address" for each device.
To have two dpdk primary processes on the same system you will also need to 
change
The hugepage prfix used be dpdk which you can also do via the dpdk-extra option.

After this patch we will still be able to specify the whitelist using
other_config:dpdk-extra correct? If not this may break ovs-dpdk support
in openstack installers. I ported our whitelist code in networking-ovs-dpdk to 
use dpdk-extra when
when we moved the dpdk params to the db and I also added it to kolla.
im pretty sure tripple0 and fule also do the same.

> > > >
> > > > However I'd suggest adding an sscanf or some such to verify that
> > > > the domain is also specified for each whitelist member. And
> either
> > > > add the default of '' or complain loudly if the domain is
> absent.
> > > [Sugesh] Will throw an error in that case then .
> > >
> > > >
> > > > Currently (without this patch) you must specify the domain when
> > > > adding
> > ports:
> > > >Vsctl add-port ... options:dpdk-devargs=:05:00.0 Or else
> an
> > > > error such as 'Cannot find unplugged device (05:00.0)'  is
> reported.
> > > >
> > > > And with the patch if you include the domain in the other_config
> (e.g.
> > > > other_config:dpdk-whitelist-pci-ids=":05:00.0") everything
> > > > works just as before.
> > > >
> > > > However with the patch if you add the whitelist *without* a
> domain e.g.
> > > > ovs-vsctl --no-wait set Open_vSwitch .
> > > > other_config:dpdk-whitelist-pci- ids="05:00.0"
> > > >
> > > > There is no immediate error. However later when doing add-port if
> > > > you include the domain (current required practice) you will get
> an error.
> > > > If you omit the domain all is well.
> > > [Sugesh] It looks to me, the dpdk-devargs need the PCI id with the
> ''.
> > > But to bind and PCI scan its not necessary.
> > > So to keep it consistent, I would add check for PCI-ID in whitelist
> > > config too, and throw error incase pci-id are mentioned wrong(means
> > > without
> > ''.
[Mooney, Sean K] don't assume it is '' it is only '' if the pci device 
is
A child of the first pci root usally connected to socket 0 unless you server is 
old
Enough to still use a FSB instead of qpi/dmi.

> > > Does it looks OK to you?
> >
> > [[BO'M]] I think the error is the right thing to do. It would be
> > tempting to insert the default '' if 

Re: [ovs-dev] [PATCH V4 2/2] netdev-dpdk: vHost IOMMU support

2017-12-07 Thread Mooney, Sean K


> -Original Message-
> From: Jan Scheurich [mailto:jan.scheur...@ericsson.com]
> Sent: Thursday, December 7, 2017 4:07 PM
> To: Mooney, Sean K <sean.k.moo...@intel.com>; Kevin Traynor
> <ktray...@redhat.com>; Kavanagh, Mark B <mark.b.kavan...@intel.com>;
> Ilya Maximets <i.maxim...@samsung.com>; Stokes, Ian
> <ian.sto...@intel.com>; d...@openvswitch.org
> Cc: maxime.coque...@redhat.com; Fischetti, Antonio
> <antonio.fische...@intel.com>; Bie, Tiwei <tiwei@intel.com>;
> Mcnamara, John <john.mcnam...@intel.com>; Guoshuai Li
> <l...@dtdream.com>; Loftus, Ciara <ciara.lof...@intel.com>; Yuanhan Liu
> <y...@fridaylinux.org>
> Subject: RE: [ovs-dev] [PATCH V4 2/2] netdev-dpdk: vHost IOMMU support
> 
> > > Would the virtio PMD bug in DPDK 17.11 in the guest actually be
> > > mitigated by running DPDK 17.05 or a fixed 17.11.1 as vhostuser
> > > backend inside OVS on the host?
> 
> > [Mooney, Sean K] from talking to mark about this eairlier I don’t
> belive so.
> > I think if you used 17.11 testpmd in the guest with kernel ovs you
> > should get the same behavior e.g. it does not forward packet. The
> > guest should not be able to know with certainty what vhost backend is
> in use on the host.
> 
> If that is the case why should OVS even consider documenting that there
> are known problems when running a DPDK 17.11 application in a guest,
> irrespective of the OVS version or datapath?
> 
> If anything, this is a critical DPDK bug and DPDK need to warn their
> virtio PMD users not to use 17.11 until this bug is fixed in a 17.11.1
> maintenance release.
> 
> The real issue I see is that OVS-DPDK may also be deployed in in a VM
> with virtio ports, and that kind of deployment will indeed be broken
> with DPDK 17.11. Can we accept and document that as a limitation in OVS
> 2.9 that will be fixed in OVS 2.9.1 as soon as DPDK 17.11.1 is out?
[Mooney, Sean K] that is a concern for me also. I run ovs-dpdk in a vm on a 
daily
Basis but I typical configure the vm with e1000 virtual nics but that then 
requires
Me to use kernel ovs on the host. I started using this partly to work around old
virtio pmd bugs that are have since been fixed.

I ran ovs-dpdk in a vm  on top of ovs-dpdk with vhost-user nics via openstack 
for years
However using Virtio-net-pci devices for ovs-dpdk requires special handeling
Due to some limitations in the choice of pf driver.

Without a virtualized iommu in a guest its not possible to use the vfio-pci
Driver with virtio-net-interfaces. qemu/kvm did not support
vIOMMUs in the past so this has only recently become an option but I have not
validated that it works myself.

Uio support is compiled out by default by Ubuntu,fedora,centos and presumable 
rhel.
That mean even on distros that package ovs-dpdk to run it in a vm you have to 
build
From source.

Igb_uio was previously known to be buggy with vitio-net-pci devices and dpdk 
had advised
against using it as you could not reliably unbind the virtio interface form 
igb_uio and
back to the kernel driver.

So igb_uio worked but you had to reboot the vm to get the kernel dirver working 
again
which was annoying for my development and ci usecases.hence my use of e1000 
vNICs
This may have been fixed.

Uio_pci_generic works with virtio-net-pci devices as far as I know but unlike
Vfio-pci or igb_uio its memory access is over privaldged so its not a great 
choice
If you care about you applications security. It did however avoid the buggy 
igb_uio
Rebinding issue and it does not need an iommu to work so has been the most 
reliable
If least secure option for nested ovs-dpdk.

Since this bug is in the dpdk virtio pmd and not the pf driver however it will 
effect
Each one equally so I think the onus remain with the vnf vendors that build on 
ovs and dpdk 
to validate their dependencies before choosing to rev the ovs and/or dpdk 
versions they use.
Having a warning about this particular issue in the ovs docs however may help 
some of them
Avoid this trap and it will give us something I can refer people to between now 
and the dpdk 17.11.1
Release.

> 
> Or should we as a temporary solution apply a patch to DPDK 17.11 to fix
> this when building OVS?
[Mooney, Sean K] dpdk is not built via the ovs make file we just link to it so
I don’t think that is a good option for upstream ovs. That said I have patching 
support
In my networking-ovs-dpdk devstack plugin for just this reason. If people are 
building
Ovs form source I think just specifying which dpdk commit fixes the issue 
should be
Enough for them to be able to work around this issue until 17.11.1 is released.
> 
> BR, Jan
> 
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH V4 2/2] netdev-dpdk: vHost IOMMU support

2017-12-07 Thread Mooney, Sean K


> -Original Message-
> From: Jan Scheurich [mailto:jan.scheur...@ericsson.com]
> Sent: Thursday, December 7, 2017 3:09 PM
> To: Kevin Traynor <ktray...@redhat.com>; Kavanagh, Mark B
> <mark.b.kavan...@intel.com>; Ilya Maximets <i.maxim...@samsung.com>;
> Stokes, Ian <ian.sto...@intel.com>; d...@openvswitch.org
> Cc: maxime.coque...@redhat.com; Mooney, Sean K
> <sean.k.moo...@intel.com>; Fischetti, Antonio
> <antonio.fische...@intel.com>; Bie, Tiwei <tiwei@intel.com>;
> Mcnamara, John <john.mcnam...@intel.com>; Guoshuai Li
> <l...@dtdream.com>; Loftus, Ciara <ciara.lof...@intel.com>; Yuanhan Liu
> <y...@fridaylinux.org>
> Subject: RE: [ovs-dev] [PATCH V4 2/2] netdev-dpdk: vHost IOMMU support
> 
> > > I think the point that both yourself and Sean has made is
> completely valid, which puts option a) back on the table.
> > >
> >
> > a) Sounds ok to me. I think an early DPDK17.11.1 before OVS 2.9 would
> > be good in addition though. It is nicer that an OVS 2.9 user doesn't
> > have to know they can't use the latest DPDK in the guest.
> >
> 
> Would the virtio PMD bug in DPDK 17.11 in the guest actually be
> mitigated by running DPDK 17.05 or a fixed 17.11.1 as vhostuser backend
> inside OVS on the host?
[Mooney, Sean K] from talking to mark about this eairlier I don’t belive so.
I think if you used 17.11 testpmd in the guest with kernel ovs you should
get the same behavior e.g. it does not forward packet. The guest should not
be able to know with certainty what vhost backend is in use on the host.
> 
> If not, I would prefer if we decoupled the DPDK life cycle of OVS and
> DPDK applications in the guest. Guests should update their DPDK version
> if  it contains a critical bug.
> 
> BR, Jan
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for vhost IOMMU feature

2017-11-27 Thread Mooney, Sean K


> -Original Message-
> From: Jan Scheurich [mailto:jan.scheur...@ericsson.com]
> Sent: Monday, November 27, 2017 2:15 PM
> To: Kavanagh, Mark B <mark.b.kavan...@intel.com>; Mooney, Sean K
> <sean.k.moo...@intel.com>; Kevin Traynor <ktray...@redhat.com>;
> d...@openvswitch.org
> Cc: maxime.coque...@redhat.com; Flavio Leitner <f...@redhat.com>; Franck
> Baudin <fbau...@redhat.com>; Ilya Maximets <i.maxim...@samsung.com>;
> Stokes, Ian <ian.sto...@intel.com>; Loftus, Ciara
> <ciara.lof...@intel.com>; Darrell Ball <db...@vmware.com>; Aaron Conole
> <acon...@redhat.com>
> Subject: RE: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for vhost
> IOMMU feature
> 
> > >> >> Hi Mark, All,
> > >> >>
> > >> >> I'm thinking about this and whether the current approach
> > >> >> provides more than what is actually needed by users at the cost
> > >> >> of making the user interface more complex.
> > >[Mooney, Sean K] I am personally split on this. To enable iommu
> > >support in openstack with the above interface we would have to do
> two
> > >things. 1 extend the image metatdata or flavor extra specs to allow
> > >requesting a vIOMMU. Second we would have to modify os-vif to
> produce
> > >the add-port command above. Using this interfaces gives us a nice
> > >parity in our api as we only enable iommu support in ovs if we
> enable
> > >for qemu. E.g. if the falvor/image does not request a virtualized
> > >iommu we wont declare its support in the options.
> > >> >>
> > >> >> As an alternative, how about having a global other_config (to
> be
> > >> >> set like vhost-socket-dir can be) for this instead of having to
> > >> >> set it for each individual interface. It would mean that it
> > >> >> would only have to be set once, instead of having this (ugly?!)
> > >> >> option every time a vhost port is added, so it's a less
> > >> >> intrusive change and I can't really think that a user would
> > >> >> require to do this per vhostclient
> > >[Mooney, Sean K]  well one taught that instantly comes to mind is if
> > >I set The global other_config option what happens if I do not
> request
> > >a iommu in qemu Or I have an old qemu.  If it would have any
> negative
> > >effect on network connectivity Or prevent the vm from functioning,
> > >that would require the nova scheduler to be Aware of which node had
> > >this option set and take that into account when placing vms.
> > >I assume if it had no negative effects  then we would not need a
> > >option, global or per Interface.
> > >> >> interface??? It's pain to have to add this at all for a bug in
> > >> >> QEMU and I'm sure in 1/2/3 years time someone will say that
> > >> >> users could still be using QEMU < 2.9.1 and we can't remove it,
> > >> >> so it would be nice to keep it as discreet as possible as we're
> > >> >> going to be stuck
> > >> with it for a while.
> > >> >>
> > >> >> I assume that a user would only use one version of QEMU at a
> > >> >> time
> > >> and
> > >> >> would either want or not want this feature. In the worst case,
> > >> >> if that proved completely wrong in the future, then a per
> > >> >> interface override could easily be added. Once there's a way to
> > >> >> maintain backwards compatibility (which there would be) I'd
> > >> >> rather err on the side of introducing just enough enough
> > >> >> functionality over increasing complexity for the user.
> > >> >>
> > >> >> What do you think?
> > >[Mooney, Sean K] I'm not sure that a single qemu version is a valid
> > >assumption to make.
> > >At least in an OpenStack context where rolling upgrades are a thing.
> > >But you are right That it will be less common however if it was no
> > >existent we would not have the issue with Live migration between
> > >nodes with different feature sets that is also trying to be
> addressed
> > >this Cycle. If we add a global config option for iommu support that
> > >is yet another thing that needs To be accounted for during live
> > >migration.
> > >> >>
> >
> > Hi Kevin, Jan,
> >
> > Any thoughts on Sean's concerns?
> >
> > I

Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for vhost IOMMU feature

2017-11-27 Thread Mooney, Sean K


> -Original Message-
> From: Kevin Traynor [mailto:ktray...@redhat.com]
> Sent: Monday, November 27, 2017 1:32 PM
> To: Kavanagh, Mark B <mark.b.kavan...@intel.com>; Mooney, Sean K
> <sean.k.moo...@intel.com>; Jan Scheurich <jan.scheur...@ericsson.com>;
> d...@openvswitch.org
> Cc: maxime.coque...@redhat.com; Flavio Leitner <f...@redhat.com>; Franck
> Baudin <fbau...@redhat.com>; Ilya Maximets <i.maxim...@samsung.com>;
> Stokes, Ian <ian.sto...@intel.com>; Loftus, Ciara
> <ciara.lof...@intel.com>; Darrell Ball <db...@vmware.com>; Aaron Conole
> <acon...@redhat.com>
> Subject: Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for vhost
> IOMMU feature
> 
> On 11/27/2017 12:55 PM, Kavanagh, Mark B wrote:
> >
> >
> >> From: Mooney, Sean K
> >> Sent: Sunday, November 26, 2017 11:28 PM
> >> To: Kavanagh, Mark B <mark.b.kavan...@intel.com>; Jan Scheurich
> >> <jan.scheur...@ericsson.com>; Kevin Traynor <ktray...@redhat.com>;
> >> d...@openvswitch.org
> >> Cc: maxime.coque...@redhat.com; Flavio Leitner <f...@redhat.com>;
> >> Franck Baudin <fbau...@redhat.com>; Ilya Maximets
> >> <i.maxim...@samsung.com>; Stokes, Ian <ian.sto...@intel.com>;
> Loftus,
> >> Ciara <ciara.lof...@intel.com>; Darrell Ball <db...@vmware.com>;
> >> Aaron Conole <acon...@redhat.com>; Mooney, Sean K
> >> <sean.k.moo...@intel.com>
> >> Subject: RE: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for
> vhost
> >> IOMMU feature
> >>
> >>
> >>
> >>> -----Original Message-
> >>> From: Kavanagh, Mark B
> >>> Sent: Friday, November 24, 2017 4:45 PM
> >>> To: Jan Scheurich <jan.scheur...@ericsson.com>; Kevin Traynor
> >>> <ktray...@redhat.com>; d...@openvswitch.org
> >>> Cc: maxime.coque...@redhat.com; Flavio Leitner <f...@redhat.com>;
> >>> Franck Baudin <fbau...@redhat.com>; Mooney, Sean K
> >>> <sean.k.moo...@intel.com>; Ilya Maximets <i.maxim...@samsung.com>;
> >>> Stokes, Ian <ian.sto...@intel.com>; Loftus, Ciara
> >>> <ciara.lof...@intel.com>; Darrell Ball <db...@vmware.com>; Aaron
> >>> Conole <acon...@redhat.com>
> >>> Subject: RE: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for
> >>> vhost IOMMU feature
> >>>
> >>> Sounds good guys - I'll get cracking on this on Monday.
> >>>
> >>> Cheers,
> >>> Mark
> >>>
> >>>> -Original Message-
> >>>> From: Jan Scheurich [mailto:jan.scheur...@ericsson.com]
> >>>> Sent: Friday, November 24, 2017 4:21 PM
> >>>> To: Kevin Traynor <ktray...@redhat.com>; Kavanagh, Mark B
> >>>> <mark.b.kavan...@intel.com>; d...@openvswitch.org
> >>>> Cc: maxime.coque...@redhat.com; Flavio Leitner <f...@redhat.com>;
> >>> Franck
> >>>> Baudin <fbau...@redhat.com>; Mooney, Sean K
> >>>> <sean.k.moo...@intel.com>; Ilya Maximets <i.maxim...@samsung.com>;
> >>>> Stokes, Ian <ian.sto...@intel.com>; Loftus, Ciara
> >>>> <ciara.lof...@intel.com>;
> >>> Darrell
> >>>> Ball <db...@vmware.com>; Aaron Conole <acon...@redhat.com>
> >>>> Subject: RE: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for
> >>>> vhost IOMMU feature
> >>>>
> >>>> +1
> >>>> Jan
> >>>>
> >>>>> -Original Message-
> >>>>> From: Kevin Traynor [mailto:ktray...@redhat.com]
> >>>>> Sent: Friday, 24 November, 2017 17:11
> >>>>> To: Mark Kavanagh <mark.b.kavan...@intel.com>;
> d...@openvswitch.org
> >>>>> Cc: maxime.coque...@redhat.com; Flavio Leitner <f...@redhat.com>;
> >>>>> Franck
> >>>> Baudin <fbau...@redhat.com>; Mooney, Sean K
> >>>>> <sean.k.moo...@intel.com>; Ilya Maximets
> <i.maxim...@samsung.com>;
> >>>>> Ian
> >>>> Stokes <ian.sto...@intel.com>; Loftus, Ciara
> >>>>> <ciara.lof...@intel.com>; Darrell Ball <db...@vmware.com>; Aaron
> >>>>> Conole
> >>>> <acon...@redhat.com>; Jan Scheurich
> >>>>> <jan.scheur...@ericsson.com>
> >>>>> Subject

Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for vhost IOMMU feature

2017-11-26 Thread Mooney, Sean K


> -Original Message-
> From: Kavanagh, Mark B
> Sent: Friday, November 24, 2017 4:45 PM
> To: Jan Scheurich <jan.scheur...@ericsson.com>; Kevin Traynor
> <ktray...@redhat.com>; d...@openvswitch.org
> Cc: maxime.coque...@redhat.com; Flavio Leitner <f...@redhat.com>; Franck
> Baudin <fbau...@redhat.com>; Mooney, Sean K <sean.k.moo...@intel.com>;
> Ilya Maximets <i.maxim...@samsung.com>; Stokes, Ian
> <ian.sto...@intel.com>; Loftus, Ciara <ciara.lof...@intel.com>; Darrell
> Ball <db...@vmware.com>; Aaron Conole <acon...@redhat.com>
> Subject: RE: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for vhost
> IOMMU feature
> 
> Sounds good guys - I'll get cracking on this on Monday.
> 
> Cheers,
> Mark
> 
> >-Original Message-
> >From: Jan Scheurich [mailto:jan.scheur...@ericsson.com]
> >Sent: Friday, November 24, 2017 4:21 PM
> >To: Kevin Traynor <ktray...@redhat.com>; Kavanagh, Mark B
> ><mark.b.kavan...@intel.com>; d...@openvswitch.org
> >Cc: maxime.coque...@redhat.com; Flavio Leitner <f...@redhat.com>;
> Franck
> >Baudin <fbau...@redhat.com>; Mooney, Sean K <sean.k.moo...@intel.com>;
> >Ilya Maximets <i.maxim...@samsung.com>; Stokes, Ian
> ><ian.sto...@intel.com>; Loftus, Ciara <ciara.lof...@intel.com>;
> Darrell
> >Ball <db...@vmware.com>; Aaron Conole <acon...@redhat.com>
> >Subject: RE: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for vhost
> >IOMMU feature
> >
> >+1
> >Jan
> >
> >> -Original Message-
> >> From: Kevin Traynor [mailto:ktray...@redhat.com]
> >> Sent: Friday, 24 November, 2017 17:11
> >> To: Mark Kavanagh <mark.b.kavan...@intel.com>; d...@openvswitch.org
> >> Cc: maxime.coque...@redhat.com; Flavio Leitner <f...@redhat.com>;
> >> Franck
> >Baudin <fbau...@redhat.com>; Mooney, Sean K
> >> <sean.k.moo...@intel.com>; Ilya Maximets <i.maxim...@samsung.com>;
> >> Ian
> >Stokes <ian.sto...@intel.com>; Loftus, Ciara
> >> <ciara.lof...@intel.com>; Darrell Ball <db...@vmware.com>; Aaron
> >> Conole
> ><acon...@redhat.com>; Jan Scheurich
> >> <jan.scheur...@ericsson.com>
> >> Subject: Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for
> vhost
> >> IOMMU
> >feature
> >>
> >> On 11/16/2017 11:01 AM, Mark Kavanagh wrote:
> >> > DPDK v17.11 introduces support for the vHost IOMMU feature.
> >> > This is a security feature, that restricts the vhost memory that a
> >> > virtio device may access.
> >> >
> >> > This feature also enables the vhost REPLY_ACK protocol, the
> >> > implementation of which is known to work in newer versions of QEMU
> >> > (i.e. v2.10.0), but is buggy in older versions (v2.7.0 - v2.9.0,
> >> > inclusive). As such, the feature is disabled by default in (and
> >> > should remain so, for the aforementioned older QEMU verions).
> >> > Starting with QEMU v2.9.1, vhost-iommu-support can safely be
> >> > enabled, even without having an IOMMU device, with no performance
> >> > penalty.
> >> >
> >> > This patch adds a new vhost port option, vhost-iommu-support, to
> >> > allow enablement of the vhost IOMMU feature:
> >> >
> >> > $ ovs-vsctl add-port br0 vhost-client-1 \
> >> > -- set Interface vhost-client-1 type=dpdkvhostuserclient \
> >> >  options:vhost-server-path=$VHOST_USER_SOCKET_PATH   \
> >> >  options:vhost-iommu-support=true
> >> >
> >>
> >> Hi Mark, All,
> >>
> >> I'm thinking about this and whether the current approach provides
> >> more than what is actually needed by users at the cost of making the
> >> user interface more complex.
[Mooney, Sean K] I am personally split on this. To enable iommu support in
openstack with the above interface we would have to do two things. 1 extend the 
image metatdata
or flavor extra specs to allow requesting a vIOMMU. Second we would have to 
modify os-vif to produce
the add-port command above. Using this interfaces gives us a nice parity in our 
api
as we only enable iommu support in ovs if we enable for qemu. E.g. if the 
falvor/image does not request
a virtualized iommu we wont declare its support in the options.
> >>
> >> As an alternative, how about having a global other_config (to be set
> >> like vhost-socket-dir can be) for this instead of having to set i

Re: [ovs-dev] [PATCH net-next v6 3/3] openvswitch: enable NSH support

2017-08-30 Thread Mooney, Sean K


> -Original Message-
> From: Hannes Frederic Sowa [mailto:han...@stressinduktion.org]
> Sent: Wednesday, August 30, 2017 4:16 PM
> To: Mooney, Sean K <sean.k.moo...@intel.com>
> Cc: Yang, Yi Y <yi.y.y...@intel.com>; d...@openvswitch.org;
> net...@vger.kernel.org; jb...@redhat.com; e...@erig.me
> Subject: Re: [ovs-dev] [PATCH net-next v6 3/3] openvswitch: enable NSH
> support
> 
> "Mooney, Sean K" <sean.k.moo...@intel.com> writes:
> 
> >> -Original Message-
> >> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> >> boun...@openvswitch.org] On Behalf Of Hannes Frederic Sowa
> >> Sent: Wednesday, August 30, 2017 10:53 AM
> >> To: Yang, Yi Y <yi.y.y...@intel.com>
> >> Cc: d...@openvswitch.org; net...@vger.kernel.org; jb...@redhat.com;
> >> e...@erig.me
> >> Subject: Re: [ovs-dev] [PATCH net-next v6 3/3] openvswitch: enable
> >> NSH support
> >>
> >> Hello,
> >>
> >> Yi Yang <yi.y.y...@intel.com> writes:
> >>
> >> [...]
> >>
> >> > +struct ovs_key_nsh {
> >> > +u8 flags;
> >> > +u8 ttl;
> >> > +u8 mdtype;
> >> > +u8 np;
> >> > +__be32 path_hdr;
> >> > +__be32 context[NSH_MD1_CONTEXT_SIZE]; };
> >> > +
> >> >  struct sw_flow_key {
> >> >  u8 tun_opts[IP_TUNNEL_OPTS_MAX];
> >> >  u8 tun_opts_len;
> >> > @@ -144,6 +154,7 @@ struct sw_flow_key {
> >> >  };
> >> >  } ipv6;
> >> >  };
> >> > +struct ovs_key_nsh nsh; /* network service header */
> >> >  struct {
> >> >  /* Connection tracking fields not packed above. */
> >> >  struct {
> >>
> >> Does it makes sense to keep the context headers as part of the flow?
> >> What is the reasoning behind it? With mdtype 2 headers this might
> >> either not work very well or will increase sw_flow_key size causing
> >> slowdowns for all protocols.
> > [Mooney, Sean K]
> > Having the nsh context headers in the flow is quite useful It would
> > allow loadblancing on values stored in the context headers Or other
> > use. I belive odl previously used context header 4 to store a Flow id
> > so this could potentialy be used with the multipath action to have
> ovs
> > Choose between several possible next hops in the chain.
> 
> In OVS, masks are a list(!) for matching. How can this work for
> different paths that might require different masks? If they can't be
> unified you even get exact matches. Thus, for OVS the context should
> not be part of the flow.
[Mooney, Sean K] I'm not sure what you mean about a list as I never made 
reference to one.
md type 1 context headers are 4 mandatory 32 bit field.
form an ovs context they should be treated the same as the 32 bit register 
fields.
We do not need to necessarily support those in md type 2 as all metadata is 
optional.
> 
> > Another example of where this is usefull is branching chains.  if I
> > assume that both the classifier and Service function forwarder are
> > collocated in ovs on the host, and is send A packet to a firewall
> > service function which tags the packet as suspicious Via setting a
> > context header metadata field to 1, I as the sdn controller can
> > Install a high priority rule that will reclassify the packet as part
> > of as separate Service function chain the will prefer dpi on the
> > packet before returning it to The original chain if demand not a
> > threat.
> 
> You can do that with different path id's, too?
[Mooney, Sean K] a service function is not allowed to alter the spi.
Only a classifier can do that. You are correct that a different spi is required 
for the branch
To the dpi sf but packets that are not droped can rejoin the original spi.
Packet are not require you to enter a service chain at the first hop.

service function chain are explicitly not A list. They are a directed graph 
composed
of service function instance or service function groups that generally 
are reducible to a directed acrylic graph and in the simple case to a simple 
list. 
branching an recovering chains are expected, as is loadblancing between 
service function instances in the same service function group which share the 
same spi. 
The spi filed in the nsh heard is intended to transport the logical service 
plane path id
not the rendered service path id.
> 
> > So while a sff dose not in general have to be able to match on the

Re: [ovs-dev] [PATCH net-next v6 3/3] openvswitch: enable NSH support

2017-08-30 Thread Mooney, Sean K


> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Hannes Frederic Sowa
> Sent: Wednesday, August 30, 2017 10:53 AM
> To: Yang, Yi Y <yi.y.y...@intel.com>
> Cc: d...@openvswitch.org; net...@vger.kernel.org; jb...@redhat.com;
> e...@erig.me
> Subject: Re: [ovs-dev] [PATCH net-next v6 3/3] openvswitch: enable NSH
> support
> 
> Hello,
> 
> Yi Yang <yi.y.y...@intel.com> writes:
> 
> [...]
> 
> > +struct ovs_key_nsh {
> > +   u8 flags;
> > +   u8 ttl;
> > +   u8 mdtype;
> > +   u8 np;
> > +   __be32 path_hdr;
> > +   __be32 context[NSH_MD1_CONTEXT_SIZE]; };
> > +
> >  struct sw_flow_key {
> > u8 tun_opts[IP_TUNNEL_OPTS_MAX];
> > u8 tun_opts_len;
> > @@ -144,6 +154,7 @@ struct sw_flow_key {
> > };
> > } ipv6;
> > };
> > +   struct ovs_key_nsh nsh; /* network service header */
> > struct {
> > /* Connection tracking fields not packed above. */
> > struct {
> 
> Does it makes sense to keep the context headers as part of the flow?
> What is the reasoning behind it? With mdtype 2 headers this might
> either not work very well or will increase sw_flow_key size causing
> slowdowns for all protocols.
[Mooney, Sean K] 
Having the nsh context headers in the flow is quite useful
It would allow loadblancing on values stored in the context headers
Or other use. I belive odl previously used context header 4 to store a
Flow id so this could potentialy be used with the multipath action to have ovs
Choose between several possible next hops in the chain.

Another example of where this is usefull is branching chains.
if I assume that both the classifier and
Service function forwarder are collocated in ovs on the host, and is send
A packet to a firewall service function which tags the packet as suspicious
Via setting a context header metadata field to 1, I as the sdn controller can
Install a high priority rule that will reclassify the packet as part of as 
separate
Service function chain the will prefer dpi on the packet before returning it to
The original chain if demand not a threat.

So while a sff dose not in general have to be able to match on the context 
header
If I assume I want to use ovs to implenet a classifier or service function(e.g. 
loadblancer)
The its desirable to be able to both match on the context headers in md type1  
and also be able
To set them(this is something classifies and service fuction are allowed to do).
> 
> [...]
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] dpdk: announce deprecation of vhost-user server ports

2017-06-09 Thread Mooney, Sean K


> -Original Message-
> From: Aaron Conole [mailto:acon...@redhat.com]
> Sent: Friday, June 9, 2017 2:42 PM
> To: Mooney, Sean K <sean.k.moo...@intel.com>; Kavanagh, Mark B
> <mark.b.kavan...@intel.com>
> Cc: Kevin Traynor <ktray...@redhat.com>; d...@openvswitch.org; Darrell
> Ball <db...@vmware.com>; Loftus, Ciara <ciara.lof...@intel.com>
> Subject: Re: [PATCH] dpdk: announce deprecation of vhost-user server
> ports
> 
> Hi Sean, and Mark,
> 
> "Mooney, Sean K" <sean.k.moo...@intel.com> writes:
> 
> >> -Original Message-
> >> From: Aaron Conole [mailto:acon...@redhat.com]
> >> Sent: Thursday, June 8, 2017 8:12 PM
> >> To: Kevin Traynor <ktray...@redhat.com>
> >> Cc: d...@openvswitch.org; Darrell Ball <db...@vmware.com>; Loftus,
> >> Ciara <ciara.lof...@intel.com>; Mooney, Sean K
> >> <sean.k.moo...@intel.com>
> >> Subject: Re: [PATCH] dpdk: announce deprecation of vhost-user server
> >> ports
> >>
> >> Hi Kevin,
> >>
> >> Kevin Traynor <ktray...@redhat.com> writes:
> >>
> >> > On 06/07/2017 11:46 PM, Aaron Conole wrote:
> >> >> Since vhost-user server mode ports are the preferred mechanism
> for
> >> >> interconnecting Open vSwitch with VMs when using DPDK, and since
> >> >> there are currently no known use cases for vhost-user server mode
> >> >> ports apart from version incompatibilities with QEMU, announce
> >> >> that server mode ports are considered deprecated and will be
> >> >> removed in a
> >> future release.
> >
> > [Mooney, Sean K] not to be pedantic but you contradicted your self
> > her. First sentence You say vhost-user server mode ports are
> preferred then you say lets remove them.
> > I would suggest you use the interface names instead and say
> > dpdkvhostuser when referring to what will be removed Server mode port
> > is ambigious since its not clear if you are referring to qemu or dpdk
> > when you say server mode.
> 
> The mistake was pointed out - unfortunately it was already applied.
> 
> Sorry for the confusion.
> 

[Mooney, Sean K] no worries the patch looked fine to me other then the commit 
message
Which isn't a big deal in the grad scheme of things. On the openstack side 
It should be noted that the default ovs agent will use dpdvhostuserclinet 
instead
Of dpdkvhostuser if the ovs installed supports it. for the odl and ovn ml2 
driver
https://review.openstack.org/#/c/344997/ will need to be ported to ensure they 
do
the same so there is no impact when dpdkvhostuser is eventrually removed if it 
is
removed in 2.9?
thank for ccing me its good to know that this change is being made. Having
qemu be the server has many advantages and I agree that there is no known reason
bar compatibility with older qemu version to continue to support dpdk as the 
server
going forward.

> >> >>
> >> >> Cc: Ciara Loftus <ciara.lof...@intel.com>
> >> >> Cc: Kevin Traynor <ktray...@redhat.com>
> >> >> Suggested-by: Darrell Ball <db...@vmware.com>
> >> >> Signed-off-by: Aaron Conole <acon...@redhat.com>
> >> >> ---
> >> >>  Documentation/topics/dpdk/vhost-user.rst | 24
> >> >> -
> >> ---
> >> >>  NEWS |  2 ++
> >> >>  lib/netdev-dpdk.c|  2 ++
> >> >>  3 files changed, 20 insertions(+), 8 deletions(-)
> >> >>
> >> >> diff --git a/Documentation/topics/dpdk/vhost-user.rst
> >> >> b/Documentation/topics/dpdk/vhost-user.rst
> >> >> index a1c19fd..9d36cf2 100644
> >> >> --- a/Documentation/topics/dpdk/vhost-user.rst
> >> >> +++ b/Documentation/topics/dpdk/vhost-user.rst
> >> >> @@ -32,13 +32,19 @@ documentation`_ on same.
> >> >>  Quick Example
> >> >>  -
> >> >>
> >> >> -This example demonstrates how to add two ``dpdkvhostuser`` ports
> >> >> to an existing -bridge called ``br0``::
> >> >> +This example demonstrates how to add two ``dpdkvhostuserclient``
> >> >> +ports to an existing bridge called ``br0``::
> >> >>
> >> >> -$ ovs-vsctl add-port br0 dpdkvhostuser0 \
> >> >> --- set Interface dpdkvhostuser0 type=dpdkvhostuser
> >> >> -$ ovs-vsctl add-port br0 dpdkvhostuser1 \
> >> >> -

Re: [ovs-dev] [PATCH] dpdk: announce deprecation of vhost-user server ports

2017-06-09 Thread Mooney, Sean K


> -Original Message-
> From: Aaron Conole [mailto:acon...@redhat.com]
> Sent: Thursday, June 8, 2017 8:12 PM
> To: Kevin Traynor <ktray...@redhat.com>
> Cc: d...@openvswitch.org; Darrell Ball <db...@vmware.com>; Loftus, Ciara
> <ciara.lof...@intel.com>; Mooney, Sean K <sean.k.moo...@intel.com>
> Subject: Re: [PATCH] dpdk: announce deprecation of vhost-user server
> ports
> 
> Hi Kevin,
> 
> Kevin Traynor <ktray...@redhat.com> writes:
> 
> > On 06/07/2017 11:46 PM, Aaron Conole wrote:
> >> Since vhost-user server mode ports are the preferred mechanism for
> >> interconnecting Open vSwitch with VMs when using DPDK, and since
> >> there are currently no known use cases for vhost-user server mode
> >> ports apart from version incompatibilities with QEMU, announce that
> >> server mode ports are considered deprecated and will be removed in a
> future release.

[Mooney, Sean K] not to be pedantic but you contradicted your self her. First 
sentence
You say vhost-user server mode ports are preferred then you say lets remove 
them.
I would suggest you use the interface names instead and say dpdkvhostuser when 
referring to what will be removed
Server mode port is ambigious since its not clear if you are referring to qemu 
or dpdk when you say server mode.

> >>
> >> Cc: Ciara Loftus <ciara.lof...@intel.com>
> >> Cc: Kevin Traynor <ktray...@redhat.com>
> >> Suggested-by: Darrell Ball <db...@vmware.com>
> >> Signed-off-by: Aaron Conole <acon...@redhat.com>
> >> ---
> >>  Documentation/topics/dpdk/vhost-user.rst | 24 -
> ---
> >>  NEWS |  2 ++
> >>  lib/netdev-dpdk.c|  2 ++
> >>  3 files changed, 20 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/Documentation/topics/dpdk/vhost-user.rst
> >> b/Documentation/topics/dpdk/vhost-user.rst
> >> index a1c19fd..9d36cf2 100644
> >> --- a/Documentation/topics/dpdk/vhost-user.rst
> >> +++ b/Documentation/topics/dpdk/vhost-user.rst
> >> @@ -32,13 +32,19 @@ documentation`_ on same.
> >>  Quick Example
> >>  -
> >>
> >> -This example demonstrates how to add two ``dpdkvhostuser`` ports to
> >> an existing -bridge called ``br0``::
> >> +This example demonstrates how to add two ``dpdkvhostuserclient``
> >> +ports to an existing bridge called ``br0``::
> >>
> >> -$ ovs-vsctl add-port br0 dpdkvhostuser0 \
> >> --- set Interface dpdkvhostuser0 type=dpdkvhostuser
> >> -$ ovs-vsctl add-port br0 dpdkvhostuser1 \
> >> --- set Interface dpdkvhostuser1 type=dpdkvhostuser
> >> +$ ovs-vsctl add-port br0 dpdkvhostclient0 \
> >> +-- set Interface dpdkvhostclient0 type=dpdkvhostuserclient
> \
> >> +   options:vhost-server-path=/tmp/dpdkvhostclient0
> >> +$ ovs-vsctl add-port br0 dpdkvhostclient1 \
> >> +-- set Interface dpdkvhostclient1 type=dpdkvhostuserclient
> \
> >> +   options:vhost-server-path=/tmp/dpdkvhostclient1
> >> +
> >> +For the above examples to work, an appropriate server socket must
> be
> >> +created at the paths specified (``/tmp/dpdkvhostclient0`` and
> >> +``/tmp/dpdkvhostclient0``).
> >
> > You could mention QEMU here. So the reader knows where to look.
> > "These can be created by QEMU. See below for details."?
> 
> Good idea.  I'll add it.
> 
> Thanks for the review!
> 
> >>  vhost-user vs. vhost-user-client
> >>  
> >> @@ -59,7 +65,8 @@ means if OVS dies, all VMs **must** be restarted.
> >> On the other hand, for  vhost-user-client ports, OVS acts as the
> >> client and QEMU the server. This means  OVS can die and be restarted
> >> without issue, and it is also possible to restart  an instance
> >> itself. For this reason, vhost-user-client ports are the preferred -
> type for most use cases.
> >> +type for most use cases.  Ports of type vhost-user are currently
> >> +deprecated and will be removed in a future release.
> >>
> >>  .. _dpdk-vhost-user:
> >>
> >> @@ -68,7 +75,8 @@ vhost-user
> >>
> >>  .. important::
> >>
> >> -   Use of vhost-user ports requires QEMU >= 2.2
> >> +   Use of vhost-user ports requires QEMU >= 2.2;  vhost-user ports
> are
> >> +   *deprecated*.
> >>
> >>  To use vhost-user ports, you must first add said port