On Thu, Jun 08, 2017 at 09:40:52AM -0400, Aaron Conole wrote:
> Hi Darrell,
> 
> Thanks so much for the review!  Comments below.
> 
> Darrell Ball <[email protected]> writes:
> 
> > On 6/7/17, 3:46 PM, "Aaron Conole" <[email protected]> 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.
> >     
> >     Cc: Ciara Loftus <[email protected]>
> >     Cc: Kevin Traynor <[email protected]>
> >     Suggested-by: Darrell Ball <[email protected]>
> >     Signed-off-by: Aaron Conole <[email protected]>
> >     ---
> >      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``).
> >      
> >      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.
> >
> > type for all known use cases; the only limitation is that vhost-user client 
> > mode ports
> > require QEMU version 2.7.  Ports of type vhost-user are currently 
> > deprecated and
> > will be removed in a future release.
> 
> Will update with this verbiage.  Thanks.
> 
> >      .. _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 ports to the switch. 
> > DPDK
> >      vhost-user ports can have arbitrary names with the exception of 
> > forward and
> >     diff --git a/NEWS b/NEWS
> >     index 82004c8..b81d033 100644
> >     --- a/NEWS
> >     +++ b/NEWS
> >     @@ -16,6 +16,8 @@ Post-v2.7.0
> >             Log level can be changed in a usual OVS way using
> >             'ovs-appctl vlog' commands for 'dpdk' module. Lower bound
> >             still can be configured via extra arguments for DPDK EAL.
> >     +     * dpdkvhostuser ports are marked as deprecated.  They will be 
> > removed
> >     +       in an upcoming release.
> >         - IPFIX now provides additional counters:
> >           * Total counters since metering process startup.
> >           * Per-flow TCP flag counters.
> >     diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >     index b770b70..9ab4aeb 100644
> >     --- a/lib/netdev-dpdk.c
> >     +++ b/lib/netdev-dpdk.c
> >     @@ -966,6 +966,8 @@ netdev_dpdk_vhost_construct(struct netdev *netdev)
> >          err = vhost_common_construct(netdev);
> >      
> >          ovs_mutex_unlock(&dpdk_mutex);
> >     +    VLOG_WARN_ONCE("dpdkvhostuser ports are considered deprecated;  "
> >     +                   "please migrate to dpdkvhostuserclient ports.");
> >
> > I think we can:
> > 1) Print the socket name and port name
> > 2) I am not sure ‘_ONCE’ is required; do you really think the log will have 
> > that many instances.
> 
> My idea to not print the socket / port name is because I figure there
> would be cases that users have many VMs, do an upgrade, and see the log
> spewed over and over.  Maybe that's a good thing though - not sure.
> 
> If you consider a deployment with 100 VMs, that means this will pop up
> 100 times.  Even more, in deployments where they are using orchestration
> software, or a cluster management solution, I figure those systems may
> still be using the old style dpdkvhostuser ports, and again didn't want
> to print it for every start of a VM - especially when there isn't
> anything the user could do about it.
> 
> If you think there's a strong value in warning on every start, and
> including the details of the port, I'll do that.  I'm not married to
> this particular code ;)

One message should be enough, please don't flood the logs.

-- 
Flavio

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to