On 6/8/17, 12:29 PM, "Flavio Leitner" <[email protected]> wrote:
On Thu, Jun 08, 2017 at 06:54:24PM +0000, Darrell Ball wrote:
>
>
> 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.
Most probably existing deployments won't change, but OVS might be
upgraded, then logging many times will just fire alarms for no good
reason.
Well, it is up to 10 unique logs repeated vs 1 log repeated on upgrade. This
implies
that 1 will be ignored but 10 would not be and would instead be
downright scary; really ?
Whoever is spinning up VM most probably will choose DPDK/"vhost-user"[1]
and not really the underlying type, so not much the user can do.
I agree on this part
It's a warning message, if someone doesn't care about that level then
repeating it most probably won't help either. Actually, the more we spam
the log, the less people will read.
If it was 10 identical vs 1, I would agree.
The reason for 10 is to provide the details on which ports (name and socket
name),
so the average user can correlate.
I am not convinced, but you guys are closer to your customers at least, so I’ll
defer to
your opinion.
[1] https://docs.openstack.org/ocata/networking-guide/config-ovs-dpdk.html
--
Flavio
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
