[dpdk-dev] doc: announce API/ABI changes for vhost

Message ID 1485176682-12379-1-git-send-email-yuanhan.liu@linux.intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation success Compilation OK

Commit Message

Yuanhan Liu Jan. 23, 2017, 1:04 p.m. UTC
I made a vhost ABI/API refactoring at v16.04, meant to avoid such issue
forever. Well, apparently, I lied.

People are looking for more vhost-user options now days, other than
vhost-user net only. For example, SPDK (Storage Performance Development
Kit) are looking for chance of vhost-user SCSI and vhost-user block.

Apparently, they also need a vhost-user backend, while DPDK already
has a (mature enough) backend, they don't want to implement it again
from scratch. They want to leverage the one DPDK provides.

However, the last refactoring hasn't done that right, at least it's
not friendly for extending vhost-user to add more devices support.
For example, different virtio devices has its own feature set, while
APIs like rte_vhost_feature_disable(feature_mask) have no option to
tell the device type. Thus, a more proper API should look like:

    rte_vhost_feature_disable(device_type, feature_mask);

Besides that, few public files and structures should be renamed, to
not let it bind to virtio-net. Specifically, they are:

- virtio_net_device_ops --> vhost_device_ops
- rte_virtio_net.h      --> rte_vhost.h

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---

I intended to send out the code first (I have already finished the
better part of it), while I'm starting vacation since tomorrow, leaving
me no time for that.
---
 doc/guides/rel_notes/deprecation.rst | 9 +++++++++
 1 file changed, 9 insertions(+)
  

Comments

Thomas Monjalon Feb. 13, 2017, 6:02 p.m. UTC | #1
2017-01-23 21:04, Yuanhan Liu:
> I made a vhost ABI/API refactoring at v16.04, meant to avoid such issue
> forever. Well, apparently, I lied.
> 
> People are looking for more vhost-user options now days, other than
> vhost-user net only. For example, SPDK (Storage Performance Development
> Kit) are looking for chance of vhost-user SCSI and vhost-user block.
> 
> Apparently, they also need a vhost-user backend, while DPDK already
> has a (mature enough) backend, they don't want to implement it again
> from scratch. They want to leverage the one DPDK provides.
> 
> However, the last refactoring hasn't done that right, at least it's
> not friendly for extending vhost-user to add more devices support.
> For example, different virtio devices has its own feature set, while
> APIs like rte_vhost_feature_disable(feature_mask) have no option to
> tell the device type. Thus, a more proper API should look like:
> 
>     rte_vhost_feature_disable(device_type, feature_mask);
> 
> Besides that, few public files and structures should be renamed, to
> not let it bind to virtio-net. Specifically, they are:
> 
> - virtio_net_device_ops --> vhost_device_ops
> - rte_virtio_net.h      --> rte_vhost.h
> 
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
  
Jerin Jacob Feb. 14, 2017, 3:21 a.m. UTC | #2
On Mon, Feb 13, 2017 at 07:02:56PM +0100, Thomas Monjalon wrote:
> 2017-01-23 21:04, Yuanhan Liu:
> > I made a vhost ABI/API refactoring at v16.04, meant to avoid such issue
> > forever. Well, apparently, I lied.
> > 
> > People are looking for more vhost-user options now days, other than
> > vhost-user net only. For example, SPDK (Storage Performance Development
> > Kit) are looking for chance of vhost-user SCSI and vhost-user block.
> > 
> > Apparently, they also need a vhost-user backend, while DPDK already
> > has a (mature enough) backend, they don't want to implement it again
> > from scratch. They want to leverage the one DPDK provides.
> > 
> > However, the last refactoring hasn't done that right, at least it's
> > not friendly for extending vhost-user to add more devices support.
> > For example, different virtio devices has its own feature set, while
> > APIs like rte_vhost_feature_disable(feature_mask) have no option to
> > tell the device type. Thus, a more proper API should look like:
> > 
> >     rte_vhost_feature_disable(device_type, feature_mask);
> > 
> > Besides that, few public files and structures should be renamed, to
> > not let it bind to virtio-net. Specifically, they are:
> > 
> > - virtio_net_device_ops --> vhost_device_ops
> > - rte_virtio_net.h      --> rte_vhost.h
> > 
> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> 
> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
  
Maxime Coquelin Feb. 14, 2017, 1:54 p.m. UTC | #3
Hi Yuanhan,

On 01/23/2017 02:04 PM, Yuanhan Liu wrote:
> I made a vhost ABI/API refactoring at v16.04, meant to avoid such issue
> forever. Well, apparently, I lied.
>
> People are looking for more vhost-user options now days, other than
> vhost-user net only. For example, SPDK (Storage Performance Development
> Kit) are looking for chance of vhost-user SCSI and vhost-user block.
>
> Apparently, they also need a vhost-user backend, while DPDK already
> has a (mature enough) backend, they don't want to implement it again
> from scratch. They want to leverage the one DPDK provides.
>
> However, the last refactoring hasn't done that right, at least it's
> not friendly for extending vhost-user to add more devices support.
> For example, different virtio devices has its own feature set, while
> APIs like rte_vhost_feature_disable(feature_mask) have no option to
> tell the device type. Thus, a more proper API should look like:
>
>     rte_vhost_feature_disable(device_type, feature_mask);

I wonder if we could also change it to be per-instance, instead of
disabling features globally:
rte_vhost_feature_disable(vid, device_type, feature_mask);

It could be useful for live-migration with different backend versions on
the hosts, as it would allow to run instances with different compat
modes (like running vhost's DPDK v17.08 with v17.05-only supported
features).
I made a proposal about cross-version migration, but we are far from a
conclusion on the design.

>
> Besides that, few public files and structures should be renamed, to
> not let it bind to virtio-net. Specifically, they are:
>
> - virtio_net_device_ops --> vhost_device_ops
> - rte_virtio_net.h      --> rte_vhost.h
>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Anyway, the change you propose is necessary:
Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime
  
Thomas Monjalon Feb. 14, 2017, 8:28 p.m. UTC | #4
Applied
  

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 755dc65..5d6e9b6 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -62,3 +62,12 @@  Deprecation Notices
   PMDs that implement the latter.
   Target release for removal of the legacy API will be defined once most
   PMDs have switched to rte_flow.
+
+* vhost: API/ABI changes are planned for 17.05, for making DPDK vhost library
+  generic enough so that applications can build different vhost-user drivers
+  (instead of vhost-user net only) on top of that.
+  Specifically, ``virtio_net_device_ops`` will be renamed to ``vhost_device_ops``.
+  Correspondingly, some API's parameter need be changed. Few more functions also
+  need be reworked to let it be device aware. For example, different virtio device
+  has different feature set, meaning functions like ``rte_vhost_feature_disable``
+  need be changed. Last, file rte_virtio_net.h will be renamed to rte_vhost.h.