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 ;) -Aaron _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
