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