On 6/8/17, 11:22 AM, "Darrell Ball" <[email protected]> wrote:
On 6/8/17, 11:13 AM, "Flavio Leitner" <[email protected]> wrote:
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.
If you think most users will take notice and understand one log, then
sure; I don’t know if that is the case.
Another alternative is to print the socket name and port name for a limited
number of
ports like 10, limited by using a static variable counter.
This would provide all the needed information in the majority of cases, but
bound the
associated logging.
--
Flavio
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
