diff mbox

[ovs-dev] dpdk: announce deprecation of vhost-user server ports

Message ID 20170607224628.22602-1-aconole@redhat.com
State Changes Requested
Headers show

Commit Message

Aaron Conole June 7, 2017, 10:46 p.m. UTC
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 <ciara.loftus@intel.com>
Cc: Kevin Traynor <ktraynor@redhat.com>
Suggested-by: Darrell Ball <dball@vmware.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 Documentation/topics/dpdk/vhost-user.rst | 24 ++++++++++++++++--------
 NEWS                                     |  2 ++
 lib/netdev-dpdk.c                        |  2 ++
 3 files changed, 20 insertions(+), 8 deletions(-)

Comments

Darrell Ball June 7, 2017, 11:25 p.m. UTC | #1
On 6/7/17, 3:46 PM, "Aaron Conole" <aconole@redhat.com> 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 <ciara.loftus@intel.com>
    Cc: Kevin Traynor <ktraynor@redhat.com>
    Suggested-by: Darrell Ball <dball@vmware.com>
    Signed-off-by: Aaron Conole <aconole@redhat.com>

    ---
     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.


     
     .. _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.



         return err;
     }
     
    -- 
    2.9.4
Aaron Conole June 8, 2017, 1:40 p.m. UTC | #2
Hi Darrell,

Thanks so much for the review!  Comments below.

Darrell Ball <dball@vmware.com> writes:

> On 6/7/17, 3:46 PM, "Aaron Conole" <aconole@redhat.com> 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 <ciara.loftus@intel.com>
>     Cc: Kevin Traynor <ktraynor@redhat.com>
>     Suggested-by: Darrell Ball <dball@vmware.com>
>     Signed-off-by: Aaron Conole <aconole@redhat.com>
>     ---
>      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
Kevin Traynor June 8, 2017, 6:05 p.m. UTC | #3
On 06/07/2017 11:46 PM, Aaron Conole 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 <ciara.loftus@intel.com>
> Cc: Kevin Traynor <ktraynor@redhat.com>
> Suggested-by: Darrell Ball <dball@vmware.com>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  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``).

You could mention QEMU here. So the reader knows where to look.
"These can be created by QEMU. See below for details."?

>  
>  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.
>  
>  .. _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.");
>      return err;
>  }
>  
>
Darrell Ball June 8, 2017, 6:13 p.m. UTC | #4
On 6/8/17, 6:40 AM, "Aaron Conole" <aconole@redhat.com> wrote:

    Hi Darrell,
    
    Thanks so much for the review!  Comments below.
    
    Darrell Ball <dball@vmware.com> writes:
    
    > On 6/7/17, 3:46 PM, "Aaron Conole" <aconole@redhat.com> 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 <ciara.loftus@intel.com>

    >     Cc: Kevin Traynor <ktraynor@redhat.com>

    >     Suggested-by: Darrell Ball <dball@vmware.com>

    >     Signed-off-by: Aaron Conole <aconole@redhat.com>

    >     ---

    >      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 ;)

I was thinking more like 20 times would be the high number in most cases, but 100 is fine in rare cases.

This should be hard to ignore (maybe even a little annoying) for the
ultimate benefit of the user experience.



    
    -Aaron
Flavio Leitner June 8, 2017, 6:13 p.m. UTC | #5
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 <dball@vmware.com> writes:
> 
> > On 6/7/17, 3:46 PM, "Aaron Conole" <aconole@redhat.com> 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 <ciara.loftus@intel.com>
> >     Cc: Kevin Traynor <ktraynor@redhat.com>
> >     Suggested-by: Darrell Ball <dball@vmware.com>
> >     Signed-off-by: Aaron Conole <aconole@redhat.com>
> >     ---
> >      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.
Darrell Ball June 8, 2017, 6:22 p.m. UTC | #6
On 6/8/17, 11:13 AM, "Flavio Leitner" <fbl@sysclose.org> 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 <dball@vmware.com> writes:

    > 

    > > On 6/7/17, 3:46 PM, "Aaron Conole" <aconole@redhat.com> 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 <ciara.loftus@intel.com>

    > >     Cc: Kevin Traynor <ktraynor@redhat.com>

    > >     Suggested-by: Darrell Ball <dball@vmware.com>

    > >     Signed-off-by: Aaron Conole <aconole@redhat.com>

    > >     ---

    > >      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.








    
    -- 
    Flavio
Darrell Ball June 8, 2017, 6:54 p.m. UTC | #7
On 6/8/17, 11:22 AM, "Darrell Ball" <dball@vmware.com> wrote:

    
    
    On 6/8/17, 11:13 AM, "Flavio Leitner" <fbl@sysclose.org> 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 <dball@vmware.com> writes:

        > 

        > > On 6/7/17, 3:46 PM, "Aaron Conole" <aconole@redhat.com> 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 <ciara.loftus@intel.com>

        > >     Cc: Kevin Traynor <ktraynor@redhat.com>

        > >     Suggested-by: Darrell Ball <dball@vmware.com>

        > >     Signed-off-by: Aaron Conole <aconole@redhat.com>

        > >     ---

        > >      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
Aaron Conole June 8, 2017, 7:11 p.m. UTC | #8
Hi Kevin,

Kevin Traynor <ktraynor@redhat.com> writes:

> On 06/07/2017 11:46 PM, Aaron Conole 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 <ciara.loftus@intel.com>
>> Cc: Kevin Traynor <ktraynor@redhat.com>
>> Suggested-by: Darrell Ball <dball@vmware.com>
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>>  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``).
>
> You could mention QEMU here. So the reader knows where to look.
> "These can be created by QEMU. See below for details."?

Good idea.  I'll add it.

Thanks for the review!

>>  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.
>>  
>>  .. _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.");
>>      return err;
>>  }
>>  
>>
Flavio Leitner June 8, 2017, 7:29 p.m. UTC | #9
On Thu, Jun 08, 2017 at 06:54:24PM +0000, Darrell Ball wrote:
> 
> 
> On 6/8/17, 11:22 AM, "Darrell Ball" <dball@vmware.com> wrote:
> 
>     
>     
>     On 6/8/17, 11:13 AM, "Flavio Leitner" <fbl@sysclose.org> 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 <dball@vmware.com> writes:
>         > 
>         > > On 6/7/17, 3:46 PM, "Aaron Conole" <aconole@redhat.com> 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 <ciara.loftus@intel.com>
>         > >     Cc: Kevin Traynor <ktraynor@redhat.com>
>         > >     Suggested-by: Darrell Ball <dball@vmware.com>
>         > >     Signed-off-by: Aaron Conole <aconole@redhat.com>
>         > >     ---
>         > >      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.

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.

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.

[1] https://docs.openstack.org/ocata/networking-guide/config-ovs-dpdk.html
Darrell Ball June 8, 2017, 8:02 p.m. UTC | #10
On 6/8/17, 12:29 PM, "Flavio Leitner" <fbl@sysclose.org> wrote:

    On Thu, Jun 08, 2017 at 06:54:24PM +0000, Darrell Ball wrote:
    > 

    > 

    > On 6/8/17, 11:22 AM, "Darrell Ball" <dball@vmware.com> wrote:

    > 

    >     

    >     

    >     On 6/8/17, 11:13 AM, "Flavio Leitner" <fbl@sysclose.org> 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 <dball@vmware.com> writes:

    >         > 

    >         > > On 6/7/17, 3:46 PM, "Aaron Conole" <aconole@redhat.com> 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 <ciara.loftus@intel.com>

    >         > >     Cc: Kevin Traynor <ktraynor@redhat.com>

    >         > >     Suggested-by: Darrell Ball <dball@vmware.com>

    >         > >     Signed-off-by: Aaron Conole <aconole@redhat.com>

    >         > >     ---

    >         > >      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
Aaron Conole June 8, 2017, 8:34 p.m. UTC | #11
Darrell Ball <dball@vmware.com> writes:

> On 6/8/17, 12:29 PM, "Flavio Leitner" <fbl@sysclose.org> wrote:
>
>     On Thu, Jun 08, 2017 at 06:54:24PM +0000, Darrell Ball wrote:
>     > 
>     > 
>     > On 6/8/17, 11:22 AM, "Darrell Ball" <dball@vmware.com> wrote:
>     > 
>     >     
>     >     
>     >     On 6/8/17, 11:13 AM, "Flavio Leitner" <fbl@sysclose.org> 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 <dball@vmware.com> writes:
>     >         > 
>     >         > > On 6/7/17, 3:46 PM, "Aaron Conole" <aconole@redhat.com> 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 <ciara.loftus@intel.com>
>     >         > >     Cc: Kevin Traynor <ktraynor@redhat.com>
>     >         > >     Suggested-by: Darrell Ball <dball@vmware.com>
>     >         > >     Signed-off-by: Aaron Conole <aconole@redhat.com>
>     >         > >     ---
>     >         > >      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
>     
>
Mooney, Sean K June 9, 2017, 10:36 a.m. UTC | #12
> -----Original Message-----
> From: Aaron Conole [mailto:aconole@redhat.com]
> Sent: Thursday, June 8, 2017 8:12 PM
> To: Kevin Traynor <ktraynor@redhat.com>
> Cc: dev@openvswitch.org; Darrell Ball <dball@vmware.com>; Loftus, Ciara
> <ciara.loftus@intel.com>; Mooney, Sean K <sean.k.mooney@intel.com>
> Subject: Re: [PATCH] dpdk: announce deprecation of vhost-user server
> ports
> 
> Hi Kevin,
> 
> Kevin Traynor <ktraynor@redhat.com> writes:
> 
> > On 06/07/2017 11:46 PM, Aaron Conole 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.

[Mooney, Sean K] not to be pedantic but you contradicted your self her. First sentence
You say vhost-user server mode ports are preferred then you say lets remove them.
I would suggest you use the interface names instead and say dpdkvhostuser when referring to what will be removed
Server mode port is ambigious since its not clear if you are referring to qemu or dpdk when you say server mode.

> >>
> >> Cc: Ciara Loftus <ciara.loftus@intel.com>
> >> Cc: Kevin Traynor <ktraynor@redhat.com>
> >> Suggested-by: Darrell Ball <dball@vmware.com>
> >> Signed-off-by: Aaron Conole <aconole@redhat.com>
> >> ---
> >>  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``).
> >
> > You could mention QEMU here. So the reader knows where to look.
> > "These can be created by QEMU. See below for details."?
> 
> Good idea.  I'll add it.
> 
> Thanks for the review!
> 
> >>  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.
> >>
> >>  .. _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.");
> >>      return err;
> >>  }
> >>
> >>
Aaron Conole June 9, 2017, 1:41 p.m. UTC | #13
Hi Sean, and Mark,

"Mooney, Sean K" <sean.k.mooney@intel.com> writes:

>> -----Original Message-----
>> From: Aaron Conole [mailto:aconole@redhat.com]
>> Sent: Thursday, June 8, 2017 8:12 PM
>> To: Kevin Traynor <ktraynor@redhat.com>
>> Cc: dev@openvswitch.org; Darrell Ball <dball@vmware.com>; Loftus, Ciara
>> <ciara.loftus@intel.com>; Mooney, Sean K <sean.k.mooney@intel.com>
>> Subject: Re: [PATCH] dpdk: announce deprecation of vhost-user server
>> ports
>> 
>> Hi Kevin,
>> 
>> Kevin Traynor <ktraynor@redhat.com> writes:
>> 
>> > On 06/07/2017 11:46 PM, Aaron Conole 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.
>
> [Mooney, Sean K] not to be pedantic but you contradicted your self her. First sentence
> You say vhost-user server mode ports are preferred then you say lets remove them.
> I would suggest you use the interface names instead and say
> dpdkvhostuser when referring to what will be removed
> Server mode port is ambigious since its not clear if you are referring
> to qemu or dpdk when you say server mode.

The mistake was pointed out - unfortunately it was already applied.

Sorry for the confusion.

>> >>
>> >> Cc: Ciara Loftus <ciara.loftus@intel.com>
>> >> Cc: Kevin Traynor <ktraynor@redhat.com>
>> >> Suggested-by: Darrell Ball <dball@vmware.com>
>> >> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> >> ---
>> >>  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``).
>> >
>> > You could mention QEMU here. So the reader knows where to look.
>> > "These can be created by QEMU. See below for details."?
>> 
>> Good idea.  I'll add it.
>> 
>> Thanks for the review!
>> 
>> >>  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.
>> >>
>> >>  .. _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.");
>> >>      return err;
>> >>  }
>> >>
>> >>
Mooney, Sean K June 9, 2017, 2:30 p.m. UTC | #14
> -----Original Message-----
> From: Aaron Conole [mailto:aconole@redhat.com]
> Sent: Friday, June 9, 2017 2:42 PM
> To: Mooney, Sean K <sean.k.mooney@intel.com>; Kavanagh, Mark B
> <mark.b.kavanagh@intel.com>
> Cc: Kevin Traynor <ktraynor@redhat.com>; dev@openvswitch.org; Darrell
> Ball <dball@vmware.com>; Loftus, Ciara <ciara.loftus@intel.com>
> Subject: Re: [PATCH] dpdk: announce deprecation of vhost-user server
> ports
> 
> Hi Sean, and Mark,
> 
> "Mooney, Sean K" <sean.k.mooney@intel.com> writes:
> 
> >> -----Original Message-----
> >> From: Aaron Conole [mailto:aconole@redhat.com]
> >> Sent: Thursday, June 8, 2017 8:12 PM
> >> To: Kevin Traynor <ktraynor@redhat.com>
> >> Cc: dev@openvswitch.org; Darrell Ball <dball@vmware.com>; Loftus,
> >> Ciara <ciara.loftus@intel.com>; Mooney, Sean K
> >> <sean.k.mooney@intel.com>
> >> Subject: Re: [PATCH] dpdk: announce deprecation of vhost-user server
> >> ports
> >>
> >> Hi Kevin,
> >>
> >> Kevin Traynor <ktraynor@redhat.com> writes:
> >>
> >> > On 06/07/2017 11:46 PM, Aaron Conole 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.
> >
> > [Mooney, Sean K] not to be pedantic but you contradicted your self
> > her. First sentence You say vhost-user server mode ports are
> preferred then you say lets remove them.
> > I would suggest you use the interface names instead and say
> > dpdkvhostuser when referring to what will be removed Server mode port
> > is ambigious since its not clear if you are referring to qemu or dpdk
> > when you say server mode.
> 
> The mistake was pointed out - unfortunately it was already applied.
> 
> Sorry for the confusion.
> 

[Mooney, Sean K] no worries the patch looked fine to me other then the commit message
Which isn't a big deal in the grad scheme of things. On the openstack side 
It should be noted that the default ovs agent will use dpdvhostuserclinet instead
Of dpdkvhostuser if the ovs installed supports it. for the odl and ovn ml2 driver
https://review.openstack.org/#/c/344997/ will need to be ported to ensure they do
the same so there is no impact when dpdkvhostuser is eventrually removed if it is
removed in 2.9?
thank for ccing me its good to know that this change is being made. Having
qemu be the server has many advantages and I agree that there is no known reason
bar compatibility with older qemu version to continue to support dpdk as the server
going forward.

> >> >>
> >> >> Cc: Ciara Loftus <ciara.loftus@intel.com>
> >> >> Cc: Kevin Traynor <ktraynor@redhat.com>
> >> >> Suggested-by: Darrell Ball <dball@vmware.com>
> >> >> Signed-off-by: Aaron Conole <aconole@redhat.com>
> >> >> ---
> >> >>  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``).
> >> >
> >> > You could mention QEMU here. So the reader knows where to look.
> >> > "These can be created by QEMU. See below for details."?
> >>
> >> Good idea.  I'll add it.
> >>
> >> Thanks for the review!
> >>
> >> >>  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.
> >> >>
> >> >>  .. _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.");
> >> >>      return err;
> >> >>  }
> >> >>
> >> >>
Darrell Ball June 9, 2017, 4:27 p.m. UTC | #15
On 6/9/17, 6:41 AM, "Aaron Conole" <aconole@redhat.com> wrote:

    Hi Sean, and Mark,
    
    "Mooney, Sean K" <sean.k.mooney@intel.com> writes:
    
    >> -----Original Message-----

    >> From: Aaron Conole [mailto:aconole@redhat.com]

    >> Sent: Thursday, June 8, 2017 8:12 PM

    >> To: Kevin Traynor <ktraynor@redhat.com>

    >> Cc: dev@openvswitch.org; Darrell Ball <dball@vmware.com>; Loftus, Ciara

    >> <ciara.loftus@intel.com>; Mooney, Sean K <sean.k.mooney@intel.com>

    >> Subject: Re: [PATCH] dpdk: announce deprecation of vhost-user server

    >> ports

    >> 

    >> Hi Kevin,

    >> 

    >> Kevin Traynor <ktraynor@redhat.com> writes:

    >> 

    >> > On 06/07/2017 11:46 PM, Aaron Conole 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.

    >

    > [Mooney, Sean K] not to be pedantic but you contradicted your self her. First sentence

    > You say vhost-user server mode ports are preferred then you say lets remove them.

    > I would suggest you use the interface names instead and say

    > dpdkvhostuser when referring to what will be removed

    > Server mode port is ambigious since its not clear if you are referring

    > to qemu or dpdk when you say server mode.

    
    The mistake was pointed out - unfortunately it was already applied.
    
    Sorry for the confusion.


Ouch.
Since this is a documentation patch for the most part, maybe most
people will just refer to the documentation itself and not notice the extra
‘server’ word in the commit message.

It is a bit odd to say
“this is the preferred method and therefore we going to nuke it” (

    >> >>

    >> >> Cc: Ciara Loftus <ciara.loftus@intel.com>

    >> >> Cc: Kevin Traynor <ktraynor@redhat.com>

    >> >> Suggested-by: Darrell Ball <dball@vmware.com>

    >> >> Signed-off-by: Aaron Conole <aconole@redhat.com>

    >> >> ---

    >> >>  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``).

    >> >

    >> > You could mention QEMU here. So the reader knows where to look.

    >> > "These can be created by QEMU. See below for details."?

    >> 

    >> Good idea.  I'll add it.

    >> 

    >> Thanks for the review!

    >> 

    >> >>  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.

    >> >>

    >> >>  .. _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.");

    >> >>      return err;

    >> >>  }

    >> >>

    >> >>
diff mbox

Patch

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.
 
 .. _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.");
     return err;
 }