Darrell Ball <[email protected]> writes: > 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.
Well, at least for the first one, that information could be correlated, as something like this would appear in the logs: 2017-06-07T22:41:45.051Z|00049|netdev_dpdk|INFO|Socket /var/run/openvswitch/vhu0 created for vhost-user port vhu0 2017-06-07T22:41:45.051Z|00050|netdev_dpdk|WARN|dpdkvhostuser ports are considered deprecated; please migrate to dpdkvhostuserclient ports. So, at least the first time, we see it. After that, I think the user should be smart enough to ovs-vsctl show and see which ports are vhost-user vs. vhost-user-client. > I am not convinced, but you guys are closer to your customers at > least, so I’ll defer to > your opinion. I'm just worried about what happens if we spam the logs. Too much or too little information makes the SNR too heavy on the 'noise' side. I'll submit a v2 with the suggestions so far, omitting a change to the VLOG_WARN_ONCE for now. Thanks everyone for the discussion. > [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
