From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [RFC] vhost-user public struct refactor (was Re: [PATCH RFC 2/4] vhost: make buf vector for scatter RX) local. Date: Tue, 05 Apr 2016 10:37:13 +0200 Message-ID: <1865360.UqzrcfEKI5@xps13> References: <1455863563-15751-1-git-send-email-i.maximets@samsung.com> <20160219070650.GS21426@yliu-dev.sh.intel.com> <20160405054733.GO3080@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org, Ilya Maximets , Dyasly Sergey , Flavio Leitner , "Xie, Huawei" To: Yuanhan Liu Return-path: Received: from mail-wm0-f42.google.com (mail-wm0-f42.google.com [74.125.82.42]) by dpdk.org (Postfix) with ESMTP id 4717AA6A for ; Tue, 5 Apr 2016 10:38:53 +0200 (CEST) Received: by mail-wm0-f42.google.com with SMTP id f198so21623630wme.0 for ; Tue, 05 Apr 2016 01:38:53 -0700 (PDT) In-Reply-To: <20160405054733.GO3080@yliu-dev.sh.intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 2016-04-05 13:47, Yuanhan Liu: > So, I was considering to add vhost-user Tx delayed-copy (or zero copy) > support recently, which comes to yet another ABI violation, as we need > add a new field to virtio_memory_regions struct to do guest phys addr > to host phys addr translation. You may ask, however, that why do we need > expose virtio_memory_regions struct to users at all? > > You are right, we don't have to. And here is the thing: we exposed way > too many fields (or even structures) than necessary. Say, vhost_virtqueue > struct should NOT be exposed to user at all: application just need to > tell the right queue id to locate a specific queue, and that's all. > The structure should be defined in an internal header file. With that, > we could do any changes to it we want, without worrying about that we > may offense the painful ABI rules. > > Similar changes could be done to virtio_net struct as well, just exposing > very few fields that are necessary and moving all others to an internal > structure. > > Huawei then suggested a more radical yet much cleaner one: just exposing > a virtio_net handle to application, just like the way kernel exposes an > fd to user for locating a specific file. However, it's more than an ABI > change; it's also an API change: some fields are referenced by applications, > such as flags, virt_qp_nb. We could expose some new functions to access > them though. > > I'd vote for this one, as it sounds very clean to me. This would also > solve the block issue of this patch. Though it would break OVS, I'm thinking > that'd be okay, as OVS has dependence on DPDK version: what we need to > do is just to send few patches to OVS, and let it points to next release, > say DPDK v16.07. Flavio, please correct me if I'm wrong. > > Thoughts/comments? Do you plan to send a deprecation notice to change API in 16.07?