[dpdk-dev] [PATCH v3 00/20] vhost ABI/API refactoring

[Thomas Monjalon]

2016-06-30 11:15, Mcnamara, John:
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Panu Matilainen
> > On 06/30/2016 10:57 AM, Yuanhan Liu wrote:
> > > On Thu, Jun 30, 2016 at 10:39:45AM +0300, Panu Matilainen wrote:
> > >> On 06/07/2016 06:51 AM, Yuanhan Liu wrote:
> > >>> v3: - adapted the new vhost ABI/API changes to tep_term example, to
> > make
> > >>>      sure not break build at least.
> > >>>    - bumped the ABI version to 3
> > >>>
> > >>> NOTE: I created a branch at dpdk.org [0] for more conveinient testing:
> > >>>
> > >>>    [0]: git://dpdk.org/next/dpdk-next-virtio for-testing
> > >>>
> > >>>
> > >>> Every time we introduce a new feature to vhost, we are likely to
> > >>> break ABI. Moreover, some cleanups (such as the one from Ilya to
> > >>> remove vec_buf
> > >> >from vhost_virtqueue struct) also break ABI.
> > >>>
> > >>> This patch set is meant to resolve above issue ultimately, by hiding
> > >>> virtio_net structure (as well as few others) internaly, and export
> > >>> the virtio_net dev strut to applications by a number, vid, like the
> > >>> way kernel exposes an fd to user space.
> > >>>
> > >>> Back to the patch set, the first part of this set makes some changes
> > >>> to vhost example, vhost-pmd and vhost, bit by bit, to remove the
> > >>> dependence to "virtio_net" struct. And then do the final change to
> > >>> make the current APIs to adapt to using "vid".
> > >>>
> > >>> After that, "vrtio_net_device_ops" is the only left open struct that
> > >>> an application can acces, therefore, it's the only place that might
> > >>> introduce potential ABI breakage in future for extension. Hence, I
> > >>> made few more
> > >>> (5) space reservation, to make sure we will not break ABI for a long
> > >>> time, and hopefuly, forever.
> > >>
> > >> Been intending to say this for a while but seems I never actually got
> > >> around to do so:
> > >>
> > >> This is a really fine example of how to refactor an API against
> > >> constant ABI breakages, thank you Yuanhan!
> > >
> > > Panu, thanks!
> > >
> > >> Exported structs are one of the biggest obstacles in keeping a stable
> > >> ABI while adding new features, and while its not always possible to
> > >> hide everything to this extent, the damage (erm,
> > >> exposure) can usually be considerably limited by careful API design.
> > >
> > > Agreed.
> > >
> > >> Since the first and the foremost objection against doing this in the
> > >> DPDK context is always "but performance!", I'm curious as to what
> > >> sort of numbers you're getting with the new API vs the old one? I'm
> > >> really hoping other libraries would follow suit after seeing that its
> > >> possible to provide a future-proof API/ABI without sacrificing
> > >> performance :)
> > >
> > > From my (limited) test, nope, I see no performance drop at all, not
> > > even a little.
> > 
> > Awesome!
> > 
> > With that, hopefully others will see the light and follow its example.
> > If nothing else, they ought to get a bit envious when you can add features
> > left and right without ever having to wait for API/ABI break periods etc
> > ;)
> 
> Agreed. We should be doing more of this type of refactoring work to make the API/ABI less easier to break.

+1
But we must check the possible performance degradation with care :)