From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yuanhan Liu Subject: Re: [PATCH 1/4] vhost: handle VHOST_USER_SET_LOG_BASE request Date: Wed, 2 Dec 2015 23:09:23 +0800 Message-ID: <20151202150923.GV2325@yliu-dev.sh.intel.com> References: <1449027793-30975-1-git-send-email-yuanhan.liu@linux.intel.com> <1449027793-30975-2-git-send-email-yuanhan.liu@linux.intel.com> <565EF7E9.6080400@redhat.com> <20151202143101.GR2325@yliu-dev.sh.intel.com> <565F04AE.2070404@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org, Victor Kaplansky , "Michael S. Tsirkin" To: Panu Matilainen Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 890FA8E73 for ; Wed, 2 Dec 2015 16:05:57 +0100 (CET) Content-Disposition: inline In-Reply-To: <565F04AE.2070404@redhat.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" On Wed, Dec 02, 2015 at 04:48:14PM +0200, Panu Matilainen wrote: ... > >>>diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h > >>>index 5687452..416dac2 100644 > >>>--- a/lib/librte_vhost/rte_virtio_net.h > >>>+++ b/lib/librte_vhost/rte_virtio_net.h > >>>@@ -127,6 +127,8 @@ struct virtio_net { > >>> #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ) > >>> char ifname[IF_NAME_SZ]; /**< Name of the tap device or socket path. */ > >>> uint32_t virt_qp_nb; /**< number of queue pair we have allocated */ > >>>+ uint64_t log_size; /**< Size of log area */ > >>>+ uint8_t *log_base; /**< Where dirty pages are logged */ > >>> void *priv; /**< private context */ > >>> struct vhost_virtqueue *virtqueue[VHOST_MAX_QUEUE_PAIRS * 2]; /**< Contains all virtqueue information. */ > >>> } __rte_cache_aligned; > >> > >>This (and other changes in patch 2 breaks the librte_vhost ABI > >>again, so you'd need to at least add a deprecation note to 2.2 to be > >>able to do it in 2.3 at all according to the ABI policy. > > > >I was thinking that adding a new field (instead of renaming it or > >removing it) isn't an ABI break. So, I was wrong? > > Adding or removing a field in the middle of a public struct is > always an ABI break. Adding to the end often is too, but not always. > Renaming a field is an API break but not an ABI break - the compiler > cares but the cpu does not. Good to know. Thanks. > > >> > >>Perhaps a better option would be adding some padding to the structs > >>now for 2.2 since the vhost ABI is broken there anyway. That would > >>at least give a chance to keep it compatible from 2.2 to 2.3. > > > >It will not be compatible, unless we add exact same fields (not > >something like uint8_t pad[xx]). Otherwise, the pad field renaming > >is also an ABI break, right? > > There's no ABI (or API) break in changing reserved unused fields to > something else, as long as care is taken with sizes and alignment. as long as we don't reference the reserved unused fields? > In any case padding is best added to the end of a struct to minimize > risks and keep things simple. The thing is that isn't it a bit aweful to (always) add pads to the end of a struct, especially when you don't know how many need to be padded? --yliu